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
2026-10-26 05:59:41 +00:00 · 2026-10-26 10:21:03 +11:00
parent 0ac0a106c0
commit 24e2572ebd
2 changed files with 156 additions and 8 deletions
@@ -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,
@@ -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