Skip to content

Commit

Permalink
[nspcc-dev#1464] ir/container: Fix verifying the operations within se…
Browse files Browse the repository at this point in the history
…ssions

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>
  • Loading branch information
Leonard Lyubich committed Jun 7, 2022
1 parent f6d121a commit 1ed3997
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 44 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -407,8 +407,8 @@ github.com/nspcc-dev/neofs-crypto v0.3.0 h1:zlr3pgoxuzrmGCxc5W8dGVfA9Rro8diFvVnB
github.com/nspcc-dev/neofs-crypto v0.3.0/go.mod h1:8w16GEJbH6791ktVqHN9YRNH3s9BEEKYxGhlFnp0cDw=
github.com/nspcc-dev/neofs-sdk-go v0.0.0-20211201182451-a5b61c4f6477/go.mod h1:dfMtQWmBHYpl9Dez23TGtIUKiFvCIxUZq/CkSIhEpz4=
github.com/nspcc-dev/neofs-sdk-go v0.0.0-20220113123743-7f3162110659/go.mod h1:/jay1lr3w7NQd/VDBkEhkJmDmyPNsu4W+QV2obsUV40=
github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.4.0.20220606070217-67ff996dc35b h1:MdgX7hk9StrJD+ksM/RPVnCChqVrJKZinaz87RWSRnE=
github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.4.0.20220606070217-67ff996dc35b/go.mod h1:k58jgszGX3pws2yiOXu9m0i32BzRgi1T6Bpd/L1KrJU=
github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.4.0.20220607185339-031eac2f48f6 h1:aQldkeCRphHVSmsPD+05+CwLPImuLwySJkigL3AiTu8=
github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.4.0.20220607185339-031eac2f48f6/go.mod h1:k58jgszGX3pws2yiOXu9m0i32BzRgi1T6Bpd/L1KrJU=
github.com/nspcc-dev/rfc6979 v0.1.0/go.mod h1:exhIh1PdpDC5vQmyEsGvc4YDM/lyQp/452QxGq/UEso=
github.com/nspcc-dev/rfc6979 v0.2.0 h1:3e1WNxrN60/6N0DW7+UYisLeZJyfqZTNOjeV/toYvOE=
github.com/nspcc-dev/rfc6979 v0.2.0/go.mod h1:exhIh1PdpDC5vQmyEsGvc4YDM/lyQp/452QxGq/UEso=
Expand Down
61 changes: 20 additions & 41 deletions pkg/innerring/processors/container/common.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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")
Expand All @@ -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))

if v.ownerContainer.Equals(idFromKey) {
verificationKeys = []neofscrypto.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) {
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)

Expand All @@ -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 {
Expand Down

0 comments on commit 1ed3997

Please sign in to comment.