diff --git a/spinifex/handlers/sts/assume_role.go b/spinifex/handlers/sts/assume_role.go index 22ccc0323..034278e41 100644 --- a/spinifex/handlers/sts/assume_role.go +++ b/spinifex/handlers/sts/assume_role.go @@ -13,8 +13,10 @@ import ( "log/slog" "regexp" "slices" + "strconv" "strings" "time" + "unicode/utf8" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" @@ -57,6 +59,9 @@ const ( maxDurationSeconds int64 = 43200 defaultDurationSeconds int64 = 3600 + minRoleSessionNameLength = 2 + maxRoleSessionNameLength = 64 + // sessionAKIDRandomBytes hex-encodes to 16 chars; with the ASIA prefix the // AKID is 20 chars total, matching the AWS public format. sessionAKIDRandomBytes = 8 @@ -70,6 +75,10 @@ const ( // ASCII range. `:` and `/` are excluded so the assumed-role ARN stays unambiguously parseable. var roleSessionNameRegex = regexp.MustCompile(`^[A-Za-z0-9_+=,.@-]{2,64}$`) +// roleSessionNameCharsetRegex is roleSessionNameRegex without the length bound, so the +// charset and length constraints are reported separately, as AWS does. +var roleSessionNameCharsetRegex = regexp.MustCompile(`^[A-Za-z0-9_+=,.@-]*$`) + // AssumeRole mints temporary credentials after evaluating the target role's // trust policy against the caller. func (s *STSServiceImpl) AssumeRole(callerAccountID, callerARN, callerIdentity string, input *sts.AssumeRoleInput) (*sts.AssumeRoleOutput, error) { @@ -82,8 +91,33 @@ func (s *STSServiceImpl) AssumeRole(callerAccountID, callerARN, callerIdentity s } sessionName := *input.RoleSessionName - if !roleSessionNameRegex.MatchString(sessionName) { - return nil, errors.New(awserrors.ErrorValidationError) + + // AWS reports every failed constraint at once, durationSeconds first, and before + // resolving the role, so a bad value against a missing role is not AccessDenied. + var violations []constraintViolation + if d := input.DurationSeconds; d != nil { + value := strconv.FormatInt(*d, 10) + if *d < minDurationSeconds { + violations = append(violations, constraintViolation{"durationSeconds", value, + fmt.Sprintf("Member must have value greater than or equal to %d", minDurationSeconds)}) + } else if *d > maxDurationSeconds { + violations = append(violations, constraintViolation{"durationSeconds", value, + fmt.Sprintf("Member must have value less than or equal to %d", maxDurationSeconds)}) + } + } + if !roleSessionNameCharsetRegex.MatchString(sessionName) { + violations = append(violations, constraintViolation{"roleSessionName", sessionName, + `Member must satisfy regular expression pattern: [\w+=,.@-]*`}) + } + if n := utf8.RuneCountInString(sessionName); n < minRoleSessionNameLength { + violations = append(violations, constraintViolation{"roleSessionName", sessionName, + fmt.Sprintf("Member must have length greater than or equal to %d", minRoleSessionNameLength)}) + } else if n > maxRoleSessionNameLength { + violations = append(violations, constraintViolation{"roleSessionName", sessionName, + fmt.Sprintf("Member must have length less than or equal to %d", maxRoleSessionNameLength)}) + } + if err := validationError(violations); err != nil { + return nil, err } if aws.StringValue(input.Policy) != "" || len(input.PolicyArns) > 0 { @@ -155,10 +189,15 @@ func (s *STSServiceImpl) AssumeRoleForInstance(accountID, roleARN, instanceID st func (s *STSServiceImpl) assumeRoleForCaller(ctx context.Context, callerARN, principalSource, roleARN, sessionName, sourceIdentity string, requestedDuration int64) (*sts.AssumeRoleOutput, error) { roleAccountID, role, err := ResolveRoleByARN(s.iamSvc, roleARN) if err != nil { - // A miss and a non-canonical ARN are both masked to AccessDenied, - // matching AWS and preventing role enumeration. + // A miss and a non-canonical ARN are both masked to AccessDenied, with the + // same text as a trust-policy refusal, preventing role enumeration. IMDS + // has no caller ARN and keeps the bare code. if errors.Is(err, ErrRoleUnresolved) { - return nil, errors.New(awserrors.ErrorAccessDenied) + if callerARN == "" { + return nil, errors.New(awserrors.ErrorAccessDenied) + } + return nil, awserrors.Errorf(awserrors.ErrorAccessDenied, + "User: %s is not authorized to perform: %s on resource: %s", callerARN, stsActionAssumeRole, roleARN) } return nil, err } @@ -172,6 +211,10 @@ func (s *STSServiceImpl) assumeRoleForCaller(ctx context.Context, callerARN, pri effectiveMax = defaultDurationSeconds } effectiveMax = min(effectiveMax, maxDurationSeconds) + if duration > effectiveMax && duration <= maxDurationSeconds { + return nil, awserrors.Errorf(awserrors.ErrorValidationError, + "The requested DurationSeconds exceeds the MaxSessionDuration set for this role.") + } if duration < minDurationSeconds || duration > effectiveMax { return nil, errors.New(awserrors.ErrorValidationError) } @@ -184,6 +227,10 @@ func (s *STSServiceImpl) assumeRoleForCaller(ctx context.Context, callerARN, pri } if err := evalTrustPolicy(aws.StringValue(role.AssumeRolePolicyDocument), callerARN, sources); err != nil { + if code, ok := awserrors.ResolveErrorCode(err); ok && code == awserrors.ErrorAccessDenied && callerARN != "" { + return nil, awserrors.Errorf(awserrors.ErrorAccessDenied, + "User: %s is not authorized to perform: %s on resource: %s", callerARN, stsActionAssumeRole, roleARN) + } return nil, err } diff --git a/spinifex/handlers/sts/assume_role_test.go b/spinifex/handlers/sts/assume_role_test.go index 00eba5ee7..ae8d16f0b 100644 --- a/spinifex/handlers/sts/assume_role_test.go +++ b/spinifex/handlers/sts/assume_role_test.go @@ -146,7 +146,7 @@ func TestAssumeRole_NonIAMRootARN_DoesNotMatch(t *testing.T) { _, err := svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, basicAssumeRoleInput(*role.Arn, "sess")) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } func TestAssumeRole_BareAccountIDPrincipal_TreatedAsRoot(t *testing.T) { @@ -195,8 +195,7 @@ func TestAssumeRole_ExplicitDenyWinsOverAllow(t *testing.T) { _, err := svc.AssumeRole(testCallerAccountID, caller, testCallerUserName, basicAssumeRoleInput(*role.Arn, "sess")) - require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + requireAWSError(t, err, awserrors.ErrorAccessDenied, assumeRoleDeniedMessage(caller, *role.Arn)) // And in the opposite order: Deny first, Allow second — same result. policyDenyThenAllow := fmt.Sprintf(`{"Version":"2012-10-17","Statement":[ @@ -207,7 +206,7 @@ func TestAssumeRole_ExplicitDenyWinsOverAllow(t *testing.T) { _, err = svc.AssumeRole(testCallerAccountID, caller, testCallerUserName, basicAssumeRoleInput(*role2.Arn, "sess")) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } func TestAssumeRole_NoMatchingAllow_AccessDenied(t *testing.T) { @@ -217,18 +216,18 @@ func TestAssumeRole_NoMatchingAllow_AccessDenied(t *testing.T) { _, err := svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, basicAssumeRoleInput(*role.Arn, "sess")) - require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + requireAWSError(t, err, awserrors.ErrorAccessDenied, assumeRoleDeniedMessage(testCallerARN(), *role.Arn)) } func TestAssumeRole_SameAccountRoleNotFound_AccessDenied(t *testing.T) { svc, _ := newTestSetup(t) - // AWS masks missing roles to AccessDenied regardless of account. + // AWS masks missing roles to AccessDenied regardless of account, naming the + // RoleArn as supplied, path included. + roleARN := fmt.Sprintf("arn:aws:iam::%s:role/team/ghost", testCallerAccountID) _, err := svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, - basicAssumeRoleInput(fmt.Sprintf("arn:aws:iam::%s:role/ghost", testCallerAccountID), "sess")) - require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + basicAssumeRoleInput(roleARN, "sess")) + requireAWSError(t, err, awserrors.ErrorAccessDenied, assumeRoleDeniedMessage(testCallerARN(), roleARN)) } func TestAssumeRole_CrossAccountRoleNotFound_AccessDenied(t *testing.T) { @@ -237,10 +236,10 @@ func TestAssumeRole_CrossAccountRoleNotFound_AccessDenied(t *testing.T) { // Caller is in callerAccountID; missing role is in testCrossAccountID. // Masking to AccessDenied (rather than NoSuchEntity) prevents // cross-account role enumeration. + roleARN := fmt.Sprintf("arn:aws:iam::%s:role/ghost", testCrossAccountID) _, err := svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, - basicAssumeRoleInput(fmt.Sprintf("arn:aws:iam::%s:role/ghost", testCrossAccountID), "sess")) - require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + basicAssumeRoleInput(roleARN, "sess")) + requireAWSError(t, err, awserrors.ErrorAccessDenied, assumeRoleDeniedMessage(testCallerARN(), roleARN)) } func TestAssumeRole_DurationBounds(t *testing.T) { @@ -248,16 +247,20 @@ func TestAssumeRole_DurationBounds(t *testing.T) { caller := testCallerARN() role := createRoleInAccount(t, svc, testCallerAccountID, "dur", trustPolicyAllowingUser(caller)) + const exceedsRoleMax = "The requested DurationSeconds exceeds the MaxSessionDuration set for this role." cases := []struct { - name string - seconds int64 - wantErr bool + name string + seconds int64 + wantErr bool + wantMessage string }{ - {"below_minimum", 899, true}, - {"at_minimum", 900, false}, - {"default_via_unset", 0, false}, - {"at_role_max", 3600, false}, - {"above_role_max", 3601, true}, + {"below_minimum", 899, true, "1 validation error detected: Value '899' at 'durationSeconds' failed to satisfy constraint: " + + "Member must have value greater than or equal to 900"}, + {"at_minimum", 900, false, ""}, + {"default_via_unset", 0, false, ""}, + {"at_role_max", 3600, false, ""}, + {"above_role_max", 3601, true, exceedsRoleMax}, + {"at_absolute_max", maxDurationSeconds, true, exceedsRoleMax}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -267,8 +270,7 @@ func TestAssumeRole_DurationBounds(t *testing.T) { } _, err := svc.AssumeRole(testCallerAccountID, caller, testCallerUserName, input) if tc.wantErr { - require.Error(t, err) - assert.Equal(t, awserrors.ErrorValidationError, err.Error()) + requireAWSError(t, err, awserrors.ErrorValidationError, tc.wantMessage) return } require.NoError(t, err) @@ -303,8 +305,32 @@ func TestAssumeRole_LongerMaxSessionDuration_CapAtTwelveHours(t *testing.T) { RoleSessionName: aws.String("sess2"), DurationSeconds: aws.Int64(maxDurationSeconds + 1), }) - require.Error(t, err) - assert.Equal(t, awserrors.ErrorValidationError, err.Error()) + requireAWSError(t, err, awserrors.ErrorValidationError, + "1 validation error detected: Value '43201' at 'durationSeconds' failed to satisfy constraint: "+ + "Member must have value less than or equal to 43200") +} + +// AWS checks input constraints before the role, so a bad value against a missing +// role is a ValidationError, and every failed constraint is reported at once. +func TestAssumeRole_ConstraintsCheckedBeforeRoleAndReportedTogether(t *testing.T) { + svc, _ := newTestSetup(t) + missing := fmt.Sprintf("arn:aws:iam::%s:role/ghost", testCallerAccountID) + + input := basicAssumeRoleInput(missing, "sess") + input.DurationSeconds = aws.Int64(899) + _, err := svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, input) + requireAWSError(t, err, awserrors.ErrorValidationError, + "1 validation error detected: Value '899' at 'durationSeconds' failed to satisfy constraint: "+ + "Member must have value greater than or equal to 900") + + input = basicAssumeRoleInput(missing, "bad name!") + input.DurationSeconds = aws.Int64(43201) + _, err = svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, input) + requireAWSError(t, err, awserrors.ErrorValidationError, + "2 validation errors detected: Value '43201' at 'durationSeconds' failed to satisfy constraint: "+ + "Member must have value less than or equal to 43200; "+ + "Value 'bad name!' at 'roleSessionName' failed to satisfy constraint: "+ + `Member must satisfy regular expression pattern: [\w+=,.@-]*`) } func TestAssumeRole_RejectsSessionPolicies(t *testing.T) { @@ -375,6 +401,32 @@ func TestAssumeRole_RejectsSessionTags(t *testing.T) { } } +// assertErrorCode resolves the registered code, which a message attached via +// awserrors.Errorf hides from err.Error(). +func assertErrorCode(t *testing.T, err error, wantCode string) { + t.Helper() + require.Error(t, err) + code, ok := awserrors.ResolveErrorCode(err) + require.True(t, ok, "error must carry a registered code: %v", err) + assert.Equal(t, wantCode, code) +} + +// requireAWSError pins the code and the exact client-facing message; an empty +// wantMessage asserts the bare code with no message attached. +func requireAWSError(t *testing.T, err error, wantCode, wantMessage string) { + t.Helper() + require.Error(t, err) + code, message, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok, "error must carry a registered code: %v", err) + assert.Equal(t, wantCode, code) + assert.Equal(t, wantMessage, message) +} + +// assumeRoleDeniedMessage is AWS's AccessDenied text, naming the RoleArn as supplied. +func assumeRoleDeniedMessage(callerARN, roleARN string) string { + return fmt.Sprintf("User: %s is not authorized to perform: sts:AssumeRole on resource: %s", callerARN, roleARN) +} + // requireUnsupportedSessionInput pins both the code and the message, since the // defect being guarded is a misleading message under a plausible code. func requireUnsupportedSessionInput(t *testing.T, err error, wantMessage string) { @@ -443,29 +495,42 @@ func TestAssumeRole_RejectsInvalidSessionName(t *testing.T) { svc, _ := newTestSetup(t) role := createRoleInAccount(t, svc, testCallerAccountID, "name", trustPolicyAllowingUser(testCallerARN())) - bad := []string{ - "", // missing → MissingParameter (handled separately below) - "a", // too short - strings.Repeat("a", 65), // too long - "has/slash", - "has:colon", - "ué", // non-ASCII; literal regex deliberately excludes Unicode + const ( + pattern = `Member must satisfy regular expression pattern: [\w+=,.@-]*` + tooShort = "Member must have length greater than or equal to 2" + tooLong = "Member must have length less than or equal to 64" + ) + violation := func(name, constraint string) string { + return fmt.Sprintf("Value '%s' at 'roleSessionName' failed to satisfy constraint: %s", name, constraint) } - for _, name := range bad { - t.Run("session_"+name, func(t *testing.T) { - input := &sts.AssumeRoleInput{ - RoleArn: role.Arn, - RoleSessionName: aws.String(name), - } - _, err := svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, input) - require.Error(t, err) - if name == "" { - assert.Equal(t, awserrors.ErrorMissingParameter, err.Error()) - } else { - assert.Equal(t, awserrors.ErrorValidationError, err.Error()) - } + patternMessage := func(name string) string { + return "1 validation error detected: " + violation(name, pattern) + } + long, longBad := strings.Repeat("a", 65), strings.Repeat("a", 64)+"!" + bad := []struct { + name string + wantMessage string + }{ + {"a", "1 validation error detected: " + violation("a", tooShort)}, + {long, "1 validation error detected: " + violation(long, tooLong)}, + {"!", "2 validation errors detected: " + violation("!", pattern) + "; " + violation("!", tooShort)}, + {longBad, "2 validation errors detected: " + violation(longBad, pattern) + "; " + violation(longBad, tooLong)}, + {"has/slash", patternMessage("has/slash")}, + {"has:colon", patternMessage("has:colon")}, + {"bad name!", patternMessage("bad name!")}, + {"ué", patternMessage("ué")}, // non-ASCII; literal regex deliberately excludes Unicode + } + for _, tc := range bad { + t.Run("session_"+tc.name, func(t *testing.T) { + _, err := svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, + basicAssumeRoleInput(*role.Arn, tc.name)) + requireAWSError(t, err, awserrors.ErrorValidationError, tc.wantMessage) }) } + + _, err := svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, + basicAssumeRoleInput(*role.Arn, "")) + requireAWSError(t, err, awserrors.ErrorMissingParameter, "") } func TestAssumeRole_RejectsMissingRequiredFields(t *testing.T) { @@ -554,7 +619,7 @@ func TestAssumeRole_ChainedAssume_LiteralSessionARN(t *testing.T) { _, err = svc.AssumeRole(testCallerAccountID, otherSession, "SourceRole", basicAssumeRoleInput(*role.Arn, "no")) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } func TestAssumeRole_PrincipalArray_AnyEntryMatches(t *testing.T) { @@ -585,7 +650,7 @@ func TestAssumeRole_ServicePrincipal_NotMatchedByUserCaller(t *testing.T) { _, err = svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, basicAssumeRoleInput(*role2.Arn, "sess")) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } // ----- ECS task-role attribution (HTTPS AssumeRole path) ----------------- @@ -636,7 +701,7 @@ func TestAssumeRole_ECSTasks_TaskRoleCallerDenied(t *testing.T) { _, err := svc.AssumeRole(testCallerAccountID, taskCaller, "attacker-task-role", basicAssumeRoleInput(*victim.Arn, "ecs-task-2")) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } func TestAssumeRole_ECSTasks_UserCallerDenied(t *testing.T) { @@ -646,7 +711,7 @@ func TestAssumeRole_ECSTasks_UserCallerDenied(t *testing.T) { _, err := svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, basicAssumeRoleInput(*role.Arn, "sess")) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } // A container instance is attributed the service principal only for roles in its @@ -658,7 +723,7 @@ func TestAssumeRole_ECSTasks_CrossAccountInstanceCallerDenied(t *testing.T) { _, err := svc.AssumeRole(testCallerAccountID, instanceRoleCallerARN(testCallerAccountID, testInstanceID), ecsInstanceRoleName, basicAssumeRoleInput(*role.Arn, "ecs-task-3")) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } // Attribution is per-principal, not blanket: a container instance is ecs-tasks @@ -670,7 +735,7 @@ func TestAssumeRole_ECSTasks_InstanceCallerCannotClaimEC2(t *testing.T) { _, err := svc.AssumeRole(testCallerAccountID, instanceRoleCallerARN(testCallerAccountID, testInstanceID), ecsInstanceRoleName, basicAssumeRoleInput(*role.Arn, "ecs-task-4")) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } func TestAssumeRole_ECSTasks_ExplicitDenyWins(t *testing.T) { @@ -683,7 +748,7 @@ func TestAssumeRole_ECSTasks_ExplicitDenyWins(t *testing.T) { _, err := svc.AssumeRole(testCallerAccountID, instanceRoleCallerARN(testCallerAccountID, testInstanceID), ecsInstanceRoleName, basicAssumeRoleInput(*role.Arn, "ecs-task-5")) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } // ----- Trust-policy Condition evaluation on sts:AssumeRole (mulga-yxd9p, B) -- @@ -720,7 +785,7 @@ func TestAssumeRole_SourceAccountCondition_NonMatchingAccountDenies(t *testing.T _, err := svc.AssumeRole(testCallerAccountID, instanceRoleCallerARN(testCallerAccountID, testInstanceID), ecsInstanceRoleName, basicAssumeRoleInput(*role.Arn, "ecs-task-sa-2")) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } // TestAssumeRole_SourceAccountCondition_DenyEvaluatedBeforeAllow pins that the @@ -738,7 +803,7 @@ func TestAssumeRole_SourceAccountCondition_DenyEvaluatedBeforeAllow(t *testing.T _, err := svc.AssumeRole(testCallerAccountID, instanceRoleCallerARN(testCallerAccountID, testInstanceID), ecsInstanceRoleName, basicAssumeRoleInput(*role.Arn, "ecs-task-sa-3")) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } // TestAssumeRoleConditionsHold_UnknownKeyFailsClosed unit-tests the evaluator @@ -839,7 +904,7 @@ func TestAssumeRoleForInstance_NonWhitelistedService_Denied(t *testing.T) { _, err := svc.AssumeRoleForInstance(testCallerAccountID, *role.Arn, testInstanceID, defaultDurationSeconds) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } // Attribution does not leak between entry points: IMDS is ec2.amazonaws.com only, @@ -850,7 +915,7 @@ func TestAssumeRoleForInstance_ECSTasksService_Denied(t *testing.T) { _, err := svc.AssumeRoleForInstance(testCallerAccountID, *role.Arn, testInstanceID, defaultDurationSeconds) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } // A role that only trusts an AWS principal (no Service entry) must not be @@ -859,9 +924,9 @@ func TestAssumeRoleForInstance_AWSOnlyPrincipal_Denied(t *testing.T) { svc, _ := newTestSetup(t) role := createRoleInAccount(t, svc, testCallerAccountID, "user-role", trustPolicyAllowingUser(testCallerARN())) + // IMDS has no caller ARN to name, so the denial carries the bare code. _, err := svc.AssumeRoleForInstance(testCallerAccountID, *role.Arn, testInstanceID, defaultDurationSeconds) - require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + requireAWSError(t, err, awserrors.ErrorAccessDenied, "") } // Regression: the EC2 service principal must remain unreachable over the HTTPS @@ -875,7 +940,7 @@ func TestAssumeRoleForInstance_EC2ServiceUnreachableViaHTTPS(t *testing.T) { _, err := svc.AssumeRole(testCallerAccountID, testCallerARN(), testCallerUserName, basicAssumeRoleInput(*role.Arn, "sess")) require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + assertErrorCode(t, err, awserrors.ErrorAccessDenied) } func TestAssumeRoleForInstance_RejectsMissingFields(t *testing.T) { @@ -907,8 +972,7 @@ func TestAssumeRoleForInstance_CrossAccountRoleMiss_AccessDenied(t *testing.T) { // missing — masked to AccessDenied to prevent cross-account enumeration. _, err := svc.AssumeRoleForInstance(testCallerAccountID, fmt.Sprintf("arn:aws:iam::%s:role/ghost", testCrossAccountID), testInstanceID, defaultDurationSeconds) - require.Error(t, err) - assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) + requireAWSError(t, err, awserrors.ErrorAccessDenied, "") } func TestAssumeRole_RetriesOnAKIDCollision(t *testing.T) { diff --git a/spinifex/handlers/sts/get_access_key_info.go b/spinifex/handlers/sts/get_access_key_info.go index 57eb192bd..4f8148b25 100644 --- a/spinifex/handlers/sts/get_access_key_info.go +++ b/spinifex/handlers/sts/get_access_key_info.go @@ -2,7 +2,10 @@ package handlers_sts import ( "errors" + "fmt" "log/slog" + "regexp" + "unicode/utf8" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/sts" @@ -16,6 +19,9 @@ const ( maxAccessKeyIDLength = 128 ) +// accessKeyIDRegex is the model's AccessKeyIdType pattern [\w]*, ASCII-only as on AWS. +var accessKeyIDRegex = regexp.MustCompile(`^\w*$`) + // GetAccessKeyInfo resolves an access key ID to the account that owns it. // AWS requires no permission for this call and does not scope it to the caller, // so no caller identity is threaded in: a key owned by another account resolves @@ -29,8 +35,21 @@ func (s *STSServiceImpl) GetAccessKeyInfo(input *sts.GetAccessKeyInfoInput) (*st if accessKeyID == "" { return nil, errors.New(awserrors.ErrorMissingParameter) } - if !isAccessKeyIDShaped(accessKeyID) { - return nil, errors.New(awserrors.ErrorValidationError) + + var violations []constraintViolation + if !accessKeyIDRegex.MatchString(accessKeyID) { + violations = append(violations, constraintViolation{"accessKeyId", accessKeyID, + `Member must satisfy regular expression pattern: [\w]*`}) + } + if n := utf8.RuneCountInString(accessKeyID); n < minAccessKeyIDLength { + violations = append(violations, constraintViolation{"accessKeyId", accessKeyID, + fmt.Sprintf("Member must have length greater than or equal to %d", minAccessKeyIDLength)}) + } else if n > maxAccessKeyIDLength { + violations = append(violations, constraintViolation{"accessKeyId", accessKeyID, + fmt.Sprintf("Member must have length less than or equal to %d", maxAccessKeyIDLength)}) + } + if err := validationError(violations); err != nil { + return nil, err } accountID, err := s.resolveAccessKeyAccount(accessKeyID) @@ -38,10 +57,10 @@ func (s *STSServiceImpl) GetAccessKeyInfo(input *sts.GetAccessKeyInfoInput) (*st return nil, err } if accountID == "" { - // AWS reports an unresolvable key ID as an invalid token. A success with a + // AWS reports an unresolvable key ID as a validation error. A success with a // blank Account would read to a caller as a mismatch rather than a failure. slog.Debug("GetAccessKeyInfo: access key ID resolved to no account", "akid", accessKeyID) - return nil, errors.New(awserrors.ErrorInvalidClientTokenId) + return nil, awserrors.Errorf(awserrors.ErrorValidationError, "Access key ID is not valid.") } return &sts.GetAccessKeyInfoOutput{Account: aws.String(accountID)}, nil @@ -87,19 +106,3 @@ func isNoSuchEntity(err error) bool { code, ok := awserrors.ResolveErrorCode(err) return ok && code == awserrors.ErrorIAMNoSuchEntity } - -// isAccessKeyIDShaped reports whether s satisfies the model's AccessKeyIdType: -// 16-128 characters, each an ASCII letter or digit. -func isAccessKeyIDShaped(s string) bool { - if len(s) < minAccessKeyIDLength || len(s) > maxAccessKeyIDLength { - return false - } - for _, r := range s { - switch { - case r >= 'A' && r <= 'Z', r >= 'a' && r <= 'z', r >= '0' && r <= '9': - default: - return false - } - } - return true -} diff --git a/spinifex/handlers/sts/get_access_key_info_test.go b/spinifex/handlers/sts/get_access_key_info_test.go index 2252fbd08..1d3eead84 100644 --- a/spinifex/handlers/sts/get_access_key_info_test.go +++ b/spinifex/handlers/sts/get_access_key_info_test.go @@ -67,7 +67,7 @@ func TestGetAccessKeyInfo_ResolvesSessionCredential(t *testing.T) { assert.Equal(t, testCallerAccountID, aws.StringValue(out.Account)) } -func TestGetAccessKeyInfo_UnknownKeyIsInvalidClientToken(t *testing.T) { +func TestGetAccessKeyInfo_UnknownKeyIsValidationError(t *testing.T) { svc, _ := newTestSetup(t) cases := []struct { @@ -77,6 +77,8 @@ func TestGetAccessKeyInfo_UnknownKeyIsInvalidClientToken(t *testing.T) { {"unknown long-lived", "AKIANOTAREALKEY00000"}, {"unknown session", SessionAccessKeyIDPrefix + "NOTAREALKEY00000"}, {"unknown prefix", "AROANOTAREALKEY00000"}, + {"underscore", "AKIA_0123456789ABCD"}, + {"lowercase", "akiaiosfodnn7example"}, } for _, tc := range cases { @@ -84,7 +86,10 @@ func TestGetAccessKeyInfo_UnknownKeyIsInvalidClientToken(t *testing.T) { out, err := svc.GetAccessKeyInfo(&sts.GetAccessKeyInfoInput{AccessKeyId: aws.String(tc.akid)}) require.Error(t, err) assert.Nil(t, out) - assert.Equal(t, awserrors.ErrorInvalidClientTokenId, err.Error()) + code, message, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok) + assert.Equal(t, awserrors.ErrorValidationError, code) + assert.Equal(t, "Access key ID is not valid.", message) }) } } @@ -100,22 +105,29 @@ func TestGetAccessKeyInfo_MalformedKeyRejectedWithoutLookup(t *testing.T) { akid string want string }{ - {"empty", "", awserrors.ErrorMissingParameter}, - {"too short", "AKIA0123", awserrors.ErrorValidationError}, - {"too long", strings.Repeat("A", maxAccessKeyIDLength+1), awserrors.ErrorValidationError}, - {"punctuation", "AKIA-0123456789ABCD", awserrors.ErrorValidationError}, - {"whitespace", "AKIA 0123456789ABCD", awserrors.ErrorValidationError}, - {"arn not a key id", "arn:aws:iam::000000000000:user/alice", awserrors.ErrorValidationError}, + {"too short", "AKIA0123", + "1 validation error detected: Value 'AKIA0123' at 'accessKeyId' failed to satisfy constraint: Member must have length greater than or equal to 16"}, + {"too long", strings.Repeat("A", 129), + "1 validation error detected: Value '" + strings.Repeat("A", 129) + "' at 'accessKeyId' failed to satisfy constraint: Member must have length less than or equal to 128"}, + {"punctuation", "AKIA-0123456789ABCD", + "1 validation error detected: Value 'AKIA-0123456789ABCD' at 'accessKeyId' failed to satisfy constraint: Member must satisfy regular expression pattern: [\\w]*"}, + {"whitespace", "AKIA 0123456789ABCD", + "1 validation error detected: Value 'AKIA 0123456789ABCD' at 'accessKeyId' failed to satisfy constraint: Member must satisfy regular expression pattern: [\\w]*"}, + {"too short and punctuation", "AKIA-012", + "2 validation errors detected: Value 'AKIA-012' at 'accessKeyId' failed to satisfy constraint: Member must satisfy regular expression pattern: [\\w]*; " + + "Value 'AKIA-012' at 'accessKeyId' failed to satisfy constraint: Member must have length greater than or equal to 16"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { out, err := svc.GetAccessKeyInfo(&sts.GetAccessKeyInfoInput{AccessKeyId: aws.String(tc.akid)}) - require.Error(t, err) assert.Nil(t, out) - assert.Equal(t, tc.want, err.Error()) + requireAWSError(t, err, awserrors.ErrorValidationError, tc.want) }) } + + _, err := svc.GetAccessKeyInfo(&sts.GetAccessKeyInfoInput{AccessKeyId: aws.String("")}) + requireAWSError(t, err, awserrors.ErrorMissingParameter, "") assert.Zero(t, stub.calls, "malformed access key ID must not reach the KV bucket") } @@ -148,12 +160,3 @@ func TestGetAccessKeyInfo_RecordWithoutAccountIsInternalError(t *testing.T) { require.Error(t, err) assert.Equal(t, awserrors.ErrorInternalError, err.Error()) } - -func TestIsAccessKeyIDShaped(t *testing.T) { - assert.True(t, isAccessKeyIDShaped("AKIA0123456789ABCDEF")) - assert.True(t, isAccessKeyIDShaped(strings.Repeat("A", minAccessKeyIDLength))) - assert.True(t, isAccessKeyIDShaped(strings.Repeat("A", maxAccessKeyIDLength))) - assert.False(t, isAccessKeyIDShaped(strings.Repeat("A", minAccessKeyIDLength-1))) - assert.False(t, isAccessKeyIDShaped(strings.Repeat("A", maxAccessKeyIDLength+1))) - assert.False(t, isAccessKeyIDShaped("AKIA_123456789ABCDE")) -} diff --git a/spinifex/handlers/sts/validation.go b/spinifex/handlers/sts/validation.go new file mode 100644 index 000000000..82ae2cbd5 --- /dev/null +++ b/spinifex/handlers/sts/validation.go @@ -0,0 +1,33 @@ +package handlers_sts + +import ( + "fmt" + "strings" + + "github.com/mulgadc/spinifex/spinifex/awserrors" +) + +// constraintViolation is one input member failing one of its model constraints. +type constraintViolation struct { + field string + value string + constraint string +} + +// validationError renders violations as the single ValidationError AWS returns for +// failed model constraints, in the order given, or nil when there are none. +func validationError(violations []constraintViolation) error { + if len(violations) == 0 { + return nil + } + parts := make([]string, len(violations)) + for i, v := range violations { + parts[i] = fmt.Sprintf("Value '%s' at '%s' failed to satisfy constraint: %s", v.value, v.field, v.constraint) + } + noun := "errors" + if len(violations) == 1 { + noun = "error" + } + return awserrors.Errorf(awserrors.ErrorValidationError, + "%d validation %s detected: %s", len(violations), noun, strings.Join(parts, "; ")) +}