Skip to content

Commit

Permalink
feat(custom-resources): enforce user opt-in when auto-generating SDK …
Browse files Browse the repository at this point in the history
…call policies

In an attempt to be more explicit about our usage of `*` permissions, we are inverting the flow.
Instead of defaulting to auto-generated policy statements that use `*` permissions, we now force users to pass the `policy` property. To make life easier, we provide factory methods that help configure this. 

Note that the `*` is now explicitly set by the user, not by the library. 

Relates to #5873

BREAKING CHANGE: `policyStatements` property was removed in favor of a required `policy` property. Refer to [Execution Policy](https://github.com/aws/aws-cdk/tree/master/packages/%40aws-cdk/custom-resources#execution-policy-1) for more details.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

<!-- 
Please read the contribution guidelines and follow the pull-request checklist:
https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md
 -->
  • Loading branch information
iliapolo authored Mar 3, 2020
1 parent b59338d commit 0f5c24e
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 34 deletions.
26 changes: 17 additions & 9 deletions packages/@aws-cdk/custom-resources/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -339,11 +339,14 @@ be `Items.0.Title.S`.

### Execution Policy

IAM policy statements required to make the API calls are derived from the calls
and allow by default the actions to be made on all resources (`*`). You can
restrict the permissions by specifying your own list of statements with the
`policyStatements` prop. The custom resource also implements `iam.IGrantable`,
making it possible to use the `grantXxx()` methods.
You must provide the `policy` property defining the IAM Policy that will be applied to the API calls.
The library provides two factory methods to quickly configure this:

- **`AwsCustomResourcePolicy.fromSdkCalls`** - Use this to auto-generate IAM Policy statements based on the configured SDK calls.
Note that you will have to either provide specific ARN's, or explicitly use `AwsCustomResourcePolicy.ANY_RESOURCE` to allow access to any resource.
- **`AwsCustomResourcePolicy.fromStatements`** - Use this to specify your own custom statements.

The custom resource also implements `iam.IGrantable`, making it possible to use the `grantXxx()` methods.

As this custom resource uses a singleton Lambda function, it's important to note
that the function's role will eventually accumulate the permissions/grants from all
Expand All @@ -356,7 +359,8 @@ const awsCustom1 = new AwsCustomResource(this, 'API1', {
service: '...',
action: '...',
physicalResourceId: PhysicalResourceId.of('...')
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

const awsCustom2 = new AwsCustomResource(this, 'API2', {
Expand All @@ -367,7 +371,8 @@ const awsCustom2 = new AwsCustomResource(this, 'API2', {
text: awsCustom1.getDataString('Items.0.text')
},
physicalResourceId: PhysicalResourceId.of('...')
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
})
```

Expand All @@ -384,7 +389,8 @@ const verifyDomainIdentity = new AwsCustomResource(this, 'VerifyDomainIdentity',
Domain: 'example.com'
},
physicalResourceId: PhysicalResourceId.fromResponse('VerificationToken') // Use the token returned by the call as physical id
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

new route53.TxtRecord(this, 'SESVerificationRecord', {
Expand All @@ -406,7 +412,8 @@ const getParameter = new AwsCustomResource(this, 'GetParameter', {
WithDecryption: true
},
physicalResourceId: PhysicalResourceId.of(Date.now().toString()) // Update physical id to always fetch the latest version
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

// Use the value in another construct with
Expand All @@ -418,3 +425,4 @@ getParameter.getData('Parameter.Value')
---

This module is part of the [AWS Cloud Development Kit](https://github.com/aws/aws-cdk) project.

Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,61 @@ export interface AwsSdkCall {
readonly outputPath?: string;
}

/**
* Options for the auto-generation of policies based on the configured SDK calls.
*/
export interface SdkCallsPolicyOptions {

/**
* The resources that the calls will have access to.
*
* It is best to use specific resource ARN's when possible. However, you can also use `AwsCustomResourcePolicy.ANY_RESOURCE`
* to allow access to all resources. For example, when `onCreate` is used to create a resource which you don't
* know the physical name of in advance.
*
* Note that will apply to ALL SDK calls.
*/
readonly resources: string[]

}

/**
* The IAM Policy that will be applied to the different calls.
*/
export class AwsCustomResourcePolicy {

/**
* Use this constant to configure access to any resource.
*/
public static readonly ANY_RESOURCE = ['*'];

/**
* Explicit IAM Policy Statements.
*
* @param statements the statements to propagate to the SDK calls.
*/
public static fromStatements(statements: iam.PolicyStatement[]) {
return new AwsCustomResourcePolicy(statements, undefined);
}

/**
* Generate IAM Policy Statements from the configured SDK calls.
*
* Each SDK call with be translated to an IAM Policy Statement in the form of: `call.service:call.action` (e.g `s3:PutObject`).
*
* @param options options for the policy generation
*/
public static fromSdkCalls(options: SdkCallsPolicyOptions) {
return new AwsCustomResourcePolicy([], options.resources);
}

/**
* @param statements statements for explicit policy.
* @param resources resources for auto-generated from SDK calls.
*/
private constructor(public readonly statements: iam.PolicyStatement[], public readonly resources?: string[]) {}
}

/**
* Properties for AwsCustomResource.
*
Expand Down Expand Up @@ -150,8 +205,7 @@ export interface AwsCustomResourceProps {
readonly onDelete?: AwsSdkCall;

/**
* The IAM policy statements to allow the different calls. Use only if
* resource restriction is needed.
* The policy to apply to the resource.
*
* The custom resource also implements `iam.IGrantable`, making it possible
* to use the `grantXxx()` methods.
Expand All @@ -160,9 +214,10 @@ export interface AwsCustomResourceProps {
* to note the that function's role will eventually accumulate the
* permissions/grants from all resources.
*
* @default - extract the permissions from the calls
* @see Policy.fromStatements
* @see Policy.fromSdkCalls
*/
readonly policyStatements?: iam.PolicyStatement[];
readonly policy: AwsCustomResourcePolicy;

/**
* The execution role for the Lambda function implementing this custom
Expand Down Expand Up @@ -220,19 +275,22 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable {
});
this.grantPrincipal = provider.grantPrincipal;

if (props.policyStatements) {
for (const statement of props.policyStatements) {
if (props.policy.statements.length !== 0) {
// Use custom statements provided by the user
for (const statement of props.policy.statements) {
provider.addToRolePolicy(statement);
}
} else { // Derive statements from AWS SDK calls
} else {
// Derive statements from AWS SDK calls
for (const call of [props.onCreate, props.onUpdate, props.onDelete]) {
if (call) {
provider.addToRolePolicy(new iam.PolicyStatement({
actions: [awsSdkToIamAction(call.service, call.action)],
resources: ['*']
resources: props.policy.resources
}));
}
}

}

const create = props.onCreate || props.onUpdate;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import '@aws-cdk/assert/jest';
import * as iam from '@aws-cdk/aws-iam';
import * as cdk from '@aws-cdk/core';
import { AwsCustomResource, PhysicalResourceId } from '../../lib';
import { AwsCustomResource, AwsCustomResourcePolicy, PhysicalResourceId } from '../../lib';

// tslint:disable:object-literal-key-quotes

Expand All @@ -27,7 +27,8 @@ test('aws sdk js custom resource with onCreate and onDelete', () => {
parameters: {
logGroupName: '/aws/lambda/loggroup',
}
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

// THEN
Expand Down Expand Up @@ -88,6 +89,7 @@ test('onCreate defaults to onUpdate', () => {
},
physicalResourceId: PhysicalResourceId.fromResponse('ETag')
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

// THEN
Expand Down Expand Up @@ -135,12 +137,12 @@ test('with custom policyStatements', () => {
},
physicalResourceId: PhysicalResourceId.fromResponse('ETag')
},
policyStatements: [
policy: AwsCustomResourcePolicy.fromStatements([
new iam.PolicyStatement({
actions: ['s3:PutObject'],
resources: ['arn:aws:s3:::my-bucket/my-key']
})
]
])
});

// THEN
Expand All @@ -160,7 +162,9 @@ test('with custom policyStatements', () => {

test('fails when no calls are specified', () => {
const stack = new cdk.Stack();
expect(() => new AwsCustomResource(stack, 'AwsSdk', {})).toThrow(/`onCreate`.+`onUpdate`.+`onDelete`/);
expect(() => new AwsCustomResource(stack, 'AwsSdk', {
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
})).toThrow(/`onCreate`.+`onUpdate`.+`onDelete`/);
});

test('fails when no physical resource method is specified', () => {
Expand All @@ -174,7 +178,8 @@ test('fails when no physical resource method is specified', () => {
logGroupName: '/aws/lambda/loggroup',
retentionInDays: 90
}
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
})).toThrow(/`physicalResourceId`/);
});

Expand All @@ -196,6 +201,7 @@ test('encodes booleans', () => {
},
physicalResourceId: PhysicalResourceId.of('id')
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

// THEN
Expand Down Expand Up @@ -226,7 +232,8 @@ test('timeout defaults to 2 minutes', () => {
service: 'service',
action: 'action',
physicalResourceId: PhysicalResourceId.of('id')
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

// THEN
Expand All @@ -246,7 +253,8 @@ test('can specify timeout', () => {
action: 'action',
physicalResourceId: PhysicalResourceId.of('id')
},
timeout: cdk.Duration.minutes(15)
timeout: cdk.Duration.minutes(15),
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

// THEN
Expand All @@ -266,7 +274,8 @@ test('implements IGrantable', () => {
service: 'service',
action: 'action',
physicalResourceId: PhysicalResourceId.of('id')
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

// WHEN
Expand Down Expand Up @@ -308,7 +317,8 @@ test('can use existing role', () => {
action: 'action',
physicalResourceId: PhysicalResourceId.of('id')
},
role
role,
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

// THEN
Expand All @@ -327,7 +337,8 @@ test('getData', () => {
service: 'service',
action: 'action',
physicalResourceId: PhysicalResourceId.of('id')
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

// WHEN
Expand All @@ -350,7 +361,8 @@ test('getDataString', () => {
service: 'service',
action: 'action',
physicalResourceId: PhysicalResourceId.of('id')
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE}),
});

// WHEN
Expand All @@ -362,7 +374,8 @@ test('getDataString', () => {
a: awsSdk.getDataString('Data')
},
physicalResourceId: PhysicalResourceId.of('id')
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

// THEN
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import * as sns from '@aws-cdk/aws-sns';
import * as ssm from '@aws-cdk/aws-ssm';
import * as cdk from '@aws-cdk/core';
import { AwsCustomResource, PhysicalResourceId } from '../../lib';
import { AwsCustomResource, AwsCustomResourcePolicy, PhysicalResourceId } from '../../lib';

const app = new cdk.App();

Expand All @@ -20,15 +20,17 @@ const snsPublish = new AwsCustomResource(stack, 'Publish', {
TopicArn: topic.topicArn
},
physicalResourceId: PhysicalResourceId.of(topic.topicArn),
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

const listTopics = new AwsCustomResource(stack, 'ListTopics', {
onUpdate: {
service: 'SNS',
action: 'listTopics',
physicalResourceId: PhysicalResourceId.fromResponse('Topics.0.TopicArn')
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});
listTopics.node.addDependency(topic);

Expand All @@ -45,7 +47,8 @@ const getParameter = new AwsCustomResource(stack, 'GetParameter', {
WithDecryption: true
},
physicalResourceId: PhysicalResourceId.fromResponse('Parameter.ARN')
}
},
policy: AwsCustomResourcePolicy.fromSdkCalls({resources: AwsCustomResourcePolicy.ANY_RESOURCE})
});

new cdk.CfnOutput(stack, 'MessageId', { value: snsPublish.getDataString('MessageId') });
Expand Down

0 comments on commit 0f5c24e

Please sign in to comment.