From 02bb7159a54a9522ad0bc97d1a5456f5cfc425e4 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 9 Oct 2024 10:50:30 +0300 Subject: [PATCH] [#1425] services/tree: Remove eACL processing Signed-off-by: Evgenii Stratonikov --- cmd/frostfs-node/tree.go | 1 - pkg/services/tree/options.go | 9 -- pkg/services/tree/signature.go | 137 +------------------------ pkg/services/tree/signature_test.go | 151 ++++++++++++++++++---------- 4 files changed, 100 insertions(+), 198 deletions(-) diff --git a/cmd/frostfs-node/tree.go b/cmd/frostfs-node/tree.go index d22e510de..192f08471 100644 --- a/cmd/frostfs-node/tree.go +++ b/cmd/frostfs-node/tree.go @@ -54,7 +54,6 @@ func initTreeService(c *cfg) { cli: c.shared.cnrClient, }), tree.WithFrostfsidSubjectProvider(c.shared.frostfsidClient), - tree.WithEACLSource(c.cfgObject.eaclSource), tree.WithNetmapSource(c.netMapSource), tree.WithPrivateKey(&c.key.PrivateKey), tree.WithLogger(c.log), diff --git a/pkg/services/tree/options.go b/pkg/services/tree/options.go index 6a20fe5cc..1db5607f6 100644 --- a/pkg/services/tree/options.go +++ b/pkg/services/tree/options.go @@ -33,7 +33,6 @@ type cfg struct { nmSource netmap.Source cnrSource ContainerSource frostfsidSubjectProvider frostfsidcore.SubjectProvider - eaclSource container.EACLSource forest pilorama.Forest // replication-related parameters replicatorChannelCapacity int @@ -65,14 +64,6 @@ func WithFrostfsidSubjectProvider(provider frostfsidcore.SubjectProvider) 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 { diff --git a/pkg/services/tree/signature.go b/pkg/services/tree/signature.go index 58cab659f..305adf2d7 100644 --- a/pkg/services/tree/signature.go +++ b/pkg/services/tree/signature.go @@ -9,10 +9,8 @@ import ( "fmt" "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/refs" - "git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs" core "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer" - "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl" cidSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" frostfscrypto "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/crypto" @@ -20,7 +18,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/eacl" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" - "go.uber.org/zap" ) type message interface { @@ -30,16 +27,11 @@ type message interface { SetSignature(*Signature) } -func basicACLErr(op acl.Op) error { - return fmt.Errorf("access to operation %s is denied by basic ACL check", op) -} - func eACLErr(op eacl.Operation, err error) error { return fmt.Errorf("access to operation %s is denied by extended ACL check: %w", op, err) } var ( - errBearerWrongOwner = errors.New("bearer token must be signed by the container owner") errBearerWrongContainer = errors.New("bearer token is created for another container") errBearerSignature = errors.New("invalid bearer token signature") ) @@ -77,56 +69,7 @@ func (s *Service) verifyClient(ctx context.Context, req message, cid cidSDK.ID, return fmt.Errorf("can't get request role: %w", err) } - basicACL := cnr.Value.BasicACL() - // Basic ACL mask can be unset, if a container operations are performed - // with strict APE checks only. - // - // FIXME(@aarifullin): tree service temporiraly performs APE checks on - // object verbs, because tree verbs have not been introduced yet. - if basicACL == 0x0 { - return s.checkAPE(ctx, bt, cnr, cid, op, role, pubKey) - } - - if !basicACL.IsOpAllowed(op, role) { - return basicACLErr(op) - } - - if !basicACL.Extendable() { - return nil - } - - var useBearer bool - if len(rawBearer) != 0 { - if !basicACL.AllowedBearerRules(op) { - s.log.Debug(logs.TreeBearerPresentedButNotAllowedByACL, - zap.String("cid", cid.EncodeToString()), - zap.Stringer("op", op), - ) - } else { - useBearer = true - } - } - - var tb eacl.Table - signer := req.GetSignature().GetKey() - if useBearer && !bt.Impersonate() { - if !bearer.ResolveIssuer(*bt).Equals(cnr.Value.Owner()) { - return eACLErr(eaclOp, errBearerWrongOwner) - } - tb = bt.EACLTable() - } else { - tbCore, err := s.eaclSource.GetEACL(cid) - if err != nil { - return handleGetEACLError(err) - } - tb = *tbCore.Value - - if useBearer && bt.Impersonate() { - signer = bt.SigningKeyBytes() - } - } - - return checkEACL(tb, signer, eACLRole(role), eaclOp) + return s.checkAPE(ctx, bt, cnr, cid, op, role, pubKey) } // Returns true iff the operation is read-only and request was signed @@ -168,14 +111,6 @@ func parseBearer(rawBearer []byte, cid cidSDK.ID, eaclOp eacl.Operation) (*beare return bt, nil } -func handleGetEACLError(err error) error { - if client.IsErrEACLNotFound(err) { - return nil - } - - return fmt.Errorf("get eACL table: %w", err) -} - func verifyMessage(m message) error { binBody, err := m.ReadSignedData(nil) if err != nil { @@ -260,73 +195,3 @@ func eACLOp(op acl.Op) eacl.Operation { 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)) - } -} - -var ( - errDENY = errors.New("DENY eACL rule") - errNoAllowRules = errors.New("not found allowing rules for the request") -) - -// checkEACL searches for the eACL rules that could be applied to the request -// (a tuple of a signer key, his FrostFS role and a request operation). -// It does not filter the request by the filters of the eACL table since tree -// requests do not contain any "object" information that could be filtered and, -// therefore, filtering leads to unexpected results. -// The code was copied with the minor updates from the SDK repo: -// https://github.com/nspcc-dev/frostfs-sdk-go/blob/43a57d42dd50dc60465bfd3482f7f12bcfcf3411/eacl/validator.go#L28. -func checkEACL(tb eacl.Table, signer []byte, role eacl.Role, op eacl.Operation) error { - for _, record := range tb.Records() { - // check type of operation - if record.Operation() != op { - continue - } - - // check target - if !targetMatches(record, role, signer) { - continue - } - - switch a := record.Action(); a { - case eacl.ActionAllow: - return nil - case eacl.ActionDeny: - return eACLErr(op, errDENY) - default: - return eACLErr(op, fmt.Errorf("unexpected action: %s", a)) - } - } - - return eACLErr(op, errNoAllowRules) -} - -func targetMatches(rec eacl.Record, role eacl.Role, signer []byte) bool { - for _, target := range rec.Targets() { - // check public key match - if pubs := target.BinaryKeys(); len(pubs) != 0 { - for _, key := range pubs { - if bytes.Equal(key, signer) { - return true - } - } - - continue - } - - // check target group match - if role == target.Role() { - return true - } - } - - return false -} diff --git a/pkg/services/tree/signature_test.go b/pkg/services/tree/signature_test.go index 3c3ebfe89..939ff170d 100644 --- a/pkg/services/tree/signature_test.go +++ b/pkg/services/tree/signature_test.go @@ -4,22 +4,30 @@ import ( "context" "crypto/ecdsa" "crypto/sha256" + "encoding/hex" "errors" "testing" aclV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/acl" + "git.frostfs.info/TrueCloudLab/frostfs-contract/frostfsid/client" containercore "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/netmap" + checkercore "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/common/ape" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger/test" + "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/ape" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test" - eaclSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/eacl" netmapSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" + "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain" + "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine" + "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine/inmemory" + "git.frostfs.info/TrueCloudLab/policy-engine/schema/native" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" + "github.com/nspcc-dev/neo-go/pkg/util" "github.com/stretchr/testify/require" ) @@ -27,6 +35,34 @@ type dummyNetmapSource struct { netmap.Source } +type dummySubjectProvider struct { + subjects map[util.Uint160]client.SubjectExtended +} + +func (s dummySubjectProvider) GetSubject(addr util.Uint160) (*client.Subject, error) { + res := s.subjects[addr] + return &client.Subject{ + PrimaryKey: res.PrimaryKey, + AdditionalKeys: res.AdditionalKeys, + Namespace: res.Namespace, + Name: res.Name, + KV: res.KV, + }, nil +} + +func (s dummySubjectProvider) GetSubjectExtended(addr util.Uint160) (*client.SubjectExtended, error) { + res := s.subjects[addr] + return &res, nil +} + +type dummyEpochSource struct { + epoch uint64 +} + +func (s dummyEpochSource) CurrentEpoch() uint64 { + return s.epoch +} + type dummyContainerSource map[string]*containercore.Container func (s dummyContainerSource) List() ([]cid.ID, error) { @@ -57,16 +93,6 @@ func (s dummyContainerSource) DeletionInfo(id cid.ID) (*containercore.DelInfo, e return &containercore.DelInfo{}, nil } -type dummyEACLSource map[string]*containercore.EACL - -func (s dummyEACLSource) GetEACL(id cid.ID) (*containercore.EACL, error) { - cntEACL, ok := s[id.String()] - if !ok { - return nil, errors.New("container not found") - } - return cntEACL, nil -} - func testContainer(owner user.ID) container.Container { var r netmapSDK.ReplicaDescriptor r.SetNumberOfObjects(1) @@ -81,6 +107,8 @@ func testContainer(owner user.ID) container.Container { return cnt } +const currentEpoch = 123 + func TestMessageSign(t *testing.T) { privs := make([]*keys.PrivateKey, 4) for i := range privs { @@ -99,6 +127,15 @@ func TestMessageSign(t *testing.T) { Value: testContainer(ownerID), } + e := inmemory.NewInMemoryLocalOverrides() + e.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, engine.Target{ + Type: engine.Container, + Name: cid1.EncodeToString(), + }, testChain(privs[0].PublicKey(), privs[1].PublicKey())) + frostfsidProvider := dummySubjectProvider{ + subjects: make(map[util.Uint160]client.SubjectExtended), + } + s := &Service{ cfg: cfg{ log: test.NewLogger(t), @@ -107,12 +144,10 @@ func TestMessageSign(t *testing.T) { cnrSource: dummyContainerSource{ cid1.String(): cnr, }, - eaclSource: dummyEACLSource{ - cid1.String(): &containercore.EACL{ - Value: testTable(cid1, privs[0].PublicKey(), privs[1].PublicKey()), - }, - }, + frostfsidSubjectProvider: frostfsidProvider, + state: dummyEpochSource{epoch: currentEpoch}, }, + apeChecker: checkercore.New(e.LocalStorage(), e.MorphRuleChainStorage(), frostfsidProvider, dummyEpochSource{}), } rawCID1 := make([]byte, sha256.Size) @@ -235,46 +270,58 @@ func TestMessageSign(t *testing.T) { func testBearerToken(cid cid.ID, forPutGet, forGet *keys.PublicKey) bearer.Token { var b bearer.Token - b.SetEACLTable(*testTable(cid, forPutGet, forGet)) + b.SetExp(currentEpoch + 1) + b.SetAPEOverride(bearer.APEOverride{ + Target: ape.ChainTarget{ + TargetType: ape.TargetTypeContainer, + Name: cid.EncodeToString(), + }, + Chains: []ape.Chain{{Raw: testChain(forPutGet, forGet).Bytes()}}, + }) return b } -func testTable(cid cid.ID, forPutGet, forGet *keys.PublicKey) *eaclSDK.Table { - tgtGet := eaclSDK.NewTarget() - tgtGet.SetRole(eaclSDK.RoleUnknown) - tgtGet.SetBinaryKeys([][]byte{forPutGet.Bytes(), forGet.Bytes()}) - - rGet := eaclSDK.NewRecord() - rGet.SetAction(eaclSDK.ActionAllow) - rGet.SetOperation(eaclSDK.OperationGet) - rGet.SetTargets(*tgtGet) - - tgtPut := eaclSDK.NewTarget() - tgtPut.SetRole(eaclSDK.RoleUnknown) - tgtPut.SetBinaryKeys([][]byte{forPutGet.Bytes()}) - - rPut := eaclSDK.NewRecord() - rPut.SetAction(eaclSDK.ActionAllow) - rPut.SetOperation(eaclSDK.OperationPut) - rPut.SetTargets(*tgtPut) - - tb := eaclSDK.NewTable() - tb.AddRecord(rGet) - tb.AddRecord(rPut) - - tgt := eaclSDK.NewTarget() - tgt.SetRole(eaclSDK.RoleOthers) - - for _, op := range []eaclSDK.Operation{eaclSDK.OperationGet, eaclSDK.OperationPut} { - r := eaclSDK.NewRecord() - r.SetAction(eaclSDK.ActionDeny) - r.SetTargets(*tgt) - r.SetOperation(op) - tb.AddRecord(r) +func testChain(forPutGet, forGet *keys.PublicKey) *chain.Chain { + ruleGet := chain.Rule{ + Status: chain.Allow, + Resources: chain.Resources{Names: []string{native.ResourceFormatAllObjects}}, + Actions: chain.Actions{Names: []string{native.MethodGetObject}}, + Any: true, + Condition: []chain.Condition{ + { + Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: native.PropertyKeyActorPublicKey, + Value: hex.EncodeToString(forPutGet.Bytes()), + }, + { + Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: native.PropertyKeyActorPublicKey, + Value: hex.EncodeToString(forGet.Bytes()), + }, + }, + } + rulePut := chain.Rule{ + Status: chain.Allow, + Resources: chain.Resources{Names: []string{native.ResourceFormatAllObjects}}, + Actions: chain.Actions{Names: []string{native.MethodPutObject}}, + Any: true, + Condition: []chain.Condition{ + { + Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: native.PropertyKeyActorPublicKey, + Value: hex.EncodeToString(forPutGet.Bytes()), + }, + }, } - tb.SetCID(cid) - - return tb + return &chain.Chain{ + Rules: []chain.Rule{ + ruleGet, + rulePut, + }, + } }