fix: authorize EC2 PassRole against the stored role ARN (#1008)

spinifex/gateway/ec2/instance/IamInstanceProfileAssociation.go

@@ -41,7 +41,10 @@ func resolveAndAuthorizeProfile(spec *ec2.IamInstanceProfileSpecification, iamSv
 		return nil, err
 	}
 	if profile.RoleName != "" && passRoleCheck != nil {
-		roleARN := arn.FormatIAMPath(arn.IAMRole, profile.AccountID, "/", profile.RoleName)
+		roleARN, err := iamSvc.CanonicalResourceARN(profile.AccountID, arn.IAMRole, profile.RoleName)
+		if err != nil {
+			return nil, err
+		}
 		if err := passRoleCheck(roleARN); err != nil {
 			return nil, err
 		}

spinifex/gateway/ec2/instance/IamInstanceProfileAssociation_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mulgadc/spinifex/spinifex/arn"
 	"github.com/mulgadc/spinifex/spinifex/awserrors"
 	handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
 	spxtypes "github.com/mulgadc/spinifex/spinifex/types"
@@ -31,14 +32,13 @@ const (
 	testProfileIDOther   = "AIPAEXAMPLE0000000002"
 )
 
-// fakeIAMService embeds the IAMService interface so it satisfies the contract
-// without hand-writing every method. The EC2 IAM-profile gateway code only
-// touches ResolveInstanceProfile; any other method nil-panics, surfacing an
-// unexpected call rather than masking it with a zero value.
+// fakeIAMService implements profile and canonical role lookups.
+// Other methods nil-panic to surface unexpected calls.
 type fakeIAMService struct {
 	handlers_iam.IAMService
 
-	resolveFn func(accountID, nameOrARN string) (*handlers_iam.InstanceProfile, error)
+	resolveFn   func(accountID, nameOrARN string) (*handlers_iam.InstanceProfile, error)
+	canonicalFn func(accountID string, kind arn.IAMResourceType, name string) (string, error)
 }
 
 func (f *fakeIAMService) ResolveInstanceProfile(accountID, nameOrARN string) (*handlers_iam.InstanceProfile, error) {
@@ -48,6 +48,16 @@ func (f *fakeIAMService) ResolveInstanceProfile(accountID, nameOrARN string) (*h
 	return nil, errors.New(awserrors.ErrorIAMNoSuchEntity)
 }
 
+func (f *fakeIAMService) CanonicalResourceARN(accountID string, kind arn.IAMResourceType, name string) (string, error) {
+	if f.canonicalFn != nil {
+		return f.canonicalFn(accountID, kind, name)
+	}
+	if accountID == testGwAccountID && kind == arn.IAMRole && name == "app-role" {
+		return testRoleARNApp, nil
+	}
+	return "", errors.New(awserrors.ErrorIAMNoSuchEntity)
+}
+
 var _ handlers_iam.IAMService = (*fakeIAMService)(nil)
 
 // profileWithRole builds a resolved profile fixture pointing at the test role.
@@ -682,3 +692,58 @@ func TestAssociationIDFormat(t *testing.T) {
 	suffix := strings.TrimPrefix(id, "iip-assoc-")
 	assert.NotEmpty(t, suffix, "association ID must carry a non-empty random suffix")
 }
+
+func TestResolveAndAuthorizeProfile_CanonicalRoleARN(t *testing.T) {
+	t.Parallel()
+	for _, path := range []string{"/", "/team/", "/team/services/"} {
+		t.Run(path, func(t *testing.T) {
+			t.Parallel()
+			storedARN := "arn:aws:iam::111122223333:role" + path + "app-role"
+			svc := &fakeIAMService{
+				resolveFn: func(string, string) (*handlers_iam.InstanceProfile, error) {
+					return profileWithRole(), nil
+				},
+				canonicalFn: func(accountID string, kind arn.IAMResourceType, name string) (string, error) {
+					require.Equal(t, testGwAccountID, accountID)
+					require.Equal(t, arn.IAMRole, kind)
+					require.Equal(t, "app-role", name)
+					return storedARN, nil
+				},
+			}
+			var checkedARN string
+			_, err := resolveAndAuthorizeProfile(
+				&ec2.IamInstanceProfileSpecification{Name: aws.String(testProfileNameApp)},
+				svc, testGwAccountID, func(roleARN string) error {
+					checkedARN = roleARN
+					return nil
+				})
+			require.NoError(t, err)
+			require.Equal(t, storedARN, checkedARN)
+		})
+	}
+}
+
+func TestResolveAndAuthorizeProfile_CanonicalLookupError(t *testing.T) {
+	t.Parallel()
+	for _, lookupErr := range []error{errors.New(awserrors.ErrorIAMNoSuchEntity), nats.ErrTimeout} {
+		t.Run(lookupErr.Error(), func(t *testing.T) {
+			t.Parallel()
+			svc := &fakeIAMService{
+				resolveFn: func(string, string) (*handlers_iam.InstanceProfile, error) {
+					return profileWithRole(), nil
+				},
+				canonicalFn: func(string, arn.IAMResourceType, string) (string, error) {
+					return "", lookupErr
+				},
+			}
+			profile, err := resolveAndAuthorizeProfile(
+				&ec2.IamInstanceProfileSpecification{Name: aws.String(testProfileNameApp)},
+				svc, testGwAccountID, func(string) error {
+					t.Fatal("PassRole must not run when the role lookup fails")
+					return nil
+				})
+			require.ErrorIs(t, err, lookupErr)
+			require.Nil(t, profile)
+		})
+	}
+}

tests/integration/run_instances_passrole_test.go

@@ -33,27 +33,30 @@ func prPolicyWithPassRole(targetRoleARN string) string {
 	]}`, targetRoleARN)
 }
 
-// TestRunInstances_DeniedWithoutPassRole proves RunInstances enforces
-// iam:PassRole (resolveAndAuthorizeInstanceProfile ->
-// resolveAndAuthorizeProfile in gateway/ec2/instance/RunInstances.go /
-// IamInstanceProfileAssociation.go) against the role inside a supplied
-// instance profile: a caller granted ec2:RunInstances but not iam:PassRole on
-// the target role is denied, and the identical session is allowed once the
-// role grants that specific PassRole. Isolating PassRole as the sole variable
-// (ec2:RunInstances is granted throughout) proves the denial is genuinely
-// gated on the missing PassRole grant rather than some other authz gap.
+// TestRunInstances_DeniedWithoutPassRole exercises stored role paths with real IAM
+// policy evaluation: missing grants and explicit denies reject the request,
+// while an allow naming the stored role ARN permits dispatch.
 func TestRunInstances_DeniedWithoutPassRole(t *testing.T) {
+	for _, path := range []string{"/", "/team/", "/team/services/"} {
+		t.Run(path, func(t *testing.T) {
+			testInstanceProfilePassRole(t, path)
+		})
+	}
+}
+
+func testInstanceProfilePassRole(t *testing.T, path string) {
+	t.Helper()
 	gw := StartGateway(t)
 	iamCli := gw.IAMClient(t)
 
-	targetRoleARN := iamRoleARN(gw.AccountID, prTargetRole)
-
 	// The role backing the instance profile the caller wants to attach.
-	_, err := iamCli.CreateRole(&iam.CreateRoleInput{
+	targetRole, err := iamCli.CreateRole(&iam.CreateRoleInput{
 		RoleName:                 aws.String(prTargetRole),
+		Path:                     aws.String(path),
 		AssumeRolePolicyDocument: aws.String(iamTrustPolicyEC2Standard),
 	})
 	require.NoError(t, err, "create-role (target)")
+	targetRoleARN := aws.StringValue(targetRole.Role.Arn)
 
 	_, err = iamCli.CreateInstanceProfile(&iam.CreateInstanceProfileInput{
 		InstanceProfileName: aws.String(prTargetProfile),
@@ -113,6 +116,33 @@ func TestRunInstances_DeniedWithoutPassRole(t *testing.T) {
 	_, err = sessionCli.EC2.RunInstances(runInput)
 	requireAWSErrorCode(t, err, "AccessDenied")
 
+	// An explicit path-scoped deny must override an otherwise unrestricted grant.
+	denyPolicy, err := iamCli.CreatePolicy(&iam.CreatePolicyInput{
+		PolicyName: aws.String("pr-deny-path"),
+		PolicyDocument: aws.String(fmt.Sprintf(`{"Version":"2012-10-17","Statement":[
+			{"Effect":"Allow","Action":"*","Resource":"*"},
+			{"Effect":"Deny","Action":"iam:PassRole","Resource":"arn:aws:iam::%s:role%s*"}
+		]}`, gw.AccountID, path)),
+	})
+	require.NoError(t, err)
+	_, err = iamCli.AttachRolePolicy(&iam.AttachRolePolicyInput{
+		RoleName:  callerRoleOut.Role.RoleName,
+		PolicyArn: denyPolicy.Policy.Arn,
+	})
+	require.NoError(t, err)
+	_, err = sessionCli.EC2.RunInstances(runInput)
+	requireAWSErrorCode(t, err, "AccessDenied")
+	_, err = sessionCli.EC2.AssociateIamInstanceProfile(&ec2.AssociateIamInstanceProfileInput{
+		InstanceId:         aws.String("i-0123456789abcdef0"),
+		IamInstanceProfile: runInput.IamInstanceProfile,
+	})
+	requireAWSErrorCode(t, err, "AccessDenied")
+	_, err = iamCli.DetachRolePolicy(&iam.DetachRolePolicyInput{
+		RoleName:  callerRoleOut.Role.RoleName,
+		PolicyArn: denyPolicy.Policy.Arn,
+	})
+	require.NoError(t, err)
+
 	// Grant iam:PassRole scoped to the exact target role ARN; the identical
 	// session must now be allowed through to a real launch.
 	withPassRolePolicy, err := iamCli.CreatePolicy(&iam.CreatePolicyInput{