diff --git a/weed/s3api/s3tables/handler.go b/weed/s3api/s3tables/handler.go index f5f4cc51cb..69e8c4f9a8 100644 --- a/weed/s3api/s3tables/handler.go +++ b/weed/s3api/s3tables/handler.go @@ -491,6 +491,17 @@ func (h *S3TablesHandler) authorizeAbsentTarget(r *http.Request, filerClient Fil } bucketARN := h.generateTableBucketARN(bucketOwner, bucketName) + // Judge the target by the canonical ARN the surviving bucket resolves to: + // the request ARN's account and region are the caller's to choose and must + // not decide whether a policy Deny applies. + canonicalARN := resourceARN + if bucketOwner != "" { + if name != "" { + canonicalARN = h.generateTableARN(bucketOwner, bucketName, namespace+"/"+name) + } else { + canonicalARN = bucketARN + } + } ctx := &PolicyContext{ TrustedIssuers: h.trustedIssuers(r), IsLocalAccountID: h.localAccountIDs, @@ -502,10 +513,10 @@ func (h *S3TablesHandler) authorizeAbsentTarget(r *http.Request, filerClient Fil DefaultAllow: h.defaultAllowFor(r), } if resourceARN != "" && - matchesDenyStatement(operation, principal, bucketPolicy, resourceARN, ctx) { + matchesDenyStatement(operation, principal, bucketPolicy, canonicalARN, ctx) { return false } - if CheckTableOrBucketPermission(operation, principal, "", "", resourceARN, + if CheckTableOrBucketPermission(operation, principal, "", "", canonicalARN, bucketOwner, bucketPolicy, bucketARN, ctx) { return true } diff --git a/weed/s3api/s3tables/handler_existence_hiding_test.go b/weed/s3api/s3tables/handler_existence_hiding_test.go index cc1e051777..187ebf093e 100644 --- a/weed/s3api/s3tables/handler_existence_hiding_test.go +++ b/weed/s3api/s3tables/handler_existence_hiding_test.go @@ -3,6 +3,7 @@ package s3tables import ( "context" "encoding/json" + "fmt" "net/http/httptest" "testing" @@ -331,3 +332,45 @@ func TestGrantedCallerLearnsAbsence(t *testing.T) { assert.Equal(t, 404, got.status) assert.Equal(t, ErrCodeNoSuchTable, got.body.Type) } + +// A request ARN that names a foreign account still resolves to the local +// names, so the absent-target answer must match the denial the canonical ARN +// gets; otherwise a reformatted ARN restores the existence oracle. +func TestAbsentTagTargetDenySurvivesForeignAccountARN(t *testing.T) { + fs, manager := startRenameManager(t) + manager.SetTrusted(false) + manager.SetDefaultAllow(false) + + bucketMeta, _ := json.Marshal(tableBucketMetadata{Name: renameTestBucket, OwnerAccountID: DefaultAccountID}) + fs.Put(TablesPath, renameTestBucket, map[string][]byte{ + ExtendedKeyTableBucket: []byte("{}"), + ExtendedKeyMetadata: bucketMeta, + ExtendedKeyPolicy: []byte(fmt.Sprintf( + `{"Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:*","Resource":%q}]}`, + mustBucketARN(t)+"/table/*")), + }) + + foreignARN := func(table string) string { + return fmt.Sprintf("arn:aws:s3tables:%s:999999999999:bucket/%s/table/ns/%s", + DefaultRegion, renameTestBucket, table) + } + requestFor := func(operation, arn string) interface{} { + switch operation { + case "TagResource": + return &TagResourceRequest{ResourceARN: arn, Tags: map[string]string{"k": "v"}} + case "UntagResource": + return &UntagResourceRequest{ResourceARN: arn, TagKeys: []string{"k"}} + } + return &ListTagsForResourceRequest{ResourceARN: arn} + } + + for _, operation := range []string{"TagResource", "ListTagsForResource", "UntagResource"} { + t.Run(operation, func(t *testing.T) { + actions := []string{"s3tables:" + operation} + denied := runRequestAs(t, manager, fs, operation, requestFor(operation, foreignARN("t")), actions) + assert.Equal(t, 403, denied.status) + got := runRequestAs(t, manager, fs, operation, requestFor(operation, foreignARN("ghost")), actions) + assert.Equal(t, denied, got) + }) + } +}