[#1628] tree: Make ACL checks the same way as for object requests

1. Do not require a request to be signed by the container owner if a
bearer token is missing
2. Do not check the system role since public requests are not expected to
be signed by IR or a container node (unlike the object requests)

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

cmd/neofs-node/tree.go

@@ -21,6 +21,7 @@ func initTreeService(c *cfg) {
 
 	c.treeService = tree.New(
 		tree.WithContainerSource(c.cfgObject.cnrSource),
+		tree.WithEACLSource(c.cfgObject.eaclSource),
 		tree.WithNetmapSource(c.netMapSource),
 		tree.WithPrivateKey(&c.key.PrivateKey),
 		tree.WithLogger(c.log),

pkg/services/tree/options.go

@@ -16,6 +16,7 @@ type cfg struct {
 	rawPub     []byte
 	nmSource   netmap.Source
 	cnrSource  container.Source
+	eaclSource container.EACLSource
 	forest     pilorama.Forest
 	// replication-related parameters
 	replicatorChannelCapacity int
@@ -34,6 +35,14 @@ func WithContainerSource(src container.Source) Option {
 	}
 }
 
+// WithEACLSource sets a eACL table source for a tree service.
+// This option is required.
+func WithEACLSource(src container.EACLSource) Option {
+	return func(c *cfg) {
+		c.eaclSource = src
+	}
+}
+
 // WithNetmapSource sets a netmap source for a tree service.
 // This option is required.
 func WithNetmapSource(src netmap.Source) Option {

pkg/services/tree/service.go

@@ -7,8 +7,8 @@ import (
 	"fmt"
 
 	"github.com/nspcc-dev/neofs-node/pkg/local_object_storage/pilorama"
+	"github.com/nspcc-dev/neofs-sdk-go/container/acl"
 	cidSDK "github.com/nspcc-dev/neofs-sdk-go/container/id"
-	"github.com/nspcc-dev/neofs-sdk-go/eacl"
 	netmapSDK "github.com/nspcc-dev/neofs-sdk-go/netmap"
 	"go.uber.org/zap"
 )
@@ -69,7 +69,7 @@ func (s *Service) Add(ctx context.Context, req *AddRequest) (*AddResponse, error
 		return nil, err
 	}
 
-	err := s.verifyClient(req, cid, b.GetBearerToken(), eacl.OperationPut)
+	err := s.verifyClient(req, cid, b.GetBearerToken(), acl.OpObjectPut)
 	if err != nil {
 		return nil, err
 	}
@@ -117,7 +117,7 @@ func (s *Service) AddByPath(ctx context.Context, req *AddByPathRequest) (*AddByP
 		return nil, err
 	}
 
-	err := s.verifyClient(req, cid, b.GetBearerToken(), eacl.OperationPut)
+	err := s.verifyClient(req, cid, b.GetBearerToken(), acl.OpObjectPut)
 	if err != nil {
 		return nil, err
 	}
@@ -177,7 +177,7 @@ func (s *Service) Remove(ctx context.Context, req *RemoveRequest) (*RemoveRespon
 		return nil, err
 	}
 
-	err := s.verifyClient(req, cid, b.GetBearerToken(), eacl.OperationPut)
+	err := s.verifyClient(req, cid, b.GetBearerToken(), acl.OpObjectPut)
 	if err != nil {
 		return nil, err
 	}
@@ -226,7 +226,7 @@ func (s *Service) Move(ctx context.Context, req *MoveRequest) (*MoveResponse, er
 		return nil, err
 	}
 
-	err := s.verifyClient(req, cid, b.GetBearerToken(), eacl.OperationPut)
+	err := s.verifyClient(req, cid, b.GetBearerToken(), acl.OpObjectPut)
 	if err != nil {
 		return nil, err
 	}
@@ -274,7 +274,7 @@ func (s *Service) GetNodeByPath(ctx context.Context, req *GetNodeByPathRequest)
 		return nil, err
 	}
 
-	err := s.verifyClient(req, cid, b.GetBearerToken(), eacl.OperationGet)
+	err := s.verifyClient(req, cid, b.GetBearerToken(), acl.OpObjectGet)
 	if err != nil {
 		return nil, err
 	}
@@ -350,7 +350,7 @@ func (s *Service) GetSubTree(req *GetSubTreeRequest, srv TreeService_GetSubTreeS
 		return err
 	}
 
-	err := s.verifyClient(req, cid, b.GetBearerToken(), eacl.OperationGet)
+	err := s.verifyClient(req, cid, b.GetBearerToken(), acl.OpObjectGet)
 	if err != nil {
 		return err
 	}

pkg/services/tree/signature.go

@@ -8,7 +8,10 @@ import (
 
 	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
 	"github.com/nspcc-dev/neofs-api-go/v2/refs"
+	core "github.com/nspcc-dev/neofs-node/pkg/core/container"
 	"github.com/nspcc-dev/neofs-sdk-go/bearer"
+	"github.com/nspcc-dev/neofs-sdk-go/client"
+	"github.com/nspcc-dev/neofs-sdk-go/container/acl"
 	cidSDK "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"
@@ -23,9 +26,20 @@ type message interface {
 	SetSignature(*Signature)
 }
 
-// verifyClient verifies that the request for a client operation was either signed by owner
+func basicACLErr(op acl.Op) error {
-// or contains a valid bearer token.
+	return fmt.Errorf("access to operation %s is denied by basic ACL check", op)
-func (s *Service) verifyClient(req message, cid cidSDK.ID, rawBearer []byte, op eacl.Operation) error {
+}
+
+func eACLErr(op eacl.Operation, err error) error {
+	return fmt.Errorf("access to operation %s is denied by extended ACL check: %w", op, err)
+}
+
+// verifyClient verifies if the request for a client operation
+// was signed by a key allowed by (e)ACL rules.
+// Operation must be one of:
+//   - 1. ObjectPut;
+//   - 2. ObjectGet.
+func (s *Service) verifyClient(req message, cid cidSDK.ID, rawBearer []byte, op acl.Op) error {
 	err := verifyMessage(req)
 	if err != nil {
 		return err
@@ -36,53 +50,66 @@ func (s *Service) verifyClient(req message, cid cidSDK.ID, rawBearer []byte, op
 		return fmt.Errorf("can't get container %s: %w", cid, err)
 	}
 
-	ownerID := cnr.Value.Owner()
+	role, err := roleFromReq(cnr, req)
-
-	if len(rawBearer) == 0 { // must be signed by the owner
-		// No error is expected because `VerifyDataWithSource` checks the signature.
-		// However, we may use different algorithms in the future, thus this check.
-		pub, err := keys.NewPublicKeyFromBytes(req.GetSignature().GetKey(), elliptic.P256())
 	if err != nil {
-			return fmt.Errorf("invalid public key: %w", err)
+		return fmt.Errorf("can't get request role: %w", err)
 	}
 
-		var actualID user.ID
+	basicACL := cnr.Value.BasicACL()
-		user.IDFromKey(&actualID, (ecdsa.PublicKey)(*pub))
 
-		if !actualID.Equals(ownerID) {
+	if !basicACL.IsOpAllowed(op, role) {
-			return errors.New("request must be signed by a container owner")
+		return basicACLErr(op)
 	}
+
+	if !basicACL.Extendable() {
 		return nil
 	}
 
+	eaclOp := eACLOp(op)
+
+	var tb eacl.Table
+	if len(rawBearer) != 0 && basicACL.AllowedBearerRules(op) {
 		var bt bearer.Token
-	if err := bt.Unmarshal(rawBearer); err != nil {
+		if err = bt.Unmarshal(rawBearer); err != nil {
-		return fmt.Errorf("invalid bearer token: %w", err)
+			return eACLErr(eaclOp, fmt.Errorf("invalid bearer token: %w", err))
 		}
-	if !bearer.ResolveIssuer(bt).Equals(ownerID) {
+		if !bearer.ResolveIssuer(bt).Equals(cnr.Value.Owner()) {
-		return errors.New("bearer token must be signed by the container owner")
+			return eACLErr(eaclOp, errors.New("bearer token must be signed by the container owner"))
 		}
 		if !bt.AssertContainer(cid) {
-		return errors.New("bearer token is created for another container")
+			return eACLErr(eaclOp, errors.New("bearer token is created for another container"))
 		}
 		if !bt.VerifySignature() {
-		return errors.New("invalid bearer token signature")
+			return eACLErr(eaclOp, errors.New("invalid bearer token signature"))
 		}
 
-	tb := bt.EACLTable()
+		tb = bt.EACLTable()
+	} else {
+		tbCore, err := s.eaclSource.GetEACL(cid)
+		if err != nil {
+			if client.IsErrEACLNotFound(err) {
+				return nil
+			}
 
-	// The default action should be DENY, so we use RoleOthers to allow token issuer
+			return fmt.Errorf("get eACL table: %w", err)
-	// to restrict everyone not affected by the previous rules.
+		}
-	// This can be simplified after nspcc-dev/neofs-sdk-go#243 .
+
+		tb = *tbCore.Value
+	}
+
+	// The default action should be DENY.
 	action, found := eacl.NewValidator().CalculateAction(new(eacl.ValidationUnit).
 		WithEACLTable(&tb).
 		WithContainerID(&cid).
-		WithRole(eacl.RoleOthers).
+		WithRole(eACLRole(role)).
 		WithSenderKey(req.GetSignature().GetKey()).
-		WithOperation(op))
+		WithOperation(eaclOp))
-	if !found || action != eacl.ActionAllow {
+	if !found {
-		return errors.New("operation denied by bearer eACL")
+		return eACLErr(eaclOp, errors.New("not found allowing rules for the request"))
+	} else if action != eacl.ActionAllow {
+		return eACLErr(eaclOp, errors.New("DENY eACL rule"))
 	}
+
 	return nil
 }
 
@@ -132,3 +159,44 @@ func signMessage(m message, key *ecdsa.PrivateKey) error {
 
 	return nil
 }
+
+func roleFromReq(cnr *core.Container, req message) (acl.Role, error) {
+	role := acl.RoleOthers
+	owner := cnr.Value.Owner()
+
+	pub, err := keys.NewPublicKeyFromBytes(req.GetSignature().GetKey(), elliptic.P256())
+	if err != nil {
+		return role, fmt.Errorf("invalid public key: %w", err)
+	}
+
+	var reqSigner user.ID
+	user.IDFromKey(&reqSigner, (ecdsa.PublicKey)(*pub))
+
+	if reqSigner.Equals(owner) {
+		role = acl.RoleOwner
+	}
+
+	return role, nil
+}
+
+func eACLOp(op acl.Op) eacl.Operation {
+	switch op {
+	case acl.OpObjectGet:
+		return eacl.OperationGet
+	case acl.OpObjectPut:
+		return eacl.OperationPut
+	default:
+		panic(fmt.Sprintf("unexpected tree service ACL operation: %s", op))
+	}
+}
+
+func eACLRole(role acl.Role) eacl.Role {
+	switch role {
+	case acl.RoleOwner:
+		return eacl.RoleUser
+	case acl.RoleOthers:
+		return eacl.RoleOthers
+	default:
+		panic(fmt.Sprintf("unexpected tree service ACL role: %s", role))
+	}
+}

pkg/services/tree/signature_test.go

@@ -7,11 +7,12 @@ import (
 	"testing"
 
 	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
-	"github.com/nspcc-dev/neofs-api-go/v2/acl"
+	aclV2 "github.com/nspcc-dev/neofs-api-go/v2/acl"
 	containercore "github.com/nspcc-dev/neofs-node/pkg/core/container"
 	"github.com/nspcc-dev/neofs-node/pkg/core/netmap"
 	"github.com/nspcc-dev/neofs-sdk-go/bearer"
 	"github.com/nspcc-dev/neofs-sdk-go/container"
+	"github.com/nspcc-dev/neofs-sdk-go/container/acl"
 	cid "github.com/nspcc-dev/neofs-sdk-go/container/id"
 	cidtest "github.com/nspcc-dev/neofs-sdk-go/container/id/test"
 	eaclSDK "github.com/nspcc-dev/neofs-sdk-go/eacl"
@@ -63,15 +64,17 @@ func TestMessageSign(t *testing.T) {
 	var ownerID user.ID
 	user.IDFromKey(&ownerID, (ecdsa.PublicKey)(*privs[0].PublicKey()))
 
+	cnr := &containercore.Container{
+		Value: testContainer(ownerID),
+	}
+
 	s := &Service{
 		cfg: cfg{
 			log:      zaptest.NewLogger(t),
 			key:      &privs[0].PrivateKey,
 			nmSource: dummyNetmapSource{},
 			cnrSource: dummyContainerSource{
-				cid1.String(): &containercore.Container{
+				cid1.String(): cnr,
-					Value: testContainer(ownerID),
-				},
 			},
 		},
 	}
@@ -90,26 +93,43 @@ func TestMessageSign(t *testing.T) {
 		},
 	}
 
+	op := acl.OpObjectPut
+	cnr.Value.SetBasicACL(acl.PublicRW)
+
 	t.Run("missing signature, no panic", func(t *testing.T) {
-		require.Error(t, s.verifyClient(req, cid2, nil, eaclSDK.OperationUnknown))
+		require.Error(t, s.verifyClient(req, cid2, nil, op))
 	})
 
 	require.NoError(t, signMessage(req, &privs[0].PrivateKey))
-	require.NoError(t, s.verifyClient(req, cid1, nil, eaclSDK.OperationUnknown))
+	require.NoError(t, s.verifyClient(req, cid1, nil, op))
 
 	t.Run("invalid CID", func(t *testing.T) {
-		require.Error(t, s.verifyClient(req, cid2, nil, eaclSDK.OperationUnknown))
+		require.Error(t, s.verifyClient(req, cid2, nil, op))
 	})
+
+	cnr.Value.SetBasicACL(acl.Private)
+
+	t.Run("extension disabled", func(t *testing.T) {
+		require.NoError(t, signMessage(req, &privs[0].PrivateKey))
+		require.Error(t, s.verifyClient(req, cid2, nil, op))
+	})
+
 	t.Run("invalid key", func(t *testing.T) {
 		require.NoError(t, signMessage(req, &privs[1].PrivateKey))
-		require.Error(t, s.verifyClient(req, cid1, nil, eaclSDK.OperationUnknown))
+		require.Error(t, s.verifyClient(req, cid1, nil, op))
 	})
 
 	t.Run("bearer", func(t *testing.T) {
+		bACL := acl.PrivateExtended
+		bACL.AllowBearerRules(op)
+		cnr.Value.SetBasicACL(bACL)
+
+		bACL.DisableExtension()
+
 		t.Run("invalid bearer", func(t *testing.T) {
 			req.Body.BearerToken = []byte{0xFF}
 			require.NoError(t, signMessage(req, &privs[0].PrivateKey))
-			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), eaclSDK.OperationPut))
+			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), acl.OpObjectPut))
 		})
 
 		t.Run("invalid bearer CID", func(t *testing.T) {
@@ -118,7 +138,7 @@ func TestMessageSign(t *testing.T) {
 			req.Body.BearerToken = bt.Marshal()
 
 			require.NoError(t, signMessage(req, &privs[1].PrivateKey))
-			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), eaclSDK.OperationPut))
+			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), acl.OpObjectPut))
 		})
 		t.Run("invalid bearer owner", func(t *testing.T) {
 			bt := testBearerToken(cid1, privs[1].PublicKey(), privs[2].PublicKey())
@@ -126,47 +146,48 @@ func TestMessageSign(t *testing.T) {
 			req.Body.BearerToken = bt.Marshal()
 
 			require.NoError(t, signMessage(req, &privs[1].PrivateKey))
-			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), eaclSDK.OperationPut))
+			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), acl.OpObjectPut))
 		})
 		t.Run("invalid bearer signature", func(t *testing.T) {
 			bt := testBearerToken(cid1, privs[1].PublicKey(), privs[2].PublicKey())
 			require.NoError(t, bt.Sign(privs[0].PrivateKey))
 
-			var bv2 acl.BearerToken
+			var bv2 aclV2.BearerToken
 			bt.WriteToV2(&bv2)
 			bv2.GetSignature().SetSign([]byte{1, 2, 3})
 			req.Body.BearerToken = bv2.StableMarshal(nil)
 
 			require.NoError(t, signMessage(req, &privs[1].PrivateKey))
-			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), eaclSDK.OperationPut))
+			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), acl.OpObjectPut))
 		})
 
 		bt := testBearerToken(cid1, privs[1].PublicKey(), privs[2].PublicKey())
 		require.NoError(t, bt.Sign(privs[0].PrivateKey))
 		req.Body.BearerToken = bt.Marshal()
