From 5ba35e41b0b5f885e72b3f75c6d2f695d2f8808a Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 28 Nov 2022 12:30:51 +0100 Subject: [PATCH] fix(route53): cross-account delegation broken in opt-in regions (#23082) This is a revert of #22370. The fix proposed there solved one very specific (but rare) use case, in exchange for breaking common use cases. - *It fixed the case of*: AWS service authors using their own global service principal in the delegation role, with both source and target account opted into the region. - *It broke the case of*: all teams that didn't have both accounts opted into the region. The second case is much more common, so revert to the old behavior. Since the regional behavior might still be useful to *some* people somewhere, it has been relegated to a context key, `@aws-cdk/aws-route53:useRegionalStsEndpoint`, instead. It can be configured, but is not advertised as 99.9% of users will not need this behavior. Since both STS and Route53 are global and regular customers cannot usefully use Service Principals in this particular trust policy anyway, there is no impact to regular customers. Fixes #23081. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../index.ts | 9 ++++--- .../@aws-cdk/aws-route53/lib/hosted-zone.ts | 12 +++++++++ .../@aws-cdk/aws-route53/lib/record-set.ts | 23 ++++++++++++++++ .../aws-route53/test/record-set.test.ts | 26 +++++++++++++++++++ 4 files changed, 66 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-route53/lib/cross-account-zone-delegation-handler/index.ts b/packages/@aws-cdk/aws-route53/lib/cross-account-zone-delegation-handler/index.ts index 39a9be2924c35..7867e7a45a725 100644 --- a/packages/@aws-cdk/aws-route53/lib/cross-account-zone-delegation-handler/index.ts +++ b/packages/@aws-cdk/aws-route53/lib/cross-account-zone-delegation-handler/index.ts @@ -8,6 +8,7 @@ interface ResourceProperties { DelegatedZoneName: string, DelegatedZoneNameServers: string[], TTL: number, + UseRegionalStsEndpoint?: string, } export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent) { @@ -23,13 +24,13 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent } async function cfnEventHandler(props: ResourceProperties, isDeleteEvent: boolean) { - const { AssumeRoleArn, ParentZoneId, ParentZoneName, DelegatedZoneName, DelegatedZoneNameServers, TTL } = props; + const { AssumeRoleArn, ParentZoneId, ParentZoneName, DelegatedZoneName, DelegatedZoneNameServers, TTL, UseRegionalStsEndpoint } = props; if (!ParentZoneId && !ParentZoneName) { throw Error('One of ParentZoneId or ParentZoneName must be specified'); } - const credentials = await getCrossAccountCredentials(AssumeRoleArn); + const credentials = await getCrossAccountCredentials(AssumeRoleArn, !!UseRegionalStsEndpoint); const route53 = new Route53({ credentials }); const parentZoneId = ParentZoneId ?? await getHostedZoneIdByName(ParentZoneName!, route53); @@ -50,8 +51,8 @@ async function cfnEventHandler(props: ResourceProperties, isDeleteEvent: boolean }).promise(); } -async function getCrossAccountCredentials(roleArn: string): Promise { - const sts = new STS({ stsRegionalEndpoints: 'regional' }); +async function getCrossAccountCredentials(roleArn: string, regionalEndpoint: boolean): Promise { + const sts = new STS(regionalEndpoint ? { stsRegionalEndpoints: 'regional' } : {}); const timestamp = (new Date()).getTime(); const { Credentials: assumedCredentials } = await sts diff --git a/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts b/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts index da459a34deff5..fc04c3f91077f 100644 --- a/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts +++ b/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts @@ -200,6 +200,18 @@ export interface PublicHostedZoneProps extends CommonHostedZoneProps { /** * A principal which is trusted to assume a role for zone delegation * + * If supplied, this will create a Role in the same account as the Hosted + * Zone, which can be assumed by the `CrossAccountZoneDelegationRecord` to + * create a delegation record to a zone in a different account. + * + * Be sure to indicate the account(s) that you trust to create delegation + * records, using either `iam.AccountPrincipal` or `iam.OrganizationPrincipal`. + * + * If you are planning to use `iam.ServicePrincipal`s here, be sure to include + * region-specific service principals for every opt-in region you are going to + * be delegating to; or don't use this feature and create separate roles + * with appropriate permissions for every opt-in region instead. + * * @default - No delegation configuration */ readonly crossAccountZoneDelegationPrincipal?: iam.IPrincipal; diff --git a/packages/@aws-cdk/aws-route53/lib/record-set.ts b/packages/@aws-cdk/aws-route53/lib/record-set.ts index 68223d52096ee..03e7bc1cf5c1c 100644 --- a/packages/@aws-cdk/aws-route53/lib/record-set.ts +++ b/packages/@aws-cdk/aws-route53/lib/record-set.ts @@ -10,6 +10,26 @@ import { determineFullyQualifiedDomainName } from './util'; const CROSS_ACCOUNT_ZONE_DELEGATION_RESOURCE_TYPE = 'Custom::CrossAccountZoneDelegation'; const DELETE_EXISTING_RECORD_SET_RESOURCE_TYPE = 'Custom::DeleteExistingRecordSet'; +/** + * Context key to control whether to use the regional STS endpoint, instead of the global one + * + * There is only exactly one use case where you want to turn this on. If: + * + * - you are building an AWS service; AND + * - would like to your own Global Service Principal in the trust policy of the delegation role; AND + * - the target account is opted in in the same region as well + * + * Then you can turn this on. For all other use cases, the global endpoint is preferable: + * + * - if you are a regular customer, your trust policy would be in terms of account ids or + * organization ids, or ARNs, not Service Principals, so you don't care about this behavior. + * - if the target account is not opted in as well, the AssumeRole call would fail + * + * Because this configuration option is so rare, turn it into a context setting instead + * of a publicly available prop. + */ +const USE_REGIONAL_STS_ENDPOINT_CONTEXT_KEY = '@aws-cdk/aws-route53:useRegionalStsEndpoint'; + /** * A record set */ @@ -749,6 +769,8 @@ export class CrossAccountZoneDelegationRecord extends Construct { resources: [props.delegationRole.roleArn], })); + const useRegionalStsEndpoint = this.node.tryGetContext(USE_REGIONAL_STS_ENDPOINT_CONTEXT_KEY); + const customResource = new CustomResource(this, 'CrossAccountZoneDelegationCustomResource', { resourceType: CROSS_ACCOUNT_ZONE_DELEGATION_RESOURCE_TYPE, serviceToken: provider.serviceToken, @@ -760,6 +782,7 @@ export class CrossAccountZoneDelegationRecord extends Construct { DelegatedZoneName: props.delegatedZone.zoneName, DelegatedZoneNameServers: props.delegatedZone.hostedZoneNameServers!, TTL: (props.ttl || Duration.days(2)).toSeconds(), + UseRegionalStsEndpoint: useRegionalStsEndpoint ? 'true' : undefined, }, }); diff --git a/packages/@aws-cdk/aws-route53/test/record-set.test.ts b/packages/@aws-cdk/aws-route53/test/record-set.test.ts index fa1309e300faa..c444cb3059e07 100644 --- a/packages/@aws-cdk/aws-route53/test/record-set.test.ts +++ b/packages/@aws-cdk/aws-route53/test/record-set.test.ts @@ -845,6 +845,32 @@ describe('record set', () => { } }); + test('Cross account zone context flag', () => { + // GIVEN + const stack = new Stack(); + stack.node.setContext('@aws-cdk/aws-route53:useRegionalStsEndpoint', true); + const parentZone = new route53.PublicHostedZone(stack, 'ParentHostedZone', { + zoneName: 'myzone.com', + crossAccountZoneDelegationPrincipal: new iam.AccountPrincipal('123456789012'), + }); + + // WHEN + const childZone = new route53.PublicHostedZone(stack, 'ChildHostedZone', { + zoneName: 'sub.myzone.com', + }); + new route53.CrossAccountZoneDelegationRecord(stack, 'Delegation', { + delegatedZone: childZone, + parentHostedZoneName: 'myzone.com', + delegationRole: parentZone.crossAccountZoneDelegationRole!, + ttl: Duration.seconds(60), + }); + + // THEN + Template.fromStack(stack).hasResourceProperties('Custom::CrossAccountZoneDelegation', { + UseRegionalStsEndpoint: 'true', + }); + }); + test('Delete existing record', () => { // GIVEN const stack = new Stack();