From c861642d44920c8819ab55deecbbb8c0ef21249a Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Wed, 14 Oct 2026 14:00:29 +1100 Subject: [PATCH] fix: require iam:PassRole on the role in AddRoleToInstanceProfile (#1198) --- spinifex/gateway/iam.go | 29 ++++++ spinifex/gateway/iam_passrole_test.go | 132 ++++++++++++++++++++++++++ 2 files changed, 161 insertions(+) create mode 100644 spinifex/gateway/iam_passrole_test.go diff --git a/spinifex/gateway/iam.go b/spinifex/gateway/iam.go index 492cf68aa..76aa3fce4 100644 --- a/spinifex/gateway/iam.go +++ b/spinifex/gateway/iam.go @@ -6,7 +6,9 @@ import ( "log/slog" "net/http" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awsec2query" "github.com/mulgadc/spinifex/spinifex/awserrors" gateway_ec2_instance "github.com/mulgadc/spinifex/spinifex/gateway/ec2/instance" @@ -370,6 +372,11 @@ func (gw *GatewayConfig) IAM_Request(w http.ResponseWriter, r *http.Request) err if err := gw.checkPolicyResources(r, "iam", action, resources); err != nil { return err } + if in, ok := input.(*iam.AddRoleToInstanceProfileInput); ok { + if err := gw.checkInstanceProfilePassRole(r, accountID, in); err != nil { + return err + } + } xmlOutput, err := handler.dispatch(action, input, gw, accountID) if err != nil { @@ -383,3 +390,25 @@ func (gw *GatewayConfig) IAM_Request(w http.ResponseWriter, r *http.Request) err } return nil } + +// checkInstanceProfilePassRole requires iam:PassRole on the role being added to +// an instance profile: once added, every instance holding the profile receives +// its credentials, and the EC2 association check never sees the role. +func (gw *GatewayConfig) checkInstanceProfilePassRole(r *http.Request, accountID string, input *iam.AddRoleToInstanceProfileInput) error { + roleName := aws.StringValue(input.RoleName) + if roleName == "" { + return nil + } + roleARN, err := gw.IAMService.CanonicalResourceARN(accountID, arn.IAMRole, roleName) + switch { + case awserrors.IsErrorCode(err, awserrors.ErrorIAMNoSuchEntity): + // Still evaluated, so a caller who cannot pass the role cannot use + // the handler's NoSuchEntity to probe which roles exist. + roleARN = arn.FormatIAMPath(arn.IAMRole, accountID, "/", roleName) + case err != nil: + slog.Error("IAM: role lookup for iam:PassRole failed, failing closed", + "account_id", accountID, "role_name", roleName, "err", err) + return errors.New(awserrors.ErrorInternalError) + } + return gw.checkPassRole(r, roleARN, ec2ServicePrincipal) +} diff --git a/spinifex/gateway/iam_passrole_test.go b/spinifex/gateway/iam_passrole_test.go new file mode 100644 index 000000000..bd72044d7 --- /dev/null +++ b/spinifex/gateway/iam_passrole_test.go @@ -0,0 +1,132 @@ +//test:in-package — exercises IAM_Request with policy and auth context helpers. + +package gateway + +import ( + "errors" + "testing" + + "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/spinifex/spinifex/arn" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + passRoleProfileARN = "arn:aws:iam::" + authzAccountID + ":instance-profile/web" + passRoleAdminARN = "arn:aws:iam::" + authzAccountID + ":role/ops/admin" + addAdminToWeb = "Action=AddRoleToInstanceProfile&InstanceProfileName=web&RoleName=admin" +) + +// addRoleIAMService records the roles the handler binds, so a test can prove a +// denied request never reached it. +type addRoleIAMService struct { + iamAuthzPolicyService + + roleLookupErr error + added []string +} + +func (s *addRoleIAMService) CanonicalResourceARN(accountID string, kind arn.IAMResourceType, name string) (string, error) { + if kind == arn.IAMRole && s.roleLookupErr != nil { + return "", s.roleLookupErr + } + return s.iamAuthzPolicyService.CanonicalResourceARN(accountID, kind, name) +} + +func (s *addRoleIAMService) AddRoleToInstanceProfile(_ string, input *iam.AddRoleToInstanceProfileInput) (*iam.AddRoleToInstanceProfileOutput, error) { + s.added = append(s.added, *input.RoleName) + return &iam.AddRoleToInstanceProfileOutput{}, nil +} + +func addRoleGateway(statements ...handlers_iam.Statement) (*GatewayConfig, *addRoleIAMService) { + base, inner := iamScopedGateway(map[string]string{ + "web": passRoleProfileARN, + "admin": passRoleAdminARN, + }, statements...) + svc := &addRoleIAMService{iamAuthzPolicyService: *inner} + base.IAMService = svc + return base, svc +} + +// The escalation: a caller denied PassRole on a role binds it to a profile its +// instance already holds, and IMDS then serves the role's credentials. +func TestAddRoleToInstanceProfile_PassRoleDenyBlocksBinding(t *testing.T) { + gw, svc := addRoleGateway( + statement("Allow", "iam:AddRoleToInstanceProfile", "*"), + statement("Allow", "iam:PassRole", "*"), + statement("Deny", "iam:PassRole", passRoleAdminARN), + ) + assertDenied(t, dispatchIAM(t, gw, addAdminToWeb)) + assert.Empty(t, svc.added) +} + +func TestAddRoleToInstanceProfile_RequiresPassRoleGrant(t *testing.T) { + gw, svc := addRoleGateway(statement("Allow", "iam:AddRoleToInstanceProfile", passRoleProfileARN)) + assertDenied(t, dispatchIAM(t, gw, addAdminToWeb)) + assert.Empty(t, svc.added) +} + +// The grant is matched against the role's stored path, as the EC2 association +// check matches it, so a path-scoped grant behaves the same on both routes. +func TestAddRoleToInstanceProfile_PassRoleOnCanonicalPath(t *testing.T) { + allowed, svc := addRoleGateway( + statement("Allow", "iam:AddRoleToInstanceProfile", "*"), + statement("Allow", "iam:PassRole", "arn:aws:iam::"+authzAccountID+":role/ops/*"), + ) + require.NoError(t, dispatchIAM(t, allowed, addAdminToWeb)) + assert.Equal(t, []string{"admin"}, svc.added) + + wrongPath, _ := addRoleGateway( + statement("Allow", "iam:AddRoleToInstanceProfile", "*"), + statement("Allow", "iam:PassRole", "arn:aws:iam::"+authzAccountID+":role/admin"), + ) + assertDenied(t, dispatchIAM(t, wrongPath, addAdminToWeb)) +} + +func TestAddRoleToInstanceProfile_PassedToServiceIsEC2(t *testing.T) { + ec2Grant, svc := addRoleGateway( + statement("Allow", "iam:AddRoleToInstanceProfile", "*"), + passRoleTo("Allow", "StringEquals", ec2ServicePrincipal), + ) + require.NoError(t, dispatchIAM(t, ec2Grant, addAdminToWeb)) + assert.Equal(t, []string{"admin"}, svc.added) + + ecsGrant, _ := addRoleGateway( + statement("Allow", "iam:AddRoleToInstanceProfile", "*"), + passRoleTo("Allow", "StringEquals", ecsTasksServicePrincipal), + ) + assertDenied(t, dispatchIAM(t, ecsGrant, addAdminToWeb)) +} + +// An unknown role is reported only to a caller allowed to pass it, so the +// reply cannot be used to enumerate roles. +func TestAddRoleToInstanceProfile_UnknownRole(t *testing.T) { + body := "Action=AddRoleToInstanceProfile&InstanceProfileName=web&RoleName=ghost" + + denied, svc := addRoleGateway(statement("Allow", "iam:AddRoleToInstanceProfile", "*")) + assertDenied(t, dispatchIAM(t, denied, body)) + assert.Empty(t, svc.added) + + allowed, svc := addRoleGateway( + statement("Allow", "iam:AddRoleToInstanceProfile", "*"), + statement("Allow", "iam:PassRole", "arn:aws:iam::"+authzAccountID+":role/ghost"), + ) + require.NoError(t, dispatchIAM(t, allowed, body)) + assert.Equal(t, []string{"ghost"}, svc.added) +} + +func TestAddRoleToInstanceProfile_RoleLookupFaultFailsClosed(t *testing.T) { + gw, svc := addRoleGateway(statement("Allow", "iam:*", "*")) + svc.roleLookupErr = errors.New("storage unavailable") + require.EqualError(t, dispatchIAM(t, gw, addAdminToWeb), awserrors.ErrorInternalError) + assert.Empty(t, svc.added) +} + +func TestAddRoleToInstanceProfile_MissingRoleStaysValidationFault(t *testing.T) { + gw, _ := addRoleGateway(statement("Allow", "iam:AddRoleToInstanceProfile", "*")) + err := dispatchIAM(t, gw, "Action=AddRoleToInstanceProfile&InstanceProfileName=web") + require.EqualError(t, err, awserrors.ErrorMissingParameter) +}