[#529] objectcore: Use common sender classifier

Use common sender classifier for ACL service and format validator.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>

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.
@@ -94,6 +91,7 @@ func NewFormatValidator(opts ...FormatValidatorOption) *FormatValidator {
 
 	return &FormatValidator{
 		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
+	}
+}

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))

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

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

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{