From 1618d613a38b9b32be05ce5ea58f99909ba9f094 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 11 Nov 2026 15:32:13 -0800 Subject: [PATCH] s3: resolve a role session's aws:username from its user, not its session name (#12572) * s3api: record the caller on AssumeRole sessions The session name is the caller's choice; the session now carries the caller itself as the user it was issued to, as the web identity, LDAP and credential paths already do. Co-Authored-By: Claude Opus 5.5 * s3api: take a role session's aws:username from its user, not its name An assumed-role ARN ends in the session name the caller picked, so naming a session after another user resolved ${aws:username} and ${aws:userid} to that user in bucket and attached policies. Resolve them from the user the session was issued to instead. Co-Authored-By: Claude Opus 5.5 * iam: take a role session's aws:username from its user, not its name IsActionAllowed set aws:username to the last segment of an assumed-role ARN, the caller-chosen session name, for role and bucket policies. Use the user the session was issued to, and drop a forwarded value when the session names none. Co-Authored-By: Claude Opus 5.5 * s3api: pass a role session caller's user on to the session it assumes A role session that calls AssumeRole is named by its session ID, so the new session recorded that ID as its user. An STS session caller now passes on its own recorded user, or none. Co-Authored-By: Claude Opus 5.5 * s3api: size the session claims map from the request context Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- weed/iam/integration/iam_manager.go | 14 ++- weed/iam/integration/session_username_test.go | 66 ++++++++++ weed/s3api/auth_credentials.go | 2 +- weed/s3api/auth_signature_v4.go | 7 ++ weed/s3api/policy_engine/engine.go | 9 +- .../policy_engine/engine_enhanced_test.go | 2 - weed/s3api/policy_engine/engine_paths_test.go | 2 - weed/s3api/s3api_bucket_policy_engine.go | 16 ++- weed/s3api/s3api_sts.go | 24 ++-- weed/s3api/s3api_sts_session_username_test.go | 113 ++++++++++++++++++ 10 files changed, 231 insertions(+), 24 deletions(-) create mode 100644 weed/iam/integration/session_username_test.go create mode 100644 weed/s3api/s3api_sts_session_username_test.go diff --git a/weed/iam/integration/iam_manager.go b/weed/iam/integration/iam_manager.go index 6e7ade42cf..0ea4a32b2c 100644 --- a/weed/iam/integration/iam_manager.go +++ b/weed/iam/integration/iam_manager.go @@ -1672,10 +1672,16 @@ func (m *IAMManager) IsActionAllowed(ctx context.Context, request *ActionRequest // Parse principal ARN to extract details for context variables (e.g. ${aws:username}) arnInfo := utils.ParsePrincipalARN(request.Principal) - if arnInfo.RoleName != "" { - // For assumed roles, AWS docs say aws:username IS the role name. - // However, for user isolation in these tests, we typically map the session name (the user who assumed the role) to aws:username. - // arn:aws:sts::account:assumed-role/RoleName/SessionName + if arnInfo.RoleName != "" && strings.HasPrefix(request.Principal, "arn:aws:sts::") { + // An assumed-role ARN ends in the session name the caller chose; the + // session's user is the one it was issued to. + if sessionInfo != nil && sessionInfo.ExternalUserId != "" { + evalCtx.RequestContext["aws:username"] = sessionInfo.ExternalUserId + } else { + delete(evalCtx.RequestContext, "aws:username") + } + evalCtx.RequestContext["aws:userid"] = arnInfo.RoleName + } else if arnInfo.RoleName != "" { awsUsername := arnInfo.RoleName if idx := strings.LastIndex(request.Principal, "/"); idx != -1 && idx < len(request.Principal)-1 { awsUsername = request.Principal[idx+1:] diff --git a/weed/iam/integration/session_username_test.go b/weed/iam/integration/session_username_test.go new file mode 100644 index 0000000000..8bb8441e1e --- /dev/null +++ b/weed/iam/integration/session_username_test.go @@ -0,0 +1,66 @@ +package integration + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/seaweedfs/seaweedfs/weed/iam/policy" + "github.com/seaweedfs/seaweedfs/weed/iam/sts" +) + +// ${aws:username} of a role session is the user the session was issued to, +// not the session name its caller chose or a value forwarded in the request +// context. +func TestAssumedRoleUsernameIsTheSessionUser(t *testing.T) { + ctx := context.Background() + manager := setupIntegratedIAMSystem(t) + require.NoError(t, manager.CreatePolicy(ctx, "", "OwnFolderPolicy", &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{{ + Effect: "Allow", + Action: []string{"s3:GetObject"}, + Resource: []string{"arn:aws:s3:::home/${aws:username}/*"}, + }}, + })) + require.NoError(t, manager.CreateRole(ctx, "", "SharedRole", &RoleDefinition{ + RoleName: "SharedRole", + 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{"OwnFolderPolicy"}, + })) + + response, err := manager.AssumeRoleWithWebIdentity(ctx, &sts.AssumeRoleWithWebIdentityRequest{ + RoleArn: "arn:aws:iam::role/SharedRole", + WebIdentityToken: createTestJWT(t, "https://test-issuer.com", "test-user-123", "test-signing-key"), + RoleSessionName: "alice", + }) + require.NoError(t, err) + session, err := manager.GetSTSService().ValidateSessionToken(ctx, response.Credentials.SessionToken) + require.NoError(t, err) + require.NotEmpty(t, session.ExternalUserId) + require.NotEqual(t, "alice", session.ExternalUserId) + + allowed := func(resource string) bool { + t.Helper() + ok, err := manager.IsActionAllowed(ctx, &ActionRequest{ + Principal: response.AssumedRoleUser.Arn, + Action: "s3:GetObject", + Resource: resource, + SessionToken: response.Credentials.SessionToken, + RequestContext: map[string]interface{}{"aws:username": "alice"}, + }) + require.NoError(t, err) + return ok + } + assert.False(t, allowed("arn:aws:s3:::home/alice/notes.txt"), "the session name must not select another user's folder") + assert.True(t, allowed("arn:aws:s3:::home/"+session.ExternalUserId+"/notes.txt"), "the session's user keeps its folder") +} diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index ee9a48698b..ad5a2c0e68 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -2755,7 +2755,7 @@ func (iam *IdentityAccessManagement) evaluateAttachedIAMPolicies(r *http.Request for key, values := range vectorIAMConditions(r) { conditions[key] = values } - for k, v := range policy_engine.ExtractPrincipalVariables(principal) { + for k, v := range principalVariables(principal, identity.Claims) { conditions[k] = v } diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index b86c14b42c..f18f12f4eb 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -532,6 +532,13 @@ func (iam *IdentityAccessManagement) validateSTSSessionToken(r *http.Request, se for k, v := range sessionInfo.RequestContext { claims[k] = v } + // The session's user is the one it was issued to, never a claim the + // identity provider forwarded. + if sessionInfo.ExternalUserId != "" { + claims["aws:username"] = sessionInfo.ExternalUserId + } else { + delete(claims, "aws:username") + } // Key the principal on the OIDC subject so a session resolves to the same // identity on the SigV4 and JWT paths (see s3_iam_middleware.go); fall back to diff --git a/weed/s3api/policy_engine/engine.go b/weed/s3api/policy_engine/engine.go index e544b227bd..c59b54ffd0 100644 --- a/weed/s3api/policy_engine/engine.go +++ b/weed/s3api/policy_engine/engine.go @@ -404,14 +404,9 @@ func ExtractPrincipalVariables(principal string) map[string][]string { vars["aws:username"] = []string{roleName} } case "assumed-role": + // The last segment is the session name, which the caller chose, so it + // names no user. vars["aws:principaltype"] = []string{"AssumedRole"} - // For assumed roles: assumed-role/RoleName/SessionName or assumed-role/path/to/RoleName/SessionName - // The session name is always the last segment - if len(resourceParts) >= 3 { - sessionName := resourceParts[len(resourceParts)-1] - vars["aws:username"] = []string{sessionName} - vars["aws:userid"] = []string{sessionName} - } } // Note: principaltype is already set correctly in the switch above based on resource type diff --git a/weed/s3api/policy_engine/engine_enhanced_test.go b/weed/s3api/policy_engine/engine_enhanced_test.go index 0a6deae726..4ad4055469 100644 --- a/weed/s3api/policy_engine/engine_enhanced_test.go +++ b/weed/s3api/policy_engine/engine_enhanced_test.go @@ -26,8 +26,6 @@ func TestExtractPrincipalVariables(t *testing.T) { expected: map[string][]string{ "aws:PrincipalAccount": {"123456789012"}, "aws:principaltype": {"AssumedRole"}, - "aws:username": {"session-alice"}, - "aws:userid": {"session-alice"}, }, }, { diff --git a/weed/s3api/policy_engine/engine_paths_test.go b/weed/s3api/policy_engine/engine_paths_test.go index 074e4e4a59..1dc3fddfd4 100644 --- a/weed/s3api/policy_engine/engine_paths_test.go +++ b/weed/s3api/policy_engine/engine_paths_test.go @@ -36,8 +36,6 @@ func TestExtractPrincipalVariablesWithPaths(t *testing.T) { expected: map[string][]string{ "aws:PrincipalAccount": {"123456789012"}, "aws:principaltype": {"AssumedRole"}, - "aws:username": {"session-name"}, - "aws:userid": {"session-name"}, }, }, } diff --git a/weed/s3api/s3api_bucket_policy_engine.go b/weed/s3api/s3api_bucket_policy_engine.go index 8552ccc3ab..6f92dfdb42 100644 --- a/weed/s3api/s3api_bucket_policy_engine.go +++ b/weed/s3api/s3api_bucket_policy_engine.go @@ -149,7 +149,7 @@ func (bpe *BucketPolicyEngine) EvaluatePolicy(bucket, object, action, principal args.Conditions = bpe.engine.ExtractConditionValuesFromRequest(r) // Extract principal-related variables (aws:username, etc.) from principal ARN - principalVars := policy_engine.ExtractPrincipalVariables(principal) + principalVars := principalVariables(principal, claims) for k, v := range principalVars { args.Conditions[k] = v } @@ -191,3 +191,17 @@ func (bpe *BucketPolicyEngine) EvaluatePolicy(bucket, object, action, principal return false, false, fmt.Errorf("unknown policy result: %v", result) } } + +// principalVariables resolves the policy variables of a principal. A role +// session's ARN ends in the session name the caller chose, so its user comes +// from the session itself: the user the session was issued to. +func principalVariables(principal string, claims map[string]interface{}) map[string][]string { + vars := policy_engine.ExtractPrincipalVariables(principal) + if types := vars["aws:principaltype"]; len(types) == 1 && types[0] == "AssumedRole" { + if user, ok := claims["aws:username"].(string); ok && user != "" { + vars["aws:username"] = []string{user} + vars["aws:userid"] = []string{user} + } + } + return vars +} diff --git a/weed/s3api/s3api_sts.go b/weed/s3api/s3api_sts.go index 9daa1cacf7..51e0c237b8 100644 --- a/weed/s3api/s3api_sts.go +++ b/weed/s3api/s3api_sts.go @@ -12,6 +12,7 @@ import ( "net/http" "regexp" "strconv" + "strings" "time" "github.com/seaweedfs/seaweedfs/weed/credential" @@ -454,14 +455,23 @@ func (h *STSHandlers) handleAssumeRole(w http.ResponseWriter, r *http.Request) { // travel into a session the caller assumed for itself — a legacy static admin // carries no IAM policies for such a session to inherit. Assuming a named role // scopes the session to that role's policies, admin caller or not. - var modifyClaims func(claims *sts.STSSessionClaims) - if assumesSelf && identity.isAdmin() { - modifyClaims = func(claims *sts.STSSessionClaims) { - if claims.RequestContext == nil { - claims.RequestContext = make(map[string]interface{}) - } - claims.RequestContext["is_admin"] = true + // The session name is the caller's choice, so the session records the + // caller itself as the user it was issued to; an STS session caller + // passes on its own user, if it has one. + sessionUser := identity.Name + if strings.HasPrefix(identity.PrincipalArn, "arn:aws:sts::") { + sessionUser, _ = identity.Claims["aws:username"].(string) + } + sessionIsAdmin := assumesSelf && identity.isAdmin() + modifyClaims := func(claims *sts.STSSessionClaims) { + claims.ExternalUserId = sessionUser + if !sessionIsAdmin { + return } + if claims.RequestContext == nil { + claims.RequestContext = make(map[string]interface{}) + } + claims.RequestContext["is_admin"] = true } // Generate common STS components diff --git a/weed/s3api/s3api_sts_session_username_test.go b/weed/s3api/s3api_sts_session_username_test.go new file mode 100644 index 0000000000..097d22de5d --- /dev/null +++ b/weed/s3api/s3api_sts_session_username_test.go @@ -0,0 +1,113 @@ +package s3api + +import ( + "context" + "encoding/xml" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/gorilla/mux" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/seaweedfs/seaweedfs/weed/iam/integration" + "github.com/seaweedfs/seaweedfs/weed/iam/policy" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" +) + +const sessionUserAccessKey, sessionUserSecretKey = "AKIABOB000000000000", "bobSecret123" + +func newSharedRoleIAM(t *testing.T) (*integration.IAMManager, *IdentityAccessManagement) { + t.Helper() + manager := newTestSTSIntegrationManager(t) + require.NoError(t, manager.CreateRole(context.Background(), "", "SharedRole", &integration.RoleDefinition{ + RoleName: "SharedRole", + TrustPolicy: &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{{Effect: "Allow", Principal: "*", Action: []string{"sts:AssumeRole"}}}, + }, + })) + iam := &IdentityAccessManagement{iamIntegration: NewS3IAMIntegration(manager, "")} + require.NoError(t, iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{{ + Name: "bob", + Credentials: []*iam_pb.Credential{{AccessKey: sessionUserAccessKey, SecretKey: sessionUserSecretKey}}, + Actions: []string{"Read:home"}, + }}, + })) + return manager, iam +} + +func assumeSharedRole(t *testing.T, manager *integration.IAMManager, iam *IdentityAccessManagement, accessKey, secretKey, sessionToken, sessionName string) AssumeRoleResult { + t.Helper() + body := url.Values{ + "Action": {"AssumeRole"}, + "Version": {"2011-06-15"}, + "RoleArn": {"arn:aws:iam::" + defaultAccountID + ":role/SharedRole"}, + "RoleSessionName": {sessionName}, + }.Encode() + req, err := newTestRequest(http.MethodPost, "http://sts.seaweedfs.test/", int64(len(body)), strings.NewReader(body)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + if sessionToken != "" { + req.Header.Set("X-Amz-Security-Token", sessionToken) + } + require.NoError(t, signRequestV4(req, accessKey, secretKey)) + + rec := httptest.NewRecorder() + NewSTSHandlers(manager.GetSTSService(), iam).handleAssumeRole(rec, req) + require.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) + var resp AssumeRoleResponse + require.NoError(t, xml.Unmarshal(rec.Body.Bytes(), &resp)) + return resp.Result +} + +// A role session's ARN ends in the session name its caller chose, so a caller +// that names its session after another user must not get that user's +// ${aws:username}. +func TestAssumeRole_SessionNameIsNotTheUsername(t *testing.T) { + ctx := context.Background() + manager, iam := newSharedRoleIAM(t) + iam.policyEngine = NewBucketPolicyEngine() + require.NoError(t, iam.policyEngine.engine.SetBucketPolicy("home", `{"Version":"2012-10-17","Statement":[ + {"Effect":"Allow","Principal":"*","Action":"s3:GetObject","Resource":"arn:aws:s3:::home/${aws:username}/*"}, + {"Effect":"Deny","Principal":"*","Action":"s3:GetObject","NotResource":"arn:aws:s3:::home/${aws:username}/*"}]}`)) + + result := assumeSharedRole(t, manager, iam, sessionUserAccessKey, sessionUserSecretKey, "", "alice") + creds := result.Credentials + assert.Equal(t, "arn:aws:sts::"+defaultAccountID+":assumed-role/SharedRole/alice", result.AssumedRoleUser.Arn) + + session, err := manager.GetSTSService().ValidateSessionToken(ctx, creds.SessionToken) + require.NoError(t, err) + assert.Equal(t, "bob", session.ExternalUserId, "the session records its caller") + + get := func(key string) s3err.ErrorCode { + t.Helper() + r, err := newTestRequest(http.MethodGet, "http://s3.seaweedfs.test/home/"+key, 0, nil) + require.NoError(t, err) + r.Header.Set("X-Amz-Security-Token", creds.SessionToken) + require.NoError(t, signRequestV4(r, creds.AccessKeyId, creds.SecretAccessKey)) + r = mux.SetURLVars(r, map[string]string{"bucket": "home", "object": key}) + _, errCode := iam.authRequest(r, s3_constants.ACTION_READ) + return errCode + } + assert.Equal(t, s3err.ErrAccessDenied, get("alice/notes.txt"), "the session name must not select another user's folder") + assert.Equal(t, s3err.ErrNone, get("bob/notes.txt"), "the caller keeps its own folder") +} + +// A role session that assumes a role passes on its own user, not its session +// ID or the name it picks for the new session. +func TestAssumeRole_ChainedSessionKeepsTheUser(t *testing.T) { + manager, iam := newSharedRoleIAM(t) + first := assumeSharedRole(t, manager, iam, sessionUserAccessKey, sessionUserSecretKey, "", "bob").Credentials + chained := assumeSharedRole(t, manager, iam, first.AccessKeyId, first.SecretAccessKey, first.SessionToken, "alice").Credentials + + session, err := manager.GetSTSService().ValidateSessionToken(context.Background(), chained.SessionToken) + require.NoError(t, err) + assert.Equal(t, "bob", session.ExternalUserId) +}