diff --git a/weed/iam/sts/parent_user_test.go b/weed/iam/sts/parent_user_test.go index 128186fb50..bae1258783 100644 --- a/weed/iam/sts/parent_user_test.go +++ b/weed/iam/sts/parent_user_test.go @@ -60,6 +60,17 @@ func TestComputeParentUserEncoding(t *testing.T) { } } +func TestIsParentUserID(t *testing.T) { + if !IsParentUserID(ComputeParentUser("alice", "https://idp.example/")) { + t.Fatal("a ComputeParentUser result must be recognized") + } + for _, s := range []string{"", "alice", "admin", "arn:aws:iam::123:user/admin", "dGVzdA=="} { + if IsParentUserID(s) { + t.Fatalf("%q is not a scoped parent user id", s) + } + } +} + func TestSessionClaimsRoundTripParentUser(t *testing.T) { parent := ComputeParentUser("alice", "https://idp.example/") claims := NewSTSSessionClaims("sid-1", "issuer", time.Now().Add(time.Hour)). diff --git a/weed/iam/sts/session_claims.go b/weed/iam/sts/session_claims.go index 376dcd9118..8a16d55273 100644 --- a/weed/iam/sts/session_claims.go +++ b/weed/iam/sts/session_claims.go @@ -24,6 +24,14 @@ func ComputeParentUser(sub, iss string) string { return base64.RawURLEncoding.EncodeToString(h[:]) } +// IsParentUserID reports whether s has the shape ComputeParentUser produces — +// a base64url-encoded SHA-256 — i.e. an already issuer-scoped id rather than a +// bare subject string. +func IsParentUserID(s string) bool { + b, err := base64.RawURLEncoding.DecodeString(s) + return err == nil && len(b) == sha256.Size +} + // identityClaimPriority is the order in which ResolveIdentityClaim looks for a // human-readable, authoritative identity attribute in an STS request context. // The context is populated at federation time from the validated OIDC token diff --git a/weed/s3api/auth_account_collapse_test.go b/weed/s3api/auth_account_collapse_test.go index 141c6440d4..b2b741a7c2 100644 --- a/weed/s3api/auth_account_collapse_test.go +++ b/weed/s3api/auth_account_collapse_test.go @@ -290,3 +290,32 @@ func TestDeclaredAccountOutranksInlineIdentityAccount(t *testing.T) { assert.Equal(t, "100000000001", iam.GetAccountIdByEmail("alice@example.com"), "the declared email must stay indexed") assert.Empty(t, iam.GetAccountIdByEmail("wrong@example.com"), "an inline block must not claim an email for a declared account") } + +// Stored owner strings that name a configured identity or account were never +// OIDC subjects; recognizing them keeps issuer re-anchoring away from local +// principals. +func TestIsConfiguredAccountID(t *testing.T) { + resetMemoryStore() + + config := `{ + "identities": [ + {"name": "alice", "credentials": [{"accessKey": "alice_ak", "secretKey": "alice_sk"}], "actions": ["Read"]}, + {"name": "bob", "account": {"id": "100000000002"}, "credentials": [{"accessKey": "bob_ak", "secretKey": "bob_sk"}], "actions": ["Read"]} + ] +}` + tmp, err := os.CreateTemp("", "s3-config-*.json") + require.NoError(t, err) + defer os.Remove(tmp.Name()) + _, err = tmp.WriteString(config) + require.NoError(t, err) + require.NoError(t, tmp.Close()) + + iam := NewIdentityAccessManagementWithStore(&S3ApiServerOption{Config: tmp.Name()}, nil, "memory") + + for _, id := range []string{"admin", "anonymous", "alice", "bob", "100000000002"} { + assert.True(t, iam.isConfiguredAccountID(id), "%q is a configured local account", id) + } + for _, id := range []string{"mallory", "alice-sub", "000000000042"} { + assert.False(t, iam.isConfiguredAccountID(id), "%q is not a configured local account", id) + } +} diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index f1f520c8bb..b8d611ce4e 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -1645,6 +1645,29 @@ func (iam *IdentityAccessManagement) GetAccountIdByEmail(email string) string { return "" } +// isConfiguredAccountID reports whether id names a configured local identity +// or account — the reserved accounts included — i.e. an owner string that was +// never an OIDC subject. +func (iam *IdentityAccessManagement) isConfiguredAccountID(id string) bool { + if id == "" || id == s3_constants.AccountAdminId || id == s3_constants.AccountAnonymousId { + return true + } + iam.m.RLock() + defer iam.m.RUnlock() + if _, ok := iam.nameToIdentity[id]; ok { + return true + } + if _, ok := iam.accounts[id]; ok { + return true + } + for _, identity := range iam.identities { + if identity.Account != nil && identity.Account.Id == id { + return true + } + } + return false +} + func (iam *IdentityAccessManagement) Auth(f http.HandlerFunc, action Action) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if !iam.isEnabled() { diff --git a/weed/s3api/s3api_tables.go b/weed/s3api/s3api_tables.go index f64e315b13..5a382d0f3f 100644 --- a/weed/s3api/s3api_tables.go +++ b/weed/s3api/s3api_tables.go @@ -52,6 +52,12 @@ func (st *S3TablesApiServer) SetIAMAuthorizer(authorizer s3tables.IAMAuthorizer) st.handler.SetIAMAuthorizer(authorizer) } +// SetLocalAccountChecker injects the predicate recognizing configured local +// account IDs for S3 Tables legacy-owner checks. +func (st *S3TablesApiServer) SetLocalAccountChecker(fn func(string) bool) { + st.handler.SetLocalAccountChecker(fn) +} + // S3TablesHandler handles S3 Tables API requests func (st *S3TablesApiServer) S3TablesHandler(w http.ResponseWriter, r *http.Request) { st.handler.HandleRequest(w, r, st) @@ -66,6 +72,9 @@ func (st *S3TablesApiServer) WithFilerClient(streamingMode bool, fn func(filer_p func (s3a *S3ApiServer) registerS3TablesRoutes(router *mux.Router) { // Create S3 Tables handler s3TablesApi := NewS3TablesApiServer(s3a) + if s3a.iam != nil { + s3TablesApi.SetLocalAccountChecker(s3a.iam.isConfiguredAccountID) + } if s3a.iam != nil && s3a.iam.iamIntegration != nil { s3TablesApi.SetDefaultAllow(s3a.iam.iamIntegration.DefaultAllow()) if s3Integration, ok := s3a.iam.iamIntegration.(*S3IAMIntegration); ok && s3Integration.iamManager != nil { diff --git a/weed/s3api/s3tables/handler.go b/weed/s3api/s3tables/handler.go index 73eb2381b5..608771a037 100644 --- a/weed/s3api/s3tables/handler.go +++ b/weed/s3api/s3tables/handler.go @@ -57,11 +57,12 @@ const ( // S3TablesHandler handles S3 Tables API requests type S3TablesHandler struct { - region string - accountID string - defaultAllow bool // Whether to allow access by default (for zero-config IAM) - trusted bool // Trusted local tooling (shell/admin) bypasses authorization - iamAuthorizer IAMAuthorizer + region string + accountID string + defaultAllow bool // Whether to allow access by default (for zero-config IAM) + trusted bool // Trusted local tooling (shell/admin) bypasses authorization + iamAuthorizer IAMAuthorizer + localAccountIDs func(string) bool } // NewS3TablesHandler creates a new S3 Tables handler @@ -98,6 +99,13 @@ func (h *S3TablesHandler) SetTrusted(trusted bool) { h.trusted = trusted } +// SetLocalAccountChecker injects the predicate that recognizes account IDs of +// configured local identities, so a stored owner that names one is never +// re-anchored to an OIDC subject. +func (h *S3TablesHandler) SetLocalAccountChecker(fn func(string) bool) { + h.localAccountIDs = fn +} + // FilerClient interface for filer operations type FilerClient interface { WithFilerClient(streamingMode bool, fn func(client filer_pb.SeaweedFilerClient) error) error diff --git a/weed/s3api/s3tables/handler_bucket_create.go b/weed/s3api/s3tables/handler_bucket_create.go index 5191db7309..acf34182b5 100644 --- a/weed/s3api/s3tables/handler_bucket_create.go +++ b/weed/s3api/s3tables/handler_bucket_create.go @@ -65,9 +65,10 @@ func (h *S3TablesHandler) handleCreateTableBucket(w http.ResponseWriter, r *http owner = DefaultAccountID } if !CheckPermissionWithContext("CreateTableBucket", principal, owner, "", "", &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to create table buckets") return NewAuthError("CreateTableBucket", principal, "not authorized to create table buckets") diff --git a/weed/s3api/s3tables/handler_bucket_get_list_delete.go b/weed/s3api/s3tables/handler_bucket_get_list_delete.go index 57fb01cf0a..ce1e8147b1 100644 --- a/weed/s3api/s3tables/handler_bucket_get_list_delete.go +++ b/weed/s3api/s3tables/handler_bucket_get_list_delete.go @@ -70,10 +70,11 @@ func (h *S3TablesHandler) handleGetTableBucket(w http.ResponseWriter, r *http.Re principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("GetTableBucket", principal, metadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName)) return ErrAccessDenied @@ -193,10 +194,11 @@ func (h *S3TablesHandler) handleListTableBuckets(w http.ResponseWriter, r *http. bucketARN := h.generateTableBucketARN(metadata.OwnerAccountID, entry.Entry.Name) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("GetTableBucket", accountID, metadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: entry.Entry.Name, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: entry.Entry.Name, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { continue } @@ -298,10 +300,11 @@ func (h *S3TablesHandler) handleDeleteTableBucket(w http.ResponseWriter, r *http principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("DeleteTableBucket", principal, metadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { return NewAuthError("DeleteTableBucket", principal, fmt.Sprintf("not authorized to delete bucket %s", bucketName)) } diff --git a/weed/s3api/s3tables/handler_maintenance.go b/weed/s3api/s3tables/handler_maintenance.go index 1bbe57b941..0d80bafdce 100644 --- a/weed/s3api/s3tables/handler_maintenance.go +++ b/weed/s3api/s3tables/handler_maintenance.go @@ -349,10 +349,11 @@ func (h *S3TablesHandler) authorizeMaintenanceBucket(r *http.Request, filerClien bucketARN := h.generateTableBucketARN(bucketMetadata.OwnerAccountID, bucketName) principal := h.getAccountID(r) if !CheckPermissionWithContext(operation, principal, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - IdentityActions: getIdentityActions(r), - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + IdentityActions: getIdentityActions(r), + DefaultAllow: h.defaultAllowFor(r), }) { return "", NewAuthError(operation, principal, "not authorized to "+operation) } @@ -386,12 +387,13 @@ func (h *S3TablesHandler) authorizeMaintenanceTable(r *http.Request, filerClient tableARN := h.generateTableARN(metadata.OwnerAccountID, bucketName, namespaceName+"/"+tableName) principal := h.getAccountID(r) if !CheckPermissionWithContext(operation, principal, metadata.OwnerAccountID, bucketPolicy, tableARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespaceName, - TableName: tableName, - IdentityActions: getIdentityActions(r), - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespaceName, + TableName: tableName, + IdentityActions: getIdentityActions(r), + DefaultAllow: h.defaultAllowFor(r), }) { return "", NewAuthError(operation, principal, "not authorized to "+operation) } diff --git a/weed/s3api/s3tables/handler_namespace.go b/weed/s3api/s3tables/handler_namespace.go index ea54c2eded..d3b38dc734 100644 --- a/weed/s3api/s3tables/handler_namespace.go +++ b/weed/s3api/s3tables/handler_namespace.go @@ -114,12 +114,13 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R identityActions := getIdentityActions(r) glog.Infof("S3Tables: CreateNamespace permission check - principal=%s, owner=%s, actions=%v", principal, bucketMetadata.OwnerAccountID, identityActions) if !CheckPermissionWithContext("CreateNamespace", principal, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespaceName, - TableBucketTags: bucketTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespaceName, + TableBucketTags: bucketTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { glog.Infof("S3Tables: Permission denied for CreateNamespace - principal=%s, owner=%s", principal, bucketMetadata.OwnerAccountID) h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to create namespace in this bucket") @@ -256,12 +257,13 @@ func (h *S3TablesHandler) handleGetNamespace(w http.ResponseWriter, r *http.Requ principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("GetNamespace", principal, metadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespaceName, - TableBucketTags: bucketTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespaceName, + TableBucketTags: bucketTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, fmt.Sprintf("namespace %s not found", flattenNamespace(req.Namespace))) return ErrAccessDenied @@ -347,12 +349,13 @@ func (h *S3TablesHandler) handleUpdateNamespace(w http.ResponseWriter, r *http.R principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("UpdateNamespace", principal, metadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespaceName, - TableBucketTags: bucketTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespaceName, + TableBucketTags: bucketTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, fmt.Sprintf("namespace %s not found", flattenNamespace(req.Namespace))) return ErrAccessDenied @@ -450,11 +453,12 @@ func (h *S3TablesHandler) handleListNamespaces(w http.ResponseWriter, r *http.Re principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("ListNamespaces", principal, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - TableBucketTags: bucketTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + TableBucketTags: bucketTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName)) return ErrAccessDenied @@ -635,12 +639,13 @@ func (h *S3TablesHandler) handleDeleteNamespace(w http.ResponseWriter, r *http.R principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("DeleteNamespace", principal, metadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespaceName, - TableBucketTags: bucketTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespaceName, + TableBucketTags: bucketTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, fmt.Sprintf("namespace %s not found", flattenNamespace(req.Namespace))) return ErrAccessDenied diff --git a/weed/s3api/s3tables/handler_policy.go b/weed/s3api/s3tables/handler_policy.go index 0f1aed19dd..3ebd71e411 100644 --- a/weed/s3api/s3tables/handler_policy.go +++ b/weed/s3api/s3tables/handler_policy.go @@ -92,10 +92,11 @@ func (h *S3TablesHandler) handlePutTableBucketPolicy(w http.ResponseWriter, r *h principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("PutTableBucketPolicy", principal, bucketMetadata.OwnerAccountID, "", bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to put table bucket policy") return NewAuthError("PutTableBucketPolicy", principal, "not authorized to put table bucket policy") @@ -171,10 +172,11 @@ func (h *S3TablesHandler) handleGetTableBucketPolicy(w http.ResponseWriter, r *h principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("GetTableBucketPolicy", principal, bucketMetadata.OwnerAccountID, string(policy), bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to get table bucket policy") return NewAuthError("GetTableBucketPolicy", principal, "not authorized to get table bucket policy") @@ -248,10 +250,11 @@ func (h *S3TablesHandler) handleDeleteTableBucketPolicy(w http.ResponseWriter, r principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("DeleteTableBucketPolicy", principal, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to delete table bucket policy") return NewAuthError("DeleteTableBucketPolicy", principal, "not authorized to delete table bucket policy") @@ -348,12 +351,13 @@ func (h *S3TablesHandler) handlePutTablePolicy(w http.ResponseWriter, r *http.Re principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("PutTablePolicy", principal, metadata.OwnerAccountID, bucketPolicy, tableARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespaceName, - TableName: tableName, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespaceName, + TableName: tableName, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to put table policy") return NewAuthError("PutTablePolicy", principal, "not authorized to put table policy") @@ -457,12 +461,13 @@ func (h *S3TablesHandler) handleGetTablePolicy(w http.ResponseWriter, r *http.Re principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("GetTablePolicy", principal, metadata.OwnerAccountID, bucketPolicy, tableARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespaceName, - TableName: tableName, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespaceName, + TableName: tableName, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to get table policy") return NewAuthError("GetTablePolicy", principal, "not authorized to get table policy") @@ -548,12 +553,13 @@ func (h *S3TablesHandler) handleDeleteTablePolicy(w http.ResponseWriter, r *http principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("DeleteTablePolicy", principal, metadata.OwnerAccountID, bucketPolicy, tableARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespaceName, - TableName: tableName, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespaceName, + TableName: tableName, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to delete table policy") return NewAuthError("DeleteTablePolicy", principal, "not authorized to delete table policy") @@ -646,14 +652,15 @@ func (h *S3TablesHandler) handleTagResource(w http.ResponseWriter, r *http.Reque principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("TagResource", principal, ownerAccountID, bucketPolicy, resourceARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - TableBucketTags: bucketTags, - RequestTags: req.Tags, - TagKeys: requestTagKeys, - ResourceTags: existingTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + TableBucketTags: bucketTags, + RequestTags: req.Tags, + TagKeys: requestTagKeys, + ResourceTags: existingTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { return NewAuthError("TagResource", principal, "not authorized to tag resource") } @@ -767,12 +774,13 @@ func (h *S3TablesHandler) handleListTagsForResource(w http.ResponseWriter, r *ht principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("ListTagsForResource", principal, ownerAccountID, bucketPolicy, resourceARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - TableBucketTags: bucketTags, - ResourceTags: tags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + TableBucketTags: bucketTags, + ResourceTags: tags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { return NewAuthError("ListTagsForResource", principal, "not authorized to list tags for resource") } @@ -875,13 +883,14 @@ func (h *S3TablesHandler) handleUntagResource(w http.ResponseWriter, r *http.Req principal := h.getAccountID(r) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("UntagResource", principal, ownerAccountID, bucketPolicy, resourceARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - TableBucketTags: bucketTags, - TagKeys: req.TagKeys, - ResourceTags: tags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + TableBucketTags: bucketTags, + TagKeys: req.TagKeys, + ResourceTags: tags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { return NewAuthError("UntagResource", principal, "not authorized to untag resource") } diff --git a/weed/s3api/s3tables/handler_table.go b/weed/s3api/s3tables/handler_table.go index 8111480f3a..7091dbda59 100644 --- a/weed/s3api/s3tables/handler_table.go +++ b/weed/s3api/s3tables/handler_table.go @@ -422,14 +422,15 @@ func (h *S3TablesHandler) handleGetTable(w http.ResponseWriter, r *http.Request, allowed := CheckTableOrBucketPermission("GetTable", accountID, metadata.OwnerAccountID, tablePolicy, tableARN, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespace, - TableName: tableName, - TableBucketTags: bucketTags, - ResourceTags: tableTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespace, + TableName: tableName, + TableBucketTags: bucketTags, + ResourceTags: tableTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) if !allowed { @@ -557,12 +558,13 @@ func (h *S3TablesHandler) handleListTables(w http.ResponseWriter, r *http.Reques if !CheckTableOrBucketPermission("ListTables", accountID, nsMeta.OwnerAccountID, namespacePolicy, bucketARN, bucketMeta.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespaceName, - TableBucketTags: bucketTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespaceName, + TableBucketTags: bucketTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { return ErrAccessDenied } @@ -603,11 +605,12 @@ func (h *S3TablesHandler) handleListTables(w http.ResponseWriter, r *http.Reques bucketARN := h.generateTableBucketARN(bucketMeta.OwnerAccountID, bucketName) identityActions := getIdentityActions(r) if !CheckPermissionWithContext("ListTables", accountID, bucketMeta.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - TableBucketTags: bucketTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + TableBucketTags: bucketTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { return ErrAccessDenied } @@ -943,14 +946,15 @@ func (h *S3TablesHandler) handleDeleteTable(w http.ResponseWriter, r *http.Reque allowed := CheckTableOrBucketPermission("DeleteTable", principal, metadata.OwnerAccountID, tablePolicy, tableARN, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespaceName, - TableName: tableName, - TableBucketTags: bucketTags, - ResourceTags: tableTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespaceName, + TableName: tableName, + TableBucketTags: bucketTags, + ResourceTags: tableTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) if !allowed { if h.canReadEntry("GetTable", principal, metadata.OwnerAccountID, bucketMetadata.OwnerAccountID, tablePolicy, bucketPolicy, tableARN, bucketARN, bucketName, namespaceName, tableName, bucketTags, tableTags, identityActions, r) { @@ -1115,14 +1119,15 @@ func (h *S3TablesHandler) readTableMetadata(ctx context.Context, client filer_pb // cannot see the entry keeps the not-found veil. func (h *S3TablesHandler) canReadEntry(readOp, principal, tableOwner, bucketOwner, tablePolicy, bucketPolicy, tableARN, bucketARN, bucketName, namespace, name string, bucketTags, tableTags map[string]string, identityActions []string, r *http.Request) bool { ctx := &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespace, - TableName: name, - TableBucketTags: bucketTags, - ResourceTags: tableTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespace, + TableName: name, + TableBucketTags: bucketTags, + ResourceTags: tableTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), } return CheckTableOrBucketPermission(readOp, principal, tableOwner, tablePolicy, tableARN, @@ -1300,14 +1305,15 @@ func (h *S3TablesHandler) renameCatalogEntry(w http.ResponseWriter, r *http.Requ allowed := CheckTableOrBucketPermission(kind.renameOp, principal, metadata.OwnerAccountID, tablePolicy, tableARN, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: srcNamespace, - TableName: srcName, - TableBucketTags: bucketTags, - ResourceTags: tableTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: srcNamespace, + TableName: srcName, + TableBucketTags: bucketTags, + ResourceTags: tableTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) if !allowed { readTags := tableTags @@ -1367,13 +1373,14 @@ func (h *S3TablesHandler) renameCatalogEntry(w http.ResponseWriter, r *http.Requ if !CheckTableOrBucketPermission(kind.createOp, principal, destNamespaceMetadata.OwnerAccountID, destNamespacePolicy, bucketARN, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: destNamespace, - TableName: destName, - TableBucketTags: bucketTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: destNamespace, + TableName: destName, + TableBucketTags: bucketTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) { 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") @@ -1572,14 +1579,15 @@ func (h *S3TablesHandler) handleUpdateTable(w http.ResponseWriter, r *http.Reque allowed := CheckTableOrBucketPermission("UpdateTable", principal, metadata.OwnerAccountID, tablePolicy, tableARN, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespaceName, - TableName: tableName, - TableBucketTags: bucketTags, - ResourceTags: tableTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespaceName, + TableName: tableName, + TableBucketTags: bucketTags, + ResourceTags: tableTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), }) if !allowed { @@ -1777,15 +1785,16 @@ func (h *S3TablesHandler) authorizeCreateTable(w http.ResponseWriter, r *http.Re bucketARN := h.generateTableBucketARN(bucketMetadata.OwnerAccountID, bucketName) identityActions := getIdentityActions(r) policyContext := &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespaceName, - TableName: tableName, - RequestTags: requestTags, - TagKeys: mapKeys(requestTags), - TableBucketTags: bucketTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespaceName, + TableName: tableName, + RequestTags: requestTags, + TagKeys: mapKeys(requestTags), + TableBucketTags: bucketTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), } if !CheckTableOrBucketPermission("CreateTable", accountID, namespaceMetadata.OwnerAccountID, namespacePolicy, bucketARN, diff --git a/weed/s3api/s3tables/handler_view.go b/weed/s3api/s3tables/handler_view.go index 7cea342da1..3d2c98e6ab 100644 --- a/weed/s3api/s3tables/handler_view.go +++ b/weed/s3api/s3tables/handler_view.go @@ -635,13 +635,14 @@ func (h *S3TablesHandler) loadAuthContext(r *http.Request, filerClient FilerClie func (h *S3TablesHandler) authorizeViewOp(r *http.Request, operation, principal, resourceOwner, bucketOwner, resourcePolicy, bucketPolicy, resourceARN, bucketName, namespace, viewName string, bucketTags map[string]string) bool { identityActions := getIdentityActions(r) ctx := &PolicyContext{ - TrustedIssuers: h.trustedIssuers(r), - TableBucketName: bucketName, - Namespace: namespace, - TableName: viewName, - TableBucketTags: bucketTags, - IdentityActions: identityActions, - DefaultAllow: h.defaultAllowFor(r), + TrustedIssuers: h.trustedIssuers(r), + IsLocalAccountID: h.localAccountIDs, + TableBucketName: bucketName, + Namespace: namespace, + TableName: viewName, + TableBucketTags: bucketTags, + IdentityActions: identityActions, + DefaultAllow: h.defaultAllowFor(r), } bucketARN := h.generateTableBucketARN(bucketOwner, bucketName) return CheckTableOrBucketPermission(operation, principal, diff --git a/weed/s3api/s3tables/permissions.go b/weed/s3api/s3tables/permissions.go index ee81400db3..bbe458875e 100644 --- a/weed/s3api/s3tables/permissions.go +++ b/weed/s3api/s3tables/permissions.go @@ -94,6 +94,11 @@ type PolicyContext struct { // account IDs. A bare subject is only resolvable when exactly one issuer // is configured; more than one leaves it ambiguous, so it must not match. TrustedIssuers []string + // IsLocalAccountID reports whether an account ID belongs to a configured + // local identity. Such owner strings were never OIDC subjects and must not + // be re-anchored; a nil predicate exempts nothing beyond the built-in + // reserved, ARN, and scoped-id checks. + IsLocalAccountID func(string) bool } // CheckPermissionWithContext checks permission with optional resource and condition context. @@ -295,8 +300,21 @@ func matchesCaller(principalSpec interface{}, principal string, ctx *PolicyConte if matchesPrincipal(principalSpec, principal) { return true } - legacy, ok := principalSpec.(string) - return ok && matchesLegacySubject(legacy, principal, ctx) + switch p := principalSpec.(type) { + case string: + return matchesLegacySubject(p, principal, ctx) + case []interface{}: + for _, item := range p { + if str, ok := item.(string); ok && matchesLegacySubject(str, principal, ctx) { + return true + } + } + case map[string]interface{}: + if aws, ok := p["AWS"]; ok { + return matchesCaller(aws, principal, ctx) + } + } + return false } // matchesLegacySubject reports whether a bare subject stored before issuer @@ -309,6 +327,17 @@ func matchesLegacySubject(legacy, principal string, ctx *PolicyContext) bool { if ctx == nil || len(ctx.TrustedIssuers) != 1 { return false } + // The reserved accounts, ARN values, and already issuer-scoped ids are never + // OIDC subjects — or, for strings that could be either a legacy subject or a + // scoped id, resolving to scoped fails closed. A configured local account + // id is excluded the same way once the caller supplies the predicate. Any + // of them could otherwise be claimed by a token asserting that string as + // sub — including the admin account or a victim's scoped id. + if legacy == s3_constants.AccountAdminId || legacy == s3_constants.AccountAnonymousId || + strings.HasPrefix(legacy, "arn:") || sts.IsParentUserID(legacy) || + (ctx.IsLocalAccountID != nil && ctx.IsLocalAccountID(legacy)) { + return false + } return sts.ComputeParentUser(legacy, ctx.TrustedIssuers[0]) == principal } diff --git a/weed/s3api/s3tables/permissions_test.go b/weed/s3api/s3tables/permissions_test.go index 25dfaf7659..62f67deabd 100644 --- a/weed/s3api/s3tables/permissions_test.go +++ b/weed/s3api/s3tables/permissions_test.go @@ -334,26 +334,97 @@ func TestCheckTableOrBucketPermissionOwnerlessFailsClosed(t *testing.T) { func TestCheckPermissionLegacyOwnerScopedToIssuer(t *testing.T) { owner := "alice-sub" principal := sts.ComputeParentUser(owner, "corp-idp") + noLocal := &PolicyContext{ + TrustedIssuers: []string{"corp-idp"}, + IsLocalAccountID: func(string) bool { return false }, + } - if !CheckPermissionWithContext("GetTable", principal, owner, "", "arn", &PolicyContext{ - TrustedIssuers: []string{"corp-idp"}, - }) { + if !CheckPermissionWithContext("GetTable", principal, owner, "", "arn", noLocal) { t.Error("single-issuer deployments must keep recognizing the legacy owner") } if CheckPermissionWithContext("GetTable", principal, owner, "", "arn", &PolicyContext{ - TrustedIssuers: []string{"corp-idp", "evil-idp"}, + TrustedIssuers: []string{"corp-idp", "evil-idp"}, + IsLocalAccountID: func(string) bool { return false }, }) { t.Error("a bare subject is ambiguous across multiple issuers and must not match") } if CheckPermissionWithContext("GetTable", sts.ComputeParentUser(owner, "evil-idp"), owner, "", "arn", &PolicyContext{ - TrustedIssuers: []string{"corp-idp", "evil-idp"}, + TrustedIssuers: []string{"corp-idp", "evil-idp"}, + IsLocalAccountID: func(string) bool { return false }, }) { t.Error("a same-sub caller from another issuer must not claim the legacy owner") } - if CheckPermissionWithContext("GetTable", principal, owner, "", "arn", &PolicyContext{}) { + if CheckPermissionWithContext("GetTable", principal, owner, "", "arn", &PolicyContext{ + IsLocalAccountID: func(string) bool { return false }, + }) { t.Error("legacy owner matching must not engage without a trusted issuer") } + + // With no local-account predicate only the unconditional exclusions apply; + // a plain legacy subject still re-anchors. + if !CheckPermissionWithContext("GetTable", principal, owner, "", "arn", &PolicyContext{ + TrustedIssuers: []string{"corp-idp"}, + }) { + t.Error("a missing local-account check keeps plain legacy subjects working") + } +} + +// A stored owner that names a reserved or configured local account was never +// an OIDC subject, and an already issuer-scoped id is not a legacy subject +// either; re-anchoring them would let a caller pick sub= and +// claim the resource. +func TestCheckPermissionLegacyOwnerExcludesLocalAndScoped(t *testing.T) { + ctx := &PolicyContext{ + TrustedIssuers: []string{"corp-idp"}, + IsLocalAccountID: func(id string) bool { return id == "admin" || id == "owner2" }, + } + + for _, owner := range []string{"admin", "owner2"} { + principal := sts.ComputeParentUser(owner, "corp-idp") + if CheckPermissionWithContext("GetTable", principal, owner, "", "arn", ctx) { + t.Errorf("sub=%q must not re-anchor onto the local account owner", owner) + } + } + + scoped := sts.ComputeParentUser("victim-sub", "corp-idp") + if CheckPermissionWithContext("GetTable", sts.ComputeParentUser(scoped, "corp-idp"), scoped, "", "arn", ctx) { + t.Error("sub equal to a stored scoped id must not re-anchor") + } + + // An ARN-shaped subject claims nothing; ARNs name IAM principals, not OIDC + // subjects, and this shape is what let a forged sub reach the admin account. + arn := "arn:aws:iam::123456789012:user/admin" + if CheckPermissionWithContext("GetTable", sts.ComputeParentUser(arn, "corp-idp"), arn, "", "arn", ctx) { + t.Error("an ARN-shaped stored owner must not re-anchor") + } +} + +// Legacy policies can carry the bare subject in the AWS map or array forms, +// not only as a top-level string; re-anchoring must reach all of them or a +// pre-scoping Deny silently stops matching the caller. +func TestCheckPermissionLegacyPolicyPrincipalShapes(t *testing.T) { + ctx := &PolicyContext{ + TrustedIssuers: []string{"corp-idp"}, + IsLocalAccountID: func(string) bool { return false }, + } + principal := sts.ComputeParentUser("alice-sub", "corp-idp") + other := "other-owner" + + // The caller owns the resource, so only a matched Deny can veto it. + for _, policy := range []string{ + `{"Statement":[{"Effect":"Deny","Principal":{"AWS":"alice-sub"},"Action":"s3tables:GetTable"}]}`, + `{"Statement":[{"Effect":"Deny","Principal":["alice-sub"],"Action":"s3tables:GetTable"}]}`, + } { + if CheckPermissionWithContext("GetTable", principal, principal, policy, "arn", ctx) { + t.Errorf("legacy Deny must veto the scoped caller's owner grant: %s", policy) + } + } + + allow := `{"Statement":[{"Effect":"Allow","Principal":["alice-sub"],"Action":"s3tables:GetTable"}]}` + if !CheckPermissionWithContext("GetTable", principal, other, allow, "arn", ctx) { + t.Error("legacy Allow with array principal must still match the scoped caller") + } }