From d8a7d7fab352f0d64d118aef479a90ab2a7073b2 Mon Sep 17 00:00:00 2001 From: Kevin Lucas Date: Thu, 11 Jan 2024 19:11:49 +0000 Subject: [PATCH] fix: Fix missing PassRole permissions required by the remediation document --- src/ec2-required-role-config-rule.ts | 9 +-- ...required-role-remediation-configuration.ts | 21 +++++-- src/ec2-required-role-remediation-document.ts | 26 +++++++- test/index.test.ts | 62 +++++++++++++------ 4 files changed, 83 insertions(+), 35 deletions(-) diff --git a/src/ec2-required-role-config-rule.ts b/src/ec2-required-role-config-rule.ts index 54bf5f0..88ef215 100644 --- a/src/ec2-required-role-config-rule.ts +++ b/src/ec2-required-role-config-rule.ts @@ -1,6 +1,6 @@ import { Duration, Resource, ResourceProps } from 'aws-cdk-lib'; import { CustomRule, ResourceType, RuleScope } from 'aws-cdk-lib/aws-config'; -import { IInstanceProfile, IRole, InstanceProfile, Role, ServicePrincipal } from 'aws-cdk-lib/aws-iam'; +import { IInstanceProfile, IRole, Role, ServicePrincipal } from 'aws-cdk-lib/aws-iam'; import { IFunction } from 'aws-cdk-lib/aws-lambda'; import { IConstruct } from 'constructs'; import { Ec2RequiredRoleRemediationConfiguration } from './ec2-required-role-remediation-configuration'; @@ -55,14 +55,9 @@ export class Ec2RequiredRoleConfigRule extends Resource { path: '/service-role/', }); - this.defaultInstanceProfile = new InstanceProfile(this, 'default-ec2-instance-profile', { - path: '/service-role/', - role: this.defaultRole, - }); - new Ec2RequiredRoleRemediationConfiguration(this, 'remediation', { automatic: props.remediation?.automatic, - instanceProfile: this.defaultInstanceProfile, + ec2DefaultRole: this.defaultRole, maxAutomaticAttempts: props.remediation?.maxAutomaticAttempts, retryPeriod: props.remediation?.retryPeriod, rule: this.rule, diff --git a/src/ec2-required-role-remediation-configuration.ts b/src/ec2-required-role-remediation-configuration.ts index 925e050..011e855 100644 --- a/src/ec2-required-role-remediation-configuration.ts +++ b/src/ec2-required-role-remediation-configuration.ts @@ -1,13 +1,13 @@ import { Duration, Resource, ResourceProps } from 'aws-cdk-lib'; import { CfnRemediationConfiguration, IRule } from 'aws-cdk-lib/aws-config'; -import { IInstanceProfile, Role, ServicePrincipal } from 'aws-cdk-lib/aws-iam'; +import { IRole, InstanceProfile, Role, ServicePrincipal } from 'aws-cdk-lib/aws-iam'; import { IConstruct } from 'constructs'; import { Ec2RequiredRoleRemediationDocument } from './ec2-required-role-remediation-document'; export interface Ec2RequiredRoleRemediationConfigurationProps extends ResourceProps { readonly automatic?: boolean; - readonly instanceProfile: IInstanceProfile; + readonly ec2DefaultRole: IRole; readonly maxAutomaticAttempts?: number; readonly retryPeriod?: Duration; readonly rule: IRule; @@ -18,7 +18,7 @@ export class Ec2RequiredRoleRemediationConfiguration extends Resource { public static readonly DEFAULT_RETRY_PERIOD: Duration = Duration.seconds(60); public readonly automatic: boolean; - public readonly instanceProfile: IInstanceProfile; + public readonly ec2DefaultRole: IRole; public readonly maxAutomaticAttempts: number; public readonly retryPeriod: Duration; public readonly rule: IRule; @@ -30,20 +30,29 @@ export class Ec2RequiredRoleRemediationConfiguration extends Resource { super(scope, id, props); this.automatic = props.automatic ?? false; - this.instanceProfile = props.instanceProfile; + this.ec2DefaultRole = props.ec2DefaultRole; this.maxAutomaticAttempts = props.maxAutomaticAttempts ?? Ec2RequiredRoleRemediationConfiguration.DEFAULT_MAX_AUTOMATIC_ATTEMPTS; this.retryPeriod = props.retryPeriod ?? Ec2RequiredRoleRemediationConfiguration.DEFAULT_RETRY_PERIOD; this.rule = props.rule; const automation = new Ec2RequiredRoleRemediationDocument(this, 'automation'); + const instanceProfile = new InstanceProfile(this, 'default-ec2-instance-profile', { + path: '/service-role/', + role: this.ec2DefaultRole, + }); + if (this.automatic) { this.automationRole = new Role(this, 'automation-role', { assumedBy: new ServicePrincipal('ssm.amazonaws.com'), path: '/service-role/', }); - automation.grantExecute(this.automationRole); + automation.grantExecute(this.automationRole, { + allowedEc2Roles: [ + this.ec2DefaultRole, + ], + }); } new CfnRemediationConfiguration(this, 'Resource', { @@ -64,7 +73,7 @@ export class Ec2RequiredRoleRemediationConfiguration extends Resource { InstanceProfileName: { StaticValue: { Values: [ - this.instanceProfile.instanceProfileName, + instanceProfile.instanceProfileName, ], }, }, diff --git a/src/ec2-required-role-remediation-document.ts b/src/ec2-required-role-remediation-document.ts index c17ed80..c57d22f 100644 --- a/src/ec2-required-role-remediation-document.ts +++ b/src/ec2-required-role-remediation-document.ts @@ -1,9 +1,13 @@ import { ArnFormat, Resource, ResourceProps } from 'aws-cdk-lib'; -import { IGrantable, PolicyStatement } from 'aws-cdk-lib/aws-iam'; +import { IGrantable, IRole, PolicyStatement } from 'aws-cdk-lib/aws-iam'; import { CfnDocument } from 'aws-cdk-lib/aws-ssm'; import { IConstruct } from 'constructs'; +export interface GrantRemediationExecuteOptions { + readonly allowedEc2Roles?: IRole[]; +} + export interface Ec2RequiredRoleRemediationDocumentProps extends ResourceProps {} export class Ec2RequiredRoleRemediationDocument extends Resource { @@ -86,7 +90,16 @@ export class Ec2RequiredRoleRemediationDocument extends Resource { return `${this.automationDefinitionArn}:${version}`; } - public grantExecute(principal: IGrantable): void { + public grantExecute(principal: IGrantable, options: GrantRemediationExecuteOptions = {}): void { + const defaultPassRoles = [ + this.stack.formatArn({ + arnFormat: ArnFormat.SLASH_RESOURCE_NAME, + resource: 'instance', + resourceName: '*', + service: 'ec2', + }), + ]; + principal.grantPrincipal.addToPrincipalPolicy(new PolicyStatement({ actions: [ 'ec2:AssociateIamInstanceProfile', @@ -100,5 +113,14 @@ export class Ec2RequiredRoleRemediationDocument extends Resource { }), ], })); + + principal.grantPrincipal.addToPrincipalPolicy(new PolicyStatement({ + actions: [ + 'iam:PassRole', + ], + resources: options.allowedEc2Roles?.map((x) => { + return x.roleArn; + }) ?? defaultPassRoles, + })); } } \ No newline at end of file diff --git a/test/index.test.ts b/test/index.test.ts index 123298d..6e1b6e9 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -47,7 +47,7 @@ test('automatic remediation should have required properties', () => { test('automatic remediation should properly configure permissions', () => { const stack = new Stack(); - const rule = new Ec2RequiredRoleConfigRule(stack, 'rule', { + new Ec2RequiredRoleConfigRule(stack, 'rule', { remediation: { automatic: true, }, @@ -55,12 +55,8 @@ test('automatic remediation should properly configure permissions', () => { const template = Template.fromStack(stack); - const remediation = rule.node.tryFindChild('remediation'); - const automationRoleL2 = remediation?.node.tryFindChild('automation-role'); - const automationRoleL1 = automationRoleL2?.node.defaultChild; - - expect(CfnResource.isCfnResource(automationRoleL1)).toBe(true); - const automationRoleResource = automationRoleL1 as CfnResource; + const automationRole = getRemediationAutomationRole(stack); + const defaultEc2Role = getDefaultEc2Role(stack); template.hasResourceProperties('AWS::IAM::Role', { AssumeRolePolicyDocument: { @@ -77,19 +73,26 @@ test('automatic remediation should properly configure permissions', () => { template.hasResourceProperties('AWS::IAM::Policy', { PolicyDocument: { - Statement: [{ - Action: 'ec2:AssociateIamInstanceProfile', - Effect: 'Allow', - Resource: stack.resolve(stack.formatArn({ - arnFormat: ArnFormat.SLASH_RESOURCE_NAME, - resource: 'instance', - resourceName: '*', - service: 'ec2', - })), - }], + Statement: [ + { + Action: 'ec2:AssociateIamInstanceProfile', + Effect: 'Allow', + Resource: stack.resolve(stack.formatArn({ + arnFormat: ArnFormat.SLASH_RESOURCE_NAME, + resource: 'instance', + resourceName: '*', + service: 'ec2', + })), + }, + { + Action: 'iam:PassRole', + Effect: 'Allow', + Resource: stack.resolve(defaultEc2Role.getAtt('Arn')), + }, + ], }, Roles: [ - stack.resolve(automationRoleResource.ref), + stack.resolve(automationRole.ref), ], }); @@ -98,10 +101,29 @@ test('automatic remediation should properly configure permissions', () => { AutomationAssumeRole: { StaticValue: { Values: [ - stack.resolve(automationRoleResource.getAtt('Arn')), + stack.resolve(automationRole.getAtt('Arn')), ], }, }, }, }); -}); \ No newline at end of file +}); + +function getRemediationAutomationRole(stack: Stack): CfnResource { + const rule = stack.node.tryFindChild('rule'); + const remediation = rule?.node.tryFindChild('remediation'); + const role = remediation?.node.tryFindChild('automation-role'); + const resource = role?.node.defaultChild; + + expect(CfnResource.isCfnResource(resource)).toBe(true); + return resource as CfnResource; +} + +function getDefaultEc2Role(stack: Stack): CfnResource { + const rule = stack.node.findChild('rule'); + const role = rule.node.findChild('default-ec2-role'); + const resource = role.node.defaultChild; + + expect(CfnResource.isCfnResource(resource)).toBe(true); + return resource as CfnResource; +} \ No newline at end of file