Merge pull request #2153 from versity/sis/policy-key-normalization

fix: normalize object keys during bucket policy evaluation

auth/access-control.go

@@ -114,7 +114,7 @@ func VerifyAccess(ctx context.Context, be backend.Backend, opts AccessOptions) e
 			return policyErr
 		}
 	} else {
-		return VerifyBucketPolicy(policy, opts.Acc.Access, opts.Bucket, opts.Object, opts.Actions...)
+		return VerifyBucketPolicy(policy, opts.Acc.Access, opts.Bucket, opts.Object, be.NormalizeObjectKey, opts.Actions...)
 	}
 
 	if err := verifyACL(opts.Acl, opts.Acc.Access, opts.AclPermission, opts.DisableACL); err != nil {
@@ -132,7 +132,7 @@ func VerifyPublicAccess(ctx context.Context, be backend.Backend, action Action,
 		return err
 	}
 	if err == nil {
-		err = VerifyPublicBucketPolicy(policy, bucket, object, action)
+		err = VerifyPublicBucketPolicy(policy, bucket, object, be.NormalizeObjectKey, action)
 		if errors.Is(err, errExplicitDeny) {
 			// Explicit public-policy Deny has higher precedence than any
 			// public ACL grant, so do not continue to ACL fallback.

auth/access-control_test.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"path/filepath"
 	"testing"
 
 	"github.com/aws/aws-sdk-go-v2/service/s3"
@@ -44,9 +45,10 @@ func (b noBucketPolicyBackend) GetBucketAcl(_ context.Context, _ *s3.GetBucketAc
 
 type publicBucketPolicyBackend struct {
 	backend.BackendUnsupported
-	policy   []byte
-	acl      ACL
-	aclCalls int
+	policy      []byte
+	acl         ACL
+	aclCalls    int
+	normalizeFn objectKeyNormalizer
 }
 
 func (b *publicBucketPolicyBackend) GetBucketPolicy(_ context.Context, _ string) ([]byte, error) {
@@ -58,6 +60,27 @@ func (b *publicBucketPolicyBackend) GetBucketAcl(_ context.Context, _ *s3.GetBuc
 	return json.Marshal(b.acl)
 }
 
+func (b *publicBucketPolicyBackend) NormalizeObjectKey(bucket, key string) string {
+	if b.normalizeFn == nil {
+		return b.BackendUnsupported.NormalizeObjectKey(bucket, key)
+	}
+
+	return b.normalizeFn(bucket, key)
+}
+
+func testNormalizeObjectKey(bucket, key string) string {
+	fullPath := filepath.Join(bucket, key)
+	normalizedKey, err := filepath.Rel(filepath.Clean(bucket), fullPath)
+	if err != nil {
+		return fullPath
+	}
+	if normalizedKey == "." {
+		return ""
+	}
+
+	return normalizedKey
+}
+
 func publicReadACL() ACL {
 	return ACL{
 		Owner: "owner",
@@ -71,6 +94,53 @@ func publicReadACL() ACL {
 	}
 }
 
+func TestVerifyAccess_NormalizesObjectKeyBeforePolicyMatch(t *testing.T) {
+	be := &publicBucketPolicyBackend{
+		normalizeFn: testNormalizeObjectKey,
+		policy: []byte(`{
+			"Statement": [{
+				"Effect": "Allow",
+				"Principal": "testuser",
+				"Action": "s3:GetObject",
+				"Resource": "arn:aws:s3:::bucket/public/*"
+			}]
+		}`),
+	}
+
+	err := VerifyAccess(context.Background(), be, AccessOptions{
+		Acc:     Account{Access: "testuser", Role: RoleUser},
+		Bucket:  "bucket",
+		Object:  "public/../private.txt",
+		Actions: []Action{GetObjectAction},
+	})
+
+	assert.Error(t, err)
+	assert.True(t, errors.Is(err, s3err.GetAPIError(s3err.ErrAccessDenied)))
+}
+
+func TestVerifyAccess_NormalizesPolicyResourceBeforeMatch(t *testing.T) {
+	be := &publicBucketPolicyBackend{
+		normalizeFn: testNormalizeObjectKey,
+		policy: []byte(`{
+			"Statement": [{
+				"Effect": "Allow",
+				"Principal": "testuser",
+				"Action": "s3:GetObject",
+				"Resource": "arn:aws:s3:::bucket/public/../private.txt"
+			}]
+		}`),
+	}
+
+	err := VerifyAccess(context.Background(), be, AccessOptions{
+		Acc:     Account{Access: "testuser", Role: RoleUser},
+		Bucket:  "bucket",
+		Object:  "private.txt",
+		Actions: []Action{GetObjectAction},
+	})
+
+	assert.NoError(t, err)
+}
+
 func TestVerifyPublicAccess_PublicPolicyDenyStopsACLFallback(t *testing.T) {
 	be := &publicBucketPolicyBackend{
 		policy: []byte(`{
@@ -110,6 +180,27 @@ func TestVerifyPublicAccess_PublicPolicyNoMatchFallsBackToACL(t *testing.T) {
 	assert.Equal(t, 1, be.aclCalls)
 }
 
+func TestVerifyPublicAccess_NormalizedDenyStopsACLFallback(t *testing.T) {
+	be := &publicBucketPolicyBackend{
+		normalizeFn: testNormalizeObjectKey,
+		policy: []byte(`{
+			"Statement": [{
+				"Effect": "Deny",
+				"Principal": "*",
+				"Action": "s3:GetObject",
+				"Resource": "arn:aws:s3:::bucket/private/*"
+			}]
+		}`),
+		acl: publicReadACL(),
+	}
+
+	err := VerifyPublicAccess(context.Background(), be, GetObjectAction, PermissionRead, "bucket", "public/../private/secret.txt")
+
+	assert.Error(t, err)
+	assert.True(t, errors.Is(err, s3err.GetAPIError(s3err.ErrAccessDenied)))
+	assert.Equal(t, 0, be.aclCalls)
+}
+
 func TestVerifyObjectCopyAccess_URLEncodedSlashSeparator(t *testing.T) {
 	const testUser = "testuser"
 

auth/bucket_policy.go

@@ -60,6 +60,8 @@ type BucketPolicy struct {
 	Statement []BucketPolicyItem `json:"Statement"`
 }
 
+type objectKeyNormalizer func(bucket, object string) string
+
 func (bp *BucketPolicy) UnmarshalJSON(data []byte) error {
 	var tmp struct {
 		Version   *PolicyVersion
@@ -101,10 +103,10 @@ func (bp *BucketPolicy) Validate(bucket string, iam IAMService) error {
 	return nil
 }
 
-func (bp *BucketPolicy) isAllowed(principal string, action Action, resource string) bool {
+func (bp *BucketPolicy) isAllowed(principal string, action Action, resource string, normalizeObjectKey objectKeyNormalizer) bool {
 	var isAllowed bool
 	for _, statement := range bp.Statement {
-		if statement.findMatch(principal, action, resource) {
+		if statement.findMatch(principal, action, resource, normalizeObjectKey) {
 			switch statement.Effect {
 			case BucketPolicyAccessTypeAllow:
 				isAllowed = true
@@ -117,10 +119,10 @@ func (bp *BucketPolicy) isAllowed(principal string, action Action, resource stri
 	return isAllowed
 }
 
-func (bp *BucketPolicy) publicDecisionFor(resource string, action Action) policyDecision {
+func (bp *BucketPolicy) publicDecisionFor(resource string, action Action, normalizeObjectKey objectKeyNormalizer) policyDecision {
 	var isAllowed bool
 	for _, statement := range bp.Statement {
-		if statement.isPublicFor(resource, action) {
+		if statement.isPublicFor(resource, action, normalizeObjectKey) {
 			switch statement.Effect {
 			case BucketPolicyAccessTypeAllow:
 				isAllowed = true
@@ -186,18 +188,18 @@ func (bpi *BucketPolicyItem) Validate(bucket string, iam IAMService) error {
 	return nil
 }
 
-func (bpi *BucketPolicyItem) findMatch(principal string, action Action, resource string) bool {
-	if bpi.Principals.Contains(principal) && bpi.Actions.FindMatch(action) && bpi.Resources.FindMatch(resource) {
+func (bpi *BucketPolicyItem) findMatch(principal string, action Action, resource string, normalizeObjectKey objectKeyNormalizer) bool {
+	if bpi.Principals.Contains(principal) && bpi.Actions.FindMatch(action) && bpi.Resources.FindMatch(resource, normalizeObjectKey) {
 		return true
 	}
 
 	return false
 }
 
-// isPublicFor checks if the bucket policy statemant grants public access
+// isPublicFor checks if the bucket policy statement grants public access
 // for given resource and action
-func (bpi *BucketPolicyItem) isPublicFor(resource string, action Action) bool {
-	return bpi.Principals.isPublic() && bpi.Actions.FindMatch(action) && bpi.Resources.FindMatch(resource)
+func (bpi *BucketPolicyItem) isPublicFor(resource string, action Action, normalizeObjectKey objectKeyNormalizer) bool {
+	return bpi.Principals.isPublic() && bpi.Actions.FindMatch(action) && bpi.Resources.FindMatch(resource, normalizeObjectKey)
 }
 
 // isPublic checks if the statement grants public access
@@ -248,7 +250,7 @@ func ValidatePolicyDocument(policyBin []byte, bucket string, iam IAMService) err
 	return nil
 }
 
-func VerifyBucketPolicy(policy []byte, access, bucket, object string, actions ...Action) error {
+func VerifyBucketPolicy(policy []byte, access, bucket, object string, normalizeObjectKey objectKeyNormalizer, actions ...Action) error {
 	if len(actions) == 0 {
 		return s3err.GetAPIError(s3err.ErrAccessDenied)
 	}
@@ -258,13 +260,10 @@ func VerifyBucketPolicy(policy []byte, access, bucket, object string, actions ..
 		return fmt.Errorf("failed to parse the bucket policy: %w", err)
 	}
 
-	resource := bucket
-	if object != "" {
-		resource += "/" + object
-	}
+	resource := makePolicyResource(bucket, object, normalizeObjectKey)
 
 	for _, action := range actions {
-		if !bucketPolicy.isAllowed(access, action, resource) {
+		if !bucketPolicy.isAllowed(access, action, resource, normalizeObjectKey) {
 			return s3err.GetAPIError(s3err.ErrAccessDenied)
 		}
 	}
@@ -273,18 +272,15 @@ func VerifyBucketPolicy(policy []byte, access, bucket, object string, actions ..
 }
 
 // Checks if the bucket policy grants public access
-func VerifyPublicBucketPolicy(policy []byte, bucket, object string, action Action) error {
+func VerifyPublicBucketPolicy(policy []byte, bucket, object string, normalizeObjectKey objectKeyNormalizer, action Action) error {
 	var bucketPolicy BucketPolicy
 	if err := json.Unmarshal(policy, &bucketPolicy); err != nil {
 		return err
 	}
 
-	resource := bucket
-	if object != "" {
-		resource += "/" + object
-	}
+	resource := makePolicyResource(bucket, object, normalizeObjectKey)
 
-	switch bucketPolicy.publicDecisionFor(resource, action) {
+	switch bucketPolicy.publicDecisionFor(resource, action, normalizeObjectKey) {
 	case policyDecisionAllow:
 		return nil
 	case policyDecisionDeny:
@@ -294,6 +290,22 @@ func VerifyPublicBucketPolicy(policy []byte, bucket, object string, action Actio
 	}
 }
 
+func makePolicyResource(bucket, object string, normalizeObjectKey objectKeyNormalizer) string {
+	if object == "" {
+		return bucket
+	}
+
+	return bucket + "/" + normalizePolicyObjectKey(bucket, object, normalizeObjectKey)
+}
+
+func normalizePolicyObjectKey(bucket, key string, normalizeObjectKey objectKeyNormalizer) string {
+	if key == "" || normalizeObjectKey == nil {
+		return key
+	}
+
+	return normalizeObjectKey(bucket, key)
+}
+
 // matchPattern checks if the input string matches the given pattern with wildcard(`*`) and any character(`?`).
 // - `?` matches exactly one occurrence of any character.
 // - `*` matches arbitrary many (including zero) occurrences of any character.

auth/bucket_policy_resources.go

@@ -100,9 +100,9 @@ func (r Resources) Validate(bucket string) error {
 	return nil
 }
 
-func (r Resources) FindMatch(resource string) bool {
+func (r Resources) FindMatch(resource string, normalizer objectKeyNormalizer) bool {
 	for res := range r {
-		if r.Match(res, resource) {
+		if r.Match(res, resource, normalizer) {
 			return true
 		}
 	}
@@ -111,8 +111,17 @@ func (r Resources) FindMatch(resource string) bool {
 }
 
 // Match matches the given input resource with the pattern
-func (r Resources) Match(pattern, input string) bool {
-	return matchPattern(pattern, input)
+func (r Resources) Match(pattern, input string, normalizer objectKeyNormalizer) bool {
+	return matchPattern(normalizePolicyResource(pattern, normalizer), input)
+}
+
+func normalizePolicyResource(resource string, normalizeObjectKey objectKeyNormalizer) string {
+	bucket, key, ok := strings.Cut(resource, "/")
+	if !ok {
+		return resource
+	}
+
+	return bucket + "/" + normalizePolicyObjectKey(bucket, key, normalizeObjectKey)
 }
 
 // Checks the resource to have arn prefix and not starting with /

auth/bucket_policy_resources_test.go

@@ -132,51 +132,194 @@ func TestValidate(t *testing.T) {
 }
 
 func TestFindMatch(t *testing.T) {
+	posixNormalizer := testNormalizeObjectKey
+
 	cases := []struct {
-		resources []string
-		input     string
-		expected  bool
+		name       string
+		resources  []string
+		input      string
+		normalizer objectKeyNormalizer
+		expected   bool
 	}{
-		{[]string{"arn:aws:s3:::my-bucket/*"}, "my-bucket/my-object", true},
-		{[]string{"arn:aws:s3:::my-bucket/object"}, "other-bucket/my-object", false},
-		{[]string{"arn:aws:s3:::my-bucket/object"}, "my-bucket/object", true},
-		{[]string{"arn:aws:s3:::my-bucket/*", "arn:aws:s3:::other-bucket/*"}, "other-bucket/something", true},
+		{
+			name:      "wildcard object match without normalizer",
+			resources: []string{"arn:aws:s3:::my-bucket/*"},
+			input:     "my-bucket/my-object",
+			expected:  true,
+		},
+		{
+			name:      "wrong bucket without normalizer",
+			resources: []string{"arn:aws:s3:::my-bucket/object"},
+			input:     "other-bucket/my-object",
+			expected:  false,
+		},
+		{
+			name:      "exact object match without normalizer",
+			resources: []string{"arn:aws:s3:::my-bucket/object"},
+			input:     "my-bucket/object",
+			expected:  true,
+		},
+		{
+			name:      "second resource matches without normalizer",
+			resources: []string{"arn:aws:s3:::my-bucket/*", "arn:aws:s3:::other-bucket/*"},
+			input:     "other-bucket/something",
+			expected:  true,
+		},
+		{
+			name:       "normalized private key does not match public prefix",
+			resources:  []string{"arn:aws:s3:::my-bucket/public/*"},
+			input:      "my-bucket/private.txt",
+			normalizer: posixNormalizer,
+			expected:   false,
+		},
+		{
+			name:       "policy resource parent segment normalizes before matching",
+			resources:  []string{"arn:aws:s3:::my-bucket/public/../private.txt"},
+			input:      "my-bucket/private.txt",
+			normalizer: posixNormalizer,
+			expected:   true,
+		},
+		{
+			name:       "policy resource escaping bucket does not normalize inside bucket",
+			resources:  []string{"arn:aws:s3:::my-bucket/../private.txt"},
+			input:      "my-bucket/private.txt",
+			normalizer: posixNormalizer,
+			expected:   false,
+		},
 	}
 
 	for _, tc := range cases {
-		r := Resources{}
-		for _, res := range tc.resources {
-			r.Add(res)
-		}
-		if r.FindMatch(tc.input) != tc.expected {
-			t.Errorf("Expected FindMatch to be %v for input %s", tc.expected, tc.input)
-		}
+		t.Run(tc.name, func(t *testing.T) {
+			r := Resources{}
+			for _, res := range tc.resources {
+				if err := r.Add(res); err != nil {
+					t.Fatalf("Add(%q): %v", res, err)
+				}
+			}
+			if r.FindMatch(tc.input, tc.normalizer) != tc.expected {
+				t.Errorf("Expected FindMatch to be %v for input %s", tc.expected, tc.input)
+			}
+		})
 	}
 }
 
 func TestMatch(t *testing.T) {
 	r := Resources{}
+	posixNormalizer := testNormalizeObjectKey
+
 	cases := []struct {
-		pattern  string
-		input    string
-		expected bool
+		name       string
+		pattern    string
+		input      string
+		normalizer objectKeyNormalizer
+		expected   bool
 	}{
-		{"my-bucket/*", "my-bucket/object", true},
-		{"my-bucket/?bject", "my-bucket/object", true},
-		{"my-bucket/*", "other-bucket/object", false},
-		{"*", "any-bucket/object", true},
-		{"my-bucket/*", "my-bucket/subdir/object", true},
-		{"my-bucket/*", "other-bucket", false},
-		{"my-bucket/*/*", "my-bucket/hello", false},
-		{"my-bucket/*/*", "my-bucket/hello/world", true},
-		{"foo/???/bar", "foo/qux/bar", true},
-		{"foo/???/bar", "foo/quxx/bar", false},
-		{"foo/???/bar/*/?", "foo/qux/bar/hello/g", true},
-		{"foo/???/bar/*/?", "foo/qux/bar/hello/smth", false},
+		{
+			name:     "wildcard object",
+			pattern:  "my-bucket/*",
+			input:    "my-bucket/object",
+			expected: true,
+		},
+		{
+			name:     "single char wildcard",
+			pattern:  "my-bucket/?bject",
+			input:    "my-bucket/object",
+			expected: true,
+		},
+		{
+			name:     "wrong bucket",
+			pattern:  "my-bucket/*",
+			input:    "other-bucket/object",
+			expected: false,
+		},
+		{
+			name:     "global wildcard",
+			pattern:  "*",
+			input:    "any-bucket/object",
+			expected: true,
+		},
+		{
+			name:     "wildcard nested object",
+			pattern:  "my-bucket/*",
+			input:    "my-bucket/subdir/object",
+			expected: true,
+		},
+		{
+			name:     "bucket only does not match object wildcard",
+			pattern:  "my-bucket/*",
+			input:    "other-bucket",
+			expected: false,
+		},
+		{
+			name:     "missing nested segment",
+			pattern:  "my-bucket/*/*",
+			input:    "my-bucket/hello",
+			expected: false,
+		},
+		{
+			name:     "nested segment",
+			pattern:  "my-bucket/*/*",
+			input:    "my-bucket/hello/world",
+			expected: true,
+		},
+		{
+			name:     "three char segment",
+			pattern:  "foo/???/bar",
+			input:    "foo/qux/bar",
+			expected: true,
+		},
+		{
+			name:     "too long for single char wildcards",
+			pattern:  "foo/???/bar",
+			input:    "foo/quxx/bar",
+			expected: false,
+		},
+		{
+			name:     "mixed wildcards",
+			pattern:  "foo/???/bar/*/?",
+			input:    "foo/qux/bar/hello/g",
+			expected: true,
+		},
+		{
+			name:     "mixed wildcards final segment too long",
+			pattern:  "foo/???/bar/*/?",
+			input:    "foo/qux/bar/hello/smth",
+			expected: false,
+		},
+		{
+			name:       "raw traversal key matches public prefix without normalization",
+			pattern:    "my-bucket/public/*",
+			input:      "my-bucket/public/../private.txt",
+			normalizer: nil,
+			expected:   true,
+		},
+		{
+			name:       "normalized traversal key does not match public prefix",
+			pattern:    "my-bucket/public/*",
+			input:      "my-bucket/private.txt",
+			normalizer: posixNormalizer,
+			expected:   false,
+		},
+		{
+			name:       "policy resource traversal normalizes to private object",
+			pattern:    "my-bucket/public/../private.txt",
+			input:      "my-bucket/private.txt",
+			normalizer: posixNormalizer,
+			expected:   true,
+		},
+		{
+			name:       "policy resource traversal escaping bucket does not match bucket object",
+			pattern:    "my-bucket/../private.txt",
+			input:      "my-bucket/private.txt",
+			normalizer: posixNormalizer,
+			expected:   false,
+		},
 	}
 	for _, tc := range cases {
-		if r.Match(tc.pattern, tc.input) != tc.expected {
-			t.Errorf("Match(%s, %s) failed, expected %v", tc.pattern, tc.input, tc.expected)
-		}
+		t.Run(tc.name, func(t *testing.T) {
+			if r.Match(tc.pattern, tc.input, tc.normalizer) != tc.expected {
+				t.Errorf("Match(%s, %s) failed, expected %v", tc.pattern, tc.input, tc.expected)
+			}
+		})
 	}
 }

auth/object_lock.go

@@ -173,7 +173,7 @@ func IsObjectLockRetentionPutAllowed(ctx context.Context, be backend.Backend, bu
 		debuglogger.Logf("failed to get the bucket policy: %v", err)
 		return s3err.GetAPIError(s3err.ErrObjectLocked)
 	}
-	err = VerifyBucketPolicy(policy, userAccess, bucket, object, BypassGovernanceRetentionAction)
+	err = VerifyBucketPolicy(policy, userAccess, bucket, object, be.NormalizeObjectKey, BypassGovernanceRetentionAction)
 	if err != nil {
 		// if user doesn't have "s3:BypassGovernanceRetention" permission
 		// return object is locked
@@ -316,9 +316,9 @@ func CheckObjectAccess(ctx context.Context, bucket, userAccess string, objects [
 							return err
 						}
 						if isBucketPublic {
-							err = VerifyPublicBucketPolicy(policy, bucket, key, BypassGovernanceRetentionAction)
+							err = VerifyPublicBucketPolicy(policy, bucket, key, be.NormalizeObjectKey, BypassGovernanceRetentionAction)
 						} else {
-							err = VerifyBucketPolicy(policy, userAccess, bucket, key, BypassGovernanceRetentionAction)
+							err = VerifyBucketPolicy(policy, userAccess, bucket, key, be.NormalizeObjectKey, BypassGovernanceRetentionAction)
 						}
 						if err != nil {
 							return s3err.GetAPIError(s3err.ErrObjectLocked)
@@ -362,9 +362,9 @@ func CheckObjectAccess(ctx context.Context, bucket, userAccess string, objects [
 						return err
 					}
 					if isBucketPublic {
-						err = VerifyPublicBucketPolicy(policy, bucket, key, BypassGovernanceRetentionAction)
+						err = VerifyPublicBucketPolicy(policy, bucket, key, be.NormalizeObjectKey, BypassGovernanceRetentionAction)
 					} else {
-						err = VerifyBucketPolicy(policy, userAccess, bucket, key, BypassGovernanceRetentionAction)
+						err = VerifyBucketPolicy(policy, userAccess, bucket, key, be.NormalizeObjectKey, BypassGovernanceRetentionAction)
 					}
 					if err != nil {
 						return s3err.GetAPIError(s3err.ErrObjectLocked)

backend/backend.go

@@ -30,6 +30,7 @@ import (
 type Backend interface {
 	fmt.Stringer
 	Shutdown()
+	NormalizeObjectKey(bucket, object string) string
 
 	// bucket operations
 	ListBuckets(context.Context, s3response.ListBucketsInput) (s3response.ListAllMyBucketsResult, error)
@@ -111,6 +112,9 @@ func (BackendUnsupported) Shutdown() {}
 func (BackendUnsupported) String() string {
 	return "Unsupported"
 }
+func (BackendUnsupported) NormalizeObjectKey(_, object string) string {
+	return object
+}
 func (BackendUnsupported) ListBuckets(context.Context, s3response.ListBucketsInput) (s3response.ListAllMyBucketsResult, error) {
 	return s3response.ListAllMyBucketsResult{}, s3err.GetAPIError(s3err.ErrNotImplemented)
 }

backend/posix/posix.go

@@ -6598,6 +6598,19 @@ func (p *Posix) ListBucketsAndOwners(ctx context.Context) (buckets []s3response.
 	return buckets, nil
 }
 
+func (p *Posix) NormalizeObjectKey(bucket, object string) string {
+	fullPath := filepath.Join(bucket, object)
+	key, err := filepath.Rel(filepath.Clean(bucket), fullPath)
+	if err != nil {
+		return fullPath
+	}
+	if key == "." {
+		return ""
+	}
+
+	return key
+}
+
 func (p *Posix) storeChecksums(f *os.File, bucket, object string, chs s3response.Checksum) error {
 	checksums, err := json.Marshal(chs)
 	if err != nil {

s3api/controllers/backend_moq_test.go

@@ -131,6 +131,9 @@ var _ backend.Backend = &BackendMock{}
 //			ListPartsFunc: func(contextMoqParam context.Context, listPartsInput *s3.ListPartsInput) (s3response.ListPartsResult, error) {
 //				panic("mock out the ListParts method")
 //			},
+//			NormalizeObjectKeyFunc: func(bucket string, object string) string {
+//				panic("mock out the NormalizeObjectKey method")
+//			},
 //			PutBucketAclFunc: func(contextMoqParam context.Context, bucket string, data []byte) error {
 //				panic("mock out the PutBucketAcl method")
 //			},
@@ -300,6 +303,9 @@ type BackendMock struct {
 	// ListPartsFunc mocks the ListParts method.
 	ListPartsFunc func(contextMoqParam context.Context, listPartsInput *s3.ListPartsInput) (s3response.ListPartsResult, error)
 
+	// NormalizeObjectKeyFunc mocks the NormalizeObjectKey method.
+	NormalizeObjectKeyFunc func(bucket string, object string) string
+
 	// PutBucketAclFunc mocks the PutBucketAcl method.
 	PutBucketAclFunc func(contextMoqParam context.Context, bucket string, data []byte) error
 
@@ -626,6 +632,13 @@ type BackendMock struct {
 			// ListPartsInput is the listPartsInput argument value.
 			ListPartsInput *s3.ListPartsInput
 		}
+		// NormalizeObjectKey holds details about calls to the NormalizeObjectKey method.
+		NormalizeObjectKey []struct {
+			// Bucket is the bucket argument value.
+			Bucket string
+			// Object is the object argument value.
+			Object string
+		}
 		// PutBucketAcl holds details about calls to the PutBucketAcl method.
 		PutBucketAcl []struct {
 			// ContextMoqParam is the contextMoqParam argument value.
@@ -813,6 +826,7 @@ type BackendMock struct {
 	lockListObjects                   sync.RWMutex
 	lockListObjectsV2                 sync.RWMutex
 	lockListParts                     sync.RWMutex
+	lockNormalizeObjectKey            sync.RWMutex
 	lockPutBucketAcl                  sync.RWMutex
 	lockPutBucketCors                 sync.RWMutex
 	lockPutBucketOwnershipControls    sync.RWMutex
@@ -2165,6 +2179,42 @@ func (mock *BackendMock) ListPartsCalls() []struct {
 	return calls
 }
 
+// NormalizeObjectKey calls NormalizeObjectKeyFunc.
+func (mock *BackendMock) NormalizeObjectKey(bucket string, object string) string {
+	if mock.NormalizeObjectKeyFunc == nil {
+		panic("BackendMock.NormalizeObjectKeyFunc: method is nil but Backend.NormalizeObjectKey was just called")
+	}
+	callInfo := struct {
+		Bucket string
+		Object string
+	}{
+		Bucket: bucket,
+		Object: object,
+	}
+	mock.lockNormalizeObjectKey.Lock()
+	mock.calls.NormalizeObjectKey = append(mock.calls.NormalizeObjectKey, callInfo)
+	mock.lockNormalizeObjectKey.Unlock()
+	return mock.NormalizeObjectKeyFunc(bucket, object)
+}
+
+// NormalizeObjectKeyCalls gets all the calls that were made to NormalizeObjectKey.
+// Check the length with:
+//
+//	len(mockedBackend.NormalizeObjectKeyCalls())
+func (mock *BackendMock) NormalizeObjectKeyCalls() []struct {
+	Bucket string
+	Object string
+} {
+	var calls []struct {
+		Bucket string
+		Object string
+	}
+	mock.lockNormalizeObjectKey.RLock()
+	calls = mock.calls.NormalizeObjectKey
+	mock.lockNormalizeObjectKey.RUnlock()
+	return calls
+}
+
 // PutBucketAcl calls PutBucketAclFunc.
 func (mock *BackendMock) PutBucketAcl(contextMoqParam context.Context, bucket string, data []byte) error {
 	if mock.PutBucketAclFunc == nil {

tests/integration/Access_Control.go

@@ -15,8 +15,10 @@
 package integration
 
 import (
+	"bytes"
 	"context"
 	"fmt"
+	"io"
 	"time"
 
 	"github.com/aws/aws-sdk-go-v2/service/s3"
@@ -455,6 +457,106 @@ func AccessControl_copy_object_with_starting_slash_for_user(s *S3Conf) error {
 	})
 }
 
+func AccessControl_policy_normalizes_object_key_for_get_put_delete(s *S3Conf) error {
+	testName := "AccessControl_policy_normalizes_object_key_for_get_put_delete"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		privateKey := "private.txt"
+		traversalKey := "public/../" + privateKey
+		privateBody := []byte("private object body")
+
+		_, err := putObjectWithData(0, &s3.PutObjectInput{
+			Bucket: &bucket,
+			Key:    &privateKey,
+			Body:   bytes.NewReader(privateBody),
+		}, s3client)
+		if err != nil {
+			return err
+		}
+
+		testuser := getUser("user")
+		if err := createUsers(s, []user{testuser}); err != nil {
+			return err
+		}
+
+		policy := genPolicyDoc("Allow", fmt.Sprintf(`"%s"`, testuser.access),
+			`["s3:GetObject","s3:PutObject","s3:DeleteObject"]`,
+			fmt.Sprintf(`"arn:aws:s3:::%s/public/*"`, bucket))
+		if err := putBucketPolicy(s3client, bucket, policy); err != nil {
+			return err
+		}
+
+		userClient := s.getUserClient(testuser)
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err = userClient.GetObject(ctx, &s3.GetObjectInput{
+			Bucket: &bucket,
+			Key:    &traversalKey,
+		})
+		cancel()
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
+			return err
+		}
+
+		copySource := fmt.Sprintf("%s/%s", bucket, traversalKey)
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		_, err = userClient.CopyObject(ctx, &s3.CopyObjectInput{
+			Bucket:     &bucket,
+			Key:        getPtr("public/copied.txt"),
+			CopySource: &copySource,
+		})
+		cancel()
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
+			return err
+		}
+
+		_, err = putObjectWithData(0, &s3.PutObjectInput{
+			Bucket: &bucket,
+			Key:    &traversalKey,
+			Body:   bytes.NewReader([]byte("overwrite")),
+		}, userClient)
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
+			return err
+		}
+
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		out, err := s3client.GetObject(ctx, &s3.GetObjectInput{
+			Bucket: &bucket,
+			Key:    &privateKey,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+		defer out.Body.Close()
+
+		gotBody, err := io.ReadAll(out.Body)
+		if err != nil {
+			return fmt.Errorf("read private object body: %w", err)
+		}
+		if !bytes.Equal(gotBody, privateBody) {
+			return fmt.Errorf("expected private object body to remain %q, instead got %q", privateBody, gotBody)
+		}
+
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		_, err = userClient.DeleteObject(ctx, &s3.DeleteObjectInput{
+			Bucket: &bucket,
+			Key:    &traversalKey,
+		})
+		cancel()
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
+			return err
+		}
+
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{
+			Bucket: &bucket,
+			Key:    &privateKey,
+		})
+		cancel()
+		return err
+	})
+}
+
 func AccessControl_PutObject_with_tagging_policy(s *S3Conf) error {
 	testName := "AccessControl_PutObject_with_tagging_policy"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {

tests/integration/group-tests.go

@@ -1036,6 +1036,9 @@ func TestAccessControl(ts *TestState) {
 	ts.Run(AccessControl_CopyObject_with_tagging_policy)
 	ts.Run(AccessControl_CopyObject_with_legal_hold_policy)
 	ts.Run(AccessControl_CopyObject_with_retention_policy)
+	if !ts.conf.azureTests {
+		ts.Run(AccessControl_policy_normalizes_object_key_for_get_put_delete)
+	}
 }
 
 func TestPublicBuckets(ts *TestState) {
@@ -1892,6 +1895,7 @@ func GetIntTests() IntTests {
 		"AccessControl_CopyObject_with_tagging_policy":                             AccessControl_CopyObject_with_tagging_policy,
 		"AccessControl_CopyObject_with_legal_hold_policy":                          AccessControl_CopyObject_with_legal_hold_policy,
 		"AccessControl_CopyObject_with_retention_policy":                           AccessControl_CopyObject_with_retention_policy,
+		"AccessControl_policy_normalizes_object_key_for_get_put_delete":            AccessControl_policy_normalizes_object_key_for_get_put_delete,
 		"PublicBucket_default_private_bucket":                                      PublicBucket_default_private_bucket,
 		"PublicBucket_public_bucket_policy":                                        PublicBucket_public_bucket_policy,
 		"PublicBucket_public_object_policy":                                        PublicBucket_public_object_policy,