Skip to content

Commit

Permalink
fix(certificatemanager): domainName not checked for length (#21807)
Browse files Browse the repository at this point in the history
## fix(AWS-CertificateManager) cap domain name to 64 characters
*fix for*: #21777
*motivation*: if a domainName that is longer than 64 characters is provided, certification creation will fail - this implements a check for string length 

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
josephedward committed Aug 30, 2022
1 parent 7472fa4 commit 3e55092
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 3 deletions.
5 changes: 5 additions & 0 deletions packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,11 @@ export class Certificate extends CertificateBase implements ICertificate {
}
}

// check if domain name is 64 characters or less
if (props.domainName.length > 64) {
throw new Error('Domain name must be 64 characters or less');
}

const allDomainNames = [props.domainName].concat(props.subjectAlternativeNames || []);

let certificateTransparencyLoggingPreference: string | undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,16 @@ export class DnsValidatedCertificate extends CertificateBase implements ICertifi
super(scope, id);

this.region = props.region;

this.domainName = props.domainName;
// check if domain name is 64 characters or less
if (this.domainName.length > 64) {
throw new Error('Domain name must be 64 characters or less');
}
this.normalizedZoneName = props.hostedZone.zoneName;
// Remove trailing `.` from zone name
if (this.normalizedZoneName.endsWith('.')) {
this.normalizedZoneName = this.normalizedZoneName.substring(0, this.normalizedZoneName.length - 1);
}

// Remove any `/hostedzone/` prefix from the Hosted Zone ID
this.hostedZoneId = props.hostedZone.hostedZoneId.replace(/^\/hostedzone\//, '');
this.tags = new cdk.TagManager(cdk.TagType.MAP, 'AWS::CertificateManager::Certificate');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,17 @@ test('can configure validation method', () => {
});
});

test('throws when domain name is longer than 64 characters', () => {
const stack = new Stack();

expect(() => {
new Certificate(stack, 'Certificate', {
domainName: 'example.com'.repeat(7),
});
}).toThrow(/Domain name must be 64 characters or less/);
});


test('needs validation domain supplied if domain contains a token', () => {
const stack = new Stack();

Expand Down Expand Up @@ -363,4 +374,5 @@ describe('Transparency logging settings', () => {
CertificateTransparencyLoggingPreference: 'DISABLED',
});
});
});
});

Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,21 @@ test('works with imported role', () => {
});
});


test('throws when domain name is longer than 64 characters', () => {
const stack = new Stack();

const exampleDotComZone = new PublicHostedZone(stack, 'ExampleDotCom', {
zoneName: 'example.com',
});
expect(() => {
new DnsValidatedCertificate(stack, 'Cert', {
domainName: 'example.com'.repeat(7),
hostedZone: exampleDotComZone,
});
}).toThrow(/Domain name must be 64 characters or less/);
}),

test('test transparency logging settings is passed to the custom resource', () => {
const stack = new Stack();

Expand Down

0 comments on commit 3e55092

Please sign in to comment.