Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(aws-s3): adds s3 bucket aws fsbp option #10970

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions packages/@aws-cdk/aws-s3/lib/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,14 @@ export interface BucketProps {
*/
readonly encryptionKey?: kms.IKey;

/**
* Enforces all of the AWS Foundational Security Best Practices Regarding S3
* Details: https://docs.aws.amazon.com/securityhub/latest/userguide/securityhub-standards-fsbp-controls.html
*
* @default false
*/
readonly enforceSecurityBestPractice?: boolean;
crashGoBoom marked this conversation as resolved.
Show resolved Hide resolved

/**
* Physical name of this bucket.
*
Expand Down Expand Up @@ -1225,6 +1233,8 @@ export class Bucket extends BucketBase {
private accessControl?: BucketAccessControl;
private readonly lifecycleRules: LifecycleRule[] = [];
private readonly versioned?: boolean;
private readonly enforceSecurityBestPractice?: boolean;
private readonly blockPublicAccess: BlockPublicAccess | undefined;
private readonly notifications: BucketNotifications;
private readonly metrics: BucketMetrics[] = [];
private readonly cors: CorsRule[] = [];
Expand All @@ -1238,6 +1248,8 @@ export class Bucket extends BucketBase {
const { bucketEncryption, encryptionKey } = this.parseEncryption(props);

this.validateBucketName(this.physicalName);
this.enforceSecurityBestPractice = props.enforceSecurityBestPractice;
this.blockPublicAccess = props.blockPublicAccess;

const websiteConfiguration = this.renderWebsiteConfiguration(props);
this.isWebsite = (websiteConfiguration !== undefined);
Expand All @@ -1248,7 +1260,7 @@ export class Bucket extends BucketBase {
versioningConfiguration: props.versioned ? { status: 'Enabled' } : undefined,
lifecycleConfiguration: Lazy.anyValue({ produce: () => this.parseLifecycleConfiguration() }),
websiteConfiguration,
publicAccessBlockConfiguration: props.blockPublicAccess,
publicAccessBlockConfiguration: this.blockPublicAccess,
metricsConfigurations: Lazy.anyValue({ produce: () => this.parseMetricConfiguration() }),
corsConfiguration: Lazy.anyValue({ produce: () => this.parseCorsConfiguration() }),
accessControl: Lazy.stringValue({ produce: () => this.accessControl }),
Expand All @@ -1275,9 +1287,18 @@ export class Bucket extends BucketBase {
this.bucketDualStackDomainName = resource.attrDualStackDomainName;
this.bucketRegionalDomainName = resource.attrRegionalDomainName;

this.disallowPublicAccess = props.blockPublicAccess && props.blockPublicAccess.blockPublicPolicy;
this.disallowPublicAccess = this.blockPublicAccess && this.blockPublicAccess.blockPublicPolicy;
this.accessControl = props.accessControl;

// Enforce AWS Foundational Security Best Practice
if (this.enforceSecurityBestPractice) {
// Require requests to use Secure Socket Layer
this.enforceSSL();
// Block all public access
this.blockPublicAccess = BlockPublicAccess.BLOCK_ALL;
resource.publicAccessBlockConfiguration = this.blockPublicAccess;
}

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

private enforceSSL() {
crashGoBoom marked this conversation as resolved.
Show resolved Hide resolved
const statement = new iam.PolicyStatement({
actions: ['s3:*'],
effect: iam.Effect.DENY,
resources: [this.bucketArn, `${this.bucketArn}/*`],
crashGoBoom marked this conversation as resolved.
Show resolved Hide resolved
principals: [new iam.AnyPrincipal()],
});
statement.addCondition('Bool', { 'aws:SecureTransport': 'false' });
crashGoBoom marked this conversation as resolved.
Show resolved Hide resolved
this.addToResourcePolicy(statement);
}

private validateBucketName(physicalName: string): void {
const bucketName = physicalName;
if (!bucketName || Token.isUnresolved(bucketName)) {
Expand Down Expand Up @@ -1453,6 +1485,11 @@ export class Bucket extends BucketBase {
throw new Error(`encryptionKey is specified, so 'encryption' must be set to KMS (value: ${encryptionType})`);
}

// Ensure SSE is enabled if best practices are enforced.
if (this.enforceSecurityBestPractice && encryptionType === BucketEncryption.UNENCRYPTED) {
encryptionType = BucketEncryption.S3_MANAGED;
}

if (encryptionType === BucketEncryption.UNENCRYPTED) {
return { bucketEncryption: undefined, encryptionKey: undefined };
}
Expand Down
72 changes: 72 additions & 0 deletions packages/@aws-cdk/aws-s3/test/test.bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,78 @@ export = {
test.done();
},

'bucket with aws foundational security best practice'(test: Test) {
const stack = new cdk.Stack();
new s3.Bucket(stack, 'MyBucket', {
enforceSecurityBestPractice: true,
});

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

test.done();
},

'forBucket returns a permission statement associated with the bucket\'s ARN'(test: Test) {
const stack = new cdk.Stack();

Expand Down