Allow a KMS Action to specify keys in the Resources of a policy (#121)

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,
 	}
 }

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 {

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
 		}

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 {

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
 	}