From 483a67b17064990d69e39e23d7b3fd6c72d9bfb3 Mon Sep 17 00:00:00 2001 From: Anton Nikiforov Date: Tue, 30 Jan 2024 17:07:05 +0300 Subject: [PATCH] [#937] ape: Validate chain resource name Signed-off-by: Anton Nikiforov --- .../internal/modules/morph/frostfsid_util.go | 22 +-- .../modules/morph/frostfsid_util_test.go | 7 +- internal/ape/util.go | 11 ++ pkg/services/control/server/ape/validate.go | 97 +++++++++++++ .../control/server/ape/validate_test.go | 132 ++++++++++++++++++ pkg/services/control/server/policy_engine.go | 8 ++ 6 files changed, 259 insertions(+), 18 deletions(-) create mode 100644 internal/ape/util.go create mode 100644 pkg/services/control/server/ape/validate.go create mode 100644 pkg/services/control/server/ape/validate_test.go diff --git a/cmd/frostfs-adm/internal/modules/morph/frostfsid_util.go b/cmd/frostfs-adm/internal/modules/morph/frostfsid_util.go index f88e4edfb..6b6bdffa9 100644 --- a/cmd/frostfs-adm/internal/modules/morph/frostfsid_util.go +++ b/cmd/frostfs-adm/internal/modules/morph/frostfsid_util.go @@ -3,9 +3,9 @@ package morph import ( "errors" "fmt" - "regexp" commonCmd "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/internal/common" + "git.frostfs.info/TrueCloudLab/frostfs-node/internal/ape" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/util" @@ -13,14 +13,6 @@ import ( "github.com/spf13/viper" ) -var ( - frostfsidSubjectNameRegexp = regexp.MustCompile(`^[\w+=,.@-]{1,64}$`) - frostfsidGroupNameRegexp = regexp.MustCompile(`^[\w+=,.@-]{1,128}$`) - - // frostfsidNamespaceNameRegexp similar to https://git.frostfs.info/TrueCloudLab/frostfs-contract/src/commit/f2a82aa635aa57d9b05092d8cf15b170b53cc324/nns/nns_contract.go#L690 - frostfsidNamespaceNameRegexp = regexp.MustCompile(`(^$)|(^[a-z0-9]{1,2}$)|(^[a-z0-9][a-z0-9-]{1,48}[a-z0-9]$)`) -) - func getFrostfsIDAdmin(v *viper.Viper) (util.Uint160, bool, error) { admin := v.GetString(frostfsIDAdminConfigKey) if admin == "" { @@ -65,9 +57,9 @@ func getFrostfsIDSubjectName(cmd *cobra.Command) string { return "" } - if !frostfsidSubjectNameRegexp.MatchString(subjectName) { + if !ape.SubjectNameRegexp.MatchString(subjectName) { commonCmd.ExitOnErr(cmd, "invalid subject name: %w", - fmt.Errorf("name must match regexp: %s", frostfsidSubjectNameRegexp.String())) + fmt.Errorf("name must match regexp: %s", ape.SubjectNameRegexp.String())) } return subjectName @@ -76,9 +68,9 @@ func getFrostfsIDSubjectName(cmd *cobra.Command) string { func getFrostfsIDGroupName(cmd *cobra.Command) string { groupName, _ := cmd.Flags().GetString(groupNameFlag) - if !frostfsidGroupNameRegexp.MatchString(groupName) { + if !ape.GroupNameRegexp.MatchString(groupName) { commonCmd.ExitOnErr(cmd, "invalid group name: %w", - fmt.Errorf("name must match regexp: %s", frostfsidGroupNameRegexp.String())) + fmt.Errorf("name must match regexp: %s", ape.GroupNameRegexp.String())) } return groupName @@ -100,9 +92,9 @@ func getFrostfsIDNamespace(cmd *cobra.Command) string { ns = "" } - if !frostfsidNamespaceNameRegexp.MatchString(ns) { + if !ape.NamespaceNameRegexp.MatchString(ns) { commonCmd.ExitOnErr(cmd, "invalid namespace: %w", - fmt.Errorf("name must match regexp: %s", frostfsidNamespaceNameRegexp.String())) + fmt.Errorf("name must match regexp: %s", ape.NamespaceNameRegexp.String())) } return ns diff --git a/cmd/frostfs-adm/internal/modules/morph/frostfsid_util_test.go b/cmd/frostfs-adm/internal/modules/morph/frostfsid_util_test.go index f73ebad08..79a0845f9 100644 --- a/cmd/frostfs-adm/internal/modules/morph/frostfsid_util_test.go +++ b/cmd/frostfs-adm/internal/modules/morph/frostfsid_util_test.go @@ -4,6 +4,7 @@ import ( "encoding/hex" "testing" + "git.frostfs.info/TrueCloudLab/frostfs-node/internal/ape" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/spf13/viper" @@ -95,7 +96,7 @@ func TestNamespaceRegexp(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - require.Equal(t, tc.matched, frostfsidNamespaceNameRegexp.MatchString(tc.namespace)) + require.Equal(t, tc.matched, ape.NamespaceNameRegexp.MatchString(tc.namespace)) }) } } @@ -128,7 +129,7 @@ func TestSubjectNameRegexp(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - require.Equal(t, tc.matched, frostfsidSubjectNameRegexp.MatchString(tc.subject)) + require.Equal(t, tc.matched, ape.SubjectNameRegexp.MatchString(tc.subject)) }) } } @@ -166,7 +167,7 @@ func TestSubjectGroupRegexp(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - require.Equal(t, tc.matched, frostfsidGroupNameRegexp.MatchString(tc.subject)) + require.Equal(t, tc.matched, ape.GroupNameRegexp.MatchString(tc.subject)) }) } } diff --git a/internal/ape/util.go b/internal/ape/util.go new file mode 100644 index 000000000..99eba95ba --- /dev/null +++ b/internal/ape/util.go @@ -0,0 +1,11 @@ +package ape + +import "regexp" + +var ( + SubjectNameRegexp = regexp.MustCompile(`^[\w+=,.@-]{1,64}$`) + GroupNameRegexp = regexp.MustCompile(`^[\w+=,.@-]{1,128}$`) + + // NamespaceNameRegexp similar to https://git.frostfs.info/TrueCloudLab/frostfs-contract/src/commit/f2a82aa635aa57d9b05092d8cf15b170b53cc324/nns/nns_contract.go#L690 + NamespaceNameRegexp = regexp.MustCompile(`(^$)|(^[a-z0-9]{1,2}$)|(^[a-z0-9][a-z0-9-]{1,48}[a-z0-9]$)`) +) diff --git a/pkg/services/control/server/ape/validate.go b/pkg/services/control/server/ape/validate.go new file mode 100644 index 000000000..f4aa0399f --- /dev/null +++ b/pkg/services/control/server/ape/validate.go @@ -0,0 +1,97 @@ +package ape + +import ( + "errors" + "fmt" + "strings" + + "git.frostfs.info/TrueCloudLab/frostfs-node/internal/ape" + cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" + oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" + "git.frostfs.info/TrueCloudLab/policy-engine/schema/native" +) + +var ( + ErrInvalidResource = errors.New("invalid resource name") + ErrUnsupportedPrefix = errors.New("unsupported resource name prefix") + ErrInvalidContainerID = errors.New("invalid container id") + ErrInvalidObjectID = errors.New("invalid object id") + ErrInvalidNamespace = fmt.Errorf("namespace must match regexp: %s", ape.NamespaceNameRegexp.String()) +) + +// ValidateResourceName validates resource name components - container and object id, namespace. +// Also validates matching resource name to templates of policy engine's native scheme. +func ValidateResourceName(name string) error { + if after, found := strings.CutPrefix(name, native.ObjectPrefix+"/"); found { + return validateObjectResourceName(after) + } else if after, found = strings.CutPrefix(name, native.ContainerPrefix+"/"); found { + return validateContainerResourceName(after) + } + return ErrUnsupportedPrefix +} + +// validateObjectResourceName validate name for object. +// Name should be without prefix `native.ObjectPrefix`. +func validateObjectResourceName(name string) error { + if name == "*" { + return nil + } + lexems := strings.Split(name, "/") + if len(lexems) == 1 && lexems[0] == "*" { + return nil + } else if len(lexems) == 2 { + // len == 2 means format `namespace(root_namespace)/*` + if lexems[0] != "" && !ape.NamespaceNameRegexp.MatchString(lexems[0]) { + return ErrInvalidNamespace + } + if lexems[1] == "*" { + return nil + } + } else if len(lexems) == 3 { + // len == 3 means format `namespace(root_namespace)/CID/OID(*)` + if lexems[0] != "" && !ape.NamespaceNameRegexp.MatchString(lexems[0]) { + return ErrInvalidNamespace + } + var cnr cid.ID + err := cnr.DecodeString(lexems[1]) + if err != nil { + return fmt.Errorf("%w: %w", ErrInvalidContainerID, err) + } + if lexems[2] == "*" { + return nil + } + var objID oid.ID + err = objID.DecodeString(lexems[2]) + if err != nil { + return fmt.Errorf("%w: %w", ErrInvalidObjectID, err) + } + return nil + } + return ErrInvalidResource +} + +// validateContainerResourceName validate resource name for container. +// Name should be without prefix `native.ContainerPrefix`. +func validateContainerResourceName(name string) error { + if name == "*" { + return nil + } + lexems := strings.Split(name, "/") + if len(lexems) == 1 && lexems[0] == "*" { + return nil + } else if len(lexems) == 2 { + // len == 2 means format `namespace(root_namespace)/CID(*)` + if lexems[0] != "" && !ape.NamespaceNameRegexp.MatchString(lexems[0]) { + return ErrInvalidNamespace + } + if lexems[1] != "*" { + var cnr cid.ID + err := cnr.DecodeString(lexems[1]) + if err != nil { + return fmt.Errorf("%w: %w", ErrInvalidContainerID, err) + } + } + return nil + } + return ErrInvalidResource +} diff --git a/pkg/services/control/server/ape/validate_test.go b/pkg/services/control/server/ape/validate_test.go new file mode 100644 index 000000000..af811efed --- /dev/null +++ b/pkg/services/control/server/ape/validate_test.go @@ -0,0 +1,132 @@ +package ape + +import ( + "testing" + + "git.frostfs.info/TrueCloudLab/policy-engine/schema/native" + "github.com/stretchr/testify/require" +) + +func TestValidationOfChainResources(t *testing.T) { + tests := [...]struct { + testName string + resourceName string + expectErr error + }{ + { + testName: "native object: all objects", + resourceName: native.ObjectPrefix + "/*", + }, + { + testName: "native object: all objects in namespace", + resourceName: native.ObjectPrefix + "/ns/*", + }, + { + testName: "native object: all objects in root namespace", + resourceName: native.ObjectPrefix + "//*", + }, + { + testName: "native object: all objects in namespace/container", + resourceName: native.ObjectPrefix + "/ns/SeHNpifDH2Fc4scNBphrbmrKi96QXj2HzYJkhSGuytH/*", + }, + { + testName: "native object: all objects in root namespace/container", + resourceName: native.ObjectPrefix + "//SeHNpifDH2Fc4scNBphrbmrKi96QXj2HzYJkhSGuytH/*", + }, + { + testName: "native object: object in namespace/container", + resourceName: native.ObjectPrefix + "/ns/SeHNpifDH2Fc4scNBphrbmrKi96QXj2HzYJkhSGuytH/BCGsUu6o92oG1UALVox1sV6YbBUKUL2xSCtAFkrsuvWY", + }, + { + testName: "native object: object in root namespace/container", + resourceName: native.ObjectPrefix + "//SeHNpifDH2Fc4scNBphrbmrKi96QXj2HzYJkhSGuytH/BCGsUu6o92oG1UALVox1sV6YbBUKUL2xSCtAFkrsuvWY", + }, + { + testName: "native object: invalid all objects", + resourceName: native.ObjectPrefix + "/*12313", + expectErr: ErrInvalidResource, + }, + { + testName: "native object: all objects in invalid namespace", + resourceName: native.ObjectPrefix + "/qwe_123123/*", + expectErr: ErrInvalidNamespace, + }, + { + testName: "native object: invalid all objects in root namespace", + resourceName: native.ObjectPrefix + "//qwe", + expectErr: ErrInvalidResource, + }, + { + testName: "native object: invalid cid in all objects in root namespace", + resourceName: native.ObjectPrefix + "//SeHNpifDH2Fc4scNBphrbmrKi96QXj2HzYJkhSGuytHqwe/*", + expectErr: ErrInvalidContainerID, + }, + { + testName: "native object: invalid cid in all objects in namespace", + resourceName: native.ObjectPrefix + "/ns/SeHNpifDH2Fc4scNBphrbmrKi96QXj2HzYJkhSGuytHqwe/*", + expectErr: ErrInvalidContainerID, + }, + { + testName: "native object: invalid object in namespace/container", + resourceName: native.ObjectPrefix + "/ns/SeHNpifDH2Fc4scNBphrbmrKi96QXj2HzYJkhSGuytH/BCGsUu6o92oG1UALVox1sV6YbBUKUL2xSCtAFkrsuvWY111", + expectErr: ErrInvalidObjectID, + }, + { + testName: "native object: invalid resource", + resourceName: native.ObjectPrefix + "/ns/SeHNpifD/AFkrsuvWY111/AFkrsuvWY222", + expectErr: ErrInvalidResource, + }, + { + testName: "native container: all containers", + resourceName: native.ContainerPrefix + "/*", + }, + { + testName: "native container: all containers in namespace", + resourceName: native.ContainerPrefix + "/ns/*", + }, + { + testName: "native container: all containers in root namespace", + resourceName: native.ContainerPrefix + "//*", + }, + { + testName: "native container: container in namespace", + resourceName: native.ContainerPrefix + "/ns/SeHNpifDH2Fc4scNBphrbmrKi96QXj2HzYJkhSGuytH", + }, + { + testName: "native container: container in root namespace", + resourceName: native.ContainerPrefix + "//SeHNpifDH2Fc4scNBphrbmrKi96QXj2HzYJkhSGuytH", + }, + { + testName: "native container: invalid all containers", + resourceName: native.ContainerPrefix + "/*asd", + expectErr: ErrInvalidResource, + }, + { + testName: "native container: invalid resource", + resourceName: native.ContainerPrefix + "/ns/cid/cid", + expectErr: ErrInvalidResource, + }, + { + testName: "native container: invalid container in root namespace", + resourceName: native.ContainerPrefix + "//*asd", + expectErr: ErrInvalidContainerID, + }, + { + testName: "native container: container in invalid namespace", + resourceName: native.ContainerPrefix + "/ns_111/SeHNpifDH2Fc4scNBphrbmrKi96QXj2HzYJkhSGuytH", + expectErr: ErrInvalidNamespace, + }, + { + testName: "unsupported prefix", + resourceName: "native:test/ns_111/SeHNpifDH2Fc4scNBphrbmrKi96QXj2HzYJkhSGuytH", + expectErr: ErrUnsupportedPrefix, + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + err := ValidateResourceName(test.resourceName) + require.ErrorIs(t, err, test.expectErr) + }) + } +} diff --git a/pkg/services/control/server/policy_engine.go b/pkg/services/control/server/policy_engine.go index 9b42f3a98..bc85c4356 100644 --- a/pkg/services/control/server/policy_engine.go +++ b/pkg/services/control/server/policy_engine.go @@ -6,6 +6,7 @@ import ( "fmt" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control/server/ape" apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain" "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine" "google.golang.org/grpc/codes" @@ -38,6 +39,13 @@ func (s *Server) AddChainLocalOverride(_ context.Context, req *control.AddChainL if err := chain.DecodeBytes(req.GetBody().GetChain()); err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } + for _, rule := range chain.Rules { + for _, name := range rule.Resources.Names { + if err := ape.ValidateResourceName(name); err != nil { + return nil, status.Error(codes.InvalidArgument, fmt.Errorf("invalid resource: %w", err).Error()) + } + } + } s.apeChainCounter.Add(1) // TODO (aarifullin): the such chain id is not well-designed yet.