From fa009db140b31fb268277b4bbf3216c078a10b43 Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Tue, 7 Jun 2022 20:58:21 +0300 Subject: [PATCH] [#1464] ir/container: Fix verifying the operations within sessions In previous implementation `verifySignature` method of container processor worked incorrectly for operations without a key and with session: processor tried to verify signature with one of the bound owner keys instead of session one. Use `VerifySessionDataSignature` method to check the signature if session is used. Refactor `verifySignature` a bit with session check highlighting for readability. Signed-off-by: Leonard Lyubich --- go.mod | 2 +- go.sum | Bin 98309 -> 98309 bytes pkg/innerring/processors/container/common.go | 59 ++++++------------- 3 files changed, 20 insertions(+), 41 deletions(-) diff --git a/go.mod b/go.mod index c24a3f26a..49ab8c4ba 100644 --- a/go.mod +++ b/go.mod @@ -19,7 +19,7 @@ require ( github.com/nspcc-dev/neo-go/pkg/interop v0.0.0-20220321144137-d5a9af5860af // indirect github.com/nspcc-dev/neofs-api-go/v2 v2.12.2 github.com/nspcc-dev/neofs-contract v0.15.1 - github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.4.0.20220606070217-67ff996dc35b + github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.4.0.20220607185339-031eac2f48f6 github.com/nspcc-dev/tzhash v1.5.2 github.com/panjf2000/ants/v2 v2.4.0 github.com/paulmach/orb v0.2.2 diff --git a/go.sum b/go.sum index 468680783dd1a9089d43d3244fa1847c278a9a9b..b2ddd869d4d4c7272f177324fc95b83e93d7744f 100644 GIT binary patch delta 111 zcmZo|U~6q)+aNZ_+1${=)Y#Zk*TC2?H8I&J&BP+jOd-S2Dlsr8B|FtQs35~5EI79~ xz(w1@RNJ}SC%`ke)Tg{M*eg3T-N)E5Go;jFa^h^&$#ru?M6j8)dD)!!#{iR;Bnbcj delta 111 zcmZo|U~6q)+aNZ_+04M)z{t>C*UUUE&C=2=CE3_CNg>0~$~PrF!aO6}GPtD3%SAi8 x*jGO&AS}<>IioPF$jdt_GcU2q!aOKEI4I9`a^h^&$#ru?M6j8)dD)!!#{iPGBp3hy diff --git a/pkg/innerring/processors/container/common.go b/pkg/innerring/processors/container/common.go index a1075b31d..89fc6dcb0 100644 --- a/pkg/innerring/processors/container/common.go +++ b/pkg/innerring/processors/container/common.go @@ -1,14 +1,12 @@ package container import ( - "bytes" "crypto/ecdsa" "errors" "fmt" "github.com/nspcc-dev/neofs-node/pkg/morph/client/neofsid" cid "github.com/nspcc-dev/neofs-sdk-go/container/id" - neofscrypto "github.com/nspcc-dev/neofs-sdk-go/crypto" neofsecdsa "github.com/nspcc-dev/neofs-sdk-go/crypto/ecdsa" "github.com/nspcc-dev/neofs-sdk-go/session" "github.com/nspcc-dev/neofs-sdk-go/user" @@ -52,7 +50,6 @@ func (cp *Processor) verifySignature(v signatureVerificationData) error { var err error var key neofsecdsa.PublicKeyRFC6979 keyProvided := v.binPublicKey != nil - withSession := len(v.binTokenSession) > 0 if keyProvided { err = key.Decode(v.binPublicKey) @@ -61,7 +58,7 @@ func (cp *Processor) verifySignature(v signatureVerificationData) error { } } - if withSession { + if len(v.binTokenSession) > 0 { var tok session.Container err = tok.Unmarshal(v.binTokenSession) @@ -74,7 +71,6 @@ func (cp *Processor) verifySignature(v signatureVerificationData) error { } // FIXME(@cthulhu-rider): #1387 check token is signed by container owner, see neofs-sdk-go#233 - // We'll get container owner's keys which is needed below, so it's worth to cache them if keyProvided && !tok.AssertAuthKey(&key) { return errors.New("signed with a non-session key") @@ -96,24 +92,27 @@ func (cp *Processor) verifySignature(v signatureVerificationData) error { if err != nil { return fmt.Errorf("check session lifetime: %w", err) } - } - var verificationKeys []neofscrypto.PublicKey + if !tok.VerifySessionDataSignature(v.signedData, v.signature) { + return errors.New("invalid signature calculated with session key") + } + + return nil + } if keyProvided { - if withSession { - verificationKeys = []neofscrypto.PublicKey{&key} - } else { - var idFromKey user.ID - user.IDFromKey(&idFromKey, (ecdsa.PublicKey)(key)) + // TODO(@cthulhu-rider): #1387 use another approach after neofs-sdk-go#233 + var idFromKey user.ID + user.IDFromKey(&idFromKey, (ecdsa.PublicKey)(key)) - if v.ownerContainer.Equals(idFromKey) { - verificationKeys = []neofscrypto.PublicKey{&key} + if v.ownerContainer.Equals(idFromKey) { + if key.Verify(v.signedData, v.signature) { + return nil } - } - } - if verificationKeys == nil { + return errors.New("invalid signature calculated by container owner's key") + } + } else { var prm neofsid.AccountKeysPrm prm.SetID(v.ownerContainer) @@ -122,34 +121,14 @@ func (cp *Processor) verifySignature(v signatureVerificationData) error { return fmt.Errorf("receive owner keys %s: %w", v.ownerContainer, err) } - if !keyProvided { - verificationKeys = make([]neofscrypto.PublicKey, 0, len(ownerKeys)) - } - for i := range ownerKeys { - if keyProvided { - // TODO(@cthulhu-rider): keys have been decoded in order to encode only, should be optimized by #1387 - if bytes.Equal(ownerKeys[i].Bytes(), v.binPublicKey) { - verificationKeys = []neofscrypto.PublicKey{(*neofsecdsa.PublicKeyRFC6979)(ownerKeys[i])} - break - } - } else { - verificationKeys = append(verificationKeys, (*neofsecdsa.PublicKeyRFC6979)(ownerKeys[i])) + if (*neofsecdsa.PublicKeyRFC6979)(ownerKeys[i]).Verify(v.signedData, v.signature) { + return nil } } } - if len(verificationKeys) == 0 { - return errors.New("key is not a container owner's key") - } - - for i := range verificationKeys { - if verificationKeys[i].Verify(v.signedData, v.signature) { - return nil - } - } - - return errors.New("invalid signature") + return errors.New("signature is invalid or calculated with the key not bound to the container owner") } func (cp *Processor) checkTokenLifetime(token session.Container) error {