From 56fb867fe20b1bf23b20f7cd3fcb43e7ee1da79d Mon Sep 17 00:00:00 2001 From: TJBK Date: Thu, 14 May 2026 22:08:55 +0100 Subject: [PATCH 1/2] Fix DKIM selector probe order (#4) Probe null DKIM first, then provider/common selectors, then wildcard fallback. Fixes #4. --- entrypoints/popup/main.ts | 2 +- lib/checkDomain.dnsError.test.ts | 138 +++++++++++++++++++++++++++++++ lib/checkDomain.ts | 106 ++++++++++++------------ lib/checks/mailInfra.ts | 2 +- lib/parse/dkim.test.ts | 14 +++- lib/parse/dkim.ts | 22 +++-- lib/score/breakdown.ts | 2 +- lib/score/index.ts | 2 +- 8 files changed, 223 insertions(+), 65 deletions(-) diff --git a/entrypoints/popup/main.ts b/entrypoints/popup/main.ts index fe7a4b3..7a3bcd6 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'; 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.` after wildcard / apex checks. - * When MX identifies a provider with configured selectors, only those are queried (e.g. M365 → selector1, selector2). - * Otherwise {@link getDkimSelectors} order is used as a best-efforts sweep. + * DKIM TXT names to probe under `{selector}._domainkey.` after the null DKIM check. + * When MX identifies a provider with configured selectors, try those first, then continue + * with the default best-efforts sweep. */ export function dkimSelectorsForDnsProbe( mxProfileSelectors?: readonly string[] | undefined, ): readonly string[] { - if (!mxProfileSelectors?.length) { - return DKIM_SELECTORS; - } const seen = new Set(); const out: string[] = []; - for (const raw of mxProfileSelectors) { + + for (const raw of mxProfileSelectors ?? []) { const s = raw.trim(); if (!s) continue; const key = s.toLowerCase(); @@ -43,7 +41,15 @@ export function dkimSelectorsForDnsProbe( seen.add(key); out.push(s); } - return out.length > 0 ? out : DKIM_SELECTORS; + + for (const s of DKIM_SELECTORS) { + const key = s.toLowerCase(); + if (seen.has(key)) continue; + seen.add(key); + out.push(s); + } + + return out; } /** diff --git a/lib/score/breakdown.ts b/lib/score/breakdown.ts index 8c6ac3b..07d21b0 100644 --- a/lib/score/breakdown.ts +++ b/lib/score/breakdown.ts @@ -209,7 +209,7 @@ export function buildDkimBreakdown( lines.push({ status: 'info', text: queryHost - ? `Probed *._domainkey.${queryHost} first, then _domainkey.${queryHost} and selectors ${selectors}.` + ? `Probed _domainkey.${queryHost} for null DKIM first, then selectors ${selectors}, then *._domainkey.${queryHost}.` : `Probed selectors: ${selectors}.`, }); if (!d.raw) { diff --git a/lib/score/index.ts b/lib/score/index.ts index f75749b..a843bcf 100644 --- a/lib/score/index.ts +++ b/lib/score/index.ts @@ -166,7 +166,7 @@ export function scoreDkim( max: DKIM_MAX, status: 'missing', detail: - 'No DKIM DNS record at *._domainkey, _domainkey, or common selectors for this hostname.', + 'No DKIM DNS record at _domainkey, common selectors, or *._domainkey for this hostname.', }; } if ( From 3887a74104d3c10694ab5cfd4704b1ff3b9c6eb2 Mon Sep 17 00:00:00 2001 From: TJBK Date: Thu, 14 May 2026 22:30:02 +0100 Subject: [PATCH 2/2] Fix DMARC report button (#5) Show the report action for any non-pass DMARC result and use neutral report wording. Fixes #5. --- entrypoints/popup/main.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/entrypoints/popup/main.ts b/entrypoints/popup/main.ts index 7a3bcd6..4624dfb 100644 --- a/entrypoints/popup/main.ts +++ b/entrypoints/popup/main.ts @@ -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 `
@@ -169,7 +173,7 @@ function renderCastShameModal(result: CheckResult): string {