From 24e2572ebd33968901ac7be8ef9286ad6457ab44 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Mon, 26 Oct 2026 10:21:03 +1100 Subject: [PATCH] fix: resolve a GetSessionToken session as its IAM user in presigned GetCallerIdentity, and reject an identity with no ARN (#1254) * fix: resolve a GetSessionToken session as its IAM user in presigned GetCallerIdentity, and reject an identity with no ARN * fix: report the account ID as root's UserId for a root session in presigned GetCallerIdentity --- spinifex/domains/sts/presigned_url_verify.go | 49 ++++++-- .../domains/sts/presigned_url_verify_test.go | 115 ++++++++++++++++++ 2 files changed, 156 insertions(+), 8 deletions(-) diff --git a/spinifex/domains/sts/presigned_url_verify.go b/spinifex/domains/sts/presigned_url_verify.go index 315561e42..a3d1da374 100644 --- a/spinifex/domains/sts/presigned_url_verify.go +++ b/spinifex/domains/sts/presigned_url_verify.go @@ -95,9 +95,26 @@ func (s *STSServiceImpl) VerifyPresignedGetCallerIdentity(presignedURL, expected // Deferred past req.Verify for the reason the SigV4 door defers it: the check // reads the IAM users and roles buckets, and an AKID travels in cleartext, so // running it earlier lets anyone holding a copy drive IAM reads unauthenticated. - if err := s.checkVerifySessionPrincipal(sessionCred, req.Credential.AccessKeyID); err != nil { + live, err := s.checkVerifySessionPrincipal(sessionCred, req.Credential.AccessKeyID) + if err != nil { return nil, err } + // A user session persists only the user name, so its ARN, path included, + // comes from the record the continuity check just read. + if sessionCred != nil && principal.PrincipalType == principalTypeUserPresigned { + principal.ARN = live.UserARN + if principal.ARN == "" { + principal.ARN = arn.FormatIAMPath(arn.IAMUser, sessionCred.AccountID, "/", sessionCred.SessionName) + } + } + + // The EKS webhook keys its access-entry lookup on this ARN, so an identity + // without one is refused here rather than handed downstream. + if principal.ARN == "" { + slog.Error("VerifyPresignedGetCallerIdentity: resolved principal has no ARN", + "akid", req.Credential.AccessKeyID, "principal_type", principal.PrincipalType) + return nil, errors.New(awserrors.ErrorInvalidIdentityToken) + } principal.XK8sAwsID = expectedClusterName return principal, nil @@ -105,27 +122,28 @@ func (s *STSServiceImpl) VerifyPresignedGetCallerIdentity(presignedURL, expected // checkVerifySessionPrincipal maps a continuity verdict onto this door's // vocabulary. A nil cred is the long-lived path, which has no session to check. -func (s *STSServiceImpl) checkVerifySessionPrincipal(cred *SessionCredential, accessKeyID string) error { +func (s *STSServiceImpl) checkVerifySessionPrincipal(cred *SessionCredential, accessKeyID string) (*SessionPrincipal, error) { if cred == nil { - return nil + return nil, nil } // Without this the session branch performs no IAM lookup at all, so even a // deleted principal leaves a working path to its cluster identity mapping. - if _, err := s.VerifySessionPrincipal(cred); err != nil { + live, err := s.VerifySessionPrincipal(cred) + if err != nil { if IsSessionPrincipalVerdict(err) { slog.Warn("VerifyPresignedGetCallerIdentity: session principal continuity check failed", "akid", accessKeyID, "reason", err) - return errors.New(awserrors.ErrorInvalidIdentityToken) + return nil, errors.New(awserrors.ErrorInvalidIdentityToken) } // The NATS responder logs every verify failure at Debug, on the assumption // that a rejection is the expected case. A fault is not, and would otherwise // take the fleet down with nothing written at production log level. slog.Error("VerifyPresignedGetCallerIdentity: session principal check faulted, refusing to authorize", "akid", accessKeyID, "err", err) - return fmt.Errorf("verify session principal: %w", err) + return nil, fmt.Errorf("verify session principal: %w", err) } - return nil + return live, nil } // mapSigv4Err maps sigv4 sentinel errors onto AWS error codes. Time-window failures @@ -140,7 +158,7 @@ func mapSigv4Err(err error) error { } // resolvePrincipalForVerify resolves an access key to its plaintext secret and a -// PresignedCallerIdentity skeleton. Branches on AKID prefix (session vs long-lived). +// PresignedCallerIdentity skeleton, branching on AKID prefix then session PrincipalType. // The session record comes back so the caller can check continuity past the signature. func (s *STSServiceImpl) resolvePrincipalForVerify(accessKeyID string) (*PresignedCallerIdentity, string, *SessionCredential, error) { switch { @@ -159,6 +177,21 @@ func (s *STSServiceImpl) resolvePrincipalForVerify(accessKeyID string) (*Presign if err != nil { return nil, "", nil, fmt.Errorf("decrypt session secret: %w", err) } + if cred.PrincipalType == principalTypeUser { + // GetSessionToken session: the caller is the IAM user itself. The ARN + // is filled in once the signature and continuity check have passed. + userID := cred.UserID + if cred.SessionName == "root" { + // AWS reports the account ID as root's UserId. + userID = cred.AccountID + } + return &PresignedCallerIdentity{ + AccountID: cred.AccountID, + UserID: userID, + PrincipalType: principalTypeUserPresigned, + }, secret, cred, nil + } + // "assumed-role", or empty for a record predating PrincipalType. return &PresignedCallerIdentity{ AccountID: cred.AccountID, ARN: cred.AssumedRoleARN, diff --git a/spinifex/domains/sts/presigned_url_verify_test.go b/spinifex/domains/sts/presigned_url_verify_test.go index 3f2f1942f..b796c5926 100644 --- a/spinifex/domains/sts/presigned_url_verify_test.go +++ b/spinifex/domains/sts/presigned_url_verify_test.go @@ -1,6 +1,7 @@ package sts import ( + "encoding/json" "errors" "net/http" "net/url" @@ -12,6 +13,7 @@ import ( "github.com/aws/aws-sdk-go/aws/credentials" v4 "github.com/aws/aws-sdk-go/aws/signer/v4" "github.com/aws/aws-sdk-go/service/iam" + "github.com/aws/aws-sdk-go/service/sts" "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -105,6 +107,119 @@ func TestVerifyPresignedGetCallerIdentity_HappyPath_SessionCred(t *testing.T) { assert.Equal(t, testCallerAccountID, got.AccountID) assert.Equal(t, cluster, got.XK8sAwsID) assert.Equal(t, principalTypeAssumedRolePresigned, got.PrincipalType) + assert.Equal(t, aws.StringValue(assumeOut.AssumedRoleUser.Arn), got.ARN) + assert.Equal(t, aws.StringValue(assumeOut.AssumedRoleUser.AssumedRoleId), got.UserID) +} + +// A GetSessionToken session is the IAM user itself, as GetCallerIdentity reports +// it on AWS: the user ARN, path included, and the user's UserId. +func TestVerifyPresignedGetCallerIdentity_UserSession_ResolvesAsUser(t *testing.T) { + svc, _ := newTestSetup(t) + out, err := svc.iamSvc.CreateUser(testCallerAccountID, &iam.CreateUserInput{ + UserName: aws.String(testCallerUserName), Path: aws.String("/eng/"), + }) + require.NoError(t, err) + tokOut, err := svc.GetSessionToken(testCallerAccountID, testCallerUserName, principalTypeUser, + testCallerAccessKeyID, &sts.GetSessionTokenInput{}) + require.NoError(t, err) + + signedAt := time.Now().UTC().Truncate(time.Second) + withFrozenTime(t, signedAt) + + const cluster = "user-session-cluster" + u := presignTestURL(t, aws.StringValue(tokOut.Credentials.AccessKeyId), + aws.StringValue(tokOut.Credentials.SecretAccessKey), cluster, signedAt, 900) + + got, err := svc.VerifyPresignedGetCallerIdentity(u, cluster) + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, principalTypeUserPresigned, got.PrincipalType) + assert.Equal(t, "arn:aws:iam::"+testCallerAccountID+":user/eng/"+testCallerUserName, got.ARN) + assert.Equal(t, aws.StringValue(out.User.UserId), got.UserID) + assert.Equal(t, testCallerAccountID, got.AccountID) +} + +// GetCallerIdentity reports the account ID as root's UserId, so the presigned +// door must agree with the SigV4 one for a root session. +func TestVerifyPresignedGetCallerIdentity_RootSession_UserIDIsAccountID(t *testing.T) { + svc, _ := newTestSetup(t) + seedUser(t, svc, testCallerAccountID, "root") + tokOut, err := svc.GetSessionToken(testCallerAccountID, "root", principalTypeUser, + testCallerAccessKeyID, &sts.GetSessionTokenInput{}) + require.NoError(t, err) + + signedAt := time.Now().UTC().Truncate(time.Second) + withFrozenTime(t, signedAt) + + const cluster = "root-session-cluster" + u := presignTestURL(t, aws.StringValue(tokOut.Credentials.AccessKeyId), + aws.StringValue(tokOut.Credentials.SecretAccessKey), cluster, signedAt, 900) + + got, err := svc.VerifyPresignedGetCallerIdentity(u, cluster) + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, principalTypeUserPresigned, got.PrincipalType) + assert.Equal(t, testCallerAccountID, got.UserID) +} + +// rewriteSession mints a role session, applies mutate to its stored record and +// returns a URL presigned with it. +func rewriteSession(t *testing.T, svc *STSServiceImpl, roleName, cluster string, signedAt time.Time, mutate func(*SessionCredential)) (string, *sts.AssumeRoleOutput) { + t.Helper() + role := createRoleInAccount(t, svc, testCallerAccountID, roleName, trustPolicyAllowingUser(testCallerARN())) + assumeOut, err := svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, + basicAssumeRoleInput(*role.Arn, "sess-"+roleName)) + require.NoError(t, err) + akid := aws.StringValue(assumeOut.Credentials.AccessKeyId) + + cred, err := svc.LookupSessionCredential(akid) + require.NoError(t, err) + require.NotNil(t, cred) + mutate(cred) + raw, err := json.Marshal(cred) + require.NoError(t, err) + _, err = sessionsKV(t, svc).Put(t.Context(), akid, raw) + require.NoError(t, err) + + return presignTestURL(t, akid, aws.StringValue(assumeOut.Credentials.SecretAccessKey), cluster, signedAt, 900), assumeOut +} + +// A record minted before PrincipalType existed carries an empty value and is a +// role session. +func TestVerifyPresignedGetCallerIdentity_LegacyEmptyPrincipalType_ResolvesAsAssumedRole(t *testing.T) { + svc, _ := newTestSetup(t) + signedAt := time.Now().UTC().Truncate(time.Second) + withFrozenTime(t, signedAt) + + const cluster = "legacy-cluster" + u, assumeOut := rewriteSession(t, svc, "irsa-legacy", cluster, signedAt, func(c *SessionCredential) { + c.PrincipalType = "" + }) + + got, err := svc.VerifyPresignedGetCallerIdentity(u, cluster) + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, principalTypeAssumedRolePresigned, got.PrincipalType) + assert.Equal(t, aws.StringValue(assumeOut.AssumedRoleUser.Arn), got.ARN) + assert.Equal(t, aws.StringValue(assumeOut.AssumedRoleUser.AssumedRoleId), got.UserID) +} + +// The EKS webhook maps the returned ARN to a Kubernetes identity, so an identity +// that resolves without one must be refused, not returned empty. +func TestVerifyPresignedGetCallerIdentity_EmptyARN_Rejected(t *testing.T) { + svc, _ := newTestSetup(t) + signedAt := time.Now().UTC().Truncate(time.Second) + withFrozenTime(t, signedAt) + + const cluster = "no-arn-cluster" + u, _ := rewriteSession(t, svc, "irsa-noarn", cluster, signedAt, func(c *SessionCredential) { + c.AssumedRoleARN = "" + }) + + got, err := svc.VerifyPresignedGetCallerIdentity(u, cluster) + require.Error(t, err) + assert.Nil(t, got) + assert.Equal(t, awserrors.ErrorInvalidIdentityToken, err.Error()) } // Before the continuity check the session branch performed no IAM lookup at