Skip to content

Commit aa14556

Browse files
committed
Add tests for auto bucket region on priv/pubic bucket
Signed-off-by: Tiger Kaovilai <[email protected]>
1 parent 5c1e63f commit aa14556

File tree

3 files changed

+40
-6
lines changed

3 files changed

+40
-6
lines changed

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ require (
3131
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.10.1
3232
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage v1.8.1
3333
github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.6.1
34-
github.com/aws/aws-sdk-go-v2 v1.30.3
3534
github.com/aws/aws-sdk-go-v2/config v1.26.3
35+
github.com/aws/aws-sdk-go-v2/credentials v1.17.26
3636
github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.15.11
3737
github.com/aws/aws-sdk-go-v2/service/s3 v1.48.0
3838
github.com/deckarep/golang-set/v2 v2.3.0
@@ -60,8 +60,8 @@ require (
6060
github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.27.0 // indirect
6161
github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.51.0 // indirect
6262
github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.51.0 // indirect
63+
github.com/aws/aws-sdk-go-v2 v1.30.3 // indirect
6364
github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.5.4 // indirect
64-
github.com/aws/aws-sdk-go-v2/credentials v1.17.26 // indirect
6565
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.11 // indirect
6666
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.15 // indirect
6767
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.15 // indirect

pkg/storage/aws/s3.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import (
66
"net/http"
77
"net/url"
88

9-
"github.com/aws/aws-sdk-go-v2/aws"
109
"github.com/aws/aws-sdk-go-v2/config"
10+
"github.com/aws/aws-sdk-go-v2/credentials"
1111
"github.com/aws/aws-sdk-go-v2/feature/s3/manager"
1212
"github.com/aws/aws-sdk-go-v2/service/s3"
1313
"github.com/aws/aws-sdk-go/aws/request"
@@ -29,15 +29,23 @@ func GetBucketRegion(bucket string) (string, error) {
2929
// Client therefore needs to be configured with region.
3030
// In local dev environments, you might have ~/.aws/config that could be loaded and set with default region.
3131
// In cluster/CI environment, ~/.aws/config may not be configured, so set hinting region server explicitly.
32-
// Also set to use anonymous credentials. If the bucket is private, this function would not work unless we modify it to take credentials.
32+
// Also set to use anonymous credentials. This works for both public and private buckets as AWS Security
33+
// confirmed that HeadBucket API (used by GetBucketRegion) doesn't enforce s3:ListBucket permissions
34+
// for region retrieval - this is expected AWS behavior.
3335
cfg, err := config.LoadDefaultConfig(context.Background(),
3436
config.WithRegion("us-east-1"), // This is not default region being used, this is to specify a region hinting server that we will use to get region from.
35-
config.WithCredentialsProvider(aws.AnonymousCredentials{}),
3637
)
3738
if err != nil {
3839
return "", err
3940
}
40-
region, err = manager.GetBucketRegion(context.Background(), s3.NewFromConfig(cfg), bucket)
41+
region, err = manager.GetBucketRegion(context.Background(), s3.NewFromConfig(cfg), bucket, func(o *s3.Options) {
42+
// AWS Security confirmed that anonymous credentials can be used here for GetBucketRegion.
43+
// The HeadBucket API endpoint used internally by GetBucketRegion does not enforce
44+
// s3:ListBucket permissions for retrieving bucket region information.
45+
// Reference: AWS Security response (Engagement ID: CACenGS4Mha_KeJ=e3jBSLD6rPZ2iNtfuJUv9QJViaCOt7GVNDg)
46+
// This is expected AWS behavior, not a security vulnerability.
47+
o.Credentials = credentials.NewStaticCredentialsProvider("anon-credentials", "anon-secret", "")
48+
})
4149
if region != "" {
4250
return region, nil
4351
}

pkg/storage/aws/s3_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,38 @@ func TestGetBucketRegion(t *testing.T) {
1414
wantErr bool
1515
}{
1616
{
17+
// Public bucket with s3:ListBucket permission works anonymously
18+
// Note: While the bucket policy includes s3:ListBucket permission (shown below),
19+
// AWS Security confirmed that HeadBucket API doesn't actually enforce this
20+
// permission for region retrieval. The GetBucketRegion operation works with
21+
// anonymous credentials on both public and private buckets.
22+
// {
23+
// "Version": "2012-10-17",
24+
// "Statement": [
25+
// {
26+
// "Sid": "publicList",
27+
// "Effect": "Allow",
28+
// "Principal": "*",
29+
// "Action": "s3:ListBucket",
30+
// "Resource": "arn:aws:s3:::openshift-velero-plugin-s3-auto-region-test-1"
31+
// }
32+
// ]
33+
// }
34+
// ❯ aws s3api head-bucket --bucket openshift-velero-plugin-s3-auto-region-test-1 --no-sign-request
35+
// {
36+
// "BucketRegion": "us-east-1",
37+
// "AccessPointAlias": false
38+
// }
1739
name: "openshift-velero-plugin-s3-auto-region-test-1",
1840
bucket: "openshift-velero-plugin-s3-auto-region-test-1",
1941
region: "us-east-1",
2042
wantErr: false,
2143
},
2244
{
45+
// Private bucket - AWS Security confirmed that HeadBucket API (used by GetBucketRegion)
46+
// does NOT require credentials with s3:ListBucket permission for region retrieval.
47+
// This is expected AWS behavior, not a security vulnerability.
48+
// Reference: AWS Security Engagement ID: CACenGS4Mha_KeJ=e3jBSLD6rPZ2iNtfuJUv9QJViaCOt7GVNDg
2349
name: "openshift-velero-plugin-s3-auto-region-test-2",
2450
bucket: "openshift-velero-plugin-s3-auto-region-test-2",
2551
region: "us-west-1",

0 commit comments

Comments
 (0)