diff --git a/policy/resource.go b/policy/resource.go index 3b8fc6c..bd3d8d6 100644 --- a/policy/resource.go +++ b/policy/resource.go @@ -26,12 +26,60 @@ import ( "github.com/minio/pkg/v3/wildcard" ) -// ResourceARNPrefix - resource ARN prefix as per AWS S3 specification. -const ResourceARNPrefix = "arn:aws:s3:::" +const ( + // ResourceARNPrefix - resource S3 ARN prefix as per S3 specification. + ResourceARNPrefix = "arn:aws:s3:::" + + // ResourceARNKMSPrefix is for KMS key resources. MinIO specific API. + ResourceARNKMSPrefix = "arn:minio:kms:::" +) + +// ResourceARNType - ARN prefix type +type ResourceARNType uint32 + +const ( + // Zero value for detecting errors + unknownARN ResourceARNType = iota + + // ResourceARNS3 is the ARN prefix type for S3 resources. + ResourceARNS3 + + // ResourceARNKMS is the ARN prefix type for MinIO KMS resources. + ResourceARNKMS +) + +// ARNTypeToPrefix maps the type to prefix string +var ARNTypeToPrefix = map[ResourceARNType]string{ + ResourceARNS3: ResourceARNPrefix, + ResourceARNKMS: ResourceARNKMSPrefix, +} + +// ARNPrefixToType maps prefix to types. +var ARNPrefixToType map[string]ResourceARNType + +func init() { + ARNPrefixToType = make(map[string]ResourceARNType) + for k, v := range ARNTypeToPrefix { + ARNPrefixToType[v] = k + } +} + +func (a ResourceARNType) String() string { + return ARNTypeToPrefix[a] +} // Resource - resource in policy statement. type Resource struct { Pattern string + Type ResourceARNType +} + +func (r Resource) isKMS() bool { + return r.Type == ResourceARNKMS +} + +func (r Resource) isS3() bool { + return r.Type == ResourceARNS3 } func (r Resource) isBucketPattern() bool { @@ -44,9 +92,21 @@ func (r Resource) isObjectPattern() bool { // IsValid - checks whether Resource is valid or not. func (r Resource) IsValid() bool { - if strings.HasPrefix(r.Pattern, "/") { + if r.Type == unknownARN { return false } + if r.isS3() { + if strings.HasPrefix(r.Pattern, "/") { + return false + } + } + if r.isKMS() { + if strings.IndexFunc(r.Pattern, func(c rune) bool { + return c == '/' || c == '\\' || c == '.' + }) >= 0 { + return false + } + } return r.Pattern != "" } @@ -83,7 +143,7 @@ func (r Resource) MarshalJSON() ([]byte, error) { } func (r Resource) String() string { - return ResourceARNPrefix + r.Pattern + return r.Type.String() + r.Pattern } // UnmarshalJSON - decodes JSON data to Resource. @@ -136,23 +196,37 @@ func (r Resource) ValidateBucket(bucketName string) error { // parseResource - parses string to Resource. func parseResource(s string) (Resource, error) { - if !strings.HasPrefix(s, ResourceARNPrefix) { - return Resource{}, Errorf("invalid resource '%v'", s) + r := Resource{} + for k, v := range ARNPrefixToType { + if rem, ok := strings.CutPrefix(s, k); ok { + r.Type = v + r.Pattern = rem + break + } + } + if r.Type == unknownARN { + return r, Errorf("invalid resource '%v'", s) } - pattern := strings.TrimPrefix(s, ResourceARNPrefix) - if strings.HasPrefix(pattern, "/") { - return Resource{}, Errorf("invalid resource '%v' - starts with '/' will not match a bucket", s) + if strings.HasPrefix(r.Pattern, "/") { + return r, Errorf("invalid resource '%v' - starts with '/' will not match a bucket", s) } + return r, nil +} + +// NewResource - creates new resource with the default ARN type of S3. +func NewResource(pattern string) Resource { return Resource{ Pattern: pattern, - }, nil + Type: ResourceARNS3, + } } -// NewResource - creates new resource. -func NewResource(pattern string) Resource { +// NewKMSResource - creates new resource with type KMS +func NewKMSResource(pattern string) Resource { return Resource{ Pattern: pattern, + Type: ResourceARNKMS, } } diff --git a/policy/resource_test.go b/policy/resource_test.go index 874da6a..9d3118b 100644 --- a/policy/resource_test.go +++ b/policy/resource_test.go @@ -36,6 +36,11 @@ func TestResourceIsBucketPattern(t *testing.T) { {NewResource("mybucket/*"), false}, {NewResource("mybucket*/myobject"), false}, {NewResource("mybucket?0/2010/photos/*"), false}, + + {NewKMSResource("*"), true}, + {NewKMSResource("mykey"), true}, + {NewKMSResource("mykey*"), true}, + {NewKMSResource("mykey?0"), true}, } for i, testCase := range testCases { @@ -86,6 +91,15 @@ func TestResourceIsValid(t *testing.T) { {NewResource("mybucket?0"), true}, {NewResource("/*"), false}, {NewResource(""), false}, + + {NewKMSResource("*"), true}, + {NewKMSResource("mykey*"), true}, + {NewKMSResource("*/*"), false}, + {NewKMSResource("mykey/*"), false}, + {NewKMSResource("mykey/"), false}, + {NewKMSResource("./mykey"), false}, + {NewKMSResource("../../mykey"), false}, + {NewKMSResource(""), false}, } for i, testCase := range testCases { diff --git a/policy/resourceset.go b/policy/resourceset.go index 0e6faa0..644ea0c 100644 --- a/policy/resourceset.go +++ b/policy/resourceset.go @@ -150,9 +150,26 @@ func (resourceSet *ResourceSet) UnmarshalJSON(data []byte) error { return nil } -// Validate - validates ResourceSet. -func (resourceSet ResourceSet) Validate() error { +// ValidateS3 - validates ResourceSet is S3. +func (resourceSet ResourceSet) ValidateS3() error { for resource := range resourceSet { + if !resource.isS3() { + return Errorf("resource '%v' type is not S3", resource) + } + if err := resource.Validate(); err != nil { + return err + } + } + + return nil +} + +// ValidateKMS - validates ResourceSet is KMS. +func (resourceSet ResourceSet) ValidateKMS() error { + for resource := range resourceSet { + if !resource.isKMS() { + return Errorf("resource '%v' type is not KMS", resource) + } if err := resource.Validate(); err != nil { return err } diff --git a/policy/resourceset_test.go b/policy/resourceset_test.go index 58a0dab..5349473 100644 --- a/policy/resourceset_test.go +++ b/policy/resourceset_test.go @@ -232,17 +232,39 @@ func TestResourceSetUnmarshalJSON(t *testing.T) { } } -func TestResourceSetValidate(t *testing.T) { +func TestResourceSetS3Validate(t *testing.T) { testCases := []struct { resourceSet ResourceSet expectErr bool }{ {NewResourceSet(NewResource("mybucket/myobject*")), false}, {NewResourceSet(NewResource("/")), true}, + {NewResourceSet(NewResource("mybucket"), NewKMSResource("mykey")), true}, // mismatching types } for i, testCase := range testCases { - err := testCase.resourceSet.Validate() + err := testCase.resourceSet.ValidateS3() + expectErr := (err != nil) + + if expectErr != testCase.expectErr { + t.Fatalf("case %v: error: expected: %v, got: %v", i+1, testCase.expectErr, expectErr) + } + } +} + +func TestResourceSetKMSValidate(t *testing.T) { + testCases := []struct { + resourceSet ResourceSet + expectErr bool + }{ + {NewResourceSet(NewKMSResource("mykey/invalid")), true}, + {NewResourceSet(NewKMSResource("/")), true}, + {NewResourceSet(NewKMSResource("mykey")), false}, + {NewResourceSet(NewKMSResource("mykey"), NewResource("mybucket")), true}, // mismatching types + } + + for i, testCase := range testCases { + err := testCase.resourceSet.ValidateKMS() expectErr := (err != nil) if expectErr != testCase.expectErr { diff --git a/policy/statement.go b/policy/statement.go index 22958a4..b594ac5 100644 --- a/policy/statement.go +++ b/policy/statement.go @@ -52,8 +52,20 @@ func (statement Statement) IsAllowed(args Args) bool { resource += "/" } - // For admin statements, resource match can be ignored. - if !statement.Resources.Match(resource, args.ConditionValues) && !statement.isAdmin() && !statement.isKMS() && !statement.isSTS() { + if statement.isKMS() { + if resource == "/" || len(statement.Resources) == 0 { + // In previous MinIO versions, KMS statements ignored Resources, so if len(statement.Resources) == 0, + // allow backward compatibility by not trying to Match. + + // When resource is "/", this allows evaluating KMS statements while explicitly excluding Resource, + // by passing Args with empty BucketName and ObjectName. This is useful when doing a + // two-phase authorization of a request. + return statement.Conditions.Evaluate(args.ConditionValues) + } + } + + // For some admin statements, resource match can be ignored. + if !statement.Resources.Match(resource, args.ConditionValues) && !statement.isAdmin() && !statement.isSTS() { return false } @@ -129,7 +141,10 @@ func (statement Statement) isValid() error { } if statement.isKMS() { - return statement.Actions.ValidateKMS() + if err := statement.Actions.ValidateKMS(); err != nil { + return err + } + return statement.Resources.ValidateKMS() } if !statement.SID.IsValid() { @@ -140,7 +155,7 @@ func (statement Statement) isValid() error { return Errorf("Resource must not be empty") } - if err := statement.Resources.Validate(); err != nil { + if err := statement.Resources.ValidateS3(); err != nil { return err }