fix: query DNS and archive in parallel instead of fallback

This commit is contained in:
Yogesh Shahi
2025-10-23 02:54:05 +05:30
parent 39b187ba74
commit 544794218a
2 changed files with 58 additions and 26 deletions

View File

@@ -1,6 +1,6 @@
import { pki } from 'node-forge'; import { pki } from 'node-forge';
import { DkimVerifier } from '../lib/mailauth/dkim-verifier'; import { DkimVerifier } from '../lib/mailauth/dkim-verifier';
import { writeToStream } from '../lib/mailauth/tools'; import { writeToStream, CustomError } from '../lib/mailauth/tools';
import sanitizers from './sanitizers'; import sanitizers from './sanitizers';
import { resolveDNSHTTP } from './dns-over-http'; import { resolveDNSHTTP } from './dns-over-http';
import { resolveDNSFromZKEmailArchive } from './dns-archive'; import { resolveDNSFromZKEmailArchive } from './dns-archive';
@@ -103,17 +103,31 @@ async function tryVerifyDKIM(
skipBodyHash = false, skipBodyHash = false,
) { ) {
const resolver = async (name: string, type: string) => { const resolver = async (name: string, type: string) => {
let dnsKeys: string[] = [];
let archiveKeys: string[] = [];
try { try {
const result = await resolveDNSHTTP(name, type); dnsKeys = await resolveDNSHTTP(name, type);
return result;
} catch (e) { } catch (e) {
if (fallbackToZKEmailDNSArchive) { // DNS failed, that's okay - will try archive
console.log('DNS over HTTP failed, falling back to ZK Email Archive');
const result = await resolveDNSFromZKEmailArchive(name, type);
return result;
}
throw e;
} }
if (fallbackToZKEmailDNSArchive) {
try {
archiveKeys = await resolveDNSFromZKEmailArchive(name, type);
} catch (e) {
// Archive failed, that's okay - will use DNS keys if available
}
}
// Combine and deduplicate keys
const allKeys = [...new Set([...dnsKeys, ...archiveKeys])];
if (allKeys.length === 0) {
throw new CustomError('No DNS records found from any source', 'ENODATA');
}
return allKeys;
}; };
const dkimVerifier = new DkimVerifier({ const dkimVerifier = new DkimVerifier({

View File

@@ -104,11 +104,9 @@ it('should fallback to ZK Email Archive if DNS over HTTP fails', async () => {
.spyOn(dnsOverHttp, 'resolveDNSHTTP') .spyOn(dnsOverHttp, 'resolveDNSHTTP')
.mockRejectedValue(new Error('Failed due to mock')); .mockRejectedValue(new Error('Failed due to mock'));
const consoleSpy = jest.spyOn(console, 'log'); // Should succeed using Archive keys even though DNS failed
await verifyDKIMSignature(email, 'icloud.com', true, true); const result = await verifyDKIMSignature(email, 'icloud.com', true, true);
expect(result.signingDomain).toBe('icloud.com');
// Check if the error was logged to ensure fallback to ZK Email Archive happened
expect(consoleSpy).toHaveBeenCalledWith('DNS over HTTP failed, falling back to ZK Email Archive');
mockResolveDNSHTTP.mockRestore(); mockResolveDNSHTTP.mockRestore();
}); });
@@ -116,19 +114,15 @@ it('should fallback to ZK Email Archive if DNS over HTTP fails', async () => {
it('should try multiple DKIM keys when archive returns multiple records for same selector', async () => { it('should try multiple DKIM keys when archive returns multiple records for same selector', async () => {
const email = fs.readFileSync(path.join(__dirname, 'test-data/multi-dkim-sig.eml')); const email = fs.readFileSync(path.join(__dirname, 'test-data/multi-dkim-sig.eml'));
// Mock resolveDNSHTTP to throw an error to force fallback to archive // Mock resolveDNSHTTP to throw an error to force using archive keys
const mockResolveDNSHTTP = jest const mockResolveDNSHTTP = jest
.spyOn(dnsOverHttp, 'resolveDNSHTTP') .spyOn(dnsOverHttp, 'resolveDNSHTTP')
.mockRejectedValue(new Error('Failed due to mock')); .mockRejectedValue(new Error('Failed due to mock'));
const consoleSpy = jest.spyOn(console, 'log');
// This email requires trying multiple keys from the archive before finding the correct one // This email requires trying multiple keys from the archive before finding the correct one
// Archive returns 7 keys for selector 'hs2', and the 6th one is the correct key // Archive returns 7 keys for selector 'hs2', and the 6th one is the correct key
const result = await verifyDKIMSignature(email, 'bf01.eu1.hubspotstarter.net', false, true); const result = await verifyDKIMSignature(email, 'bf01.eu1.hubspotstarter.net', false, true);
// Verify fallback happened
expect(consoleSpy).toHaveBeenCalledWith('DNS over HTTP failed, falling back to ZK Email Archive');
// Verify signature passed with one of the multiple keys // Verify signature passed with one of the multiple keys
expect(result.signingDomain).toBe('bf01.eu1.hubspotstarter.net'); expect(result.signingDomain).toBe('bf01.eu1.hubspotstarter.net');
expect(result.selector).toBe('hs2'); expect(result.selector).toBe('hs2');
@@ -136,6 +130,34 @@ it('should try multiple DKIM keys when archive returns multiple records for same
mockResolveDNSHTTP.mockRestore(); mockResolveDNSHTTP.mockRestore();
}, 20000); }, 20000);
it('should query both DNS and Archive in parallel and succeed with historical key', async () => {
const email = fs.readFileSync(path.join(__dirname, 'test-data/multi-dkim-sig.eml'));
// NO MOCKING - This test verifies the fix with real API calls
// The email was signed with an old rotated key (key #6 from May 2025)
// DNS will return the CURRENT key (Oct 2025) which is WRONG for this email
// Archive will return 7 historical keys including the correct one
// With the fix, both sources are queried and keys are combined
// Verification should succeed with the correct historical key from archive
// Spy on both functions to verify they are both called
const dnsHttpSpy = jest.spyOn(dnsOverHttp, 'resolveDNSHTTP');
const archiveSpy = jest.spyOn(dnsArchive, 'resolveDNSFromZKEmailArchive');
const result = await verifyDKIMSignature(email, 'bf01.eu1.hubspotstarter.net', false, true);
// Verify signature passed
expect(result.signingDomain).toBe('bf01.eu1.hubspotstarter.net');
expect(result.selector).toBe('hs2');
// Verify both DNS and Archive were called (not just archive as fallback)
expect(dnsHttpSpy).toHaveBeenCalled();
expect(archiveSpy).toHaveBeenCalled();
dnsHttpSpy.mockRestore();
archiveSpy.mockRestore();
}, 20000);
it('should fail on DNS over HTTP failure if fallback is not enabled', async () => { it('should fail on DNS over HTTP failure if fallback is not enabled', async () => {
const email = fs.readFileSync(path.join(__dirname, 'test-data/email-good.eml')); const email = fs.readFileSync(path.join(__dirname, 'test-data/email-good.eml'));
@@ -148,9 +170,7 @@ it('should fail on DNS over HTTP failure if fallback is not enabled', async () =
try { try {
await verifyDKIMSignature(email, 'icloud.com', true, false); await verifyDKIMSignature(email, 'icloud.com', true, false);
} catch (e) { } catch (e) {
expect(e.message).toBe( expect(e.message).toBe('DKIM signature verification failed for domain icloud.com. Reason: no key');
'DKIM signature verification failed for domain icloud.com. Reason: DNS failure: Failed due to mock',
);
} }
mockResolveDNSHTTP.mockRestore(); mockResolveDNSHTTP.mockRestore();
}); });
@@ -168,11 +188,9 @@ it('should fail if both DNS over HTTP and ZK Email Archive fail', async () => {
expect.assertions(1); expect.assertions(1);
try { try {
await verifyDKIMSignature(email, 'icloud.com', true, false); await verifyDKIMSignature(email, 'icloud.com', true, true);
} catch (e) { } catch (e) {
expect(e.message).toBe( expect(e.message).toBe('DKIM signature verification failed for domain icloud.com. Reason: no key');
'DKIM signature verification failed for domain icloud.com. Reason: DNS failure: Failed due to mock',
);
} }
mockResolveDNSHTTP.mockRestore(); mockResolveDNSHTTP.mockRestore();