Skip to content

Commit

Permalink
fix: Do not allow broken session token by SN
Browse files Browse the repository at this point in the history
If container session token is incorrect, there is no need in Alphabet's check,
the error can be returned immediately to a client; otherwise he has to wait for
the TX to persist infinitely with not feedback. Closes #2466.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
  • Loading branch information
carpawell committed Jan 23, 2024
1 parent c9ddb54 commit 71fc7ee
Show file tree
Hide file tree
Showing 3 changed files with 205 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Changelog for NeoFS Node
- Created files are not group writable (#2589)
- IR does not create new notary requests for the SN's bootstraps but signs the received ones instead (#2717)
- IR can handle third-party notary requests without reliance on receiving the original one (#2715)
- SN validates container session token's issuer to be container's owner (#2466)

### Removed
- Deprecated `neofs-adm [...] inspect` commands (#2603)
Expand Down
55 changes: 55 additions & 0 deletions pkg/services/container/morph/executor.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package container

import (
"bytes"
"context"
"errors"
"fmt"

"github.com/mr-tron/base58"
"github.com/nspcc-dev/neofs-api-go/v2/container"
"github.com/nspcc-dev/neofs-api-go/v2/refs"
sessionV2 "github.com/nspcc-dev/neofs-api-go/v2/session"
Expand Down Expand Up @@ -80,6 +82,11 @@ func (s *morphExecutor) Put(_ context.Context, tokV2 *sessionV2.Token, body *con
if err != nil {
return nil, fmt.Errorf("invalid session token: %w", err)
}

err = s.validateToken(tokV2, nil, sessionV2.ContainerVerbPut)
if err != nil {
return nil, fmt.Errorf("session token validation: %w", err)

Check warning on line 88 in pkg/services/container/morph/executor.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/container/morph/executor.go#L88

Added line #L88 was not covered by tests
}
}

idCnr, err := s.wrt.Put(cnr)
Expand Down Expand Up @@ -120,6 +127,11 @@ func (s *morphExecutor) Delete(_ context.Context, tokV2 *sessionV2.Token, body *
if err != nil {
return nil, fmt.Errorf("invalid session token: %w", err)
}

err = s.validateToken(tokV2, body.GetContainerID(), sessionV2.ContainerVerbDelete)
if err != nil {
return nil, fmt.Errorf("session token validation: %w", err)
}
}

var rmWitness containercore.RemovalWitness
Expand Down Expand Up @@ -228,6 +240,11 @@ func (s *morphExecutor) SetExtendedACL(_ context.Context, tokV2 *sessionV2.Token
if err != nil {
return nil, fmt.Errorf("invalid session token: %w", err)
}

err = s.validateToken(tokV2, body.GetEACL().GetContainerID(), sessionV2.ContainerVerbSetEACL)
if err != nil {
return nil, fmt.Errorf("session token validation: %w", err)

Check warning on line 246 in pkg/services/container/morph/executor.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/container/morph/executor.go#L246

Added line #L246 was not covered by tests
}
}

err = s.wrt.PutEACL(eaclInfo)
Expand Down Expand Up @@ -274,3 +291,41 @@ func (s *morphExecutor) GetExtendedACL(_ context.Context, body *container.GetExt

return res, nil
}

func (s *morphExecutor) validateToken(t *sessionV2.Token, cID *refs.ContainerID, op sessionV2.ContainerSessionVerb) error {
c := t.GetBody().GetContext()
cc, ok := c.(*sessionV2.ContainerSessionContext)
if !ok {
return errors.New("session is not container-related")

Check warning on line 299 in pkg/services/container/morph/executor.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/container/morph/executor.go#L299

Added line #L299 was not covered by tests
}

if verb := cc.Verb(); verb != op {
return fmt.Errorf("wrong container session operation: %s", verb)
}

if cID == nil { // can be nil for PUT or wildcard may be true
return nil
}

if sessionCID := cc.ContainerID().GetValue(); !bytes.Equal(sessionCID, cID.GetValue()) {
return fmt.Errorf("wrong container: %s", base58.Encode(sessionCID))
}

var cIDSDK cid.ID

err := cIDSDK.ReadFromV2(*cID)
if err != nil {
return fmt.Errorf("invalid container ID: %w", err)

Check warning on line 318 in pkg/services/container/morph/executor.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/container/morph/executor.go#L318

Added line #L318 was not covered by tests
}

cnr, err := s.rdr.Get(cIDSDK)
if err != nil {
return fmt.Errorf("reading container from the network: %w", err)

Check warning on line 323 in pkg/services/container/morph/executor.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/container/morph/executor.go#L323

Added line #L323 was not covered by tests
}

if issuer := t.GetBody().GetOwnerID().GetValue(); !bytes.Equal(cnr.Value.Owner().WalletBytes(), issuer) {
return fmt.Errorf("session was not issued by the container owner, issuer: %q", issuer)
}

return nil
}
155 changes: 149 additions & 6 deletions pkg/services/container/morph/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package container_test

import (
"context"
"crypto/sha256"
"testing"

"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
Expand All @@ -11,17 +12,32 @@ import (
containerCore "github.com/nspcc-dev/neofs-node/pkg/core/container"
containerSvc "github.com/nspcc-dev/neofs-node/pkg/services/container"
containerSvcMorph "github.com/nspcc-dev/neofs-node/pkg/services/container/morph"
containerSDK "github.com/nspcc-dev/neofs-sdk-go/container"
cid "github.com/nspcc-dev/neofs-sdk-go/container/id"
cidtest "github.com/nspcc-dev/neofs-sdk-go/container/id/test"
containertest "github.com/nspcc-dev/neofs-sdk-go/container/test"
neofscrypto "github.com/nspcc-dev/neofs-sdk-go/crypto"
sessionsdk "github.com/nspcc-dev/neofs-sdk-go/session"
sessiontest "github.com/nspcc-dev/neofs-sdk-go/session/test"
"github.com/nspcc-dev/neofs-sdk-go/user"
usertest "github.com/nspcc-dev/neofs-sdk-go/user/test"
"github.com/stretchr/testify/require"
)

type mock struct {
containerSvcMorph.Reader
cnr containerSDK.Container
}

func (m mock) Get(_ cid.ID) (*containerCore.Container, error) {
return &containerCore.Container{Value: m.cnr}, nil
}

func (m mock) GetEACL(id cid.ID) (*containerCore.EACL, error) {
return nil, nil
}

func (m mock) List(id *user.ID) ([]cid.ID, error) {
return nil, nil
}

func (m mock) Put(_ containerCore.Container) (*cid.ID, error) {
Expand All @@ -36,10 +52,7 @@ func (m mock) PutEACL(_ containerCore.EACL) error {
return nil
}

func TestInvalidToken(t *testing.T) {
m := mock{}
e := containerSvcMorph.NewExecutor(m, m)

func TestExecutor(t *testing.T) {
cnr := cidtest.ID()

var cnrV2 refs.ContainerID
Expand All @@ -62,8 +75,18 @@ func TestInvalidToken(t *testing.T) {
reqBody.SetSignature(&sigV2)
}

tok := sessiontest.Container()
tok.ApplyOnlyTo(cnr)
require.NoError(t, tok.Sign(signer))

var tokV2 session.Token
sessiontest.ContainerSigned(signer).WriteToV2(&tokV2)
tok.WriteToV2(&tokV2)

realContainer := containertest.Container(t)
realContainer.SetOwner(tok.Issuer())

m := mock{cnr: realContainer}
e := containerSvcMorph.NewExecutor(m, m)

tests := []struct {
name string
Expand Down Expand Up @@ -92,6 +115,11 @@ func TestInvalidToken(t *testing.T) {
var reqBody container.DeleteRequestBody
reqBody.SetContainerID(&cnrV2)

cc, ok := tokV2.GetBody().GetContext().(*session.ContainerSessionContext)
if ok {
cc.SetVerb(session.ContainerVerbDelete)
}

_, err = e.Delete(context.TODO(), tokV2, &reqBody)
return
},
Expand All @@ -103,6 +131,11 @@ func TestInvalidToken(t *testing.T) {
reqBody.SetSignature(new(refs.Signature))
sign(&reqBody)

cc, ok := tokV2.GetBody().GetContext().(*session.ContainerSessionContext)
if ok {
cc.SetVerb(session.ContainerVerbSetEACL)
}

_, err = e.SetExtendedACL(context.TODO(), tokV2, &reqBody)
return
},
Expand All @@ -121,6 +154,116 @@ func TestInvalidToken(t *testing.T) {
}
}

func TestValidateToken(t *testing.T) {
cID := cidtest.ID()
var cIDV2 refs.ContainerID
cID.WriteToV2(&cIDV2)

priv, err := keys.NewPrivateKey()
require.NoError(t, err)

signer := user.NewAutoIDSigner(priv.PrivateKey)

tok := sessiontest.Container()
tok.ApplyOnlyTo(cID)
tok.ForVerb(sessionsdk.VerbContainerDelete)
require.NoError(t, tok.Sign(signer))

cnr := containertest.Container(t)
cnr.SetOwner(tok.Issuer())

var cnrV2 container.Container
cnr.WriteToV2(&cnrV2)

m := mock{cnr: cnr}
e := containerSvcMorph.NewExecutor(m, m)

t.Run("non-container token", func(t *testing.T) {
var reqBody container.DeleteRequestBody
reqBody.SetContainerID(&cIDV2)

var tokV2 session.Token
objectSession := sessiontest.Object()
require.NoError(t, objectSession.Sign(signer))

objectSession.WriteToV2(&tokV2)

_, err = e.Delete(context.TODO(), &tokV2, &reqBody)
require.Error(t, err)

return
})

t.Run("wrong verb token", func(t *testing.T) {
var reqBody container.DeleteRequestBody
reqBody.SetContainerID(&cIDV2)

var tokCopy sessionsdk.Container
tok.CopyTo(&tokCopy)
tokCopy.ForVerb(sessionsdk.VerbContainerPut)

var tokV2 session.Token
tokCopy.WriteToV2(&tokV2)

_, err = e.Delete(context.TODO(), &tokV2, &reqBody)
require.Error(t, err)

return
})

t.Run("incorrect cID", func(t *testing.T) {
var reqBody container.DeleteRequestBody
reqBody.SetContainerID(&cIDV2)

var tokV2 session.Token
var cIDV2 refs.ContainerID
cc := new(session.ContainerSessionContext)
b := new(session.TokenBody)

cIDV2.SetValue(make([]byte, sha256.Size+1))
cc.SetContainerID(&cIDV2)
b.SetContext(cc)
tokV2.SetBody(b)

_, err = e.Delete(context.TODO(), &tokV2, &reqBody)
require.Error(t, err)
})

t.Run("different container ID", func(t *testing.T) {
var reqBody container.DeleteRequestBody
reqBody.SetContainerID(&cIDV2)

var tokCopy sessionsdk.Container
tok.CopyTo(&tokCopy)
tokCopy.ApplyOnlyTo(cidtest.ID())

require.NoError(t, tokCopy.Sign(signer))

var tokV2 session.Token
tokCopy.WriteToV2(&tokV2)

_, err = e.Delete(context.TODO(), &tokV2, &reqBody)
require.Error(t, err)
})

t.Run("different issuer", func(t *testing.T) {
var reqBody container.DeleteRequestBody
reqBody.SetContainerID(&cIDV2)

var tokV2 session.Token
tok.WriteToV2(&tokV2)

var ownerV2Wrong refs.OwnerID
ownerWrong := usertest.ID(t)
ownerWrong.WriteToV2(&ownerV2Wrong)

tokV2.GetBody().SetOwnerID(&ownerV2Wrong)

_, err = e.Delete(context.TODO(), &tokV2, &reqBody)
require.Error(t, err)
})
}

func generateToken(ctx session.TokenContext) *session.Token {
body := new(session.TokenBody)
body.SetContext(ctx)
Expand Down

0 comments on commit 71fc7ee

Please sign in to comment.