fix: require iam:PassRole on the role in AddRoleToInstanceProfile (#1198)

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)
+}

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)
+}