Skip to content

Commit

Permalink
feat(certificatemanager): DnsValidatedCertificate DNS record cleanup (a…
Browse files Browse the repository at this point in the history 
…ws#18311)

Adds an option to DnsValidatedCertificate to automatically cleanup the related
DNS validation records when the Certificate is deleted.

This is an opt-in property and discouraged for production use, as there are edge
cases that can cause unintended side effects. The most obvious is that if two or
more certificates exist with the same domain, the same validation record is used
for both. If one certificate is deleted (and deletes the validation record), the
second certificate (with the same domain name) will be unable to automatically
renew.

closes aws#3333
closes aws#7063

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@njlynch @TikiTDO
njlynch authored and TikiTDO committed Feb 21, 2022
1 parent f8fd7de commit 5af7c1f
Show file tree
Hide file tree
Showing 4 changed files with 281 additions and 52 deletions.
148 changes: 96 additions & 52 deletions ...cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/lib/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,69 +110,28 @@ const requestCertificate = async function (requestId, domainName, subjectAlterna

console.log('Waiting for ACM to provide DNS records for validation...');

let records;
for (let attempt = 0; attempt < maxAttempts && !records; attempt++) {
let records = [];
for (let attempt = 0; attempt < maxAttempts && !records.length; attempt++) {
const { Certificate } = await acm.describeCertificate({
CertificateArn: reqCertResponse.CertificateArn
}).promise();
const options = Certificate.DomainValidationOptions || [];
// Ensure all records are ready; there is (at least a theory there's) a chance of a partial response here in rare cases.
if (options.length > 0 && options.every(opt => opt && !!opt.ResourceRecord)) {
// some alternative names will produce the same validation record
// as the main domain (eg. example.com + *.example.com)
// filtering duplicates to avoid errors with adding the same record
// to the route53 zone twice
const unique = options
.map((val) => val.ResourceRecord)
.reduce((acc, cur) => {
acc[cur.Name] = cur;
return acc;
}, {});
records = Object.keys(unique).sort().map(key => unique[key]);
} else {

records = getDomainValidationRecords(Certificate);
if (!records.length) {
// Exponential backoff with jitter based on 200ms base
// component of backoff fixed to ensure minimum total wait time on
// slow targets.
const base = Math.pow(2, attempt);
await sleep(random() * base * 50 + base * 150);
}
}
if (!records) {
if (!records.length) {
throw new Error(`Response from describeCertificate did not contain DomainValidationOptions after ${maxAttempts} attempts.`)
}


console.log(`Upserting ${records.length} DNS records into zone ${hostedZoneId}:`);

const changeBatch = await route53.changeResourceRecordSets({
ChangeBatch: {
Changes: records.map((record) => {
console.log(`${record.Name} ${record.Type} ${record.Value}`)
return {
Action: 'UPSERT',
ResourceRecordSet: {
Name: record.Name,
Type: record.Type,
TTL: 60,
ResourceRecords: [{
Value: record.Value
}]
}
};
}),
},
HostedZoneId: hostedZoneId
}).promise();

console.log('Waiting for DNS records to commit...');
await route53.waitFor('resourceRecordSetsChanged', {
// Wait up to 5 minutes
$waiter: {
delay: 30,
maxAttempts: 10
},
Id: changeBatch.ChangeInfo.Id
}).promise();
await commitRoute53Records(route53, records, hostedZoneId);

console.log('Waiting for validation...');
await acm.waitFor('certificateValidated', {
Expand All @@ -193,47 +152,126 @@ const requestCertificate = async function (requestId, domainName, subjectAlterna
*
* @param {string} arn The certificate ARN
*/
const deleteCertificate = async function (arn, region) {
const deleteCertificate = async function (arn, region, hostedZoneId, route53Endpoint, cleanupRecords) {
const acm = new aws.ACM({ region });
const route53 = route53Endpoint ? new aws.Route53({ endpoint: route53Endpoint }) : new aws.Route53();
if (waiter) {
// Used by the test suite, since waiters aren't mockable yet
route53.waitFor = acm.waitFor = waiter;
}

try {
console.log(`Waiting for certificate ${arn} to become unused`);

let inUseByResources;
let records = [];
for (let attempt = 0; attempt < maxAttempts; attempt++) {
const { Certificate } = await acm.describeCertificate({
CertificateArn: arn
}).promise();

if (cleanupRecords) {
records = getDomainValidationRecords(Certificate);
}
inUseByResources = Certificate.InUseBy || [];

if (inUseByResources.length) {
if (inUseByResources.length || !records.length) {
// Exponential backoff with jitter based on 200ms base
// component of backoff fixed to ensure minimum total wait time on
// slow targets.
const base = Math.pow(2, attempt);
await sleep(random() * base * 50 + base * 150);
} else {
break
break;
}
}

if (inUseByResources.length) {
throw new Error(`Response from describeCertificate did not contain an empty InUseBy list after ${maxAttempts} attempts.`)
}
if (cleanupRecords && !records.length) {
throw new Error(`Response from describeCertificate did not contain DomainValidationOptions after ${maxAttempts} attempts.`)
}

console.log(`Deleting certificate ${arn}`);

await acm.deleteCertificate({
CertificateArn: arn
}).promise();

if (cleanupRecords) {
console.log(`Deleting ${records.length} DNS records from zone ${hostedZoneId}:`);

await commitRoute53Records(route53, records, hostedZoneId, 'DELETE');
}

} catch (err) {
if (err.name !== 'ResourceNotFoundException') {
throw err;
}
}
};

/**
* Retrieve the unique domain validation options as records to be upserted (or deleted) from Route53.
*
* Returns an empty array ([]) if the domain validation options is empty or the records are not yet ready.
*/
function getDomainValidationRecords(certificate) {
const options = certificate.DomainValidationOptions || [];
// Ensure all records are ready; there is (at least a theory there's) a chance of a partial response here in rare cases.
if (options.length > 0 && options.every(opt => opt && !!opt.ResourceRecord)) {
// some alternative names will produce the same validation record
// as the main domain (eg. example.com + *.example.com)
// filtering duplicates to avoid errors with adding the same record
// to the route53 zone twice
const unique = options
.map((val) => val.ResourceRecord)
.reduce((acc, cur) => {
acc[cur.Name] = cur;
return acc;
}, {});
return Object.keys(unique).sort().map(key => unique[key]);
}
return [];
}

/**
* Execute Route53 ChangeResourceRecordSets for a set of records within a Hosted Zone,
* and wait for the records to commit. Defaults to an 'UPSERT' action.
*/
async function commitRoute53Records(route53, records, hostedZoneId, action = 'UPSERT') {
const changeBatch = await route53.changeResourceRecordSets({
ChangeBatch: {
Changes: records.map((record) => {
console.log(`${record.Name} ${record.Type} ${record.Value}`);
return {
Action: action,
ResourceRecordSet: {
Name: record.Name,
Type: record.Type,
TTL: 60,
ResourceRecords: [{
Value: record.Value
}]
}
};
}),
},
HostedZoneId: hostedZoneId
}).promise();

console.log('Waiting for DNS records to commit...');
await route53.waitFor('resourceRecordSetsChanged', {
// Wait up to 5 minutes
$waiter: {
delay: 30,
maxAttempts: 10
},
Id: changeBatch.ChangeInfo.Id
}).promise();
}

/**
* Main handler, invoked by Lambda
*/
Expand Down Expand Up @@ -262,7 +300,13 @@ exports.certificateRequestHandler = async function (event, context) {
// If the resource didn't create correctly, the physical resource ID won't be the
// certificate ARN, so don't try to delete it in that case.
if (physicalResourceId.startsWith('arn:')) {
await deleteCertificate(physicalResourceId, event.ResourceProperties.Region);
await deleteCertificate(
physicalResourceId,
event.ResourceProperties.Region,
event.ResourceProperties.HostedZoneId,
event.ResourceProperties.Route53Endpoint,
event.ResourceProperties.CleanupRecords === "true",
);
}
break;
default:
Expand Down
168 changes: 168 additions & 0 deletions ...certificatemanager/lambda-packages/dns_validated_certificate_handler/test/handler.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -996,4 +996,172 @@ describe('DNS Validated Certificate Handler', () => {
expect(request.isDone()).toBe(true);
});
});

describe('Delete option record cleanup', () => {
let describeCertificateFake;
let deleteCertificateFake;
let changeResourceRecordSetsFake;

beforeEach(() => {
deleteCertificateFake = sinon.fake.resolves({});
AWS.mock('ACM', 'deleteCertificate', deleteCertificateFake);
changeResourceRecordSetsFake = sinon.fake.resolves({
ChangeInfo: {
Id: 'bogus'
}
});
AWS.mock('Route53', 'changeResourceRecordSets', changeResourceRecordSetsFake);

describeCertificateFake = sinon.fake.resolves({
Certificate: {
CertificateArn: testCertificateArn,
DomainValidationOptions: [{
ValidationStatus: 'SUCCESS',
ResourceRecord: {
Name: testRRName,
Type: 'CNAME',
Value: testRRValue
}
}]
}
});
AWS.mock('ACM', 'describeCertificate', describeCertificateFake);
});

test('ignores records if CleanupRecords is not set', () => {
const request = nock(ResponseURL).put('/', body => {
return body.Status === 'SUCCESS';
}).reply(200);

return LambdaTester(handler.certificateRequestHandler)
.event({
RequestType: 'Delete',
RequestId: testRequestId,
PhysicalResourceId: testCertificateArn,
ResourceProperties: {
Region: 'us-east-1',
HostedZoneId: testHostedZoneId,
}
})
.expectResolve(() => {
sinon.assert.calledWith(describeCertificateFake, sinon.match({
CertificateArn: testCertificateArn
}));
sinon.assert.calledWith(deleteCertificateFake, sinon.match({
CertificateArn: testCertificateArn
}));
sinon.assert.notCalled(changeResourceRecordSetsFake);
expect(request.isDone()).toBe(true);
});
});

test('ignores records if CleanupRecords is not set to "true"', () => {
const request = nock(ResponseURL).put('/', body => {
return body.Status === 'SUCCESS';
}).reply(200);

return LambdaTester(handler.certificateRequestHandler)
.event({
RequestType: 'Delete',
RequestId: testRequestId,
PhysicalResourceId: testCertificateArn,
ResourceProperties: {
Region: 'us-east-1',
HostedZoneId: testHostedZoneId,
CleanupRecords: 'TRUE', // Not "true"
}
})
.expectResolve(() => {
sinon.assert.calledWith(describeCertificateFake, sinon.match({
CertificateArn: testCertificateArn
}));
sinon.assert.calledWith(deleteCertificateFake, sinon.match({
CertificateArn: testCertificateArn
}));
sinon.assert.notCalled(changeResourceRecordSetsFake);
expect(request.isDone()).toBe(true);
});
});

test('deletes records if CleanupRecords is set to true and records are present', () => {
const request = nock(ResponseURL).put('/', body => {
return body.Status === 'SUCCESS';
}).reply(200);

AWS.mock('Route53', 'changeResourceRecordSets', changeResourceRecordSetsFake);

return LambdaTester(handler.certificateRequestHandler)
.event({
RequestType: 'Delete',
RequestId: testRequestId,
PhysicalResourceId: testCertificateArn,
ResourceProperties: {
Region: 'us-east-1',
HostedZoneId: testHostedZoneId,
CleanupRecords: 'true',
},
})
.expectResolve(() => {
sinon.assert.calledWith(describeCertificateFake, sinon.match({
CertificateArn: testCertificateArn
}));
sinon.assert.calledWith(deleteCertificateFake, sinon.match({
CertificateArn: testCertificateArn
}));
sinon.assert.calledWith(changeResourceRecordSetsFake, sinon.match({
ChangeBatch: {
Changes: [{
Action: 'DELETE',
ResourceRecordSet: {
Name: testRRName,
Type: 'CNAME',
TTL: 60,
ResourceRecords: [{
Value: testRRValue
}]
}
}]
},
HostedZoneId: testHostedZoneId
}));
expect(request.isDone()).toBe(true);
});
});

test('fails if CleanupRecords is set to true and records are not present', () => {
describeCertificateFake = sinon.fake.resolves({
Certificate: {
CertificateArn: testCertificateArn,
}
});
AWS.remock('ACM', 'describeCertificate', describeCertificateFake);

const request = nock(ResponseURL).put('/', body => {
return body.Status === 'FAILED' &&
body.Reason.startsWith('Response from describeCertificate did not contain DomainValidationOptions');
}).reply(200);

AWS.mock('Route53', 'changeResourceRecordSets', changeResourceRecordSetsFake);

return LambdaTester(handler.certificateRequestHandler)
.event({
RequestType: 'Delete',
RequestId: testRequestId,
PhysicalResourceId: testCertificateArn,
ResourceProperties: {
Region: 'us-east-1',
HostedZoneId: testHostedZoneId,
CleanupRecords: 'true',
},
})
.expectResolve(() => {
sinon.assert.calledWith(describeCertificateFake, sinon.match({
CertificateArn: testCertificateArn
}));
sinon.assert.notCalled(deleteCertificateFake);
sinon.assert.notCalled(changeResourceRecordSetsFake);
expect(request.isDone()).toBe(true);
});
});
});
});
Loading

0 comments on commit 5af7c1f

Please sign in to comment.