From 55b82e744b822a60f4b84c8b389e08ad13d1cca1 Mon Sep 17 00:00:00 2001 From: Dmitrii Stepanov Date: Mon, 7 Aug 2023 09:54:47 +0300 Subject: [PATCH] [#529] objectcore: Use common sender classifier Use common sender classifier for ACL service and format validator. Signed-off-by: Dmitrii Stepanov --- pkg/core/object/fmt.go | 98 +++---------------- pkg/core/object/fmt_test.go | 27 ++++- .../object/sender_classifier.go} | 70 +++++++------ pkg/services/object/acl/v2/service.go | 19 ++-- pkg/services/object/put/service.go | 1 + 5 files changed, 95 insertions(+), 120 deletions(-) rename pkg/{services/object/acl/v2/classifier.go => core/object/sender_classifier.go} (68%) diff --git a/pkg/core/object/fmt.go b/pkg/core/object/fmt.go index 90365cea..e6576772 100644 --- a/pkg/core/object/fmt.go +++ b/pkg/core/object/fmt.go @@ -1,10 +1,8 @@ package object import ( - "bytes" "context" "crypto/ecdsa" - "crypto/elliptic" "crypto/sha256" "errors" "fmt" @@ -14,18 +12,20 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/refs" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/netmap" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger" + "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" frostfsecdsa "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/crypto/ecdsa" - netmapSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap" objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" - "github.com/nspcc-dev/neo-go/pkg/crypto/keys" ) // FormatValidator represents an object format validator. type FormatValidator struct { *cfg + + senderClassifier SenderClassifier } // FormatValidatorOption represents a FormatValidator constructor option. @@ -37,13 +37,10 @@ type cfg struct { ir InnerRing netmap netmap.Source containers container.Source + log *logger.Logger verifyTokenIssuer bool } -type InnerRing interface { - InnerRingKeys() ([][]byte, error) -} - // DeleteHandler is an interface of delete queue processor. type DeleteHandler interface { // DeleteObjects places objects to a removal queue. @@ -93,7 +90,8 @@ func NewFormatValidator(opts ...FormatValidatorOption) *FormatValidator { } return &FormatValidator{ - cfg: cfg, + cfg: cfg, + senderClassifier: NewSenderClassifier(cfg.ir, cfg.netmap, cfg.log), } } @@ -193,42 +191,6 @@ func (v *FormatValidator) validateSignatureKey(obj *objectSDK.Object) error { } func (v *FormatValidator) isIROrContainerNode(obj *objectSDK.Object, signerKey []byte) (bool, error) { - pKey, err := keys.NewPublicKeyFromBytes(signerKey, elliptic.P256()) - if err != nil { - return false, fmt.Errorf("(%T) failed to unmarshal signer public key: %w", v, err) - } - - isIR, err := v.isInnerRingKey(pKey.Bytes()) - if err != nil { - return false, fmt.Errorf("(%T) failed to check if signer is inner ring node: %w", v, err) - } - if isIR { - return true, nil - } - - isContainerNode, err := v.isContainerNode(pKey.Bytes(), obj) - if err != nil { - return false, fmt.Errorf("(%T) failed to check if signer is container node: %w", v, err) - } - return isContainerNode, nil -} - -func (v *FormatValidator) isInnerRingKey(key []byte) (bool, error) { - innerRingKeys, err := v.ir.InnerRingKeys() - if err != nil { - return false, err - } - - for i := range innerRingKeys { - if bytes.Equal(innerRingKeys[i], key) { - return true, nil - } - } - - return false, nil -} - -func (v *FormatValidator) isContainerNode(key []byte, obj *objectSDK.Object) (bool, error) { cnrID, containerIDSet := obj.ContainerID() if !containerIDSet { return false, errNilCID @@ -242,46 +204,11 @@ func (v *FormatValidator) isContainerNode(key []byte, obj *objectSDK.Object) (bo return false, fmt.Errorf("failed to get container (id=%s): %w", cnrID.EncodeToString(), err) } - lastNetmap, err := netmap.GetLatestNetworkMap(v.netmap) - if err != nil { - return false, fmt.Errorf("failed to get latest netmap: %w", err) - } - - isContainerNode, err := v.isContainerNodeKey(lastNetmap, cnr, cnrIDBin, key) - if err != nil { - return false, fmt.Errorf("failed to check latest netmap for container nodes: %w", err) - } - if isContainerNode { - return true, nil - } - - previousNetmap, err := netmap.GetPreviousNetworkMap(v.netmap) - if err != nil { - return false, fmt.Errorf("failed to get previous netmap: %w", err) - } - - isContainerNode, err = v.isContainerNodeKey(previousNetmap, cnr, cnrIDBin, key) - if err != nil { - return false, fmt.Errorf("failed to check previous netmap for container nodes: %w", err) - } - return isContainerNode, nil -} - -func (v *FormatValidator) isContainerNodeKey(nm *netmapSDK.NetMap, cnr *container.Container, cnrIDBin, key []byte) (bool, error) { - cnrVectors, err := nm.ContainerNodes(cnr.Value.PlacementPolicy(), cnrIDBin) + res, err := v.senderClassifier.IsInnerRingOrContainerNode(signerKey, cnrID, cnr.Value) if err != nil { return false, err } - - for i := range cnrVectors { - for j := range cnrVectors[i] { - if bytes.Equal(cnrVectors[i][j].PublicKey(), key) { - return true, nil - } - } - } - - return false, nil + return res.Role == acl.RoleContainer || res.Role == acl.RoleInnerRing, nil } func (v *FormatValidator) checkOwnerKey(id user.ID, key frostfsecdsa.PublicKey) error { @@ -533,3 +460,10 @@ func WithVerifySessionTokenIssuer(verifySessionTokenIssuer bool) FormatValidator c.verifyTokenIssuer = verifySessionTokenIssuer } } + +// WithLogger return option to set logger. +func WithLogger(l *logger.Logger) FormatValidatorOption { + return func(c *cfg) { + c.log = l + } +} diff --git a/pkg/core/object/fmt_test.go b/pkg/core/object/fmt_test.go index fbd82906..2a5b5690 100644 --- a/pkg/core/object/fmt_test.go +++ b/pkg/core/object/fmt_test.go @@ -9,6 +9,7 @@ import ( objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger" containerSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test" @@ -22,6 +23,7 @@ import ( "github.com/google/uuid" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" ) func blankValidObject(key *ecdsa.PrivateKey) *objectSDK.Object { @@ -63,6 +65,7 @@ func TestFormatValidator_Validate(t *testing.T) { epoch: curEpoch, }), WithLockSource(ls), + WithLogger(&logger.Logger{Logger: zaptest.NewLogger(t)}), ) ownerKey, err := keys.NewPrivateKey() @@ -285,6 +288,7 @@ func TestFormatValidator_ValidateTokenIssuer(t *testing.T) { }), WithLockSource(ls), WithVerifySessionTokenIssuer(false), + WithLogger(&logger.Logger{Logger: zaptest.NewLogger(t)}), ) tok := sessiontest.Object() @@ -307,6 +311,14 @@ func TestFormatValidator_ValidateTokenIssuer(t *testing.T) { t.Run("different issuer and owner, issuer is IR node, verify issuer enabled", func(t *testing.T) { t.Parallel() + + cnrID := cidtest.ID() + cont := containerSDK.Container{} + cont.Init() + pp := netmap.PlacementPolicy{} + require.NoError(t, pp.DecodeString("REP 1")) + cont.SetPlacementPolicy(pp) + v := NewFormatValidator( WithNetState(testNetState{ epoch: curEpoch, @@ -316,6 +328,16 @@ func TestFormatValidator_ValidateTokenIssuer(t *testing.T) { WithInnerRing(&testIRSource{ irNodes: [][]byte{signer.PublicKey().Bytes()}, }), + WithContainersSource( + &testContainerSource{ + containers: map[cid.ID]*container.Container{ + cnrID: { + Value: cont, + }, + }, + }, + ), + WithLogger(&logger.Logger{Logger: zaptest.NewLogger(t)}), ) tok := sessiontest.Object() @@ -328,7 +350,7 @@ func TestFormatValidator_ValidateTokenIssuer(t *testing.T) { require.NoError(t, tok.Sign(signer.PrivateKey)) obj := objectSDK.New() - obj.SetContainerID(cidtest.ID()) + obj.SetContainerID(cnrID) obj.SetSessionToken(tok) obj.SetOwnerID(&owner) require.NoError(t, objectSDK.SetIDWithSignature(signer.PrivateKey, obj)) @@ -393,6 +415,7 @@ func TestFormatValidator_ValidateTokenIssuer(t *testing.T) { currentEpoch: curEpoch, }, ), + WithLogger(&logger.Logger{Logger: zaptest.NewLogger(t)}), ) require.NoError(t, v.Validate(context.Background(), obj, false)) @@ -466,6 +489,7 @@ func TestFormatValidator_ValidateTokenIssuer(t *testing.T) { currentEpoch: curEpoch, }, ), + WithLogger(&logger.Logger{Logger: zaptest.NewLogger(t)}), ) require.NoError(t, v.Validate(context.Background(), obj, false)) @@ -541,6 +565,7 @@ func TestFormatValidator_ValidateTokenIssuer(t *testing.T) { currentEpoch: curEpoch, }, ), + WithLogger(&logger.Logger{Logger: zaptest.NewLogger(t)}), ) require.Error(t, v.Validate(context.Background(), obj, false)) diff --git a/pkg/services/object/acl/v2/classifier.go b/pkg/core/object/sender_classifier.go similarity index 68% rename from pkg/services/object/acl/v2/classifier.go rename to pkg/core/object/sender_classifier.go index cdc5fb62..79bf12ce 100644 --- a/pkg/services/object/acl/v2/classifier.go +++ b/pkg/core/object/sender_classifier.go @@ -1,4 +1,4 @@ -package v2 +package object import ( "bytes" @@ -11,50 +11,64 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap" + "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" + "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "go.uber.org/zap" ) -type senderClassifier struct { +type InnerRing interface { + InnerRingKeys() ([][]byte, error) +} + +type SenderClassifier struct { log *logger.Logger - innerRing InnerRingFetcher + innerRing InnerRing netmap core.Source } -type classifyResult struct { - role acl.Role - key []byte +func NewSenderClassifier(innerRing InnerRing, netmap core.Source, log *logger.Logger) SenderClassifier { + return SenderClassifier{ + log: log, + innerRing: innerRing, + netmap: netmap, + } } -func (c senderClassifier) classify( - req MetaWithToken, - idCnr cid.ID, - cnr container.Container) (res *classifyResult, err error) { - ownerID, ownerKey, err := req.RequestOwner() - if err != nil { - return nil, err - } +type ClassifyResult struct { + Role acl.Role + Key []byte +} +func (c SenderClassifier) Classify( + ownerID *user.ID, + ownerKey *keys.PublicKey, + idCnr cid.ID, + cnr container.Container) (res *ClassifyResult, err error) { ownerKeyInBytes := ownerKey.Bytes() // TODO: #767 get owner from frostfs.id if present // if request owner is the same as container owner, return RoleUser if ownerID.Equals(cnr.Owner()) { - return &classifyResult{ - role: acl.RoleOwner, - key: ownerKeyInBytes, + return &ClassifyResult{ + Role: acl.RoleOwner, + Key: ownerKeyInBytes, }, nil } + return c.IsInnerRingOrContainerNode(ownerKeyInBytes, idCnr, cnr) +} + +func (c SenderClassifier) IsInnerRingOrContainerNode(ownerKeyInBytes []byte, idCnr cid.ID, cnr container.Container) (*ClassifyResult, error) { isInnerRingNode, err := c.isInnerRingKey(ownerKeyInBytes) if err != nil { // do not throw error, try best case matching c.log.Debug(logs.V2CantCheckIfRequestFromInnerRing, zap.String("error", err.Error())) } else if isInnerRingNode { - return &classifyResult{ - role: acl.RoleInnerRing, - key: ownerKeyInBytes, + return &ClassifyResult{ + Role: acl.RoleInnerRing, + Key: ownerKeyInBytes, }, nil } @@ -69,20 +83,20 @@ func (c senderClassifier) classify( c.log.Debug(logs.V2CantCheckIfRequestFromContainerNode, zap.String("error", err.Error())) } else if isContainerNode { - return &classifyResult{ - role: acl.RoleContainer, - key: ownerKeyInBytes, + return &ClassifyResult{ + Role: acl.RoleContainer, + Key: ownerKeyInBytes, }, nil } // if none of above, return RoleOthers - return &classifyResult{ - role: acl.RoleOthers, - key: ownerKeyInBytes, + return &ClassifyResult{ + Role: acl.RoleOthers, + Key: ownerKeyInBytes, }, nil } -func (c senderClassifier) isInnerRingKey(owner []byte) (bool, error) { +func (c SenderClassifier) isInnerRingKey(owner []byte) (bool, error) { innerRingKeys, err := c.innerRing.InnerRingKeys() if err != nil { return false, err @@ -98,7 +112,7 @@ func (c senderClassifier) isInnerRingKey(owner []byte) (bool, error) { return false, nil } -func (c senderClassifier) isContainerKey( +func (c SenderClassifier) isContainerKey( owner, idCnr []byte, cnr container.Container) (bool, error) { nm, err := core.GetLatestNetworkMap(c.netmap) // first check current netmap diff --git a/pkg/services/object/acl/v2/service.go b/pkg/services/object/acl/v2/service.go index 271c4d20..8239403a 100644 --- a/pkg/services/object/acl/v2/service.go +++ b/pkg/services/object/acl/v2/service.go @@ -9,6 +9,7 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/session" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/netmap" + objectCore "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/object" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger" apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" @@ -24,7 +25,7 @@ import ( type Service struct { *cfg - c senderClassifier + c objectCore.SenderClassifier } type putStreamBasicChecker struct { @@ -95,11 +96,7 @@ func New(next object.ServiceServer, return Service{ cfg: cfg, - c: senderClassifier{ - log: cfg.log, - innerRing: cfg.irFetcher, - netmap: cfg.nm, - }, + c: objectCore.NewSenderClassifier(cfg.irFetcher, cfg.nm, cfg.log), } } @@ -652,20 +649,24 @@ func (b Service) findRequestInfo(req MetaWithToken, idCnr cid.ID, op acl.Op) (in } // find request role and key - res, err := b.c.classify(req, idCnr, cnr.Value) + ownerID, ownerKey, err := req.RequestOwner() + if err != nil { + return info, err + } + res, err := b.c.Classify(ownerID, ownerKey, idCnr, cnr.Value) if err != nil { return info, err } info.basicACL = cnr.Value.BasicACL() - info.requestRole = res.role + info.requestRole = res.Role info.operation = op info.cnrOwner = cnr.Value.Owner() info.idCnr = idCnr // it is assumed that at the moment the key will be valid, // otherwise the request would not pass validation - info.senderKey = res.key + info.senderKey = res.Key // add bearer token if it is present in request info.bearer = req.bearer diff --git a/pkg/services/object/put/service.go b/pkg/services/object/put/service.go index 4095cf12..3a7dcefd 100644 --- a/pkg/services/object/put/service.go +++ b/pkg/services/object/put/service.go @@ -98,6 +98,7 @@ func NewService(ks *objutil.KeyStorage, object.WithNetmapSource(ns), object.WithContainersSource(cs), object.WithVerifySessionTokenIssuer(c.verifySessionTokenIssuer), + object.WithLogger(c.log), ) return &Service{