From 10247a958cf74c6cb89799db32edf10365fc08f8 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Thu, 1 Oct 2026 13:50:22 +1000 Subject: [PATCH] fix(iam): report a pathed IAM user's stored ARN as its caller ARN (#1149) --- spinifex/gateway/auth.go | 9 ++- .../conditionkeys_completeness_test.go | 2 +- spinifex/gateway/conditionkeys_test.go | 50 +++++++++++--- spinifex/gateway/ecr_getauthorizationtoken.go | 3 +- .../gateway/ecr_getauthorizationtoken_test.go | 43 ++++++++++++ spinifex/gateway/ecr_principal.go | 9 ++- spinifex/gateway/ecr_principal_test.go | 61 ++++++++++++++++- spinifex/gateway/ecr_token.go | 2 +- spinifex/gateway/eks.go | 3 +- spinifex/gateway/gateway.go | 21 +++--- spinifex/gateway/sts.go | 11 +++- spinifex/gateway/sts/GetCallerIdentity.go | 33 +++++----- spinifex/gateway/sts_request_test.go | 65 +++++++++++++++++++ spinifex/handlers/sts/session_principal.go | 3 +- .../handlers/sts/session_principal_test.go | 15 +++++ 15 files changed, 282 insertions(+), 48 deletions(-) diff --git a/spinifex/gateway/auth.go b/spinifex/gateway/auth.go index f72e61a48..93e3e9949 100644 --- a/spinifex/gateway/auth.go +++ b/spinifex/gateway/auth.go @@ -184,12 +184,13 @@ func (gw *GatewayConfig) SigV4AuthMiddleware() func(http.Handler) http.Handler { // Resolved here, once, rather than per policy check: a fault must fail // the request rather than authorize it against a context missing the key. - userID, err := gw.principalUserID(principal) + userID, userARN, err := gw.principalUser(principal) if err != nil { gw.writeSigV4Error(w, r, err.Error(), "") return } principal.userID = userID + principal.userARN = userARN // Parse rewound the body; re-read it for query-arg parsing, then rewind // again for the downstream handler. @@ -222,6 +223,9 @@ func (gw *GatewayConfig) SigV4AuthMiddleware() func(http.Handler) http.Handler { if principal.userID != "" { ctx = context.WithValue(ctx, ctxUserID, principal.userID) } + if principal.userARN != "" { + ctx = context.WithValue(ctx, ctxUserARN, principal.userARN) + } // Parse once; dispatchers reuse via ctxQueryArgs. On error the // dispatcher re-parses and returns MalformedQueryString. @@ -348,6 +352,9 @@ type principalContext struct { // userID is aws:userid, resolved where the principal is so the value cannot // differ between two policy checks in the same request. userID string + // userARN is an IAM user's stored ARN, path included, from the same read as + // userID. Empty for every other principal type. + userARN string // sessionCred is the rehydrated STS record on the ASIA path, nil on the // long-lived one. It carries the continuity check past sig.Verify. sessionCred *handlers_sts.SessionCredential diff --git a/spinifex/gateway/conditionkeys_completeness_test.go b/spinifex/gateway/conditionkeys_completeness_test.go index a3970ce55..14ab297ab 100644 --- a/spinifex/gateway/conditionkeys_completeness_test.go +++ b/spinifex/gateway/conditionkeys_completeness_test.go @@ -126,7 +126,7 @@ func emittedKeys(t *testing.T) map[string]string { r.RemoteAddr = "10.4.1.9:52344" // Resolved the way the middleware resolves it, so the gate cannot pass on // a value only the test supplies. - userID, err := gw.principalUserID(principal) + userID, _, err := gw.principalUser(principal) require.NoError(t, err) principal.userID = userID for key := range requestConditionKeys(r, principal) { diff --git a/spinifex/gateway/conditionkeys_test.go b/spinifex/gateway/conditionkeys_test.go index f723d1041..7caeb7ffc 100644 --- a/spinifex/gateway/conditionkeys_test.go +++ b/spinifex/gateway/conditionkeys_test.go @@ -109,19 +109,19 @@ func TestRequestConditionKeys_OmitsUnresolvableUserID(t *testing.T) { assert.NotContains(t, keys, iampolicy.KeyUserID) } -// principalUserID resolves the ID once per request, at the point the principal +// principalUser resolves the ID once per request, at the point the principal // is resolved. A user with no record and a session minted before the ID was // recorded both yield an omitted key, not a failed request. func TestPrincipalUserID_ResolvesAndOmits(t *testing.T) { gw := &GatewayConfig{DisableLogging: true, IAMService: &mockIAMService{}} - userID, err := gw.principalUserID(principalContext{ + userID, _, err := gw.principalUser(principalContext{ identity: "alice", accountID: "000000000001", principalType: principalTypeUser, }) require.NoError(t, err) assert.Equal(t, "AIDAALICE", userID) - userID, err = gw.principalUserID(principalContext{ + userID, _, err = gw.principalUser(principalContext{ identity: "session", accountID: "000000000001", principalType: principalTypeAssumedRole, assumedRoleID: "AROASHAREDOPS:session", }) @@ -130,7 +130,7 @@ func TestPrincipalUserID_ResolvesAndOmits(t *testing.T) { // Root's aws:userid is the account ID, matching what GetCallerIdentity // reports for the same principal. - userID, err = gw.principalUserID(principalContext{ + userID, _, err = gw.principalUser(principalContext{ identity: "root", accountID: "000000000000", principalType: principalTypeUser, }) require.NoError(t, err) @@ -141,7 +141,7 @@ func TestPrincipalUserID_ResolvesAndOmits(t *testing.T) { return nil, errors.New(awserrors.ErrorIAMNoSuchEntity) }, }} - userID, err = missing.principalUserID(principalContext{ + userID, _, err = missing.principalUser(principalContext{ identity: "alice", accountID: "000000000001", principalType: principalTypeUser, }) require.NoError(t, err, "a deleted user omits the key rather than failing the request") @@ -153,14 +153,14 @@ func TestPrincipalUserID_ResolvesAndOmits(t *testing.T) { return &iam.GetUserOutput{User: &iam.User{UserName: aws.String("alice")}}, nil }, }} - userID, err = legacy.principalUserID(principalContext{ + userID, _, err = legacy.principalUser(principalContext{ identity: "alice", accountID: "000000000001", principalType: principalTypeUser, }) require.NoError(t, err) assert.Empty(t, userID) // A session minted before assumed_role_id was recorded takes the same arm. - userID, err = gw.principalUserID(principalContext{ + userID, _, err = gw.principalUser(principalContext{ identity: "session", accountID: "000000000001", principalType: principalTypeAssumedRole, }) require.NoError(t, err) @@ -176,13 +176,47 @@ func TestPrincipalUserID_DependencyFaultFailsClosed(t *testing.T) { }, }} - userID, err := faulty.principalUserID(principalContext{ + userID, userARN, err := faulty.principalUser(principalContext{ identity: "alice", accountID: "000000000001", principalType: principalTypeUser, }) require.Error(t, err) assert.Equal(t, awserrors.ErrorInternalError, err.Error()) assert.Empty(t, userID) + assert.Empty(t, userARN) +} + +// An IAM user's ARN comes from its record, path included, in the same read as +// its ID. Root and role sessions carry no user ARN. +func TestPrincipalUser_ResolvesStoredUserARN(t *testing.T) { + const pathed = "arn:aws:iam::000000000001:user/eng/alice" + gw := &GatewayConfig{DisableLogging: true, IAMService: &mockIAMService{ + getUserFn: func(string, *iam.GetUserInput) (*iam.GetUserOutput, error) { + return &iam.GetUserOutput{User: &iam.User{ + UserName: aws.String("alice"), UserId: aws.String("AIDAALICE"), Arn: aws.String(pathed), + }}, nil + }, + }} + + userID, userARN, err := gw.principalUser(principalContext{ + identity: "alice", accountID: "000000000001", principalType: principalTypeUser, + }) + require.NoError(t, err) + assert.Equal(t, "AIDAALICE", userID) + assert.Equal(t, pathed, userARN) + + _, userARN, err = gw.principalUser(principalContext{ + identity: "root", accountID: "000000000000", principalType: principalTypeUser, + }) + require.NoError(t, err) + assert.Empty(t, userARN) + + _, userARN, err = gw.principalUser(principalContext{ + identity: "session", accountID: "000000000001", principalType: principalTypeAssumedRole, + assumedRoleID: "AROASHAREDOPS:session", + }) + require.NoError(t, err) + assert.Empty(t, userARN) } // RoleSessionName is chosen by the caller of AssumeRole, so aws:username must diff --git a/spinifex/gateway/ecr_getauthorizationtoken.go b/spinifex/gateway/ecr_getauthorizationtoken.go index 0db8a41cb..a860c44a7 100644 --- a/spinifex/gateway/ecr_getauthorizationtoken.go +++ b/spinifex/gateway/ecr_getauthorizationtoken.go @@ -31,12 +31,13 @@ func (gw *GatewayConfig) handleGetAuthorizationToken(w http.ResponseWriter, r *h identity, _ := ctx.Value(ctxIdentity).(string) principalType, _ := ctx.Value(ctxPrincipalType).(string) assumedRoleARN, _ := ctx.Value(ctxAssumedRoleARN).(string) + userARN, _ := ctx.Value(ctxUserARN).(string) // The minted token names the exact IAM/STS record every /v2/* request will // later rehydrate against, so its subject must be the same canonical ARN // buildCallerARN produces everywhere else in the gateway (STS included) — // not a best-effort approximation that could omit an IAM path. - callerARN, err := buildCallerARN(accountID, identity, principalType, assumedRoleARN) + callerARN, err := buildCallerARN(accountID, identity, principalType, assumedRoleARN, userARN) if err != nil { slog.Error("GetAuthorizationToken: cannot build canonical caller ARN", "err", err) return errors.New(awserrors.ErrorServerInternal) diff --git a/spinifex/gateway/ecr_getauthorizationtoken_test.go b/spinifex/gateway/ecr_getauthorizationtoken_test.go index 2079d3fab..c508dc908 100644 --- a/spinifex/gateway/ecr_getauthorizationtoken_test.go +++ b/spinifex/gateway/ecr_getauthorizationtoken_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/aws/aws-sdk-go/aws" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -57,6 +58,48 @@ func TestHandleGetAuthorizationToken_MintsUsableToken(t *testing.T) { assert.Equal(t, ecrTestAccount, claims.AccountID) } +// The mint and verify halves agree on a pathed user: the token is minted for the +// stored ARN the middleware resolved, and verifies against the same record. +func TestHandleGetAuthorizationToken_PathedUserRoundTrips(t *testing.T) { + iamSvc := newECRMockIAMService() + seedECRTestUser(iamSvc, ecrPrincipalTestAccount, "dev", ecrPrincipalTestAKID) + iamSvc.users[ecrPrincipalTestAccount+"|dev"].Arn = aws.String(ecrPathedUserARN) + iss, verify := newECRAuth(t) + gw := &GatewayConfig{ + Region: ecrTestRegion, InternalSuffix: ecrTestSuffix, + ECRTokenIssuer: iss, ECRTokenVerifier: verify, DisableLogging: true, + IAMService: iamSvc, + } + + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader("{}")) + ctx := context.WithValue(req.Context(), ctxAccountID, ecrPrincipalTestAccount) + ctx = context.WithValue(ctx, ctxPrincipalType, principalTypeUser) + ctx = context.WithValue(ctx, ctxIdentity, "dev") + ctx = context.WithValue(ctx, ctxUserARN, ecrPathedUserARN) + ctx = context.WithValue(ctx, ctxAccessKey, ecrPrincipalTestAKID) + w := httptest.NewRecorder() + require.NoError(t, gw.handleGetAuthorizationToken(w, req.WithContext(ctx))) + require.Equal(t, http.StatusOK, w.Code) + + var out struct { + AuthorizationData []struct { + AuthorizationToken string `json:"authorizationToken"` + } `json:"authorizationData"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &out)) + require.Len(t, out.AuthorizationData, 1) + decoded, err := base64.StdEncoding.DecodeString(out.AuthorizationData[0].AuthorizationToken) + require.NoError(t, err) + _, jwtStr, _ := strings.Cut(string(decoded), ":") + + claims, err := verify.Verify(jwtStr) + require.NoError(t, err) + assert.Equal(t, ecrPathedUserARN, claims.Subject) + got, err := gw.resolveECRPrincipal(claims) + require.NoError(t, err) + assert.Equal(t, ecrPathedUserARN, got.userARN) +} + func TestHandleGetAuthorizationToken_ProxyEndpointCarriesPort(t *testing.T) { iss, verify := newECRAuth(t) gw := &GatewayConfig{ diff --git a/spinifex/gateway/ecr_principal.go b/spinifex/gateway/ecr_principal.go index 1808f5cad..3ec95d6b7 100644 --- a/spinifex/gateway/ecr_principal.go +++ b/spinifex/gateway/ecr_principal.go @@ -119,11 +119,12 @@ func (gw *GatewayConfig) resolveECRLongLivedPrincipal(claims *gateway_ecrauth.Cl return principalContext{}, classifyIAMLookupErr(err, "user") } identity := aws.StringValue(userOut.User.UserName) + userARN := aws.StringValue(userOut.User.Arn) if claims.PrincipalType != principalTypeUser { return principalContext{}, ecrInvalidPrincipal("principalType claim %q does not match resolved user", claims.PrincipalType) } - canonicalARN, err := buildCallerARN(ak.AccountID, identity, principalTypeUser, "") + canonicalARN, err := buildCallerARN(ak.AccountID, identity, principalTypeUser, "", userARN) if err != nil { return principalContext{}, ecrInvalidPrincipal("cannot build canonical ARN: %w", err) } @@ -136,6 +137,7 @@ func (gw *GatewayConfig) resolveECRLongLivedPrincipal(claims *gateway_ecrauth.Cl accountID: ak.AccountID, principalType: principalTypeUser, userID: aws.StringValue(userOut.User.UserId), + userARN: userARN, }, nil } @@ -186,7 +188,7 @@ func (gw *GatewayConfig) resolveECRSessionPrincipal(claims *gateway_ecrauth.Clai if claims.PrincipalType != principalTypeUser { return principalContext{}, ecrInvalidPrincipal("principalType claim %q does not match resolved session", claims.PrincipalType) } - canonicalARN, err := buildCallerARN(cred.AccountID, cred.SessionName, principalTypeUser, "") + canonicalARN, err := buildCallerARN(cred.AccountID, cred.SessionName, principalTypeUser, "", live.UserARN) if err != nil { return principalContext{}, ecrInvalidPrincipal("cannot build canonical ARN: %w", err) } @@ -199,6 +201,7 @@ func (gw *GatewayConfig) resolveECRSessionPrincipal(claims *gateway_ecrauth.Clai accountID: cred.AccountID, principalType: principalTypeUser, userID: live.UserID, + userARN: live.UserARN, }, nil } @@ -208,7 +211,7 @@ func (gw *GatewayConfig) resolveECRSessionPrincipal(claims *gateway_ecrauth.Clai return principalContext{}, ecrInvalidPrincipal("principalType claim %q does not match resolved session", claims.PrincipalType) } - canonicalARN, err := buildCallerARN(cred.AccountID, cred.SessionName, principalTypeAssumedRole, cred.AssumedRoleARN) + canonicalARN, err := buildCallerARN(cred.AccountID, cred.SessionName, principalTypeAssumedRole, cred.AssumedRoleARN, "") if err != nil { return principalContext{}, ecrInvalidPrincipal("cannot build canonical ARN: %w", err) } diff --git a/spinifex/gateway/ecr_principal_test.go b/spinifex/gateway/ecr_principal_test.go index 6abffde0c..961d73152 100644 --- a/spinifex/gateway/ecr_principal_test.go +++ b/spinifex/gateway/ecr_principal_test.go @@ -119,6 +119,9 @@ type ecrMockSTSService struct { // returns. Detection itself is exercised against real IAM/STS services in // handlers/sts; what these tests pin is how this door maps the answer. principalErr error + + // liveUserARN is the stored ARN the continuity read returns for a user session. + liveUserARN string } func newECRMockSTSService() *ecrMockSTSService { @@ -133,7 +136,11 @@ func (m *ecrMockSTSService) VerifySessionPrincipal(cred *handlers_sts.SessionCre if m.principalErr != nil { return nil, m.principalErr } - return stubSessionPrincipal(cred) + live, err := stubSessionPrincipal(cred) + if err == nil && cred.PrincipalType == principalTypeUser { + live.UserARN = m.liveUserARN + } + return live, err } const ( @@ -172,7 +179,34 @@ func TestResolveECRPrincipal_LongLivedUser(t *testing.T) { got, err := gw.resolveECRPrincipal(claims) require.NoError(t, err) assert.Equal(t, principalContext{identity: "dev", accountID: ecrPrincipalTestAccount, - principalType: principalTypeUser, userID: "AIDADEV"}, got) + principalType: principalTypeUser, userID: "AIDADEV", userARN: claims.Subject}, got) +} + +const ecrPathedUserARN = "arn:aws:iam::" + ecrPrincipalTestAccount + ":user/eng/dev" + +// A user at a non-root path verifies against its stored ARN, path included. A +// token whose subject drops the path, as one minted before the path was carried +// would, no longer names the principal and is rejected. +func TestResolveECRPrincipal_LongLivedUser_Pathed(t *testing.T) { + iamSvc := newECRMockIAMService() + seedECRTestUser(iamSvc, ecrPrincipalTestAccount, "dev", ecrPrincipalTestAKID) + iamSvc.users[ecrPrincipalTestAccount+"|dev"].Arn = aws.String(ecrPathedUserARN) + gw := &GatewayConfig{IAMService: iamSvc} + + claims := &gateway_ecrauth.Claims{ + AccountID: ecrPrincipalTestAccount, + PrincipalType: principalTypeUser, + AccessKeyID: ecrPrincipalTestAKID, + } + claims.Subject = ecrPathedUserARN + got, err := gw.resolveECRPrincipal(claims) + require.NoError(t, err) + assert.Equal(t, ecrPathedUserARN, got.userARN) + + claims.Subject = "arn:aws:iam::" + ecrPrincipalTestAccount + ":user/dev" + _, err = gw.resolveECRPrincipal(claims) + require.Error(t, err) + assert.False(t, isECRDependencyFailure(err)) } func TestResolveECRPrincipal_GlobalRoot(t *testing.T) { @@ -324,6 +358,29 @@ func TestResolveECRPrincipal_SessionToken_User(t *testing.T) { principalType: principalTypeUser, userID: "AIDADEV"}, got) } +func TestResolveECRPrincipal_SessionToken_User_Pathed(t *testing.T) { + iamSvc := newECRMockIAMService() + stsSvc := newECRMockSTSService() + seedECRSessionUser(iamSvc, stsSvc, ecrPrincipalTestAccount, "dev", ecrPrincipalTestASID, time.Now().Add(time.Hour)) + stsSvc.liveUserARN = ecrPathedUserARN + gw := &GatewayConfig{IAMService: iamSvc, STSService: stsSvc} + + claims := &gateway_ecrauth.Claims{ + AccountID: ecrPrincipalTestAccount, + PrincipalType: principalTypeUser, + AccessKeyID: ecrPrincipalTestASID, + } + claims.Subject = ecrPathedUserARN + got, err := gw.resolveECRPrincipal(claims) + require.NoError(t, err) + assert.Equal(t, ecrPathedUserARN, got.userARN) + + claims.Subject = "arn:aws:iam::" + ecrPrincipalTestAccount + ":user/dev" + _, err = gw.resolveECRPrincipal(claims) + require.Error(t, err) + assert.False(t, isECRDependencyFailure(err)) +} + // The user branch had no continuity check at all before: a GetSessionToken // session survived its user being deleted and recreated under the same name. func TestResolveECRPrincipal_SessionToken_User_StalePrincipalRejected(t *testing.T) { diff --git a/spinifex/gateway/ecr_token.go b/spinifex/gateway/ecr_token.go index 5d425b1e5..f038d188f 100644 --- a/spinifex/gateway/ecr_token.go +++ b/spinifex/gateway/ecr_token.go @@ -82,7 +82,7 @@ func (gw *GatewayConfig) handleECRToken(w http.ResponseWriter, r *http.Request) return } - callerARN, err := buildCallerARN(principal.accountID, principal.identity, principal.principalType, principal.assumedRoleARN) + callerARN, err := buildCallerARN(principal.accountID, principal.identity, principal.principalType, principal.assumedRoleARN, principal.userARN) if err != nil { slog.Error("ECR token endpoint: cannot build canonical caller ARN", "err", err) gateway_ecr.WriteError(w, http.StatusInternalServerError, "SERVER_ERROR", "token mint failed") diff --git a/spinifex/gateway/eks.go b/spinifex/gateway/eks.go index b0fac48af..574334e87 100644 --- a/spinifex/gateway/eks.go +++ b/spinifex/gateway/eks.go @@ -380,7 +380,8 @@ func eksCallerPrincipalARN(r *http.Request) string { identity, _ := ctx.Value(ctxIdentity).(string) principalType, _ := ctx.Value(ctxPrincipalType).(string) assumedRoleARN, _ := ctx.Value(ctxAssumedRoleARN).(string) - arn, err := buildCallerARN(accountID, identity, principalType, assumedRoleARN) + userARN, _ := ctx.Value(ctxUserARN).(string) + arn, err := buildCallerARN(accountID, identity, principalType, assumedRoleARN, userARN) if err != nil { slog.DebugContext(r.Context(), "EKS_Request: could not resolve caller principal ARN", "err", err) return "" diff --git a/spinifex/gateway/gateway.go b/spinifex/gateway/gateway.go index 1fc04e5e5..d655537cb 100644 --- a/spinifex/gateway/gateway.go +++ b/spinifex/gateway/gateway.go @@ -64,6 +64,8 @@ const ( // ctxUserID carries aws:userid, resolved once by the SigV4 middleware so // every policy check in a request evaluates the same value. ctxUserID contextKey = "sigv4.userID" + // ctxUserARN carries an IAM user's stored ARN, path included, from the same read. + ctxUserARN contextKey = "sigv4.userARN" // ctxUnderlyingRoleARN carries the IAM role ARN backing an assumed-role session. // Policy enforcement resolves the role name from this, never from ctxIdentity // (attacker-influenced RoleSessionName). @@ -637,6 +639,7 @@ func (gw *GatewayConfig) checkPolicyResourcesWithKeys(r *http.Request, service, assumedRoleID: mustCtxString(r, ctxAssumedRoleID), underlyingRoleARN: mustCtxString(r, ctxUnderlyingRoleARN), userID: mustCtxString(r, ctxUserID), + userARN: mustCtxString(r, ctxUserARN), } keys := requestConditionKeys(r, principal) maps.Copy(keys, actionKeys) @@ -702,19 +705,19 @@ func principalTypeCondition(principalType string) (string, bool) { } } -// principalUserID resolves aws:userid: an IAM user's unique ID, the role ID and +// principalUser resolves aws:userid: an IAM user's unique ID, the role ID and // session name STS minted for a role session, or the account ID for root. Both // halves of a session's ID come from the resolved role, so unlike aws:username -// it is not caller-chosen. +// it is not caller-chosen. An IAM user's stored ARN comes from the same read. // // A principal with no ID on record returns empty and the door omits the key. A // dependency fault returns InternalError instead: authorizing against a context // missing the key silently narrows an Allow and widens a Deny. -func (gw *GatewayConfig) principalUserID(principal principalContext) (string, error) { +func (gw *GatewayConfig) principalUser(principal principalContext) (userID, userARN string, err error) { if principal.identity == "" || principal.accountID == "" { - return "", nil + return "", "", nil } - userID, err := gateway_sts.ResolveCallerUserID(principal.accountID, principal.principalType, + userID, userARN, err = gateway_sts.ResolveCallerUser(principal.accountID, principal.principalType, principal.identity, principal.assumedRoleID, gw.IAMService) switch { case err == nil: @@ -723,15 +726,15 @@ func (gw *GatewayConfig) principalUserID(principal principalContext) (string, er "accountID", principal.accountID, "identity", principal.identity, "principalType", principal.principalType) } - return userID, nil + return userID, userARN, nil case strings.Contains(err.Error(), awserrors.ErrorIAMNoSuchEntity): slog.Warn("aws:userid unavailable: no such IAM user", "accountID", principal.accountID, "user", principal.identity) - return "", nil + return "", "", nil default: slog.Error("aws:userid: IAM dependency fault, refusing to authorize on a degraded context", "accountID", principal.accountID, "identity", principal.identity, "err", err) - return "", errors.New(awserrors.ErrorInternalError) + return "", "", errors.New(awserrors.ErrorInternalError) } } @@ -837,7 +840,7 @@ func (d *identityPolicyDenialError) Error() string { func (d *identityPolicyDenialError) detailedError() error { callerARN, err := buildCallerARN(d.principal.accountID, d.principal.identity, - d.principal.principalType, d.principal.assumedRoleARN) + d.principal.principalType, d.principal.assumedRoleARN, d.principal.userARN) if err != nil { callerARN = d.logIdentity } diff --git a/spinifex/gateway/sts.go b/spinifex/gateway/sts.go index 66907ae04..827abcbe1 100644 --- a/spinifex/gateway/sts.go +++ b/spinifex/gateway/sts.go @@ -195,8 +195,9 @@ func (gw *GatewayConfig) resolveSTSCaller(r *http.Request) (stsCaller, error) { assumedRoleARN, _ := ctx.Value(ctxAssumedRoleARN).(string) assumedRoleID, _ := ctx.Value(ctxAssumedRoleID).(string) accessKey, _ := ctx.Value(ctxAccessKey).(string) + userARN, _ := ctx.Value(ctxUserARN).(string) - arn, err := buildCallerARN(accountID, identity, principalType, assumedRoleARN) + arn, err := buildCallerARN(accountID, identity, principalType, assumedRoleARN, userARN) if err != nil { return stsCaller{}, err } @@ -212,8 +213,9 @@ func (gw *GatewayConfig) resolveSTSCaller(r *http.Request) (stsCaller, error) { } // buildCallerARN composes the caller ARN: assumed-role uses ctxAssumedRoleARN, -// root uses arn:aws:iam::{aid}:root, user uses arn:aws:iam::{aid}:user/{identity}. -func buildCallerARN(accountID, identity, principalType, assumedRoleARN string) (string, error) { +// root uses arn:aws:iam::{aid}:root, user uses its stored userARN, path included, +// or arn:aws:iam::{aid}:user/{identity} when there is no record to read it from. +func buildCallerARN(accountID, identity, principalType, assumedRoleARN, userARN string) (string, error) { switch principalType { case principalTypeAssumedRole: if assumedRoleARN == "" { @@ -231,6 +233,9 @@ func buildCallerARN(accountID, identity, principalType, assumedRoleARN string) ( slog.Error("STS_Request: user principal without identity") return "", errors.New(awserrors.ErrorInternalError) } + if userARN != "" { + return userARN, nil + } return spxarn.FormatIAMPath(spxarn.IAMUser, accountID, "/", identity), nil default: slog.Error("STS_Request: unknown principal type", "principalType", principalType) diff --git a/spinifex/gateway/sts/GetCallerIdentity.go b/spinifex/gateway/sts/GetCallerIdentity.go index a49ee32d1..0bf2a84d1 100644 --- a/spinifex/gateway/sts/GetCallerIdentity.go +++ b/spinifex/gateway/sts/GetCallerIdentity.go @@ -35,7 +35,7 @@ func GetCallerIdentity( iamSvc handlers_iam.IAMService, stsSvc handlers_sts.STSService, ) (*sts.GetCallerIdentityOutput, error) { - userID, err := ResolveCallerUserID(accountID, callerPrincipalType, identity, assumedRoleID, iamSvc) + userID, _, err := ResolveCallerUser(accountID, callerPrincipalType, identity, assumedRoleID, iamSvc) if err != nil { return nil, err } @@ -52,38 +52,37 @@ func GetCallerIdentity( return stsSvc.GetCallerIdentity(accountID, callerARN, userID, input) } -// ResolveCallerUserID resolves the UserId of an authenticated principal, which -// is also aws:userid at the authorization door. An error is a dependency fault; -// a principal with no ID to report returns empty, which each caller reads on its -// own terms. -func ResolveCallerUserID( +// ResolveCallerUser resolves the UserId of an authenticated principal, which is +// also aws:userid at the authorization door, and for an IAM user its stored ARN. +// An error is a dependency fault; a missing value returns empty. +func ResolveCallerUser( accountID, callerPrincipalType, identity, assumedRoleID string, iamSvc handlers_iam.IAMService, -) (string, error) { +) (userID, userARN string, err error) { switch callerPrincipalType { case PrincipalTypeRoot: - return accountID, nil + return accountID, "", nil case PrincipalTypeUser: // Root is encoded as principalType=user + identity="root"; UserId is the account ID. if identity == "root" { - return accountID, nil + return accountID, "", nil } if iamSvc == nil { - slog.Error("ResolveCallerUserID: IAM service not initialized") - return "", errors.New(awserrors.ErrorInternalError) + slog.Error("ResolveCallerUser: IAM service not initialized") + return "", "", errors.New(awserrors.ErrorInternalError) } out, err := iamSvc.GetUser(accountID, &iam.GetUserInput{UserName: aws.String(identity)}) if err != nil { - return "", err + return "", "", err } if out == nil || out.User == nil { - return "", nil + return "", "", nil } - return aws.StringValue(out.User.UserId), nil + return aws.StringValue(out.User.UserId), aws.StringValue(out.User.Arn), nil case PrincipalTypeAssumedRole: - return assumedRoleID, nil + return assumedRoleID, "", nil default: - slog.Error("ResolveCallerUserID: unknown principal type", "principalType", callerPrincipalType) - return "", errors.New(awserrors.ErrorInternalError) + slog.Error("ResolveCallerUser: unknown principal type", "principalType", callerPrincipalType) + return "", "", errors.New(awserrors.ErrorInternalError) } } diff --git a/spinifex/gateway/sts_request_test.go b/spinifex/gateway/sts_request_test.go index d2c440509..d4855962c 100644 --- a/spinifex/gateway/sts_request_test.go +++ b/spinifex/gateway/sts_request_test.go @@ -442,3 +442,68 @@ func TestSTSRequest_GetCallerIdentity_User_LookupIAM(t *testing.T) { assert.Contains(t, xmlStr, "arn:aws:iam::000000000000:user/alice") assert.Contains(t, xmlStr, "AIDAALICE000") } + +// A user at a non-root path is reported with its stored ARN, as AWS does, +// resolved by the SigV4 middleware rather than reformatted at "/". +func TestSTSRequest_GetCallerIdentity_PathedUserThroughMiddleware(t *testing.T) { + const pathed = "arn:aws:iam::123456789012:user/eng/alice" + encryptedSecret, err := handlers_iam.EncryptSecret(testSecretKey, testMasterKey) + require.NoError(t, err) + gw := &GatewayConfig{ + DisableLogging: true, + Region: testRegion, + STSService: &flexMockSTSService{}, + IAMService: &mockIAMService{ + masterKey: testMasterKey, + accessKeys: map[string]*handlers_iam.AccessKey{testAccessKey: { + AccessKeyID: testAccessKey, SecretAccessKey: encryptedSecret, + UserName: "alice", AccountID: "123456789012", Status: "Active", + }}, + getUserFn: func(string, *iam.GetUserInput) (*iam.GetUserOutput, error) { + return &iam.GetUserOutput{User: &iam.User{ + UserName: aws.String("alice"), UserId: aws.String("AIDAALICE"), Arn: aws.String(pathed), + }}, nil + }, + }, + } + handler := gw.SigV4AuthMiddleware()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := gw.STS_Request(w, r); err != nil { + gw.ErrorHandler(w, r, err) + } + })) + + body := []byte("Action=GetCallerIdentity") + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(string(body))) + req.Host = "localhost:9999" + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + signTestRequest(t, req, body, testAccessKey, testSecretKey) + + resp := doRequest(handler, req) + b, _ := io.ReadAll(resp.Body) + require.Equalf(t, http.StatusOK, resp.StatusCode, "body: %s", string(b)) + assert.Contains(t, string(b), ""+pathed+"") +} + +func TestBuildCallerARN(t *testing.T) { + const acct = "123456789012" + cases := []struct { + name, accountID, identity, principalType, assumedRoleARN, userARN, want string + }{ + {"pathed user takes its stored ARN", acct, "alice", principalTypeUser, "", + "arn:aws:iam::" + acct + ":user/eng/alice", "arn:aws:iam::" + acct + ":user/eng/alice"}, + {"user with no record formats at the root path", acct, "alice", principalTypeUser, "", "", + "arn:aws:iam::" + acct + ":user/alice"}, + {"global root ignores a user ARN", utils.GlobalAccountID, "root", principalTypeUser, "", + "arn:aws:iam::" + utils.GlobalAccountID + ":user/root", "arn:aws:iam::" + utils.GlobalAccountID + ":root"}, + {"root principal", acct, "", principalTypeRoot, "", "", "arn:aws:iam::" + acct + ":root"}, + {"assumed role keeps its session ARN", acct, "s", principalTypeAssumedRole, + "arn:aws:sts::" + acct + ":assumed-role/Ops/s", "", "arn:aws:sts::" + acct + ":assumed-role/Ops/s"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := buildCallerARN(tc.accountID, tc.identity, tc.principalType, tc.assumedRoleARN, tc.userARN) + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/spinifex/handlers/sts/session_principal.go b/spinifex/handlers/sts/session_principal.go index b20a7ed3c..7cdb7a5e9 100644 --- a/spinifex/handlers/sts/session_principal.go +++ b/spinifex/handlers/sts/session_principal.go @@ -42,6 +42,7 @@ func IsSessionPrincipalVerdict(err error) bool { // SessionPrincipal is the live IAM record a session credential still resolves to. type SessionPrincipal struct { UserID string // user sessions + UserARN string // user sessions RoleID string // role sessions RoleName string RoleARN string @@ -85,7 +86,7 @@ func (s *STSServiceImpl) verifySessionUser(cred *SessionCredential) (*SessionPri if liveUserID != cred.UserID { return nil, ErrSessionPrincipalReplaced } - return &SessionPrincipal{UserID: liveUserID}, nil + return &SessionPrincipal{UserID: liveUserID, UserARN: aws.StringValue(out.User.Arn)}, nil } // verifySessionRole resolves the session's underlying role ARN and compares the diff --git a/spinifex/handlers/sts/session_principal_test.go b/spinifex/handlers/sts/session_principal_test.go index 00aa664bd..efc955031 100644 --- a/spinifex/handlers/sts/session_principal_test.go +++ b/spinifex/handlers/sts/session_principal_test.go @@ -77,6 +77,21 @@ func TestVerifySessionPrincipal_UnchangedUser_ReturnsLiveID(t *testing.T) { assert.Empty(t, got.RoleID) } +// A user session reports the stored ARN, path included, so a door naming the +// session's principal agrees with the IAM record. +func TestVerifySessionPrincipal_PathedUser_ReturnsStoredARN(t *testing.T) { + svc, _ := newTestSetup(t) + _, err := svc.iamSvc.CreateUser(testCallerAccountID, &iam.CreateUserInput{ + UserName: aws.String(testCallerUserName), Path: aws.String("/eng/"), + }) + require.NoError(t, err) + cred := mintUserSession(t, svc, testCallerAccountID, testCallerUserName) + + got, err := svc.VerifySessionPrincipal(cred) + require.NoError(t, err) + assert.Equal(t, "arn:aws:iam::"+testCallerAccountID+":user/eng/"+testCallerUserName, got.UserARN) +} + func TestVerifySessionPrincipal_UnchangedRole_ReturnsLiveRecord(t *testing.T) { svc, _ := newTestSetup(t) role := createRoleInAccount(t, svc, testCallerAccountID, "app", trustPolicyAllowingUser(testCallerARN()))