mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-22 22:11:39 +02:00
s3tables: keep absent-target answers identical to denied ones (#11905)
* s3tables: authorize before revealing a list or tag target exists ListTables, ListViews and the tag operations read the target's catalog metadata before any permission check, so a denied caller could tell an existing name (403 or the resource's own error) from an absent one (an empty list, or 404). Evaluate the operation's permission with whatever bucket context could be read and answer a denied caller the same way for absent, damaged and denied targets. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: answer bucket reads uniformly when the entry is not a table bucket A plain S3 bucket directory answers GetTableBucket and ListNamespaces with 500 attribute-not-found while a missing name gets 404, and a denied CreateNamespace is 403 while a missing bucket is 404. Run the permission check before the absence answer and read not-a-table-bucket as NoSuchTableBucket. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: cover absent-target answers matching denied ones 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:
1 parent
bb1ba8b247
commit
8c3d8fcea0
7 files changed
+238
-10
No files matched your search
@@ -459,6 +459,59 @@ func isAuthError(err error) bool {
|
||||
return errors.As(err, &authErr) || errors.Is(err, ErrAccessDenied)
|
||||
}
|
||||
|
||||
// authorizeAbsentTarget reports whether the caller may learn the target
|
||||
// resolved to nothing, so a caller without a grant gets the same response a
|
||||
// denied existing target would give. The bucket named by the target still
|
||||
// applies whatever metadata and policy load; with nothing readable to
|
||||
// evaluate, only an identity-level grant on the operation, the admin, or a
|
||||
// default-allow deployment passes.
|
||||
func (h *S3TablesHandler) authorizeAbsentTarget(r *http.Request, filerClient FilerClient, operation, bucketName, namespace, name, resourceARN string) bool {
|
||||
principal := h.getAccountID(r)
|
||||
if principal == "" {
|
||||
return false
|
||||
}
|
||||
|
||||
var bucketOwner, bucketPolicy string
|
||||
var bucketTags map[string]string
|
||||
if bucketName != "" {
|
||||
bucketPath := GetTableBucketPath(bucketName)
|
||||
_ = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
||||
if data, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyMetadata); err == nil {
|
||||
var meta tableBucketMetadata
|
||||
if json.Unmarshal(data, &meta) == nil {
|
||||
bucketOwner = meta.OwnerAccountID
|
||||
}
|
||||
}
|
||||
if policy, err := h.readPolicy(r, client, bucketPath); err == nil {
|
||||
bucketPolicy = policy
|
||||
}
|
||||
bucketTags, _ = h.readTags(r.Context(), client, bucketPath)
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
bucketARN := h.generateTableBucketARN(bucketOwner, bucketName)
|
||||
ctx := &PolicyContext{
|
||||
TrustedIssuers: h.trustedIssuers(r),
|
||||
IsLocalAccountID: h.localAccountIDs,
|
||||
TableBucketName: bucketName,
|
||||
Namespace: namespace,
|
||||
TableName: name,
|
||||
TableBucketTags: bucketTags,
|
||||
IdentityActions: getIdentityActions(r),
|
||||
DefaultAllow: h.defaultAllowFor(r),
|
||||
}
|
||||
if resourceARN != "" &&
|
||||
matchesDenyStatement(operation, principal, bucketPolicy, resourceARN, ctx) {
|
||||
return false
|
||||
}
|
||||
if CheckTableOrBucketPermission(operation, principal, "", "", resourceARN,
|
||||
bucketOwner, bucketPolicy, bucketARN, ctx) {
|
||||
return true
|
||||
}
|
||||
return bucketOwner == "" && checkPermission(operation, principal, "", "", resourceARN, ctx)
|
||||
}
|
||||
|
||||
func (h *S3TablesHandler) readTags(ctx context.Context, client filer_pb.SeaweedFilerClient, path string) (map[string]string, error) {
|
||||
data, err := h.getExtendedAttribute(ctx, client, path, ExtendedKeyTags)
|
||||
if err != nil {
|
||||
|
||||
@@ -58,7 +58,7 @@ func (h *S3TablesHandler) handleGetTableBucket(w http.ResponseWriter, r *http.Re
|
||||
})
|
||||
|
||||
if err != nil {
|
||||
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||
if errors.Is(err, filer_pb.ErrNotFound) || errors.Is(err, ErrAttributeNotFound) {
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName))
|
||||
} else {
|
||||
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to get table bucket: %v", err))
|
||||
|
||||
@@ -211,3 +211,123 @@ func TestDeniedTableWritesDoNotRevealViews(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A caller without a grant must get the same answer whether a list target
|
||||
// exists or not, and whether the bucket behind it exists at all.
|
||||
func TestAbsentListTargetsMatchDeniedAnswers(t *testing.T) {
|
||||
existing, manager := startRenameManager(t)
|
||||
missing := s3tablestest.Start(t)
|
||||
manager.SetTrusted(false)
|
||||
manager.SetDefaultAllow(false)
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
fs *s3tablestest.MemFiler
|
||||
operation string
|
||||
denied interface{}
|
||||
request interface{}
|
||||
}{
|
||||
{"ListTables missing namespace", existing, "ListTables",
|
||||
&ListTablesRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ns"}},
|
||||
&ListTablesRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ghost"}}},
|
||||
{"ListTables missing bucket", missing, "ListTables",
|
||||
&ListTablesRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ns"}},
|
||||
&ListTablesRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ns"}}},
|
||||
{"ListTables missing bucket, all namespaces", missing, "ListTables",
|
||||
&ListTablesRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ns"}},
|
||||
&ListTablesRequest{TableBucketARN: mustBucketARN(t)}},
|
||||
{"ListViews missing namespace", existing, "ListViews",
|
||||
&ListViewsRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ns"}},
|
||||
&ListViewsRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ghost"}}},
|
||||
{"CreateNamespace missing bucket", missing, "CreateNamespace",
|
||||
&CreateNamespaceRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"newns"}},
|
||||
&CreateNamespaceRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"newns"}}},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
denied := runUnauthorizedRequest(t, manager, existing, tc.operation, tc.denied)
|
||||
assert.Equal(t, 403, denied.status)
|
||||
got := runUnauthorizedRequest(t, manager, tc.fs, tc.operation, tc.request)
|
||||
assert.Equal(t, denied, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A denied tag operation and the same operation on a missing table must answer
|
||||
// alike; a granted caller still gets the truthful not-found.
|
||||
func TestAbsentTagTargetsMatchDeniedAnswers(t *testing.T) {
|
||||
existing, manager := startRenameManager(t)
|
||||
manager.SetTrusted(false)
|
||||
manager.SetDefaultAllow(false)
|
||||
|
||||
existingARN, err := BuildTableARN(DefaultRegion, DefaultAccountID, renameTestBucket, "ns", "t")
|
||||
require.NoError(t, err)
|
||||
missingARN, err := BuildTableARN(DefaultRegion, DefaultAccountID, renameTestBucket, "ns", "ghost")
|
||||
require.NoError(t, err)
|
||||
|
||||
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) {
|
||||
denied := runUnauthorizedRequest(t, manager, existing, operation, requestFor(operation, existingARN))
|
||||
assert.Equal(t, 403, denied.status)
|
||||
got := runUnauthorizedRequest(t, manager, existing, operation, requestFor(operation, missingARN))
|
||||
assert.Equal(t, denied, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A directory that holds a plain S3 bucket name is not a table bucket: every
|
||||
// caller gets the same 404 an absent name gets, not a 500 that reveals the
|
||||
// name resolves in the filer.
|
||||
func TestPlainBucketReadsAsAbsent(t *testing.T) {
|
||||
existing, manager := startRenameManager(t)
|
||||
manager.SetTrusted(false)
|
||||
manager.SetDefaultAllow(false)
|
||||
existing.Put(TablesPath, "plainbkt", nil)
|
||||
|
||||
plainARN, err := BuildBucketARN(DefaultRegion, DefaultAccountID, "plainbkt")
|
||||
require.NoError(t, err)
|
||||
|
||||
for _, tc := range []struct {
|
||||
operation string
|
||||
request interface{}
|
||||
}{
|
||||
{"GetTableBucket", &GetTableBucketRequest{TableBucketARN: plainARN}},
|
||||
{"ListNamespaces", &ListNamespacesRequest{TableBucketARN: plainARN}},
|
||||
} {
|
||||
t.Run(tc.operation, func(t *testing.T) {
|
||||
got := runUnauthorizedRequest(t, manager, existing, tc.operation, tc.request)
|
||||
assert.Equal(t, 404, got.status)
|
||||
assert.Equal(t, ErrCodeNoSuchBucket, got.body.Type)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A caller with an identity-level grant still learns that a name is absent.
|
||||
func TestGrantedCallerLearnsAbsence(t *testing.T) {
|
||||
missing := s3tablestest.Start(t)
|
||||
manager := NewManager()
|
||||
manager.SetTrusted(false)
|
||||
manager.SetDefaultAllow(false)
|
||||
|
||||
got := runRequestAs(t, manager, missing, "ListTables", &ListTablesRequest{
|
||||
TableBucketARN: mustBucketARN(t), Namespace: []string{"ns"}},
|
||||
[]string{"s3tables:ListTables"})
|
||||
assert.Equal(t, 200, got.status)
|
||||
|
||||
missingTableARN, err := BuildTableARN(DefaultRegion, DefaultAccountID, renameTestBucket, "ns", "ghost")
|
||||
require.NoError(t, err)
|
||||
got = runRequestAs(t, manager, missing, "ListTagsForResource",
|
||||
&ListTagsForResourceRequest{ResourceARN: missingTableARN},
|
||||
[]string{"s3tables:ListTagsForResource"})
|
||||
assert.Equal(t, 404, got.status)
|
||||
assert.Equal(t, ErrCodeNoSuchTable, got.body.Type)
|
||||
}
|
||||
@@ -140,6 +140,10 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R
|
||||
return ErrAccessDenied
|
||||
}
|
||||
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||
if !h.authorizeAbsentTarget(r, filerClient, "CreateNamespace", bucketName, namespaceName, "", "") {
|
||||
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to create namespace in this bucket")
|
||||
return ErrAccessDenied
|
||||
}
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName))
|
||||
} else {
|
||||
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to check table bucket: %v", err))
|
||||
@@ -481,7 +485,7 @@ func (h *S3TablesHandler) handleListNamespaces(w http.ResponseWriter, r *http.Re
|
||||
})
|
||||
|
||||
if err != nil {
|
||||
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||
if errors.Is(err, filer_pb.ErrNotFound) || errors.Is(err, ErrAttributeNotFound) {
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName))
|
||||
} else {
|
||||
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to list namespaces: %v", err))
|
||||
|
||||
@@ -659,6 +659,9 @@ func (h *S3TablesHandler) handleTagResource(w http.ResponseWriter, r *http.Reque
|
||||
// Read metadata for ownership check
|
||||
data, err := h.getExtendedAttribute(r.Context(), client, resourcePath, ExtendedKeyMetadata)
|
||||
if err != nil {
|
||||
if errors.Is(err, filer_pb.ErrNotFound) || errors.Is(err, ErrAttributeNotFound) {
|
||||
return h.absentTagTargetError(r, client, "TagResource", "not authorized to tag resource", req.ResourceARN, rType)
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -793,6 +796,9 @@ func (h *S3TablesHandler) handleListTagsForResource(w http.ResponseWriter, r *ht
|
||||
// Read metadata for ownership check
|
||||
data, err := h.getExtendedAttribute(r.Context(), client, resourcePath, ExtendedKeyMetadata)
|
||||
if err != nil {
|
||||
if errors.Is(err, filer_pb.ErrNotFound) || errors.Is(err, ErrAttributeNotFound) {
|
||||
return h.absentTagTargetError(r, client, "ListTagsForResource", "not authorized to list tags for resource", req.ResourceARN, rType)
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -913,6 +919,9 @@ func (h *S3TablesHandler) handleUntagResource(w http.ResponseWriter, r *http.Req
|
||||
// Read metadata for ownership check
|
||||
data, err := h.getExtendedAttribute(r.Context(), client, resourcePath, ExtendedKeyMetadata)
|
||||
if err != nil {
|
||||
if errors.Is(err, filer_pb.ErrNotFound) || errors.Is(err, ErrAttributeNotFound) {
|
||||
return h.absentTagTargetError(r, client, "UntagResource", "not authorized to untag resource", req.ResourceARN, rType)
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -1017,6 +1026,22 @@ func (h *S3TablesHandler) handleUntagResource(w http.ResponseWriter, r *http.Req
|
||||
return nil
|
||||
}
|
||||
|
||||
// absentTagTargetError resolves a tag operation when the resource's catalog
|
||||
// metadata did not load: a caller the operation would refuse gets its denial,
|
||||
// everyone else the same not-found an absent name gets.
|
||||
func (h *S3TablesHandler) absentTagTargetError(r *http.Request, client filer_pb.SeaweedFilerClient, operation, deniedMessage, resourceARN string, rType ResourceType) error {
|
||||
var bucketName, namespace, name string
|
||||
if rType == ResourceTypeTable {
|
||||
bucketName, namespace, name, _ = parseTableFromARN(resourceARN)
|
||||
} else {
|
||||
bucketName, _ = parseBucketNameFromARN(resourceARN)
|
||||
}
|
||||
if !h.authorizeAbsentTarget(r, &singleClient{client}, operation, bucketName, namespace, name, resourceARN) {
|
||||
return NewAuthError(operation, h.getAccountID(r), deniedMessage)
|
||||
}
|
||||
return filer_pb.ErrNotFound
|
||||
}
|
||||
|
||||
// resolveResourcePath determines the resource path and extended attribute key from a resource ARN
|
||||
func (h *S3TablesHandler) resolveResourcePath(resourceARN string) (path string, key string, rType ResourceType, err error) {
|
||||
// Try parsing as table ARN first
|
||||
|
||||
@@ -515,11 +515,14 @@ func (h *S3TablesHandler) handleListTables(w http.ResponseWriter, r *http.Reques
|
||||
bucketTags := map[string]string{}
|
||||
|
||||
// Fetch namespace metadata and policy
|
||||
targetAbsent := false
|
||||
data, err := h.getExtendedAttribute(r.Context(), client, namespacePath, ExtendedKeyMetadata)
|
||||
if err != nil {
|
||||
return err // Not Found handled by caller
|
||||
}
|
||||
if err := json.Unmarshal(data, &nsMeta); err != nil {
|
||||
if !errors.Is(err, filer_pb.ErrNotFound) && !errors.Is(err, ErrAttributeNotFound) {
|
||||
return err
|
||||
}
|
||||
targetAbsent = true
|
||||
} else if err := json.Unmarshal(data, &nsMeta); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -527,7 +530,7 @@ func (h *S3TablesHandler) handleListTables(w http.ResponseWriter, r *http.Reques
|
||||
policyData, err := h.getExtendedAttribute(r.Context(), client, namespacePath, ExtendedKeyPolicy)
|
||||
if err == nil {
|
||||
namespacePolicy = string(policyData)
|
||||
} else if !errors.Is(err, ErrAttributeNotFound) {
|
||||
} else if !errors.Is(err, ErrAttributeNotFound) && !errors.Is(err, filer_pb.ErrNotFound) {
|
||||
return fmt.Errorf("failed to fetch namespace policy: %w", err)
|
||||
}
|
||||
|
||||
@@ -537,6 +540,8 @@ func (h *S3TablesHandler) handleListTables(w http.ResponseWriter, r *http.Reques
|
||||
if err := json.Unmarshal(data, &bucketMeta); err != nil {
|
||||
return fmt.Errorf("failed to unmarshal bucket metadata: %w", err)
|
||||
}
|
||||
} else if errors.Is(err, filer_pb.ErrNotFound) {
|
||||
targetAbsent = true
|
||||
} else if !errors.Is(err, ErrAttributeNotFound) {
|
||||
return fmt.Errorf("failed to fetch bucket metadata: %w", err)
|
||||
}
|
||||
@@ -544,15 +549,24 @@ func (h *S3TablesHandler) handleListTables(w http.ResponseWriter, r *http.Reques
|
||||
policyData, err = h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy)
|
||||
if err == nil {
|
||||
bucketPolicy = string(policyData)
|
||||
} else if !errors.Is(err, ErrAttributeNotFound) {
|
||||
} else if !errors.Is(err, ErrAttributeNotFound) && !errors.Is(err, filer_pb.ErrNotFound) {
|
||||
return fmt.Errorf("failed to fetch bucket policy: %w", err)
|
||||
}
|
||||
if tags, err := h.readTags(r.Context(), client, bucketPath); err != nil {
|
||||
return fmt.Errorf("failed to read bucket tags: %w", err)
|
||||
if !errors.Is(err, filer_pb.ErrNotFound) {
|
||||
return fmt.Errorf("failed to read bucket tags: %w", err)
|
||||
}
|
||||
} else if tags != nil {
|
||||
bucketTags = tags
|
||||
}
|
||||
|
||||
if targetAbsent {
|
||||
if !h.authorizeAbsentTarget(r, &singleClient{client}, "ListTables", bucketName, namespaceName, "", "") {
|
||||
return ErrAccessDenied
|
||||
}
|
||||
return filer_pb.ErrNotFound
|
||||
}
|
||||
|
||||
bucketARN := h.generateTableBucketARN(bucketMeta.OwnerAccountID, bucketName)
|
||||
identityActions := getIdentityActions(r)
|
||||
if !CheckTableOrBucketPermission("ListTables", accountID,
|
||||
@@ -583,7 +597,13 @@ func (h *S3TablesHandler) handleListTables(w http.ResponseWriter, r *http.Reques
|
||||
// Fetch bucket metadata and policy
|
||||
data, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyMetadata)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to fetch bucket metadata: %w", err)
|
||||
if !errors.Is(err, filer_pb.ErrNotFound) && !errors.Is(err, ErrAttributeNotFound) {
|
||||
return fmt.Errorf("failed to fetch bucket metadata: %w", err)
|
||||
}
|
||||
if !h.authorizeAbsentTarget(r, &singleClient{client}, "ListTables", bucketName, "", "", "") {
|
||||
return ErrAccessDenied
|
||||
}
|
||||
return filer_pb.ErrNotFound
|
||||
}
|
||||
if err := json.Unmarshal(data, &bucketMeta); err != nil {
|
||||
return fmt.Errorf("failed to unmarshal bucket metadata: %w", err)
|
||||
|
||||
@@ -279,7 +279,13 @@ func (h *S3TablesHandler) handleListViews(w http.ResponseWriter, r *http.Request
|
||||
var nsMeta namespaceMetadata
|
||||
data, err := h.getExtendedAttribute(r.Context(), client, namespacePath, ExtendedKeyMetadata)
|
||||
if err != nil {
|
||||
return err
|
||||
if !errors.Is(err, filer_pb.ErrNotFound) && !errors.Is(err, ErrAttributeNotFound) {
|
||||
return err
|
||||
}
|
||||
if !h.authorizeAbsentTarget(r, &singleClient{client}, "ListViews", bucketName, namespaceName, "", "") {
|
||||
return ErrAccessDenied
|
||||
}
|
||||
return filer_pb.ErrNotFound
|
||||
}
|
||||
if err := json.Unmarshal(data, &nsMeta); err != nil {
|
||||
return err
|
||||
|
||||
Reference in new issue
Block a user