s3tables: scope OIDC-derived account IDs to the token issuer (#11774)

* s3tables: scope OIDC-derived account IDs to the token issuer

Table ownership keyed off the bare sub claim, so two identity providers
issuing the same sub collided on one account - a caller could act on
tables owned by a different provider's user. OIDC bearer tokens now
carry the same issuer-namespaced (sub, iss) hash STS sessions already
expose as aws:userid, and getAccountID prefers it over the raw claims.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3tables: keep legacy owners working and scrub client-subject claims

- OIDC bearer tokens without a subject are rejected, and a token-supplied
  aws:userid claim is always overwritten with the verified (sub, iss)
  hash, so a forged claim can no longer stand in for the principal.
- Owners written before issuer scoping hold a bare subject; permission
  checks now re-anchor that subject under the caller's issuer, so the
  same (sub, iss) pair keeps its ownership while scoped owners remain
  issuer-isolated. The token issuer is asserted into claims alongside
  sub and carried through PolicyContext.

* s3tables: anchor legacy subjects to the configured issuer set

* s3: enumerate trusted issuers from registered providers too

* s3: fail closed when the OIDC provider store cannot be listed

trustedOIDCIssuers kept the registered issuers when ListOIDCProviders
failed, presenting a partial set as complete. A deployment trusting one
registered issuer A plus a stored issuer B would, on a listing error,
report only A — the single-issuer shape under which a legacy bare
subject re-anchors — letting a caller from A claim a stored subject
that belongs to B and gain owner access.

With no provider store the registered providers are provably complete,
so keep returning them; with a store, a failed listing means the real
set is unknown, so return none.

---------

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Chris LuandDevin authored and GitHub committed 2026-10-16 19:48:32 -07:00
1 parent 348116b2c4
commit 446e6f3444
13 files changed
+393 -17

No files matched your search

@@ -6,8 +6,10 @@ import (
"crypto/rsa"
"encoding/base64"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"sync/atomic"
"testing"
"time"
@@ -148,3 +150,159 @@ func TestOIDCBearerEnforcesRoleTrustPolicy(t *testing.T) {
require.Equal(t, s3err.ErrNone, errCode)
assert.Equal(t, "arn:aws:iam::role/BearerTrustedRole", identity.Principal)
}
// An OIDC bearer token's aws:userid claim is server-asserted from the
// verified (sub, iss) pair: a token-supplied value is overwritten, and a
// token without a subject is rejected outright.
func TestOIDCBearerUserIDIsIssuerScoped(t *testing.T) {
ctx := context.Background()
key, err := rsa.GenerateKey(rand.Reader, 2048)
require.NoError(t, err)
jwks := map[string]interface{}{
"keys": []map[string]interface{}{{
"kty": "RSA",
"kid": "bearer-test-key",
"use": "sig",
"alg": "RS256",
"n": base64.RawURLEncoding.EncodeToString(key.PublicKey.N.Bytes()),
"e": "AQAB",
}},
}
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/jwks" {
json.NewEncoder(w).Encode(jwks)
}
}))
defer server.Close()
provider := oidc.NewOIDCProvider("test-oidc")
require.NoError(t, provider.Initialize(&oidc.OIDCConfig{
Issuer: server.URL,
ClientID: "test-client",
JWKSUri: server.URL + "/jwks",
RoleMapping: &providers.RoleMapping{
Rules: []providers.MappingRule{
{Claim: "groups", Value: "trusted-group", Role: "arn:aws:iam::role/BearerTrustedRole"},
},
},
}))
iamManager := integration.NewIAMManager()
require.NoError(t, iamManager.Initialize(&integration.IAMConfig{
STS: &sts.STSConfig{
TokenDuration: sts.FlexibleDuration{Duration: time.Hour},
MaxSessionLength: sts.FlexibleDuration{Duration: 12 * time.Hour},
Issuer: "test-sts",
SigningKey: []byte("test-signing-key-32-characters-long"),
},
Policy: &policy.PolicyEngineConfig{DefaultEffect: "Deny", StoreType: "memory"},
Roles: &integration.RoleStoreConfig{StoreType: "memory"},
}, func() string { return "localhost:8888" }))
require.NoError(t, iamManager.RegisterIdentityProvider(provider))
require.NoError(t, iamManager.CreatePolicy(ctx, "", "BearerTestPolicy", &policy.PolicyDocument{
Version: "2012-10-17",
Statement: []policy.Statement{{
Effect: "Allow",
Action: []string{"s3:*"},
Resource: []string{"arn:aws:s3:::*", "arn:aws:s3:::*/*"},
}},
}))
require.NoError(t, iamManager.CreateRole(ctx, "", "BearerTrustedRole", &integration.RoleDefinition{
RoleName: "BearerTrustedRole",
TrustPolicy: &policy.PolicyDocument{
Version: "2012-10-17",
Statement: []policy.Statement{{
Effect: "Allow",
Principal: map[string]interface{}{"Federated": "test-oidc"},
Action: []string{"sts:AssumeRoleWithWebIdentity"},
}},
},
AttachedPolicies: []string{"BearerTestPolicy"},
}))
sign := func(extra map[string]interface{}) string {
claims := jwt.MapClaims{
"iss": server.URL,
"aud": "test-client",
"exp": time.Now().Add(time.Hour).Unix(),
"iat": time.Now().Unix(),
"groups": []string{"trusted-group"},
}
for k, v := range extra {
claims[k] = v
}
token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims)
token.Header["kid"] = "bearer-test-key"
signed, err := token.SignedString(key)
require.NoError(t, err)
return signed
}
s3iam := NewS3IAMIntegration(iamManager, "localhost:8888")
authenticate := func(token string) (*IAMIdentity, s3err.ErrorCode) {
req := httptest.NewRequest("GET", "/test-bucket/test-object", http.NoBody)
req.Header.Set("Authorization", "Bearer "+token)
return s3iam.AuthenticateJWT(ctx, req)
}
identity, errCode := authenticate(sign(map[string]interface{}{
"sub": "bearer-user",
"aws:userid": "admin",
}))
require.Equal(t, s3err.ErrNone, errCode)
assert.Equal(t,
sts.ComputeParentUser("bearer-user", server.URL),
identity.Claims["aws:userid"],
"the token-supplied aws:userid must be overwritten with the verified (sub, iss) hash")
assert.Equal(t, "bearer-user", identity.Claims["sub"])
assert.Equal(t, server.URL, identity.Claims["iss"])
assert.Equal(t, server.URL, identity.Claims["seaweed:oidc-issuers"],
"the registered provider's issuer must be asserted as trusted for legacy-owner anchoring")
_, errCode = authenticate(sign(map[string]interface{}{"sub": ""}))
assert.Equal(t, s3err.ErrAccessDenied, errCode, "a token with no subject must not authenticate")
}
// flakyListOIDCStore fails ListProviders on demand, standing in for a
// filer-backed provider store that cannot be read.
type flakyListOIDCStore struct {
integration.OIDCProviderStore
fail atomic.Bool
}
func (s *flakyListOIDCStore) ListProviders(ctx context.Context, filerAddress string) ([]*integration.OIDCProviderRecord, error) {
if s.fail.Load() {
return nil, fmt.Errorf("list OIDC providers: simulated outage")
}
return s.OIDCProviderStore.ListProviders(ctx, filerAddress)
}
// A failed store listing must not pass the registered providers off as the
// complete trusted set: hidden issuers could shrink the reported set to one
// entry, the only shape under which a legacy bare subject re-anchors.
func TestTrustedOIDCIssuersFailsClosedOnStoreError(t *testing.T) {
ctx := context.Background()
iamManager := integration.NewIAMManager()
s3iam := NewS3IAMIntegration(iamManager, "")
// No provider store at all: registered providers are the complete set —
// there is nowhere an issuer could hide behind a listing failure.
assert.Empty(t, s3iam.trustedOIDCIssuers(ctx))
mem := integration.NewMemoryOIDCProviderStore()
require.NoError(t, mem.StoreProvider(ctx, "", &integration.OIDCProviderRecord{
ARN: "arn:aws:iam::111122223333:oidc-provider/idp.example.com",
URL: "https://idp.example.com",
}))
store := &flakyListOIDCStore{OIDCProviderStore: mem}
iamManager.SetOIDCProviderStore(store)
assert.Equal(t, []string{"https://idp.example.com"}, s3iam.trustedOIDCIssuers(ctx))
store.fail.Store(true)
assert.Empty(t, s3iam.trustedOIDCIssuers(ctx),
"a failed listing must report no trusted issuers, not the partial registered set")
}
+72
View File
@@ -126,6 +126,12 @@ func (s3iam *S3IAMIntegration) AuthenticateJWT(ctx context.Context, r *http.Requ
return nil, s3err.ErrAccessDenied
}
// A token without a subject has no issuer-scoped principal; letting it
// through would make the client-supplied claims below stand in for one.
if identity.UserID == "" {
return nil, s3err.ErrAccessDenied
}
// Extract role from OIDC identity
if identity.RoleArn == "" {
return nil, s3err.ErrAccessDenied
@@ -162,6 +168,22 @@ func (s3iam *S3IAMIntegration) AuthenticateJWT(ctx context.Context, r *http.Requ
// Set critical claims explicitly, overwriting any from attributes to ensure correctness.
claims["sub"] = identity.UserID
claims["role"] = identity.RoleArn
claims["iss"] = identity.Issuer
// The bare sub collides across issuers; aws:userid carries the same
// (sub, iss) hash STS sessions already expose, so downstream ownership
// keys stay namespaced per identity provider. A claim the token itself
// supplies is always scrubbed, never adopted.
if parentUser := sts.ComputeParentUser(identity.UserID, identity.Issuer); parentUser != "" {
claims["aws:userid"] = parentUser
} else {
delete(claims, "aws:userid")
}
if issuers := s3iam.trustedOIDCIssuers(ctx); len(issuers) > 0 {
claims["seaweed:oidc-issuers"] = strings.Join(issuers, " ")
} else {
delete(claims, "seaweed:oidc-issuers")
}
// Use real email address if available
emailAddress := identity.UserID + "@oidc.local"
@@ -425,6 +447,7 @@ type OIDCIdentity struct {
UserID string
RoleArn string
Provider string
Issuer string
Email string
DisplayName string
Groups []string
@@ -488,6 +511,7 @@ func (s3iam *S3IAMIntegration) validateExternalOIDCToken(ctx context.Context, to
UserID: externalIdentity.UserID,
RoleArn: roleArn,
Provider: fmt.Sprintf("%T", provider), // Use provider type as identifier
Issuer: externalIdentity.Issuer,
Email: externalIdentity.Email,
DisplayName: externalIdentity.DisplayName,
Groups: externalIdentity.Groups,
@@ -495,6 +519,54 @@ func (s3iam *S3IAMIntegration) validateExternalOIDCToken(ctx context.Context, to
}, nil
}
// trustedOIDCIssuers returns the issuer URLs of the OIDC providers the
// deployment is configured to trust. It is what a legacy (pre-scoping) bare
// subject can be re-anchored to: exactly one issuer makes the mapping
// unambiguous; anything else leaves it unknown, which callers must treat as
// fail-closed rather than guessing from the caller's issuer.
//
// The list is only safe while complete. Without a provider store the
// registered providers are the whole set — there is nowhere an issuer could
// hide. With a store, a failed listing leaves the real set unknown, and
// passing the registered subset off as complete could collapse a
// multi-issuer deployment to a single issuer: the one shape a bare subject
// re-anchors under, so a caller from that issuer could claim a stored
// subject that belongs to a provider hidden behind the error. Fail closed.
func (s3iam *S3IAMIntegration) trustedOIDCIssuers(ctx context.Context) []string {
if s3iam.iamManager == nil {
return nil
}
seen := make(map[string]bool)
var issuers []string
// Configured identity providers registered with STS cover deployments
// whose OIDC setup lives in config rather than the provider store.
if stsService := s3iam.iamManager.GetSTSService(); stsService != nil {
for _, p := range stsService.GetProviders() {
if issuer, ok := p.(interface{ GetIssuer() string }); ok {
if iss := issuer.GetIssuer(); iss != "" && !seen[iss] {
seen[iss] = true
issuers = append(issuers, iss)
}
}
}
}
if s3iam.iamManager.GetOIDCProviderStore() == nil {
return issuers
}
records, err := s3iam.iamManager.ListOIDCProviders(ctx)
if err != nil {
glog.V(2).Infof("trustedOIDCIssuers: %v", err)
return nil
}
for _, rec := range records {
if rec.URL != "" && !seen[rec.URL] {
seen[rec.URL] = true
issuers = append(issuers, rec.URL)
}
}
return issuers
}
// selectPrimaryRole simply picks the first role from the list
// The OIDC provider should return roles in priority order (most important first)
func (s3iam *S3IAMIntegration) selectPrimaryRole(roles []string, externalIdentity *providers.ExternalIdentity) string {
+46 -14
View File
@@ -238,20 +238,12 @@ func (h *S3TablesHandler) getAccountID(r *http.Request) string {
// Prefer stable claims from JWT/STS identities. Only "sub" is guaranteed durable per OIDC;
// preferred_username is ergonomic but can rotate and may orphan ownership data, while email
// is explicitly excluded to avoid storing PII in metadata.
claimsField := val.FieldByName("Claims")
if claimsField.IsValid() && claimsField.Kind() == reflect.Map && !claimsField.IsNil() && claimsField.Type().Key().Kind() == reflect.String {
for _, claimKey := range []string{"sub", "preferred_username"} {
claimVal := claimsField.MapIndex(reflect.ValueOf(claimKey))
if !claimVal.IsValid() {
continue
}
if claimVal.Kind() == reflect.Interface && !claimVal.IsNil() {
claimVal = claimVal.Elem()
}
if claimVal.Kind() == reflect.String {
if principal := normalizePrincipalID(claimVal.String()); principal != "" {
return principal
}
if claims := identityClaimsMap(val); claims != nil {
// aws:userid is the issuer-namespaced (sub, iss) hash; a bare
// sub or preferred_username collides across providers.
for _, claimKey := range []string{"aws:userid", "sub", "preferred_username"} {
if principal := normalizePrincipalID(claimString(claims, claimKey)); principal != "" {
return principal
}
}
}
@@ -291,6 +283,46 @@ func (h *S3TablesHandler) getAccountID(r *http.Request) string {
return h.accountID
}
// identityClaimsMap extracts the Claims map from an identity value, using
// reflection to avoid an import cycle with the s3api identity type.
func identityClaimsMap(val reflect.Value) map[string]interface{} {
claimsField := val.FieldByName("Claims")
if !claimsField.IsValid() || claimsField.Kind() != reflect.Map || claimsField.IsNil() || claimsField.Type().Key().Kind() != reflect.String {
return nil
}
claims, ok := claimsField.Interface().(map[string]interface{})
if !ok {
return nil
}
return claims
}
func claimString(claims map[string]interface{}, key string) string {
v, ok := claims[key]
if !ok || v == nil {
return ""
}
str, _ := v.(string)
return str
}
// trustedIssuers returns the OIDC issuers the auth layer asserted into the
// identity's claims; they re-anchor legacy owner checks in checkPermission.
func (h *S3TablesHandler) trustedIssuers(r *http.Request) []string {
identityRaw := s3_constants.GetIdentityFromContext(r)
if identityRaw == nil {
return nil
}
val := reflect.ValueOf(identityRaw)
if val.Kind() == reflect.Ptr {
val = val.Elem()
}
if val.Kind() != reflect.Struct {
return nil
}
return strings.Fields(claimString(identityClaimsMap(val), "seaweed:oidc-issuers"))
}
// normalizePrincipalID collapses ARN and identity strings to a key that is stable within a single account.
// WARNING: this assumes identity names are unique per account; distinct principals such as
// arn:aws:iam::111:user/alice and arn:aws:iam::222:user/alice will both normalize to "alice".
@@ -65,6 +65,7 @@ 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),
}) {
@@ -70,6 +70,7 @@ 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),
@@ -192,6 +193,7 @@ 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),
@@ -296,6 +298,7 @@ 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),
@@ -171,3 +171,18 @@ func TestGetAccountIDNormalizesAccountIDARN(t *testing.T) {
assert.Equal(t, "bob", h.getAccountID(req), "expected ARN account ID to be normalized to the suffix")
}
func TestGetAccountIDPrefersIssuerScopedUserID(t *testing.T) {
h := NewS3TablesHandler()
id := &testIdentity{
Claims: map[string]interface{}{
"aws:userid": "issuerA-hash",
"sub": "alice-sub",
"preferred_username": "alice",
},
}
req := httptest.NewRequest(http.MethodGet, "/", nil)
req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), id))
assert.Equal(t, "issuerA-hash", h.getAccountID(req), "aws:userid should win over a bare sub")
}
@@ -349,6 +349,7 @@ 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),
@@ -385,6 +386,7 @@ 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,
+5
View File
@@ -114,6 +114,7 @@ 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,
@@ -255,6 +256,7 @@ 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,
@@ -345,6 +347,7 @@ 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,
@@ -447,6 +450,7 @@ 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,
@@ -631,6 +635,7 @@ 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,
+9
View File
@@ -92,6 +92,7 @@ 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),
@@ -170,6 +171,7 @@ 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),
@@ -246,6 +248,7 @@ 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),
@@ -345,6 +348,7 @@ 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,
@@ -453,6 +457,7 @@ 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,
@@ -543,6 +548,7 @@ 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,
@@ -640,6 +646,7 @@ 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,
@@ -760,6 +767,7 @@ 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,
@@ -867,6 +875,7 @@ 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,
+15
View File
@@ -420,6 +420,7 @@ func (h *S3TablesHandler) handleGetTable(w http.ResponseWriter, r *http.Request,
bucketARN := h.generateTableBucketARN(bucketMetadata.OwnerAccountID, bucketName)
identityActions := getIdentityActions(r)
tableAllowed := CheckPermissionWithContext("GetTable", accountID, metadata.OwnerAccountID, tablePolicy, tableARN, &PolicyContext{
TrustedIssuers: h.trustedIssuers(r),
TableBucketName: bucketName,
Namespace: namespace,
TableName: tableName,
@@ -429,6 +430,7 @@ func (h *S3TablesHandler) handleGetTable(w http.ResponseWriter, r *http.Request,
DefaultAllow: h.defaultAllowFor(r),
})
bucketAllowed := CheckPermissionWithContext("GetTable", accountID, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{
TrustedIssuers: h.trustedIssuers(r),
TableBucketName: bucketName,
Namespace: namespace,
TableName: tableName,
@@ -561,6 +563,7 @@ func (h *S3TablesHandler) handleListTables(w http.ResponseWriter, r *http.Reques
bucketARN := h.generateTableBucketARN(bucketMeta.OwnerAccountID, bucketName)
identityActions := getIdentityActions(r)
nsAllowed := CheckPermissionWithContext("ListTables", accountID, nsMeta.OwnerAccountID, namespacePolicy, bucketARN, &PolicyContext{
TrustedIssuers: h.trustedIssuers(r),
TableBucketName: bucketName,
Namespace: namespaceName,
TableBucketTags: bucketTags,
@@ -568,6 +571,7 @@ func (h *S3TablesHandler) handleListTables(w http.ResponseWriter, r *http.Reques
DefaultAllow: h.defaultAllowFor(r),
})
bucketAllowed := CheckPermissionWithContext("ListTables", accountID, bucketMeta.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{
TrustedIssuers: h.trustedIssuers(r),
TableBucketName: bucketName,
Namespace: namespaceName,
TableBucketTags: bucketTags,
@@ -614,6 +618,7 @@ 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,
@@ -951,6 +956,7 @@ func (h *S3TablesHandler) handleDeleteTable(w http.ResponseWriter, r *http.Reque
principal := h.getAccountID(r)
identityActions := getIdentityActions(r)
tableAllowed := CheckPermissionWithContext("DeleteTable", principal, metadata.OwnerAccountID, tablePolicy, tableARN, &PolicyContext{
TrustedIssuers: h.trustedIssuers(r),
TableBucketName: bucketName,
Namespace: namespaceName,
TableName: tableName,
@@ -960,6 +966,7 @@ func (h *S3TablesHandler) handleDeleteTable(w http.ResponseWriter, r *http.Reque
DefaultAllow: h.defaultAllowFor(r),
})
bucketAllowed := CheckPermissionWithContext("DeleteTable", principal, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{
TrustedIssuers: h.trustedIssuers(r),
TableBucketName: bucketName,
Namespace: namespaceName,
TableName: tableName,
@@ -1131,6 +1138,7 @@ 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,
@@ -1312,6 +1320,7 @@ func (h *S3TablesHandler) renameCatalogEntry(w http.ResponseWriter, r *http.Requ
principal := h.getAccountID(r)
identityActions := getIdentityActions(r)
tableAllowed := CheckPermissionWithContext(kind.renameOp, principal, metadata.OwnerAccountID, tablePolicy, tableARN, &PolicyContext{
TrustedIssuers: h.trustedIssuers(r),
TableBucketName: bucketName,
Namespace: srcNamespace,
TableName: srcName,
@@ -1321,6 +1330,7 @@ func (h *S3TablesHandler) renameCatalogEntry(w http.ResponseWriter, r *http.Requ
DefaultAllow: h.defaultAllowFor(r),
})
bucketAllowed := CheckPermissionWithContext(kind.renameOp, principal, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{
TrustedIssuers: h.trustedIssuers(r),
TableBucketName: bucketName,
Namespace: srcNamespace,
TableName: srcName,
@@ -1385,6 +1395,7 @@ func (h *S3TablesHandler) renameCatalogEntry(w http.ResponseWriter, r *http.Requ
// 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{
TrustedIssuers: h.trustedIssuers(r),
TableBucketName: bucketName,
Namespace: destNamespace,
TableName: destName,
@@ -1393,6 +1404,7 @@ func (h *S3TablesHandler) renameCatalogEntry(w http.ResponseWriter, r *http.Requ
DefaultAllow: h.defaultAllowFor(r),
})
destBucketAllowed := CheckPermissionWithContext(kind.createOp, principal, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{
TrustedIssuers: h.trustedIssuers(r),
TableBucketName: bucketName,
Namespace: destNamespace,
TableName: destName,
@@ -1596,6 +1608,7 @@ func (h *S3TablesHandler) handleUpdateTable(w http.ResponseWriter, r *http.Reque
identityActions := getIdentityActions(r)
tableAllowed := CheckPermissionWithContext("UpdateTable", principal, metadata.OwnerAccountID, tablePolicy, tableARN, &PolicyContext{
TrustedIssuers: h.trustedIssuers(r),
TableBucketName: bucketName,
Namespace: namespaceName,
TableName: tableName,
@@ -1605,6 +1618,7 @@ func (h *S3TablesHandler) handleUpdateTable(w http.ResponseWriter, r *http.Reque
DefaultAllow: h.defaultAllowFor(r),
})
bucketAllowed := CheckPermissionWithContext("UpdateTable", principal, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{
TrustedIssuers: h.trustedIssuers(r),
TableBucketName: bucketName,
Namespace: namespaceName,
TableName: tableName,
@@ -1809,6 +1823,7 @@ 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,
+1
View File
@@ -635,6 +635,7 @@ 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,
+33 -3
View File
@@ -6,6 +6,7 @@ import (
"strings"
"github.com/seaweedfs/seaweedfs/weed/glog"
"github.com/seaweedfs/seaweedfs/weed/iam/sts"
"github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
"github.com/seaweedfs/seaweedfs/weed/util/wildcard"
@@ -88,6 +89,11 @@ type PolicyContext struct {
KMSKeyArn string
StorageClass string
DefaultAllow bool
// TrustedIssuers lists the OIDC issuers the deployment is configured to
// trust, used to re-anchor owners/principals written before issuer-scoped
// 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
}
// CheckPermissionWithContext checks permission with optional resource and condition context.
@@ -109,8 +115,8 @@ func CheckPermissionWithContext(operation, principal, owner, resourcePolicy, res
func checkPermission(operation, principal, owner, resourcePolicy, resourceARN string, ctx *PolicyContext) bool {
// Owner always has permission
if principal == owner {
// Owner always has permission.
if principal == owner || matchesLegacySubject(owner, principal, ctx) {
return true
}
@@ -145,7 +151,7 @@ func checkPermission(operation, principal, owner, resourcePolicy, resourceARN st
for _, stmt := range policy.Statement {
// Check if principal matches
if !matchesPrincipal(stmt.Principal, principal) {
if !matchesCaller(stmt.Principal, principal, ctx) {
continue
}
@@ -225,6 +231,30 @@ func hasAdminAction(actions []string) bool {
return false
}
// matchesCaller matches a statement's Principal against the caller's scoped
// principal or, for policies written before issuer scoping, a bare subject
// that resolves to it under the deployment's single trusted issuer.
func matchesCaller(principalSpec interface{}, principal string, ctx *PolicyContext) bool {
if matchesPrincipal(principalSpec, principal) {
return true
}
legacy, ok := principalSpec.(string)
return ok && matchesLegacySubject(legacy, principal, ctx)
}
// matchesLegacySubject reports whether a bare subject stored before issuer
// scoping belongs to the caller. Re-anchoring is only sound when exactly one
// OIDC issuer is configured: the stored subject is hashed under that issuer
// and compared to the caller's scoped principal. With multiple issuers a bare
// subject is ambiguous — matching it under the caller's own issuer would let
// a same-sub identity from any configured issuer claim it.
func matchesLegacySubject(legacy, principal string, ctx *PolicyContext) bool {
if ctx == nil || len(ctx.TrustedIssuers) != 1 {
return false
}
return sts.ComputeParentUser(legacy, ctx.TrustedIssuers[0]) == principal
}
// matchesPrincipal checks if the principal matches the statement's principal
func matchesPrincipal(principalSpec interface{}, principal string) bool {
if principalSpec == nil {
+33
View File
@@ -3,6 +3,8 @@ package s3tables
import (
"encoding/json"
"testing"
"github.com/seaweedfs/seaweedfs/weed/iam/sts"
)
func TestMatchesActionPattern(t *testing.T) {
@@ -251,3 +253,34 @@ func TestCheckPermissionWithDefaultAllow(t *testing.T) {
})
}
}
// Owners written before issuer scoping hold a bare subject. Re-anchoring it
// is only unambiguous when exactly one OIDC issuer is configured; under two
// or more issuers a bare subject could belong to either, so it must not
// match anyone.
func TestCheckPermissionLegacyOwnerScopedToIssuer(t *testing.T) {
owner := "alice-sub"
principal := sts.ComputeParentUser(owner, "corp-idp")
if !CheckPermissionWithContext("GetTable", principal, owner, "", "arn", &PolicyContext{
TrustedIssuers: []string{"corp-idp"},
}) {
t.Error("single-issuer deployments must keep recognizing the legacy owner")
}
if CheckPermissionWithContext("GetTable", principal, owner, "", "arn", &PolicyContext{
TrustedIssuers: []string{"corp-idp", "evil-idp"},
}) {
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"},
}) {
t.Error("a same-sub caller from another issuer must not claim the legacy owner")
}
if CheckPermissionWithContext("GetTable", principal, owner, "", "arn", &PolicyContext{}) {
t.Error("legacy owner matching must not engage without a trusted issuer")
}
}