+		cnr.Value.SetBasicACL(acl.PublicRWExtended)
 
 		t.Run("put and get", func(t *testing.T) {
 			require.NoError(t, signMessage(req, &privs[1].PrivateKey))
-			require.NoError(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), eaclSDK.OperationPut))
+			require.NoError(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), acl.OpObjectPut))
-			require.NoError(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), eaclSDK.OperationGet))
+			require.NoError(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), acl.OpObjectGet))
 		})
 		t.Run("only get", func(t *testing.T) {
 			require.NoError(t, signMessage(req, &privs[2].PrivateKey))
-			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), eaclSDK.OperationPut))
+			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), acl.OpObjectPut))
-			require.NoError(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), eaclSDK.OperationGet))
+			require.NoError(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), acl.OpObjectGet))
 		})
 		t.Run("none", func(t *testing.T) {
 			require.NoError(t, signMessage(req, &privs[3].PrivateKey))
-			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), eaclSDK.OperationPut))
+			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), acl.OpObjectPut))
-			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), eaclSDK.OperationGet))
+			require.Error(t, s.verifyClient(req, cid1, req.GetBody().GetBearerToken(), acl.OpObjectGet))
 		})
 	})
 }
 
-func testBearerToken(cid cid.ID, forPut, forGet *keys.PublicKey) bearer.Token {
+func testBearerToken(cid cid.ID, forPutGet, forGet *keys.PublicKey) bearer.Token {
 	tgtGet := eaclSDK.NewTarget()
 	tgtGet.SetRole(eaclSDK.RoleUnknown)
-	tgtGet.SetBinaryKeys([][]byte{forPut.Bytes(), forGet.Bytes()})
+	tgtGet.SetBinaryKeys([][]byte{forPutGet.Bytes(), forGet.Bytes()})
 
 	rGet := eaclSDK.NewRecord()
 	rGet.SetAction(eaclSDK.ActionAllow)
@@ -175,7 +196,7 @@ func testBearerToken(cid cid.ID, forPut, forGet *keys.PublicKey) bearer.Token {
 
 	tgtPut := eaclSDK.NewTarget()
 	tgtPut.SetRole(eaclSDK.RoleUnknown)
-	tgtPut.SetBinaryKeys([][]byte{forPut.Bytes()})
+	tgtPut.SetBinaryKeys([][]byte{forPutGet.Bytes()})
 
 	rPut := eaclSDK.NewRecord()
 	rPut.SetAction(eaclSDK.ActionAllow)