Mirror
/
seaweedfs
mirror of https://github.com/seaweedfs/seaweedfs.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Fix S3 bucket policy ARN validation to accept AWS ARNs and simplified formats 

Fixes #7252

The bucket policy validation was rejecting valid AWS-style ARNs and
simplified resource formats, causing validation failures with the
error 'resource X does not match bucket X' even when they were
identical strings.

Changes:
- Updated validateResourceForBucket() to accept three formats:
  1. AWS-style ARNs: arn:aws:s3:::bucket-name[/*|/path]
  2. SeaweedFS ARNs: arn:seaweed:s3:::bucket-name[/*|/path]
  3. Simplified formats: bucket-name[/*|/path]

- Added comprehensive test coverage for all three formats
- Added specific test cases from issue #7252 to prevent regression

This ensures compatibility with standard AWS S3 bucket policies
while maintaining support for SeaweedFS-specific ARN format.
pull/7409/head
Chris Lu 2 months ago
parent
8a032bf57d
commit
2c9243a8f8
2 changed files with 219 additions and 17 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 177
      weed/s3api/s3_bucket_policy_simple_test.go
  2. 59
      weed/s3api/s3api_bucket_policy_handlers.go

177
weed/s3api/s3_bucket_policy_simple_test.go
View File

@ -143,42 +143,106 @@ func TestBucketResourceValidation(t *testing.T) {
bucket string
valid bool
}{
// SeaweedFS ARN format
{
name: "Exact bucket ARN",
name: "Exact bucket ARN (SeaweedFS)",
resource: "arn:seaweed:s3:::test-bucket",
bucket: "test-bucket",
valid: true,
},
{
name: "Bucket wildcard ARN",
name: "Bucket wildcard ARN (SeaweedFS)",
resource: "arn:seaweed:s3:::test-bucket/*",
bucket: "test-bucket",
valid: true,
},
{
name: "Specific object ARN",
name: "Specific object ARN (SeaweedFS)",
resource: "arn:seaweed:s3:::test-bucket/path/to/object.txt",
bucket: "test-bucket",
valid: true,
},
// AWS ARN format (compatibility)
{
name: "Different bucket ARN",
name: "Exact bucket ARN (AWS)",
resource: "arn:aws:s3:::test-bucket",
bucket: "test-bucket",
valid: true,
},
{
name: "Bucket wildcard ARN (AWS)",
resource: "arn:aws:s3:::test-bucket/*",
bucket: "test-bucket",
valid: true,
},
{
name: "Specific object ARN (AWS)",
resource: "arn:aws:s3:::test-bucket/path/to/object.txt",
bucket: "test-bucket",
valid: true,
},
// Simplified format (without ARN prefix)
{
name: "Simplified bucket name",
resource: "test-bucket",
bucket: "test-bucket",
valid: true,
},
{
name: "Simplified bucket wildcard",
resource: "test-bucket/*",
bucket: "test-bucket",
valid: true,
},
{
name: "Simplified specific object",
resource: "test-bucket/path/to/object.txt",
bucket: "test-bucket",
valid: true,
},
// Invalid cases
{
name: "Different bucket ARN (SeaweedFS)",
resource: "arn:seaweed:s3:::other-bucket/*",
bucket: "test-bucket",
valid: false,
},
{
name: "Global S3 wildcard",
name: "Different bucket ARN (AWS)",
resource: "arn:aws:s3:::other-bucket/*",
bucket: "test-bucket",
valid: false,
},
{
name: "Different bucket simplified",
resource: "other-bucket/*",
bucket: "test-bucket",
valid: false,
},
{
name: "Global S3 wildcard (SeaweedFS)",
resource: "arn:seaweed:s3:::*",
bucket: "test-bucket",
valid: false,
},
{
name: "Global S3 wildcard (AWS)",
resource: "arn:aws:s3:::*",
bucket: "test-bucket",
valid: false,
},
{
name: "Invalid ARN format",
resource: "invalid-arn",
bucket: "test-bucket",
valid: false,
},
{
name: "Bucket name prefix match but different bucket",
resource: "test-bucket-different/*",
bucket: "test-bucket",
valid: false,
},
}
for _, tt := range tests {
@ -226,3 +290,106 @@ func marshalPolicy(t *testing.T, policyDoc *policy.PolicyDocument) []byte {
require.NoError(t, err)
return data
}
// TestIssue7252Examples tests the specific examples from GitHub issue #7252
func TestIssue7252Examples(t *testing.T) {
s3Server := &S3ApiServer{}
tests := []struct {
name string
policy *policy.PolicyDocument
bucket string
expectedValid bool
description string
}{
{
name: "Issue #7252 - Standard ARN with wildcard",
policy: &policy.PolicyDocument{
Version: "2012-10-17",
Statement: []policy.Statement{
{
Effect: "Allow",
Principal: map[string]interface{}{
"AWS": "*",
},
Action: []string{"s3:GetObject"},
Resource: []string{"arn:aws:s3:::main-bucket/*"},
},
},
},
bucket: "main-bucket",
expectedValid: true,
description: "AWS ARN format should be accepted",
},
{
name: "Issue #7252 - Simplified resource with wildcard",
policy: &policy.PolicyDocument{
Version: "2012-10-17",
Statement: []policy.Statement{
{
Effect: "Allow",
Principal: map[string]interface{}{
"AWS": "*",
},
Action: []string{"s3:GetObject"},
Resource: []string{"main-bucket/*"},
},
},
},
bucket: "main-bucket",
expectedValid: true,
description: "Simplified format with wildcard should be accepted",
},
{
name: "Issue #7252 - Resource as exact bucket name",
policy: &policy.PolicyDocument{
Version: "2012-10-17",
Statement: []policy.Statement{
{
Effect: "Allow",
Principal: map[string]interface{}{
"AWS": "*",
},
Action: []string{"s3:GetObject"},
Resource: []string{"main-bucket"},
},
},
},
bucket: "main-bucket",
expectedValid: true,
description: "Exact bucket name should be accepted",
},
{
name: "Public read policy with AWS ARN",
policy: &policy.PolicyDocument{
Version: "2012-10-17",
Statement: []policy.Statement{
{
Sid: "PublicReadGetObject",
Effect: "Allow",
Principal: map[string]interface{}{
"AWS": "*",
},
Action: []string{"s3:GetObject"},
Resource: []string{"arn:aws:s3:::my-public-bucket/*"},
},
},
},
bucket: "my-public-bucket",
expectedValid: true,
description: "Standard public read policy with AWS ARN should work",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := s3Server.validateBucketPolicy(tt.policy, tt.bucket)
if tt.expectedValid {
assert.NoError(t, err, "Policy should be valid: %s", tt.description)
} else {
assert.Error(t, err, "Policy should be invalid: %s", tt.description)
}
})
}
}

59
weed/s3api/s3api_bucket_policy_handlers.go
View File

@ -274,18 +274,53 @@ func (s3a *S3ApiServer) validateBucketPolicy(policyDoc *policy.PolicyDocument, b
// validateResourceForBucket checks if a resource ARN is valid for the given bucket
func (s3a *S3ApiServer) validateResourceForBucket(resource, bucket string) bool {
// Expected formats:
// arn:seaweed:s3:::bucket-name
// arn:seaweed:s3:::bucket-name/*
// arn:seaweed:s3:::bucket-name/path/to/object
expectedBucketArn := fmt.Sprintf("arn:seaweed:s3:::%s", bucket)
expectedBucketWildcard := fmt.Sprintf("arn:seaweed:s3:::%s/*", bucket)
expectedBucketPath := fmt.Sprintf("arn:seaweed:s3:::%s/", bucket)
return resource == expectedBucketArn ||
resource == expectedBucketWildcard ||
strings.HasPrefix(resource, expectedBucketPath)
// Accepted formats for S3 bucket policies:
// AWS-style ARNs:
// arn:aws:s3:::bucket-name
// arn:aws:s3:::bucket-name/*
// arn:aws:s3:::bucket-name/path/to/object
// SeaweedFS ARNs:
// arn:seaweed:s3:::bucket-name
// arn:seaweed:s3:::bucket-name/*
// arn:seaweed:s3:::bucket-name/path/to/object
// Simplified formats (for convenience):
// bucket-name
// bucket-name/*
// bucket-name/path/to/object
// Check AWS-style ARN
awsBucketArn := fmt.Sprintf("arn:aws:s3:::%s", bucket)
awsBucketWildcard := fmt.Sprintf("arn:aws:s3:::%s/*", bucket)
awsBucketPath := fmt.Sprintf("arn:aws:s3:::%s/", bucket)
if resource == awsBucketArn ||
resource == awsBucketWildcard ||
strings.HasPrefix(resource, awsBucketPath) {
return true
}
// Check SeaweedFS-style ARN
seaweedBucketArn := fmt.Sprintf("arn:seaweed:s3:::%s", bucket)
seaweedBucketWildcard := fmt.Sprintf("arn:seaweed:s3:::%s/*", bucket)
seaweedBucketPath := fmt.Sprintf("arn:seaweed:s3:::%s/", bucket)
if resource == seaweedBucketArn ||
resource == seaweedBucketWildcard ||
strings.HasPrefix(resource, seaweedBucketPath) {
return true
}
// Check simplified format (bucket name without ARN prefix)
simplifiedBucketWildcard := fmt.Sprintf("%s/*", bucket)
simplifiedBucketPath := fmt.Sprintf("%s/", bucket)
if resource == bucket ||
resource == simplifiedBucketWildcard ||
strings.HasPrefix(resource, simplifiedBucketPath) {
return true
}
return false
}
// IAM integration functions

Write Preview
Loading…
Cancel
Save