diff --git a/weed/s3api/s3tables/handler.go b/weed/s3api/s3tables/handler.go index 608771a037..f5f4cc51cb 100644 --- a/weed/s3api/s3tables/handler.go +++ b/weed/s3api/s3tables/handler.go @@ -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 { diff --git a/weed/s3api/s3tables/handler_bucket_get_list_delete.go b/weed/s3api/s3tables/handler_bucket_get_list_delete.go index ce1e8147b1..1645194ca5 100644 --- a/weed/s3api/s3tables/handler_bucket_get_list_delete.go +++ b/weed/s3api/s3tables/handler_bucket_get_list_delete.go @@ -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)) diff --git a/weed/s3api/s3tables/handler_existence_hiding_test.go b/weed/s3api/s3tables/handler_existence_hiding_test.go index dc04e09423..cc1e051777 100644 --- a/weed/s3api/s3tables/handler_existence_hiding_test.go +++ b/weed/s3api/s3tables/handler_existence_hiding_test.go @@ -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) +} diff --git a/weed/s3api/s3tables/handler_namespace.go b/weed/s3api/s3tables/handler_namespace.go index 8e6a7ee6d5..613a85045d 100644 --- a/weed/s3api/s3tables/handler_namespace.go +++ b/weed/s3api/s3tables/handler_namespace.go @@ -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)) diff --git a/weed/s3api/s3tables/handler_policy.go b/weed/s3api/s3tables/handler_policy.go index bea4f3fc19..c152b209a9 100644 --- a/weed/s3api/s3tables/handler_policy.go +++ b/weed/s3api/s3tables/handler_policy.go @@ -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 diff --git a/weed/s3api/s3tables/handler_table.go b/weed/s3api/s3tables/handler_table.go index b9ce7d8834..5ee6f01391 100644 --- a/weed/s3api/s3tables/handler_table.go +++ b/weed/s3api/s3tables/handler_table.go @@ -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) diff --git a/weed/s3api/s3tables/handler_view.go b/weed/s3api/s3tables/handler_view.go index d3327783f8..57bad00c5c 100644 --- a/weed/s3api/s3tables/handler_view.go +++ b/weed/s3api/s3tables/handler_view.go @@ -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