diff --git a/src/aws-foundational-security-best-practices.ts b/src/aws-foundational-security-best-practices.ts index 7e8af8b..431c338 100644 --- a/src/aws-foundational-security-best-practices.ts +++ b/src/aws-foundational-security-best-practices.ts @@ -17,6 +17,8 @@ import { DynamoDBComplianceChecker, DynamoDBConfig } from "./dynamo-db-checker"; import { IAMConfig, IAMPolicyComplianceChecker } from "./iam-policy-checker"; import { LambdaComplianceChecker, LambdaConfig } from "./lambda-checker"; import { RDSComplianceChecker, RDSConfig } from "./rds-compliance-checker"; +import { S3ComplianceChecker, S3Config } from "./s3-checker"; +import { CfnBucket } from "@aws-cdk/aws-s3"; export interface FSBPConfig { apigateway?: ApiGatewayConfig; @@ -25,6 +27,7 @@ export interface FSBPConfig { iam?: IAMConfig; lambda?: LambdaConfig; rds?: RDSConfig; + s3?: S3Config; } /** @@ -37,6 +40,7 @@ export class AWSFoundationalSecurityBestPracticesChecker implements IAspect { #IAMPolicyComplianceChecker: IAMPolicyComplianceChecker; #LambdaComplianceChecker: LambdaComplianceChecker; #RDSComplianceChecker: RDSComplianceChecker; + #S3ComplianceChecker: S3ComplianceChecker; /** * Initialise a new Checker. All values in the configuration default to true if not provided. @@ -67,6 +71,7 @@ export class AWSFoundationalSecurityBestPracticesChecker implements IAspect { copyTagsToSnapshot: true, }, dynamodb: { autoScaling: true, pointInTimeRecovery: true }, + s3: { serverSideEncryption: true }, } ) { this.#ApiGatewayComplianceChecker = new ApiGatewayComplianceChecker( @@ -82,6 +87,7 @@ export class AWSFoundationalSecurityBestPracticesChecker implements IAspect { ); this.#LambdaComplianceChecker = new LambdaComplianceChecker(config.lambda!); this.#RDSComplianceChecker = new RDSComplianceChecker(config.rds!); + this.#S3ComplianceChecker = new S3ComplianceChecker(config.s3!); } public visit(node: IConstruct): void { @@ -97,6 +103,8 @@ export class AWSFoundationalSecurityBestPracticesChecker implements IAspect { this.#LambdaComplianceChecker.checkCompliance(node); } else if (node instanceof CfnDBInstance || node instanceof CfnDBCluster) { this.#RDSComplianceChecker.checkCompliance(node); + } else if (node instanceof CfnBucket) { + this.#S3ComplianceChecker.checkCompliance(node); } } } diff --git a/src/s3-checker.ts b/src/s3-checker.ts new file mode 100644 index 0000000..fceb51d --- /dev/null +++ b/src/s3-checker.ts @@ -0,0 +1,89 @@ +import { CfnBucket, CfnBucketPolicy } from "@aws-cdk/aws-s3"; +import { isResolvableObject, Annotations } from "@aws-cdk/core"; +import { BaseComplianceChecker } from "./compliance-checker"; + +export interface S3Config { + serverSideEncryption?: boolean; + ssl?: boolean; +} + +export class S3ComplianceChecker extends BaseComplianceChecker< + CfnBucket | CfnBucketPolicy +> { + constructor(config: S3Config) { + super({ s3: config }); + } + + checkCompliance(node: CfnBucket | CfnBucketPolicy): void { + if (this.config.s3?.serverSideEncryption) { + this.checkServerSideEncryption(node); + } else if (this.config.s3?.ssl) { + this.checkSSL(node); + } + } + + /** + * [S3.4] S3 buckets should have server-side encryption enabled + * Ref: https://docs.aws.amazon.com/securityhub/latest/userguide/securityhub-standards-fsbp-controls.html#fsbp-s3-4 + */ + private checkServerSideEncryption(node: CfnBucket | CfnBucketPolicy) { + if (node instanceof CfnBucket) { + if ( + !node.hasOwnProperty("bucketEncryption") || + node.bucketEncryption === undefined || + isResolvableObject(node.bucketEncryption) + ) { + Annotations.of(node).addError( + "[S3.4] S3 buckets should have server-side encryption enabled" + ); + } else if ( + node.bucketEncryption.serverSideEncryptionConfiguration === undefined || + isResolvableObject( + node.bucketEncryption.serverSideEncryptionConfiguration + ) || + node.bucketEncryption.serverSideEncryptionConfiguration.length === 0 || + isResolvableObject( + node.bucketEncryption.serverSideEncryptionConfiguration[0] + ) + ) { + Annotations.of(node).addError( + "[S3.4] S3 buckets should have server-side encryption enabled.2" + ); + } else { + const ssec = node.bucketEncryption.serverSideEncryptionConfiguration[0]; + if ( + isResolvableObject(ssec.bucketKeyEnabled) || + ssec.bucketKeyEnabled === false + ) { + Annotations.of(node).addError( + "[S3.4] S3 buckets should have server-side encryption enabled.3" + ); + } else if ( + ssec.serverSideEncryptionByDefault === undefined || + isResolvableObject(ssec.serverSideEncryptionByDefault) || + !["AES-256", "aws:kms"].includes( + ssec.serverSideEncryptionByDefault.sseAlgorithm + ) + ) { + Annotations.of(node).addError( + "[S3.4] S3 buckets should have server-side encryption enabled.4" + ); + } + } + } + } + + /** + * [S3.5] S3 buckets should require requests to use Secure Socket Layer + * Ref: https://docs.aws.amazon.com/securityhub/latest/userguide/securityhub-standards-fsbp-controls.html#fsbp-s3-5 + */ + private checkSSL(node: CfnBucket | CfnBucketPolicy) { + if (node instanceof CfnBucketPolicy) { + if (node.policyDocument?.Statement?.length === 0) { + Annotations.of(node).addError( + "[S3.5] S3 buckets should require requests to use Secure Socket Layer" + ); + } + } + } +} diff --git a/test/lambda/lambda.test.ts b/test/lambda/lambda.test.ts index 1aa8a55..f198ba7 100644 --- a/test/lambda/lambda.test.ts +++ b/test/lambda/lambda.test.ts @@ -1,11 +1,15 @@ import { AssetCode, Function, Runtime } from "@aws-cdk/aws-lambda"; import { Bucket } from "@aws-cdk/aws-s3"; import { App, Aspects, Stack } from "@aws-cdk/core"; -import { AWSFoundationalSecurityBestPracticesChecker } from "../../src/aws-foundational-security-best-practices"; +import { + AWSFoundationalSecurityBestPracticesChecker, + FSBPConfig, +} from "../../src/aws-foundational-security-best-practices"; import { LambdaBuilder } from "./lambda-builder"; describe("Lambda", () => { let app: App; + let config: FSBPConfig = { s3: { serverSideEncryption: false } }; beforeEach(() => { app = new App(); @@ -16,7 +20,9 @@ describe("Lambda", () => { // Arrange const stack = new Stack(app, "test-lambda-2-stack-fail", {}); new LambdaBuilder(stack).withRuntime(Runtime.NODEJS).build(); - Aspects.of(app).add(new AWSFoundationalSecurityBestPracticesChecker()); + Aspects.of(app).add( + new AWSFoundationalSecurityBestPracticesChecker(config) + ); // Act const synthMessages = app .synth({ validateOnSynthesis: true, force: true }) @@ -35,6 +41,7 @@ describe("Lambda", () => { new LambdaBuilder(stack).withRuntime(Runtime.NODEJS).build(); Aspects.of(app).add( new AWSFoundationalSecurityBestPracticesChecker({ + ...config, lambda: { supportedRuntimes: false }, }) ); @@ -50,7 +57,9 @@ describe("Lambda", () => { // Arrange const stack = new Stack(app, "test-lambda-2-stack-pass", {}); new LambdaBuilder(stack).withRuntime(Runtime.NODEJS_14_X).build(); - Aspects.of(app).add(new AWSFoundationalSecurityBestPracticesChecker()); + Aspects.of(app).add( + new AWSFoundationalSecurityBestPracticesChecker(config) + ); // Act const synthMessages = app .synth({ validateOnSynthesis: true, force: true }) diff --git a/test/s3/S3Builder.ts b/test/s3/S3Builder.ts new file mode 100644 index 0000000..b9568da --- /dev/null +++ b/test/s3/S3Builder.ts @@ -0,0 +1,36 @@ +import { AnyPrincipal, Effect, PolicyStatement } from "@aws-cdk/aws-iam"; +import { Bucket, BucketEncryption } from "@aws-cdk/aws-s3"; +import { Stack } from "@aws-cdk/core"; +import { IBuilder } from "../IBuilder"; + +export class S3Builder implements IBuilder { + private readonly _stack: Stack; + + private _encryption: BucketEncryption = BucketEncryption.KMS_MANAGED; + + constructor(stack: Stack) { + this._stack = stack; + } + + withEncryption(encryption: BucketEncryption): S3Builder { + this._encryption = encryption; + return this; + } + + build(): Bucket { + const bucket = new Bucket(this._stack, "Bucket", { + encryption: this._encryption, + }); + bucket.addToResourcePolicy( + new PolicyStatement({ + actions: ["s3:*"], + effect: Effect.DENY, + conditions: { + Bool: { "aws:SecureTransport": false }, + }, + principals: [new AnyPrincipal()], + }) + ); + return bucket; + } +} diff --git a/test/s3/s3.test.ts b/test/s3/s3.test.ts new file mode 100644 index 0000000..2450450 --- /dev/null +++ b/test/s3/s3.test.ts @@ -0,0 +1,90 @@ +import { BucketEncryption } from "@aws-cdk/aws-s3"; +import { App, Stack, Aspects } from "@aws-cdk/core"; +import { AWSFoundationalSecurityBestPracticesChecker } from "../../src/aws-foundational-security-best-practices"; +import { S3Builder } from "./S3Builder"; + +describe("S3", () => { + let app: App; + + beforeEach(() => { + app = new App(); + }); + + describe("[S3.4] S3 buckets should have server-side encryption enabled", () => { + test("Given an S3 Bucket with server-side encryption disabled, When synth is run, Then synth should fail", () => { + // Arrange + const stack = new Stack(app, "test-s3-4-stack-fail", {}); + new S3Builder(stack).withEncryption(BucketEncryption.UNENCRYPTED).build(); + Aspects.of(app).add(new AWSFoundationalSecurityBestPracticesChecker()); + + // Act + const synthMessages = app + .synth({ validateOnSynthesis: true, force: true }) + .getStackByName("test-s3-4-stack-fail").messages; + + // Assert + expect(synthMessages.length).toBe(1); + expect(synthMessages[0].level).toBe("error"); + expect(synthMessages[0].entry.data).toBe( + "[S3.4] S3 buckets should have server-side encryption enabled" + ); + }); + + test("Given an S3 Bucket with server-side encryption enabled, When synth is run, Then synth should pass", () => { + // Arrange + const stack = new Stack(app, "test-s3-4-stack-pass", {}); + new S3Builder(stack).withEncryption(BucketEncryption.KMS_MANAGED).build(); + Aspects.of(app).add(new AWSFoundationalSecurityBestPracticesChecker()); + + // Act + const synthMessages = app + .synth({ validateOnSynthesis: true, force: true }) + .getStackByName("test-s3-4-stack-pass").messages; + + // Assert + expect(synthMessages.length).toBe(0); + }); + + test("Given an S3 Bucket with server-side encryption disabled and S3.4 is ignored, When synth is run, Then synth should pass", () => { + // Arrange + const stack = new Stack(app, "test-s3-4-stack-ignore-pass", {}); + new S3Builder(stack).withEncryption(BucketEncryption.UNENCRYPTED).build(); + Aspects.of(app).add( + new AWSFoundationalSecurityBestPracticesChecker({ + s3: { serverSideEncryption: false }, + }) + ); + + // Act + const synthMessages = app + .synth({ validateOnSynthesis: true, force: true }) + .getStackByName("test-s3-4-stack-ignore-pass").messages; + + // Assert + expect(synthMessages.length).toBe(0); + }); + }); + + // describe("[S3.5] S3 buckets should require requests to use Secure Socket Layer", () => { + // test("Given an S3 Bucket that doesn't require SSL, When synth is run, Then synth should fail", () => { + // // Arrange + // const stack = new Stack(app, "test-s3-5-stack-fail", {}); + // new S3Builder(stack).build(); + // Aspects.of(app).add(new AWSFoundationalSecurityBestPracticesChecker()); + + // // Act + // const synthApp = app + // .synth({ validateOnSynthesis: true, force: true }) + // .getStackByName("test-s3-5-stack-fail"); + // console.log(JSON.stringify(synthApp.template)); + // const synthMessages = synthApp.messages; + + // // Assert + // expect(synthMessages.length).toBe(1); + // expect(synthMessages[0].level).toBe("error"); + // expect(synthMessages[0].entry.data).toBe( + // "[S3.5] S3 buckets should require requests to use Secure Socket Layer" + // ); + // }); + // }); +});