diff --git a/auth/access-control.go b/auth/access-control.go index 75b6efa3..93d71743 100644 --- a/auth/access-control.go +++ b/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. diff --git a/auth/access-control_test.go b/auth/access-control_test.go index fd5dd961..04aa1e6a 100644 --- a/auth/access-control_test.go +++ b/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" diff --git a/auth/bucket_policy.go b/auth/bucket_policy.go index 5c47764f..7c463ad5 100644 --- a/auth/bucket_policy.go +++ b/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. diff --git a/auth/bucket_policy_resources.go b/auth/bucket_policy_resources.go index 2dfd2494..f3b5f50d 100644 --- a/auth/bucket_policy_resources.go +++ b/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 / diff --git a/auth/bucket_policy_resources_test.go b/auth/bucket_policy_resources_test.go index 9b362fc9..42b49dc2 100644 --- a/auth/bucket_policy_resources_test.go +++ b/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) + } + }) } } diff --git a/auth/object_lock.go b/auth/object_lock.go index bea03618..7a91e6aa 100644 --- a/auth/object_lock.go +++ b/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) diff --git a/backend/backend.go b/backend/backend.go index 2f9f69cf..021bca2e 100644 --- a/backend/backend.go +++ b/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) } diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 17ded144..9bff308a 100644 --- a/backend/posix/posix.go +++ b/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 { diff --git a/s3api/controllers/backend_moq_test.go b/s3api/controllers/backend_moq_test.go index 1a0b0de4..530df639 100644 --- a/s3api/controllers/backend_moq_test.go +++ b/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 { diff --git a/tests/integration/Access_Control.go b/tests/integration/Access_Control.go index f5f1962c..ad7d44df 100644 --- a/tests/integration/Access_Control.go +++ b/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: ©Source, + }) + 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 { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 0bd7254b..7195b656 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -1035,6 +1035,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) { @@ -1890,6 +1893,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,