feat(aws-s3): adds s3 bucket AWS FSBP option

This adds an option to enforce aws foundational best practices for s3 buckets. Closes #10969 Signed-off-by: Christopher Mundus <chris@kindlyops.com>
aws · Oct 19, 2020 · bd5b894 · bd5b894
1 parent 10de355
commit bd5b894
Show file tree

Hide file tree

Showing 2 changed files with 111 additions and 2 deletions.
diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts
@@ -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;
+
   /**
    * Physical name of this bucket.
    *
@@ -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[] = [];
@@ -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);
@@ -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 }),
@@ -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();
     }
@@ -1392,6 +1413,17 @@ export class Bucket extends BucketBase {
     this.inventories.push(inventory);
   }
 
+  private enforceSSL() {
+    const statement = new iam.PolicyStatement({
+      actions: ['s3:*'],
+      effect: iam.Effect.DENY,
+      resources: [this.bucketArn, `${this.bucketArn}/*`],
+      principals: [new iam.AnyPrincipal()],
+    });
+    statement.addCondition('Bool', { 'aws:SecureTransport': 'false' });
+    this.addToResourcePolicy(statement);
+  }
+
   private validateBucketName(physicalName: string): void {
     const bucketName = physicalName;
     if (!bucketName || Token.isUnresolved(bucketName)) {
@@ -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 };
     }

diff --git a/packages/@aws-cdk/aws-s3/test/test.bucket.ts b/packages/@aws-cdk/aws-s3/test/test.bucket.ts
@@ -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();