mirror of
https://github.com/versity/versitygw.git
synced 2026-07-19 22:42:20 +00:00
fix: normalize object keys during bucket policy evaluation
Object key validation allowed internal parent-directory segments such as `public/../private.txt`. Bucket policy and auth checks evaluated the raw key, so a policy allowing bucket/public/* could match the request while posix backend later resolved the key with `filepath.Join` and accessed `bucket/private.txt`. Add backend-specific object key normalization to close that mismatch. The Backend interface now exposes `NormalizeObjectKey` so authorization can evaluate resources using the same key shape a backend will use for storage access. Backends that do not collapse object paths, including Azure and the S3 proxy, inherit `BackendUnsupported.NormalizeObjectKey`. That implementation returns the input key unchanged, avoiding unnecessary normalization and keeping policy evaluation unpolluted for object stores where ../ is part of the key name. posix/scoutfs normalize keys with filepath.Join so policy resources and request keys are compared after internal dot segments are collapsed. Bucket policy evaluation now normalizes both the incoming object key and object resource patterns from the policy before matching. Object lock governance bypass policy checks use the same backend normalizer as well, so retention and legal hold authorization cannot diverge from backend path resolution.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
+33
-21
@@ -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.
|
||||
|
||||
@@ -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 /
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+5
-5
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user