mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-09-19 22:44:18 +00:00
s3tables: hide inaccessible catalog resources (#11365)
* s3tables: hide inaccessible table buckets * s3tables: hide inaccessible namespaces * s3tables: hide inaccessible tables * s3tables: hide inaccessible resources in rename and namespace delete RenameTable/RenameView denied on the source now report the same not-found as a missing source, and the destination name conflict is checked only after destination authorization so a denied caller cannot distinguish an existing destination namespace or name from a missing one. DeleteNamespace denials use the same formatted message as a missing namespace.
This commit is contained in:
@@ -74,7 +74,7 @@ func (h *S3TablesHandler) handleGetTableBucket(w http.ResponseWriter, r *http.Re
|
||||
IdentityActions: identityActions,
|
||||
DefaultAllow: h.defaultAllowFor(r),
|
||||
}) {
|
||||
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to get table bucket details")
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName))
|
||||
return ErrAccessDenied
|
||||
}
|
||||
|
||||
@@ -333,7 +333,7 @@ func (h *S3TablesHandler) handleDeleteTableBucket(w http.ResponseWriter, r *http
|
||||
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName))
|
||||
} else if isAuthError(err) {
|
||||
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, err.Error())
|
||||
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 delete table bucket: %v", err))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
package s3tables
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3tables/s3tablestest"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
type s3TablesHTTPError struct {
|
||||
status int
|
||||
body S3TablesError
|
||||
}
|
||||
|
||||
func runUnauthorizedRequest(t *testing.T, m *Manager, fs *s3tablestest.MemFiler, operation string, input interface{}) s3TablesHTTPError {
|
||||
t.Helper()
|
||||
|
||||
body, err := json.Marshal(input)
|
||||
require.NoError(t, err)
|
||||
identity := &testIdentity{Name: "attacker", Account: &testIdentityAccount{Id: "attacker"}, Actions: []string{"Read"}}
|
||||
ctx := s3_constants.SetIdentityInContext(context.Background(), identity)
|
||||
req, err := newManagerRequest(ctx, operation, body, "attacker")
|
||||
require.NoError(t, err)
|
||||
|
||||
recorder := httptest.NewRecorder()
|
||||
m.handler.HandleRequest(recorder, req, NewManagerClient(fs.Client))
|
||||
result := recorder.Result()
|
||||
defer result.Body.Close()
|
||||
|
||||
var response S3TablesError
|
||||
require.NoError(t, json.NewDecoder(result.Body).Decode(&response))
|
||||
return s3TablesHTTPError{status: result.StatusCode, body: response}
|
||||
}
|
||||
|
||||
func TestTableBucketAuthorizationDenialMatchesMissing(t *testing.T) {
|
||||
existing, manager := startRenameManager(t)
|
||||
missing := s3tablestest.Start(t)
|
||||
manager.SetTrusted(false)
|
||||
manager.SetDefaultAllow(false)
|
||||
|
||||
for _, operation := range []string{"GetTableBucket", "DeleteTableBucket"} {
|
||||
t.Run(operation, func(t *testing.T) {
|
||||
var request interface{} = &GetTableBucketRequest{TableBucketARN: mustBucketARN(t)}
|
||||
if operation == "DeleteTableBucket" {
|
||||
request = &DeleteTableBucketRequest{TableBucketARN: mustBucketARN(t)}
|
||||
}
|
||||
|
||||
want := runUnauthorizedRequest(t, manager, missing, operation, request)
|
||||
got := runUnauthorizedRequest(t, manager, existing, operation, request)
|
||||
assert.Equal(t, want, got)
|
||||
assert.Equal(t, 404, got.status)
|
||||
assert.Equal(t, ErrCodeNoSuchBucket, got.body.Type)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNamespaceAuthorizationDenialMatchesMissing(t *testing.T) {
|
||||
existing, manager := startRenameManager(t)
|
||||
missing := s3tablestest.Start(t)
|
||||
manager.SetTrusted(false)
|
||||
manager.SetDefaultAllow(false)
|
||||
|
||||
for _, operation := range []string{"GetNamespace", "UpdateNamespace", "DeleteNamespace"} {
|
||||
t.Run(operation, func(t *testing.T) {
|
||||
var request interface{} = &GetNamespaceRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ns"}}
|
||||
switch operation {
|
||||
case "UpdateNamespace":
|
||||
request = &UpdateNamespaceRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ns"}}
|
||||
case "DeleteNamespace":
|
||||
request = &DeleteNamespaceRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ns"}}
|
||||
}
|
||||
|
||||
want := runUnauthorizedRequest(t, manager, missing, operation, request)
|
||||
got := runUnauthorizedRequest(t, manager, existing, operation, request)
|
||||
assert.Equal(t, want, got)
|
||||
assert.Equal(t, 404, got.status)
|
||||
assert.Equal(t, ErrCodeNoSuchNamespace, got.body.Type)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestTableAuthorizationDenialMatchesMissing(t *testing.T) {
|
||||
existing, manager := startRenameManager(t)
|
||||
missing := s3tablestest.Start(t)
|
||||
manager.SetTrusted(false)
|
||||
manager.SetDefaultAllow(false)
|
||||
|
||||
for _, operation := range []string{"GetTable", "UpdateTable", "DeleteTable", "RenameTable"} {
|
||||
t.Run(operation, func(t *testing.T) {
|
||||
var request interface{} = &GetTableRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ns"}, Name: "t"}
|
||||
switch operation {
|
||||
case "UpdateTable":
|
||||
request = &UpdateTableRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ns"}, Name: "t", VersionToken: "wrong"}
|
||||
case "DeleteTable":
|
||||
request = &DeleteTableRequest{TableBucketARN: mustBucketARN(t), Namespace: []string{"ns"}, Name: "t", VersionToken: "wrong"}
|
||||
case "RenameTable":
|
||||
request = &RenameTableRequest{TableBucketARN: mustBucketARN(t), SourceNamespace: []string{"ns"}, SourceName: "t", DestNamespace: []string{"ns"}, DestName: "t2"}
|
||||
}
|
||||
|
||||
want := runUnauthorizedRequest(t, manager, missing, operation, request)
|
||||
got := runUnauthorizedRequest(t, manager, existing, operation, request)
|
||||
assert.Equal(t, want, got)
|
||||
assert.Equal(t, 404, got.status)
|
||||
assert.Equal(t, ErrCodeNoSuchTable, got.body.Type)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeleteTableAuthorizedVersionMismatchStillConflicts(t *testing.T) {
|
||||
fs, manager := startRenameManager(t)
|
||||
err := manager.Execute(context.Background(), NewManagerClient(fs.Client), "DeleteTable", &DeleteTableRequest{
|
||||
TableBucketARN: mustBucketARN(t),
|
||||
Namespace: []string{"ns"},
|
||||
Name: "t",
|
||||
VersionToken: "wrong",
|
||||
}, nil, "")
|
||||
|
||||
require.Error(t, err)
|
||||
var s3Err *S3TablesError
|
||||
require.ErrorAs(t, err, &s3Err)
|
||||
assert.Equal(t, ErrCodeConflict, s3Err.Type)
|
||||
assert.NotNil(t, fs.Get(GetNamespacePath(renameTestBucket, "ns"), "t"))
|
||||
}
|
||||
@@ -261,7 +261,7 @@ func (h *S3TablesHandler) handleGetNamespace(w http.ResponseWriter, r *http.Requ
|
||||
IdentityActions: identityActions,
|
||||
DefaultAllow: h.defaultAllowFor(r),
|
||||
}) {
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, "namespace not found")
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, fmt.Sprintf("namespace %s not found", flattenNamespace(req.Namespace)))
|
||||
return ErrAccessDenied
|
||||
}
|
||||
|
||||
@@ -351,7 +351,7 @@ func (h *S3TablesHandler) handleUpdateNamespace(w http.ResponseWriter, r *http.R
|
||||
IdentityActions: identityActions,
|
||||
DefaultAllow: h.defaultAllowFor(r),
|
||||
}) {
|
||||
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to update namespace")
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, fmt.Sprintf("namespace %s not found", flattenNamespace(req.Namespace)))
|
||||
return ErrAccessDenied
|
||||
}
|
||||
|
||||
@@ -637,7 +637,7 @@ func (h *S3TablesHandler) handleDeleteNamespace(w http.ResponseWriter, r *http.R
|
||||
IdentityActions: identityActions,
|
||||
DefaultAllow: h.defaultAllowFor(r),
|
||||
}) {
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, "namespace not found")
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, fmt.Sprintf("namespace %s not found", flattenNamespace(req.Namespace)))
|
||||
return ErrAccessDenied
|
||||
}
|
||||
|
||||
|
||||
@@ -169,7 +169,9 @@ func TestRenameTableDestNamespaceMissing(t *testing.T) {
|
||||
}
|
||||
|
||||
// A principal allowed to rename the source must still be denied when it cannot
|
||||
// create a table in the destination namespace.
|
||||
// create a table in the destination namespace. The denial reports the same
|
||||
// not-found as a missing destination namespace so the caller cannot probe for
|
||||
// namespaces it may not touch.
|
||||
func TestRenameTableDestNamespaceUnauthorized(t *testing.T) {
|
||||
fs, m := startRenameManager(t)
|
||||
m.SetTrusted(false)
|
||||
@@ -204,7 +206,7 @@ func TestRenameTableDestNamespaceUnauthorized(t *testing.T) {
|
||||
require.Error(t, err)
|
||||
var s3Err *S3TablesError
|
||||
require.ErrorAs(t, err, &s3Err)
|
||||
assert.Equal(t, ErrCodeAccessDenied, s3Err.Type)
|
||||
assert.Equal(t, ErrCodeNoSuchNamespace, s3Err.Type)
|
||||
|
||||
assert.NotNil(t, fs.Get(GetNamespacePath(renameTestBucket, "ns"), "t"), "source must be untouched")
|
||||
assert.Nil(t, fs.Get(GetNamespacePath(renameTestBucket, "dest"), "t2"), "destination must not be written")
|
||||
|
||||
@@ -898,12 +898,6 @@ func (h *S3TablesHandler) handleDeleteTable(w http.ResponseWriter, r *http.Reque
|
||||
return fmt.Errorf("failed to unmarshal table metadata: %w", err)
|
||||
}
|
||||
|
||||
if req.VersionToken != "" {
|
||||
if metadata.VersionToken != req.VersionToken {
|
||||
return ErrVersionTokenMismatch
|
||||
}
|
||||
}
|
||||
|
||||
// Fetch table policy if it exists
|
||||
policyData, err := h.getExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyPolicy)
|
||||
if err != nil {
|
||||
@@ -980,9 +974,13 @@ func (h *S3TablesHandler) handleDeleteTable(w http.ResponseWriter, r *http.Reque
|
||||
DefaultAllow: h.defaultAllowFor(r),
|
||||
})
|
||||
if !tableAllowed && !bucketAllowed {
|
||||
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to delete table")
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchTable, fmt.Sprintf("table %s not found", tableName))
|
||||
return NewAuthError("DeleteTable", principal, "not authorized to delete table")
|
||||
}
|
||||
if req.VersionToken != "" && metadata.VersionToken != req.VersionToken {
|
||||
h.writeError(w, http.StatusConflict, ErrCodeConflict, "version token mismatch")
|
||||
return ErrVersionTokenMismatch
|
||||
}
|
||||
|
||||
// Delete the table
|
||||
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
||||
@@ -1268,14 +1266,17 @@ func (h *S3TablesHandler) renameCatalogEntry(w http.ResponseWriter, r *http.Requ
|
||||
DefaultAllow: h.defaultAllowFor(r),
|
||||
})
|
||||
if !tableAllowed && !bucketAllowed {
|
||||
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to rename "+kind.noun)
|
||||
h.writeError(w, http.StatusNotFound, kind.notFoundCode, fmt.Sprintf("%s %s not found", kind.noun, srcName))
|
||||
return NewAuthError(kind.renameOp, principal, "not authorized to rename "+kind.noun)
|
||||
}
|
||||
|
||||
// Require the destination namespace to exist and the destination table to be free.
|
||||
// Require the destination namespace to exist. Whether the destination name
|
||||
// is taken is only recorded here; reporting the conflict before the
|
||||
// destination authorization check would disclose it to denied callers.
|
||||
destNamespacePath := GetNamespacePath(bucketName, destNamespace)
|
||||
var destNamespaceMetadata namespaceMetadata
|
||||
var destNamespacePolicy string
|
||||
var destExists bool
|
||||
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
|
||||
data, err := h.getExtendedAttribute(r.Context(), client, destNamespacePath, ExtendedKeyMetadata)
|
||||
if err != nil {
|
||||
@@ -1291,7 +1292,7 @@ func (h *S3TablesHandler) renameCatalogEntry(w http.ResponseWriter, r *http.Requ
|
||||
return fmt.Errorf("failed to fetch destination namespace policy: %w", err)
|
||||
}
|
||||
if _, err := h.getExtendedAttribute(r.Context(), client, destPath, ExtendedKeyMetadata); err == nil {
|
||||
return ErrTableAlreadyExists
|
||||
destExists = true
|
||||
} else if !errors.Is(err, filer_pb.ErrNotFound) && !errors.Is(err, ErrAttributeNotFound) {
|
||||
return err
|
||||
}
|
||||
@@ -1299,9 +1300,7 @@ func (h *S3TablesHandler) renameCatalogEntry(w http.ResponseWriter, r *http.Requ
|
||||
})
|
||||
|
||||
if err != nil {
|
||||
if errors.Is(err, ErrTableAlreadyExists) {
|
||||
h.writeError(w, http.StatusConflict, kind.existsCode, fmt.Sprintf("%s %s already exists", kind.noun, destName))
|
||||
} else if errors.Is(err, filer_pb.ErrNotFound) {
|
||||
if errors.Is(err, filer_pb.ErrNotFound) {
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, fmt.Sprintf("namespace %s not found", destNamespace))
|
||||
} else {
|
||||
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to check destination: %v", err))
|
||||
@@ -1311,7 +1310,8 @@ func (h *S3TablesHandler) renameCatalogEntry(w http.ResponseWriter, r *http.Requ
|
||||
|
||||
// Renaming places the table into the destination namespace, so the principal
|
||||
// must also be allowed to create a table there (the source check alone lets a
|
||||
// caller move tables into namespaces they don't control).
|
||||
// caller move tables into namespaces they don't control). Denials report the
|
||||
// same not-found as a missing destination namespace.
|
||||
destNamespaceAllowed := CheckPermissionWithContext(kind.createOp, principal, destNamespaceMetadata.OwnerAccountID, destNamespacePolicy, bucketARN, &PolicyContext{
|
||||
TableBucketName: bucketName,
|
||||
Namespace: destNamespace,
|
||||
@@ -1329,10 +1329,15 @@ func (h *S3TablesHandler) renameCatalogEntry(w http.ResponseWriter, r *http.Requ
|
||||
DefaultAllow: h.defaultAllowFor(r),
|
||||
})
|
||||
if !destNamespaceAllowed && !destBucketAllowed {
|
||||
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to create "+kind.noun+" in the destination namespace")
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, fmt.Sprintf("namespace %s not found", destNamespace))
|
||||
return NewAuthError(kind.renameOp, principal, "not authorized to create "+kind.noun+" in the destination namespace")
|
||||
}
|
||||
|
||||
if destExists {
|
||||
h.writeError(w, http.StatusConflict, kind.existsCode, fmt.Sprintf("%s %s already exists", kind.noun, destName))
|
||||
return ErrTableAlreadyExists
|
||||
}
|
||||
|
||||
metadata.Name = destName
|
||||
metadata.Namespace = destNamespace
|
||||
metadata.ModifiedAt = time.Now()
|
||||
@@ -1538,7 +1543,7 @@ func (h *S3TablesHandler) handleUpdateTable(w http.ResponseWriter, r *http.Reque
|
||||
})
|
||||
|
||||
if !tableAllowed && !bucketAllowed {
|
||||
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to update table")
|
||||
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchTable, "table not found")
|
||||
return NewAuthError("UpdateTable", principal, "not authorized to update table")
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user