diff --git a/entrypoints/popup/main.ts b/entrypoints/popup/main.ts index fe7a4b3..4624dfb 100644 --- a/entrypoints/popup/main.ts +++ b/entrypoints/popup/main.ts @@ -116,7 +116,7 @@ function mxtoolboxEmailHealthUrl(domain: string): string { } const DNS_TECHNIQUE_DISCLOSURE = - 'DNS queries use DNS-over-HTTPS (Cloudflare / Google). Entra probe uses HTTPS only; no MTA-STS policy files or cert inspection. DKIM probes *._domainkey first, then _domainkey and common selectors.'; + 'DNS queries use DNS-over-HTTPS (Cloudflare / Google). Entra probe uses HTTPS only; no MTA-STS policy files or cert inspection. DKIM probes _domainkey for null DKIM, then provider/common selectors, then *._domainkey.'; const WALL_OF_SHAME_REPO = 'jkerai1/DMARC-WallOfShame'; @@ -148,10 +148,14 @@ function openUrlInNewTab(url: string): void { a.remove(); } +function hasReportableDmarcIssue(result: CheckResult): boolean { + return result.full.dmarc.status !== 'pass'; +} + function renderResultFooterActions(result: CheckResult): string { - const showCastShame = result.full.dmarc.status === 'fail'; + const showCastShame = hasReportableDmarcIssue(result); const castShameBtn = showCastShame - ? `` + ? `` : ''; return `
Please submit the company name for the DMARC issue.
@@ -463,7 +467,7 @@ function renderResult(result: CheckResult): void { const tabDiffers = result.tabHostname !== result.queryHostname; const detailedBreakdown = settings.detailedBreakdown; const castShameModal = - full.dmarc.status === 'fail' ? renderCastShameModal(result) : ''; + hasReportableDmarcIssue(result) ? renderCastShameModal(result) : ''; const spfSupplement = result.spfMailProviderHint && diff --git a/lib/checkDomain.dnsError.test.ts b/lib/checkDomain.dnsError.test.ts index dc3ab2f..481f6c6 100644 --- a/lib/checkDomain.dnsError.test.ts +++ b/lib/checkDomain.dnsError.test.ts @@ -20,6 +20,13 @@ beforeEach(() => { vi.mocked(queryTxt.resolveTxtDetailed).mockReset(); }); +function dkimLookupNames(): string[] { + return vi.mocked(queryTxt.resolveTxtDetailed) + .mock.calls + .map(([name]) => name) + .filter((name) => name.includes('_domainkey')); +} + describe('runDnsCheck DNS resolution errors', () => { it('marks SPF as fail when TXT lookup is non-definitive and strict mode is on', async () => { vi.mocked(queryTxt.resolveTxtDetailed).mockImplementation( @@ -97,3 +104,134 @@ describe('runDnsCheck DNS resolution errors', () => { ); }); }); + +describe('runDnsCheck DKIM probe order', () => { + it('checks fallback selectors when there is no wildcard record', async () => { + vi.mocked(queryTxt.resolveTxtDetailed).mockImplementation( + async (name: string) => { + if (name === 'example.com') { + return { strings: ['v=spf1 -all'], dnsState: 'ok' }; + } + if (name === '_dmarc.example.com') { + return { strings: ['v=DMARC1; p=reject;'], dnsState: 'ok' }; + } + if (name === 'default._domainkey.example.com') { + return { strings: ['v=DKIM1; k=rsa; p=MII'], dnsState: 'ok' }; + } + if (name.includes('_domainkey')) { + return { strings: [], dnsState: 'nxdomain' }; + } + return { strings: [], dnsState: 'ok' }; + }, + ); + + const r = await runDnsCheck('example.com'); + + expect(r.dkim.selector).toBe('default'); + expect(r.full.dkim.status).toBe('pass'); + expect(dkimLookupNames()).toEqual([ + '_domainkey.example.com', + 'google._domainkey.example.com', + 'default._domainkey.example.com', + ]); + }); + + it('chooses a valid selector before considering wildcard DKIM', async () => { + vi.mocked(queryTxt.resolveTxtDetailed).mockImplementation( + async (name: string) => { + if (name === 'example.com') { + return { strings: ['v=spf1 -all'], dnsState: 'ok' }; + } + if (name === '_dmarc.example.com') { + return { strings: ['v=DMARC1; p=reject;'], dnsState: 'ok' }; + } + if (name === 'google._domainkey.example.com') { + return { strings: ['v=DKIM1; k=rsa; p=MII'], dnsState: 'ok' }; + } + if (name === '*._domainkey.example.com') { + return { strings: ['not a DKIM record'], dnsState: 'ok' }; + } + if (name.includes('_domainkey')) { + return { strings: [], dnsState: 'nxdomain' }; + } + return { strings: [], dnsState: 'ok' }; + }, + ); + + const r = await runDnsCheck('example.com'); + + expect(r.dkim.selector).toBe('google'); + expect(dkimLookupNames()).toEqual([ + '_domainkey.example.com', + 'google._domainkey.example.com', + ]); + }); + + it('stops at null DKIM on _domainkey', async () => { + vi.mocked(queryTxt.resolveTxtDetailed).mockImplementation( + async (name: string) => { + if (name === 'example.com') { + return { strings: ['v=spf1 -all'], dnsState: 'ok' }; + } + if (name === '_dmarc.example.com') { + return { strings: ['v=DMARC1; p=reject;'], dnsState: 'ok' }; + } + if (name === '_domainkey.example.com') { + return { strings: ['v=DKIM1; p='], dnsState: 'ok' }; + } + if (name.includes('_domainkey')) { + return { strings: [], dnsState: 'nxdomain' }; + } + return { strings: [], dnsState: 'ok' }; + }, + ); + + const r = await runDnsCheck('example.com'); + + expect(r.dkim.selector).toBe('_domainkey'); + expect(r.full.dkim.status).toBe('pass'); + expect(dkimLookupNames()).toEqual(['_domainkey.example.com']); + }); + + it('tries MX provider selectors before fallback selectors', async () => { + vi.mocked(mailInfra.runMailInfraChecks).mockResolvedValue([ + { + id: 'mx', + title: 'MX', + status: 'pass', + summary: 'Microsoft 365', + lines: [], + providerProfile: { + name: 'Microsoft 365', + dkimSelectors: ['selector1', 'selector2'], + }, + }, + ]); + vi.mocked(queryTxt.resolveTxtDetailed).mockImplementation( + async (name: string) => { + if (name === 'example.com') { + return { strings: ['v=spf1 -all'], dnsState: 'ok' }; + } + if (name === '_dmarc.example.com') { + return { strings: ['v=DMARC1; p=reject;'], dnsState: 'ok' }; + } + if (name === 'selector2._domainkey.example.com') { + return { strings: ['v=DKIM1; k=rsa; p=MII'], dnsState: 'ok' }; + } + if (name.includes('_domainkey')) { + return { strings: [], dnsState: 'nxdomain' }; + } + return { strings: [], dnsState: 'ok' }; + }, + ); + + const r = await runDnsCheck('example.com'); + + expect(r.dkim.selector).toBe('selector2'); + expect(dkimLookupNames()).toEqual([ + '_domainkey.example.com', + 'selector1._domainkey.example.com', + 'selector2._domainkey.example.com', + ]); + }); +}); diff --git a/lib/checkDomain.ts b/lib/checkDomain.ts index fe8fb8a..1aa96c3 100644 --- a/lib/checkDomain.ts +++ b/lib/checkDomain.ts @@ -84,12 +84,12 @@ const DNS_FAIL_DMARC = (org: string): GradeLine[] => [ function dnsFailDkimLines(mxProfileSelectors?: readonly string[]): GradeLine[] { const probeHint = mxProfileSelectors && mxProfileSelectors.length > 0 - ? `configured selectors (${mxProfileSelectors.join(', ')})` + ? `configured selectors (${mxProfileSelectors.join(', ')}) or fallback selectors` : 'any probed selector'; return [ { status: 'fail', - text: `Could not resolve DKIM TXT at *._domainkey, _domainkey, or ${probeHint} (DNS error or non-definitive response).`, + text: `Could not resolve DKIM TXT at _domainkey, ${probeHint}, or *._domainkey (DNS error or non-definitive response).`, }, ]; } @@ -165,64 +165,66 @@ export async function runDnsCheck( let dkimBest: (DkimRecordAnalysis & { selector: string }) | null = null; let hadDefinitiveDkimLookup = false; - const wildcardDkimDet = await resolveTxtDetailed( - dkimDnsWildcardFqdn(queryHost), - dnsTxt, - ); + const apexDkimFqdn = `_domainkey.${queryHost}`; + const apexDkimDet = await resolveTxtDetailed(apexDkimFqdn, dnsTxt); if ( - wildcardDkimDet.dnsState === 'ok' || - wildcardDkimDet.dnsState === 'nxdomain' + apexDkimDet.dnsState === 'ok' || + apexDkimDet.dnsState === 'nxdomain' ) { hadDefinitiveDkimLookup = true; } - const wildcardTxts = analysisStrings(wildcardDkimDet, treatDnsAsFail); - const wildcardMerged = mergeTxtForDkim(wildcardTxts); - const wildcardRec = analyzeDkimRecord(wildcardMerged); - const wildcardOverrides = - isNullDkimDeclaration(wildcardRec) || - wildcardRec.valid || - Boolean(wildcardRec.raw); + const apexDkimTxts = analysisStrings(apexDkimDet, treatDnsAsFail); + const apexDkimMerged = mergeTxtForDkim(apexDkimTxts); + const apexDkimRec = analyzeDkimRecord(apexDkimMerged); - if (wildcardOverrides) { - dkimBest = { ...wildcardRec, selector: '*' }; + if (isNullDkimDeclaration(apexDkimRec)) { + dkimBest = { ...apexDkimRec, selector: '_domainkey' }; } else { - const apexDkimFqdn = `_domainkey.${queryHost}`; - const apexDkimDet = await resolveTxtDetailed(apexDkimFqdn, dnsTxt); - if ( - apexDkimDet.dnsState === 'ok' || - apexDkimDet.dnsState === 'nxdomain' - ) { - hadDefinitiveDkimLookup = true; + if (apexDkimRec.raw) { + dkimBest = { ...apexDkimRec, selector: '_domainkey' }; } - const apexDkimTxts = analysisStrings(apexDkimDet, treatDnsAsFail); - const apexDkimMerged = mergeTxtForDkim(apexDkimTxts); - const apexDkimRec = analyzeDkimRecord(apexDkimMerged); - if (isNullDkimDeclaration(apexDkimRec)) { - dkimBest = { ...apexDkimRec, selector: '_domainkey' }; - } else if (apexDkimRec.valid) { - dkimBest = { ...apexDkimRec, selector: '_domainkey' }; - } else { - for (const sel of dkimProbeSelectors) { - const name = `${sel}._domainkey.${queryHost}`; - const det = await resolveTxtDetailed(name, dnsTxt); - if (det.dnsState === 'ok' || det.dnsState === 'nxdomain') { - hadDefinitiveDkimLookup = true; - } - const txts = analysisStrings(det, treatDnsAsFail); - const merged = mergeTxtForDkim(txts); - const rec = analyzeDkimRecord(merged); - const tagged: DkimRecordAnalysis & { selector: string } = { - ...rec, - selector: sel, - }; - if (rec.valid) { - dkimBest = tagged; - break; - } - if (!dkimBest && rec.raw) { - dkimBest = tagged; - } + for (const sel of dkimProbeSelectors) { + const name = `${sel}._domainkey.${queryHost}`; + const det = await resolveTxtDetailed(name, dnsTxt); + if (det.dnsState === 'ok' || det.dnsState === 'nxdomain') { + hadDefinitiveDkimLookup = true; + } + const txts = analysisStrings(det, treatDnsAsFail); + const merged = mergeTxtForDkim(txts); + const rec = analyzeDkimRecord(merged); + const tagged: DkimRecordAnalysis & { selector: string } = { + ...rec, + selector: sel, + }; + if (rec.valid) { + dkimBest = tagged; + break; + } + if (!dkimBest && rec.raw) { + dkimBest = tagged; + } + } + + if (!dkimBest || !dkimBest.valid) { + const wildcardDkimDet = await resolveTxtDetailed( + dkimDnsWildcardFqdn(queryHost), + dnsTxt, + ); + if ( + wildcardDkimDet.dnsState === 'ok' || + wildcardDkimDet.dnsState === 'nxdomain' + ) { + hadDefinitiveDkimLookup = true; + } + const wildcardTxts = analysisStrings(wildcardDkimDet, treatDnsAsFail); + const wildcardMerged = mergeTxtForDkim(wildcardTxts); + const wildcardRec = analyzeDkimRecord(wildcardMerged); + + if (wildcardRec.valid || isNullDkimDeclaration(wildcardRec)) { + dkimBest = { ...wildcardRec, selector: '*' }; + } else if (!dkimBest && wildcardRec.raw) { + dkimBest = { ...wildcardRec, selector: '*' }; } } } diff --git a/lib/checks/mailInfra.ts b/lib/checks/mailInfra.ts index cd305f2..bbca7ad 100644 --- a/lib/checks/mailInfra.ts +++ b/lib/checks/mailInfra.ts @@ -40,7 +40,7 @@ export type MailInfraCheck = { providerProfile?: { name: string; expectedSpfInclude?: string; - /** MX provider profile selectors; DKIM DNS probes use these exclusively when present. */ + /** MX provider profile selectors; DKIM DNS probes prefer these before common fallbacks. */ dkimSelectors?: string[]; }; }; diff --git a/lib/parse/dkim.test.ts b/lib/parse/dkim.test.ts index ba6a678..0001510 100644 --- a/lib/parse/dkim.test.ts +++ b/lib/parse/dkim.test.ts @@ -63,10 +63,16 @@ describe('dkimSelectorsForDnsProbe', () => { expect(dkimSelectorsForDnsProbe([])).toEqual(fallback); }); - it('uses only MX profile selectors when provided', () => { + it('uses MX profile selectors before fallback selectors when provided', () => { expect(dkimSelectorsForDnsProbe(['selector1', 'selector2'])).toEqual([ 'selector1', 'selector2', + 'google', + 'default', + 'k1', + 's1', + 'dkim', + 'mail', ]); }); @@ -74,6 +80,12 @@ describe('dkimSelectorsForDnsProbe', () => { expect(dkimSelectorsForDnsProbe(['selector1 ', ' Selector1', 'selector2'])).toEqual([ 'selector1', 'selector2', + 'google', + 'default', + 'k1', + 's1', + 'dkim', + 'mail', ]); }); diff --git a/lib/parse/dkim.ts b/lib/parse/dkim.ts index 102e9fd..08d5108 100644 --- a/lib/parse/dkim.ts +++ b/lib/parse/dkim.ts @@ -23,19 +23,17 @@ export function getDkimSelectors(): readonly string[] { } /** - * DKIM TXT names to probe under `{selector}._domainkey.