mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-21 22:12:36 +00:00
s3tables: judge an absent tag target by its canonical ARN (#11912)
* 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>
This commit is contained in:
co-authored by
Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
parent
e73c23a4ee
commit
4bdbfbaaa8
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user