aws-ses: DKIM records are created with incorrect name from EmailIdentity

[Booligoosh]

Describe the bug

When creating a new ses.EmailIdentity construct, the names of the DKIM records that get created in Route53 are incorrect.

Expected Behavior

If the DKIM record name is x.y.example.com, and the hosted zone is y.example.com, the name of the record that gets set in Route53 should be x.y.example.com

Current Behavior

If the DKIM record name is x.y.example.com, and the hosted zone is y.example.com, the name of the record that gets set in Route53 should is x.y.example.com.y.example.com

Reproduction Steps

Here is a simple reproduction case:

const myHostedZone = ...; // However you're grabbing or creating your hosted zone

const sesIdentity = new ses.EmailIdentity(this, "ses-identity", {
  identity: ses.Identity.publicHostedZone(hostedZone),
  mailFromDomain: `mail.${hostedZoneDomain}`,
});

Possible Solution

The reason for the incorrect record names is because of the function here in the source code:.

aws-cdk/packages/@aws-cdk/aws-ses/lib/email-identity.ts

Lines 220 to 237 in ac32340

	if (hostedZone) {
	new route53.CnameRecord(emailIdentity, 'DkimDnsToken1', {
	zone: hostedZone,
	recordName: Lazy.string({ produce: () => emailIdentity.dkimDnsTokenName1 }),
	domainName: Lazy.string({ produce: () => emailIdentity.dkimDnsTokenValue1 }),
	});
	new route53.CnameRecord(hostedZone, 'DkimDnsToken2', {
	zone: hostedZone,
	recordName: Lazy.string({ produce: () => emailIdentity.dkimDnsTokenName2 }),
	domainName: Lazy.string({ produce: () => emailIdentity.dkimDnsTokenValue2 }),
	});
	new route53.CnameRecord(hostedZone, 'DkimDnsToken3', {
	zone: hostedZone,
	recordName: Lazy.string({ produce: () => emailIdentity.dkimDnsTokenName3 }),
	domainName: Lazy.string({ produce: () => emailIdentity.dkimDnsTokenValue3 }),
	});

It calls route53.CnameRecord, which accepts a record name excluding the hosted zone suffix - eg. for x.y.example.com where the hosted zone is y.example.com, the record name should just be x.

However, it's passing in dkimDnsTokenName1 etc, which according to the Cloudformation docs, is the entire host for the record, including the hosted zone.

This means that route53.CnameRecord tried to append the hosted zone name onto the end of the record name which already includes the hosted zone name, leading to the duplication.

This can be seen in a cdk synth:

...
    Type: AWS::Route53::RecordSet
    Properties:
      Name:
        Fn::Join:
          - ""
          - - Fn::GetAtt:
                - <Our SES Entity ID>
                - DkimDNSTokenName1
            - .y.example.com.
      Type: CNAME
...

There are 2 fixes that I can think of:

1. Call CfnRecordSet rather than CnameRecord - eg:

new route53.CfnRecordSet(emailIdentity, "DkimDnsToken1", {
  hostedZoneName: hostedZone.zoneName + ".",
  name: Lazy.string({ produce: () => emailIdentity.dkimDnsTokenName1 }),
  type: "CNAME",
  resourceRecords: [Lazy.string({ produce: () => emailIdentity.dkimDnsTokenValue1 })],
  ttl: "1800",
})

2. Use a hacky workaround to lop off the suffix before passing to CnameRecord - by combining cdk.Fn.select and cdk.Fn.split to remove the last part of the string. Here's how that would look:

// This function removes a string from the end of another string using Cfn functions.
// For example, calling it on "abcd" with suffix "cd" would return "ab".
// We accomplish this by splitting by the suffix (which gives ["ab"]), then selecting the 0th value.
const hackyCfnRemoveSuffix = (sourceString: string, suffixToRemove: string) =>
  cdk.Fn.select(0, cdk.Fn.split(suffixToRemove, sourceString));

// Then use it in the bind function
new route53.CnameRecord(emailIdentity, 'DkimDnsToken1', {
  zone: hostedZone,
  recordName: hackyCfnRemoveSuffix(Lazy.string({ produce: () => emailIdentity.dkimDnsTokenName1 }), "." + hostedZone.zoneName),
  domainName: Lazy.string({ produce: () => emailIdentity.dkimDnsTokenValue1 }),
});

Additional Information/Context

No response

CDK CLI Version

2.29.0 (build 47d7ec4)

Framework Version

2.33.0

Node.js Version

v16.15.1

OS

MacOS

Language

Typescript

Language Version

No response

Other information

Workaround

For anyone else experiencing this issue - we've opted to go with the CfnRecordSet option as it's a bit simpler to see what's going on. Simply map through each of the dkimRecords after creating the EmailIdentity and create the correct records - note that this won't remove the incorrect ones unfortunately.

Code below:

// Create SES email identity based on Route53 hosted zone
const sesIdentity = new ses.EmailIdentity(this, "ses-identity", {
  identity: ses.Identity.publicHostedZone(hostedZone),
  mailFromDomain: `mail.${hostedZone.zoneName}`,
});

// Temporary workaround: There is a bug with the EmailIdentity construct where if the DKIM record is x.y.example.com,
// and the hosted zone is y.example.com, the record that gets set is x.y.example.com.y.example.com. For now, we are
// manually creating the correct records.
const dkimCnameRecords = sesIdentity.dkimRecords.map(
  (dkimRecord, index) =>
    new route53.CfnRecordSet(this, `dkim-record-${index}`, {
      hostedZoneName: hostedZone.zoneName + ".",
      name: dkimRecord.name,
      type: "CNAME",
      resourceRecords: [dkimRecord.value],
      ttl: "1800",
    })
);

[jogold]

Hi @Booligoosh,

Thanks for reporting this.

How are you creating/importing your hosted zone here?

The integ test at https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-ses/test/integ.email-identity.ts deploys successfully.

[jogold]

OK, got it, I see the duplication in the record.

[derekmurawsky]

I am also having this same issue. Thanks, @Booligoosh , for the work around!

[TheRealAmazonKendra]

See issue #20389 for the ongoing conversation about this issue. I think that by fixing that issue we'll also be fixing this one. @jogold I'm curious about your opinion about the best path forward on that one.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

[Booligoosh]

Thanks for the fix! Just a note for @derekmurawsky and anyone else that was using the workaround I mentioned in the original issue and is wanting to upgrade to the fixed version of CDK.

When upgrading the version to get rid of the need for the workaround, we weren't able to do it all in 1 deployment, as the DKIM records created by the workaround got deleted, but did not get re-created by the now fixed EmailIdentity.

We had to do the following:

1. Remove the workaround of manually creating the CfnRecordSets (see above) and deploy. This will temporarily remove your correct DKIM records.
2. Upgrade the CDK version and deploy. This will remove the incorrect DKIM records, and re-create the correct ones (but as part of the EmailIdentity).

[tqhoughton]

Is it intended that if the mailFromDomain does not end in a . that it will still create the MX and TXT records with the name mail.example.com.example.com? I assumed that based off of the discussion here both cases were fixed but it appears that this is an issue if you do not include the trailing .. The CDK docs even show an example without a trailing . which leads to more confusion: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ses.EmailIdentity.html#example

On cdk version 2.72.1

[derekmurawsky]

@tqhoughton This is actually DNS level stuff. The . makes it an FQDN, otherwise it's assumed to be a sub record of the zone it's in. See:
https://serverfault.com/questions/18113/dns-trailing-periods
and from RFC1035, section 5.1. Format:
<domain-name>s make up a large share of the data in the master file. The labels in the domain name are expressed as character strings and separated by dots. Quoting conventions allow arbitrary characters to be stored in domain names. Domain names that end in a dot are called absolute, and are taken as complete. Domain names which do not end in a dot are called relative; the actual domain name is the concatenation of the relative part with an origin specified in a $ORIGIN, $INCLUDE, or as an argument to the master file loading routine. A relative name is an error when no origin is available. <character-string> is expressed in one or two ways: as a contiguous set of characters without interior spaces, or as a string beginning with a " and ending with a ". Inside a " delimited string any character can occur, except for a " itself, which must be quoted using \ (back slash).

Basically, . is the root of the DNS namespace, and to be a really fully qualified domain name, you need to end in a ..

So changing this may make some level of sense to some users, but it would make this implementation "quirky" for the way DNS works under the hood. I would suggest we stick with the specs and maybe document it a bit better for folks that aren't as familiar with the nuances of DNS.