From 00f8d32324e4115d300f6bffde72a5d4fc3077f1 Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Tue, 28 Jul 2020 13:45:44 +0100 Subject: [PATCH 1/2] fix(certificatemanager): Fix DNS validation for wildcard certificates If a certificate with automatic (Route53) DNS validation contains both a base domain name and the wildcard for that domain (e.g., `example.com` and `*.example.com`), the corresponding DNS validation records are identical. This seems to have caused problems for the automated CloudFormation DNS validation. Solving the problem by removing the redundant wildcard entries from the DomainValidationOption. fixes #9248 --- .../aws-certificatemanager/lib/certificate.ts | 11 +- .../test/certificate.test.ts | 130 ++++++++++++++---- 2 files changed, 111 insertions(+), 30 deletions(-) diff --git a/packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts b/packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts index 77e6247f38198..84cc6febc0f48 100644 --- a/packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts +++ b/packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts @@ -238,7 +238,7 @@ function renderDomainValidation(validation: CertificateValidation, domainNames: switch (validation.method) { case ValidationMethod.DNS: - for (const domainName of domainNames) { + for (const domainName of getUniqueDnsDomainNames(domainNames)) { const hostedZone = validation.props.hostedZones?.[domainName] ?? validation.props.hostedZone; if (hostedZone) { domainValidation.push({ domainName, hostedZoneId: hostedZone.hostedZoneId }); @@ -260,3 +260,12 @@ function renderDomainValidation(validation: CertificateValidation, domainNames: return domainValidation.length !== 0 ? domainValidation : undefined; } + +/** + * Removes wildcard domains (*.example.com) where the base domain (example.com) is present. + * This is because the DNS validation treats them as the same thing, and the automated CloudFormation + * DNS validation errors out with the duplicate records. + */ +function getUniqueDnsDomainNames(domainNames: string[]) { + return domainNames.filter(domain => !domain.startsWith('*.') || !domainNames.includes(domain.replace('*.', ''))); +} diff --git a/packages/@aws-cdk/aws-certificatemanager/test/certificate.test.ts b/packages/@aws-cdk/aws-certificatemanager/test/certificate.test.ts index 4b122b7eae754..5bbeadf007f1e 100644 --- a/packages/@aws-cdk/aws-certificatemanager/test/certificate.test.ts +++ b/packages/@aws-cdk/aws-certificatemanager/test/certificate.test.ts @@ -121,46 +121,118 @@ test('CertificateValidation.fromEmail', () => { }); }); -test('CertificateValidation.fromDns', () => { - const stack = new Stack(); +describe('CertificateValidation.fromDns', () => { - new Certificate(stack, 'Certificate', { - domainName: 'test.example.com', - subjectAlternativeNames: ['extra.example.com'], - validation: CertificateValidation.fromDns(), - }); + test('without a hosted zone', () => { + const stack = new Stack(); - expect(stack).toHaveResource('AWS::CertificateManager::Certificate', { - DomainName: 'test.example.com', - SubjectAlternativeNames: ['extra.example.com'], - ValidationMethod: 'DNS', + new Certificate(stack, 'Certificate', { + domainName: 'test.example.com', + subjectAlternativeNames: ['extra.example.com'], + validation: CertificateValidation.fromDns(), + }); + + expect(stack).toHaveResource('AWS::CertificateManager::Certificate', { + DomainName: 'test.example.com', + SubjectAlternativeNames: ['extra.example.com'], + ValidationMethod: 'DNS', + }); }); -}); -test('CertificateValidation.fromDns with hosted zone', () => { - const stack = new Stack(); + test('with a hosted zone', () => { + const stack = new Stack(); - const exampleCom = new route53.HostedZone(stack, 'ExampleCom', { - zoneName: 'example.com', + const exampleCom = new route53.HostedZone(stack, 'ExampleCom', { + zoneName: 'example.com', + }); + + new Certificate(stack, 'Certificate', { + domainName: 'test.example.com', + validation: CertificateValidation.fromDns(exampleCom), + }); + + expect(stack).toHaveResource('AWS::CertificateManager::Certificate', { + DomainName: 'test.example.com', + DomainValidationOptions: [ + { + DomainName: 'test.example.com', + HostedZoneId: { + Ref: 'ExampleCom20E1324B', + }, + }, + ], + ValidationMethod: 'DNS', + }); }); - new Certificate(stack, 'Certificate', { - domainName: 'test.example.com', - validation: CertificateValidation.fromDns(exampleCom), + test('with hosted zone and a wildcard name', () => { + const stack = new Stack(); + + const exampleCom = new route53.HostedZone(stack, 'ExampleCom', { + zoneName: 'example.com', + }); + + new Certificate(stack, 'Certificate', { + domainName: 'test.example.com', + validation: CertificateValidation.fromDns(exampleCom), + subjectAlternativeNames: ['*.test.example.com'], + }); + + //Wildcard domain names are de-duped. + expect(stack).toHaveResource('AWS::CertificateManager::Certificate', { + DomainName: 'test.example.com', + DomainValidationOptions: [ + { + DomainName: 'test.example.com', + HostedZoneId: { + Ref: 'ExampleCom20E1324B', + }, + }, + ], + ValidationMethod: 'DNS', + }); }); - expect(stack).toHaveResource('AWS::CertificateManager::Certificate', { - DomainName: 'test.example.com', - DomainValidationOptions: [ - { - DomainName: 'test.example.com', - HostedZoneId: { - Ref: 'ExampleCom20E1324B', + test('with hosted zone and multiple wildcard names', () => { + const stack = new Stack(); + + const exampleCom = new route53.HostedZone(stack, 'ExampleCom', { + zoneName: 'example.com', + }); + + new Certificate(stack, 'Certificate', { + domainName: 'test.example.com', + validation: CertificateValidation.fromDns(exampleCom), + subjectAlternativeNames: ['*.test.example.com', '*.foo.test.example.com', 'bar.test.example.com'], + }); + + //Wildcard domain names are de-duped. + expect(stack).toHaveResource('AWS::CertificateManager::Certificate', { + DomainName: 'test.example.com', + DomainValidationOptions: [ + { + DomainName: 'test.example.com', + HostedZoneId: { + Ref: 'ExampleCom20E1324B', + }, }, - }, - ], - ValidationMethod: 'DNS', + { + DomainName: '*.foo.test.example.com', + HostedZoneId: { + Ref: 'ExampleCom20E1324B', + }, + }, + { + DomainName: 'bar.test.example.com', + HostedZoneId: { + Ref: 'ExampleCom20E1324B', + }, + }, + ], + ValidationMethod: 'DNS', + }); }); + }); test('CertificateValidation.fromDnsMultiZone', () => { From d558786033dce1de21f3cefbf3ed73e7b402a53c Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Tue, 28 Jul 2020 19:48:46 +0100 Subject: [PATCH 2/2] Skip domainName if unresolved token --- packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts b/packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts index 84cc6febc0f48..9bd10343e1321 100644 --- a/packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts +++ b/packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts @@ -267,5 +267,7 @@ function renderDomainValidation(validation: CertificateValidation, domainNames: * DNS validation errors out with the duplicate records. */ function getUniqueDnsDomainNames(domainNames: string[]) { - return domainNames.filter(domain => !domain.startsWith('*.') || !domainNames.includes(domain.replace('*.', ''))); + return domainNames.filter(domain => { + return Token.isUnresolved(domain) || !domain.startsWith('*.') || !domainNames.includes(domain.replace('*.', '')); + }); }