From 4bdbfbaaa8c2f30ca937bb021cb115c7115ae1f8 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 21 Oct 2026 13:31:04 -0700 Subject: [PATCH] s3tables: judge an absent tag target by its canonical ARN (#11912) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * s3tables: judge an absent tag target by its canonical ARN authorizeAbsentTarget matched the bucket policy's Deny statements against the request's raw resource ARN. The ARN parsers accept any account and region but resolve the target by name, so a caller with an identity grant could reformat the ARN, miss the Deny match, and learn whether a denied name exists — the oracle the absent-target check was meant to close. Rebuild the ARN from the resolved names and the real bucket owner and evaluate the Deny against that, matching what the existing-target path checks. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: cover a foreign-account ARN on an absent denied table Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: keep the absent-target deny check behind a supplied ARN Callers that pass no resource ARN (ListTables, ListViews, CreateNamespace) must reach CheckTableOrBucketPermission's admin shortcut unchanged; running the bucket-policy Deny match first would deny the admin lists those callers take. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/s3api/s3tables/handler.go | 15 ++++++- .../s3tables/handler_existence_hiding_test.go | 43 +++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) 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) + }) + } +}