Skip to content

Commit

Permalink
feat(aws-s3): adds s3 bucket AWS FSBP option (#12804)
Browse files Browse the repository at this point in the history
This adds an option to enforce ssl for s3 buckets.

Closes #10969

Signed-off-by: crashGoBoom <crashGoBoom@users.noreply.github.com>

Replaces the PR  #10970 as it was created with an ORG fork which is not compatible with the required option "Allow edits by maintainers". 

FYI @NetaNir 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
crashGoBoom committed Feb 23, 2021
1 parent 520c263 commit b9cdd52
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 0 deletions.
12 changes: 12 additions & 0 deletions packages/@aws-cdk/aws-s3/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,18 @@ bucket.grantReadWrite(lambda);
Will give the Lambda's execution role permissions to read and write
from the bucket.

## AWS Foundational Security Best Practices

### Enforcing SSL

To require all requests use Secure Socket Layer (SSL):

```ts
const bucket = new Bucket(this, 'Bucket', {
enforceSSL: true
});
```

## Sharing buckets between stacks

To use a bucket in a different stack in the same CDK application, pass the object to the other stack:
Expand Down
29 changes: 29 additions & 0 deletions packages/@aws-cdk/aws-s3/lib/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,14 @@ export interface BucketProps {
*/
readonly encryptionKey?: kms.IKey;

/**
* Enforces SSL for requests. S3.5 of the AWS Foundational Security Best Practices Regarding S3.
* @see https://docs.aws.amazon.com/config/latest/developerguide/s3-bucket-ssl-requests-only.html
*
* @default false
*/
readonly enforceSSL?: boolean;

/**
* Specifies whether Amazon S3 should use an S3 Bucket Key with server-side
* encryption using KMS (SSE-KMS) for new objects in the bucket.
Expand Down Expand Up @@ -1357,6 +1365,11 @@ export class Bucket extends BucketBase {
this.disallowPublicAccess = props.blockPublicAccess && props.blockPublicAccess.blockPublicPolicy;
this.accessControl = props.accessControl;

// Enforce AWS Foundational Security Best Practice
if (props.enforceSSL) {
this.enforceSSLStatement();
}

if (props.serverAccessLogsBucket instanceof Bucket) {
props.serverAccessLogsBucket.allowLogDelivery();
}
Expand Down Expand Up @@ -1479,6 +1492,22 @@ export class Bucket extends BucketBase {
this.inventories.push(inventory);
}

/**
* Adds an iam statement to enforce SSL requests only.
*/
private enforceSSLStatement() {
const statement = new iam.PolicyStatement({
actions: ['s3:*'],
conditions: {
Bool: { 'aws:SecureTransport': 'false' },
},
effect: iam.Effect.DENY,
resources: [this.arnForObjects('*')],
principals: [new iam.AnyPrincipal()],
});
this.addToResourcePolicy(statement);
}

private validateBucketName(physicalName: string): void {
const bucketName = physicalName;
if (!bucketName || Token.isUnresolved(bucketName)) {
Expand Down
52 changes: 52 additions & 0 deletions packages/@aws-cdk/aws-s3/test/bucket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,58 @@ describe('bucket', () => {

});

test('enforceSsl can be enabled', () => {
const stack = new cdk.Stack();
new s3.Bucket(stack, 'MyBucket', { enforceSSL: true });

expect(stack).toMatchTemplate({
'Resources': {
'MyBucketF68F3FF0': {
'Type': 'AWS::S3::Bucket',
'UpdateReplacePolicy': 'Retain',
'DeletionPolicy': 'Retain',
},
'MyBucketPolicyE7FBAC7B': {
'Type': 'AWS::S3::BucketPolicy',
'Properties': {
'Bucket': {
'Ref': 'MyBucketF68F3FF0',
},
'PolicyDocument': {
'Statement': [
{
'Action': 's3:*',
'Condition': {
'Bool': {
'aws:SecureTransport': 'false',
},
},
'Effect': 'Deny',
'Principal': '*',
'Resource': {
'Fn::Join': [
'',
[
{
'Fn::GetAtt': [
'MyBucketF68F3FF0',
'Arn',
],
},
'/*',
],
],
},
},
],
'Version': '2012-10-17',
},
},
},
},
});
});

test('bucketKeyEnabled can be enabled', () => {
const stack = new cdk.Stack();

Expand Down

0 comments on commit b9cdd52

Please sign in to comment.