From f8102650bc3a82a583fe4783a3a76c305ff9f30e Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar <16217941+nija-at@users.noreply.github.com> Date: Tue, 16 Apr 2019 08:58:44 +0100 Subject: [PATCH] fix(s3): Add validations for S3 bucket names (#2256) Bucket names are verified to conform with rules published by S3 - https://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html Fixes #1308 --- packages/@aws-cdk/aws-s3/lib/bucket.ts | 38 +++++ packages/@aws-cdk/aws-s3/test/test.bucket.ts | 137 +++++++++++++++++++ 2 files changed, 175 insertions(+) diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts index 5357dbe754dc4..ed071d793276b 100644 --- a/packages/@aws-cdk/aws-s3/lib/bucket.ts +++ b/packages/@aws-cdk/aws-s3/lib/bucket.ts @@ -3,6 +3,7 @@ import iam = require('@aws-cdk/aws-iam'); import kms = require('@aws-cdk/aws-kms'); import { IBucketNotificationDestination } from '@aws-cdk/aws-s3-notifications'; import cdk = require('@aws-cdk/cdk'); +import { EOL } from 'os'; import { BucketPolicy } from './bucket-policy'; import { BucketNotifications } from './notifications-resource'; import perms = require('./perms'); @@ -669,6 +670,9 @@ export class Bucket extends BucketBase { super(scope, id); const { bucketEncryption, encryptionKey } = this.parseEncryption(props); + if (props.bucketName && !cdk.Token.unresolved(props.bucketName)) { + this.validateBucketName(props.bucketName); + } const resource = new CfnBucket(this, 'Resource', { bucketName: props && props.bucketName, @@ -776,6 +780,40 @@ export class Bucket extends BucketBase { return this.onEvent(EventType.ObjectRemoved, dest, ...filters); } + private validateBucketName(bucketName: string) { + const errors: string[] = []; + + // Rules codified from https://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html + if (bucketName.length < 3 || bucketName.length > 63) { + errors.push('Bucket name must be at least 3 and no more than 63 characters'); + } + const charsetMatch = bucketName.match(/[^a-z0-9.-]/); + if (charsetMatch) { + errors.push('Bucket name must only contain lowercase characters and the symbols, period (.) and dash (-) ' + + `(offset: ${charsetMatch.index})`); + } + if (!/[a-z0-9]/.test(bucketName.charAt(0))) { + errors.push('Bucket name must start and end with a lowercase character or number ' + + '(offset: 0)'); + } + if (!/[a-z0-9]/.test(bucketName.charAt(bucketName.length - 1))) { + errors.push('Bucket name must start and end with a lowercase character or number ' + + `(offset: ${bucketName.length - 1})`); + } + const consecSymbolMatch = bucketName.match(/\.-|-\.|\.\./); + if (consecSymbolMatch) { + errors.push('Bucket name must not have dash next to period, or period next to dash, or consecutive periods ' + + `(offset: ${consecSymbolMatch.index})`); + } + if (/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(bucketName)) { + errors.push('Bucket name must not resemble an IP address'); + } + + if (errors.length > 0) { + throw new Error(`Invalid S3 bucket name (value: ${bucketName})${EOL}${errors.join(EOL)}`); + } + } + /** * Set up key properties and return the Bucket encryption property from the * user's configuration. diff --git a/packages/@aws-cdk/aws-s3/test/test.bucket.ts b/packages/@aws-cdk/aws-s3/test/test.bucket.ts index 8789d97ceb91e..eb23be105fc90 100644 --- a/packages/@aws-cdk/aws-s3/test/test.bucket.ts +++ b/packages/@aws-cdk/aws-s3/test/test.bucket.ts @@ -3,6 +3,7 @@ import iam = require('@aws-cdk/aws-iam'); import kms = require('@aws-cdk/aws-kms'); import cdk = require('@aws-cdk/cdk'); import { Test } from 'nodeunit'; +import { EOL } from 'os'; import s3 = require('../lib'); // to make it easy to copy & paste from output: @@ -72,6 +73,142 @@ export = { test.done(); }, + 'valid bucket names'(test: Test) { + const stack = new cdk.Stack(); + + test.doesNotThrow(() => new s3.Bucket(stack, 'MyBucket1', { + bucketName: 'abc.xyz-34ab' + })); + + test.doesNotThrow(() => new s3.Bucket(stack, 'MyBucket2', { + bucketName: '124.pp--33' + })); + + test.done(); + }, + + 'bucket validation skips tokenized values'(test: Test) { + const stack = new cdk.Stack(); + + test.doesNotThrow(() => new s3.Bucket(stack, 'MyBucket', { + bucketName: new cdk.Token(() => '_BUCKET').toString() + })); + + test.done(); + }, + + 'fails with message on invalid bucket names'(test: Test) { + const stack = new cdk.Stack(); + const bucket = `-buckEt.-${new Array(65).join('$')}`; + const expectedErrors = [ + `Invalid S3 bucket name (value: ${bucket})`, + 'Bucket name must be at least 3 and no more than 63 characters', + 'Bucket name must only contain lowercase characters and the symbols, period (.) and dash (-) (offset: 5)', + 'Bucket name must start and end with a lowercase character or number (offset: 0)', + `Bucket name must start and end with a lowercase character or number (offset: ${bucket.length - 1})`, + 'Bucket name must not have dash next to period, or period next to dash, or consecutive periods (offset: 7)', + ].join(EOL); + + test.throws(() => new s3.Bucket(stack, 'MyBucket', { + bucketName: bucket + // tslint:disable-next-line:only-arrow-functions + }), function(err: Error) { + return expectedErrors === err.message; + }); + + test.done(); + }, + + 'fails if bucket name has less than 3 or more than 63 characters'(test: Test) { + const stack = new cdk.Stack(); + + test.throws(() => new s3.Bucket(stack, 'MyBucket1', { + bucketName: 'a' + }), /at least 3/); + + test.throws(() => new s3.Bucket(stack, 'MyBucket2', { + bucketName: new Array(65).join('x') + }), /no more than 63/); + + test.done(); + }, + + 'fails if bucket name has invalid characters'(test: Test) { + const stack = new cdk.Stack(); + + test.throws(() => new s3.Bucket(stack, 'MyBucket1', { + bucketName: 'b@cket' + }), /offset: 1/); + + test.throws(() => new s3.Bucket(stack, 'MyBucket2', { + bucketName: 'bucKet' + }), /offset: 3/); + + test.throws(() => new s3.Bucket(stack, 'MyBucket3', { + bucketName: 'bučket' + }), /offset: 2/); + + test.done(); + }, + + 'fails if bucket name does not start or end with lowercase character or number'(test: Test) { + const stack = new cdk.Stack(); + + test.throws(() => new s3.Bucket(stack, 'MyBucket1', { + bucketName: '-ucket' + }), /offset: 0/); + + test.throws(() => new s3.Bucket(stack, 'MyBucket2', { + bucketName: 'bucke.' + }), /offset: 5/); + + test.done(); + }, + + 'fails only if bucket name has the consecutive symbols (..), (.-), (-.)'(test: Test) { + const stack = new cdk.Stack(); + + test.throws(() => new s3.Bucket(stack, 'MyBucket1', { + bucketName: 'buc..ket' + }), /offset: 3/); + + test.throws(() => new s3.Bucket(stack, 'MyBucket2', { + bucketName: 'buck.-et' + }), /offset: 4/); + + test.throws(() => new s3.Bucket(stack, 'MyBucket3', { + bucketName: 'b-.ucket' + }), /offset: 1/); + + test.doesNotThrow(() => new s3.Bucket(stack, 'MyBucket4', { + bucketName: 'bu--cket' + })); + + test.done(); + }, + + 'fails only if bucket name resembles IP address'(test: Test) { + const stack = new cdk.Stack(); + + test.throws(() => new s3.Bucket(stack, 'MyBucket1', { + bucketName: '1.2.3.4' + }), /must not resemble an IP address/); + + test.doesNotThrow(() => new s3.Bucket(stack, 'MyBucket2', { + bucketName: '1.2.3' + })); + + test.doesNotThrow(() => new s3.Bucket(stack, 'MyBucket3', { + bucketName: '1.2.3.a' + })); + + test.doesNotThrow(() => new s3.Bucket(stack, 'MyBucket4', { + bucketName: '1000.2.3.4' + })); + + test.done(); + }, + 'fails if encryption key is used with managed encryption'(test: Test) { const stack = new cdk.Stack(); const myKey = new kms.EncryptionKey(stack, 'MyKey');