Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sl): warn ops regarding CAA records #1076

Merged
merged 2 commits into from
Dec 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 39 additions & 4 deletions src/routes/formsg/formsgSiteLaunch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
getDNSRecordsEmailBody,
getErrorEmailBody,
} from "@root/services/utilServices/SendDNSRecordEmailClient"
import TRUSTED_AMPLIFY_CAA_RECORDS from "@root/types/caaAmplify"
import { DigResponse, DigType } from "@root/types/dig"
import UsersService from "@services/identity/UsersService"
import InfraService from "@services/infra/InfraService"
Expand Down Expand Up @@ -231,7 +232,7 @@ export class FormsgSiteLaunchRouter {
await mailer.sendMail(email, subject, html)
}

private digDomainForQuadARecords = async (
digDomainRecords = async (
domain: string,
digType: DigType
): Promise<DigResponse | null> =>
Expand Down Expand Up @@ -332,13 +333,18 @@ export class FormsgSiteLaunchRouter {
for (const launchResult of launchResults) {
if (launchResult.isOk()) {
// check for AAAA records
const digResponse = await this.digDomainForQuadARecords(
const quadADigResponse = await this.digDomainRecords(
launchResult.value.primaryDomainSource,
"AAAA"
)
const caaDigResponse = await this.digDomainRecords(
launchResult.value.primaryDomainSource,
"CAA"
)

const successResult: DnsRecordsEmailProps = launchResult.value
if (digResponse && digResponse.answer) {
const quadARecords = digResponse.answer
if (quadADigResponse && quadADigResponse.answer) {
const quadARecords = quadADigResponse.answer
successResult.quadARecords = quadARecords.map((record) => ({
domain: record.domain,
class: record.class,
Expand All @@ -350,6 +356,35 @@ export class FormsgSiteLaunchRouter {
`Unable to get dig response for domain: ${launchResult.value.primaryDomainSource}. Skipping check for AAAA records`
)
}

if (!caaDigResponse) {
logger.info(
`Unable to get dig response for domain: ${launchResult.value.primaryDomainSource}. Skipping check for CAA records`
)
} else if (caaDigResponse.answer) {
const caaRecords = caaDigResponse.answer

/**
* NOTE: If there exists more than one CAA Record, we need to
* 1. check if they have whitelisted Amazon CAA
* 2. if not, send email to inform them to whitelist Amazon CAA
*/
const hasAmazonCAAWhitelisted = caaRecords.some((record) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the assumption we're making here is that if there's only one CAA record, it's the whitelisted amazon one right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really, the condition to check here is: if it has any CAA record, at least one has to be the amazon one
in the case where there is no CAA record, actually all this is ok already

const isAmazonCAA = TRUSTED_AMPLIFY_CAA_RECORDS.some(
(trustedCAA) => trustedCAA === record.value
)
return isAmazonCAA
})
if (caaRecords.length > 0 && !hasAmazonCAAWhitelisted) {
successResult.addCAARecord = true
} else {
successResult.addCAARecord = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we need the else if asAmazonCAAWhitelisted check here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh my conidtion is wrong i think that caused the confusion ps

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed, hopefully this clearer

Comment on lines +378 to +381
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (caaRecords.length > 0 && !hasAmazonCAAWhitelisted) {
successResult.addCAARecord = true
} else {
successResult.addCAARecord = false
successResult.addCAARecord = caaRecords.length > 0 && !hasAmazonCAAWhitelisted

}
} else {
logger.info(
`${launchResult.value.primaryDomainSource} Domain does not have any CAA records.`
)
}
// Create better uptime monitor
await this.createMonitor(launchResult.value.primaryDomainSource)
successResults.push(successResult)
Expand Down
45 changes: 41 additions & 4 deletions src/services/utilServices/SendDNSRecordEmailClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ import { groupBy } from "lodash"

import { DigType } from "@root/types/dig"

export interface QuadARecord {
export interface DigDNSRecord {
domain: string
class: string
type: DigType
value: string
}

export interface DnsRecordsEmailProps {
requesterEmail: string
repoName: string
Expand All @@ -18,7 +19,8 @@ export interface DnsRecordsEmailProps {
indirectionDomain: string
redirectionDomainSource?: string
redirectionDomainTarget?: string
quadARecords?: QuadARecord[]
quadARecords?: DigDNSRecord[]
addCAARecord?: boolean
}

export interface LaunchFailureEmailProps {
Expand Down Expand Up @@ -109,7 +111,43 @@ export function getDNSRecordsEmailBody(
</table>`

Object.keys(groupedDnsRecords).forEach((repoName) => {
const allQuadARecordsForRepo: QuadARecord[] = []
groupedDnsRecords[repoName].forEach((dnsRecord) => {
if (dnsRecord.addCAARecord) {
html += `<p style="${bodyFooterStyle}">Please add CAA records for the following repo: <b>${repoName}</b>.`

html += ` <table style="${tableStyle}">
<thead>
<tr style="${headerRowStyle}">
<th style="${thStyle}">Repo Name</th>
<th style="${thStyle}">Type</th>
<th style="${thStyle}">Flags</th>
<th style="${thStyle}">Tag</th>
<th style="${thStyle}">Value</th>
</tr>
</thead>
<tbody>
<tr style="${tdStyle}">
<td style="${tdStyle} background-color: #f2f2f2" rowspan="2" >${repoName}</td>
<td style="${tdStyle}">${dnsRecord.primaryDomainSource}</td>
<td style="${tdStyle}">0</td>
<td style="${tdStyle}">issue</td>
<td style="${tdStyle}">amazontrust.com</td>
</tr>
<tr style="${tdStyle}">
<td style="${tdStyle}">${dnsRecord.primaryDomainSource}</td>
<td style="${tdStyle}">0</td>
<td style="${tdStyle}">issue</td>
<td style="${tdStyle}">amazontrust.com</td>
</tr>

</tbody>
</table>`
}
})
})

Object.keys(groupedDnsRecords).forEach((repoName) => {
const allQuadARecordsForRepo: DigDNSRecord[] = []
groupedDnsRecords[repoName].forEach((dnsRecord) => {
if (dnsRecord.quadARecords) {
allQuadARecordsForRepo.push(...dnsRecord.quadARecords)
Expand Down Expand Up @@ -147,7 +185,6 @@ export function getDNSRecordsEmailBody(
})

html += `<p style="${bodyFooterStyle}">This email was sent from the Isomer CMS backend.</p>`

return html
}

Expand Down
11 changes: 11 additions & 0 deletions src/types/caaAmplify.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* This is taken from https://docs.aws.amazon.com/acm/latest/userguide/setup-caa.html
*/
const TRUSTED_AMPLIFY_CAA_RECORDS = [
"amazon.com",
"amazontrust.com",
"awstrust.com",
"amazonaws.com",
]

export default TRUSTED_AMPLIFY_CAA_RECORDS
1 change: 1 addition & 0 deletions src/types/dig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ export type DigType =
| "SOA"
| "SRV"
| "TXT"
| "CAA"
Loading