[#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 <leonard@nspcc.ru>
This commit is contained in:
Leonard Lyubich 2022-06-07 20:58:21 +03:00 committed by LeL
parent f6d121a4e4
commit fa009db140
3 changed files with 20 additions and 41 deletions

2
go.mod
View file

@ -19,7 +19,7 @@ require (
github.com/nspcc-dev/neo-go/pkg/interop v0.0.0-20220321144137-d5a9af5860af // indirect 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-api-go/v2 v2.12.2
github.com/nspcc-dev/neofs-contract v0.15.1 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/nspcc-dev/tzhash v1.5.2
github.com/panjf2000/ants/v2 v2.4.0 github.com/panjf2000/ants/v2 v2.4.0
github.com/paulmach/orb v0.2.2 github.com/paulmach/orb v0.2.2

BIN
go.sum

Binary file not shown.

View file

@ -1,14 +1,12 @@
package container package container
import ( import (
"bytes"
"crypto/ecdsa" "crypto/ecdsa"
"errors" "errors"
"fmt" "fmt"
"github.com/nspcc-dev/neofs-node/pkg/morph/client/neofsid" "github.com/nspcc-dev/neofs-node/pkg/morph/client/neofsid"
cid "github.com/nspcc-dev/neofs-sdk-go/container/id" 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" 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/session"
"github.com/nspcc-dev/neofs-sdk-go/user" "github.com/nspcc-dev/neofs-sdk-go/user"
@ -52,7 +50,6 @@ func (cp *Processor) verifySignature(v signatureVerificationData) error {
var err error var err error
var key neofsecdsa.PublicKeyRFC6979 var key neofsecdsa.PublicKeyRFC6979
keyProvided := v.binPublicKey != nil keyProvided := v.binPublicKey != nil
withSession := len(v.binTokenSession) > 0
if keyProvided { if keyProvided {
err = key.Decode(v.binPublicKey) 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 var tok session.Container
err = tok.Unmarshal(v.binTokenSession) 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 // 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) { if keyProvided && !tok.AssertAuthKey(&key) {
return errors.New("signed with a non-session key") return errors.New("signed with a non-session key")
@ -96,24 +92,27 @@ func (cp *Processor) verifySignature(v signatureVerificationData) error {
if err != nil { if err != nil {
return fmt.Errorf("check session lifetime: %w", err) return fmt.Errorf("check session lifetime: %w", err)
} }
if !tok.VerifySessionDataSignature(v.signedData, v.signature) {
return errors.New("invalid signature calculated with session key")
} }
var verificationKeys []neofscrypto.PublicKey return nil
}
if keyProvided { if keyProvided {
if withSession { // TODO(@cthulhu-rider): #1387 use another approach after neofs-sdk-go#233
verificationKeys = []neofscrypto.PublicKey{&key}
} else {
var idFromKey user.ID var idFromKey user.ID
user.IDFromKey(&idFromKey, (ecdsa.PublicKey)(key)) user.IDFromKey(&idFromKey, (ecdsa.PublicKey)(key))
if v.ownerContainer.Equals(idFromKey) { if v.ownerContainer.Equals(idFromKey) {
verificationKeys = []neofscrypto.PublicKey{&key} 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 var prm neofsid.AccountKeysPrm
prm.SetID(v.ownerContainer) 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) return fmt.Errorf("receive owner keys %s: %w", v.ownerContainer, err)
} }
if !keyProvided {
verificationKeys = make([]neofscrypto.PublicKey, 0, len(ownerKeys))
}
for i := range ownerKeys { for i := range ownerKeys {
if keyProvided { if (*neofsecdsa.PublicKeyRFC6979)(ownerKeys[i]).Verify(v.signedData, v.signature) {
// 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 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 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 { func (cp *Processor) checkTokenLifetime(token session.Container) error {