Skip to content

Commit

Permalink
Merge pull request #34 from FizzBuzz791/feat/s3-support-2
Browse files Browse the repository at this point in the history
Feat/s3 support 2
  • Loading branch information
FizzBuzz791 authored Nov 1, 2021
2 parents 256ec1e + 2f32078 commit edf23ee
Show file tree
Hide file tree
Showing 5 changed files with 235 additions and 3 deletions.
8 changes: 8 additions & 0 deletions src/aws-foundational-security-best-practices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -25,6 +27,7 @@ export interface FSBPConfig {
iam?: IAMConfig;
lambda?: LambdaConfig;
rds?: RDSConfig;
s3?: S3Config;
}

/**
Expand All @@ -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.
Expand Down Expand Up @@ -67,6 +71,7 @@ export class AWSFoundationalSecurityBestPracticesChecker implements IAspect {
copyTagsToSnapshot: true,
},
dynamodb: { autoScaling: true, pointInTimeRecovery: true },
s3: { serverSideEncryption: true },
}
) {
this.#ApiGatewayComplianceChecker = new ApiGatewayComplianceChecker(
Expand All @@ -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 {
Expand All @@ -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);
}
}
}
89 changes: 89 additions & 0 deletions src/s3-checker.ts
Original file line number Diff line number Diff line change
@@ -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"
);
}
}
}
}
15 changes: 12 additions & 3 deletions test/lambda/lambda.test.ts
Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -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 })
Expand All @@ -35,6 +41,7 @@ describe("Lambda", () => {
new LambdaBuilder(stack).withRuntime(Runtime.NODEJS).build();
Aspects.of(app).add(
new AWSFoundationalSecurityBestPracticesChecker({
...config,
lambda: { supportedRuntimes: false },
})
);
Expand All @@ -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 })
Expand Down
36 changes: 36 additions & 0 deletions test/s3/S3Builder.ts
Original file line number Diff line number Diff line change
@@ -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<Bucket> {
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;
}
}
90 changes: 90 additions & 0 deletions test/s3/s3.test.ts
Original file line number Diff line number Diff line change
@@ -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"
// );
// });
// });
});

0 comments on commit edf23ee

Please sign in to comment.