Centralize check for DNS errors

dodmi · Jan 19, 2025 · 9d4d391 · 9d4d391
1 parent 7b26b27
commit 9d4d391
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 29 deletions.
diff --git a/modules/dkim/dmarc.mjs.js b/modules/dkim/dmarc.mjs.js
@@ -4,7 +4,7 @@
  *
  * This module is NOT conform to DMARC.
  *
- * Copyright (c) 2014-2019;2021-2023 Philippe Lieser
+ * Copyright (c) 2014-2019;2021-2023;2025 Philippe Lieser
  *
  * This software is licensed under the terms of the MIT License.
  *
@@ -18,7 +18,7 @@
 /* eslint-disable no-magic-numbers */
 /* eslint-disable no-use-before-define */
 
-import { DKIM_Error, DKIM_TempError } from "../error.mjs.js";
+import { DKIM_Error } from "../error.mjs.js";
 import DNS from "../dns.mjs.js";
 import Logging from "../logging.mjs.js";
 import RfcParser from "../rfcParser.mjs.js";
@@ -125,7 +125,7 @@ export default class DMARC {
  * @param {queryDnsTxtCallback} queryDnsTxt
  * @returns {Promise<DMARCPolicy|null>}
  * @throws {DKIM_Error}
- * @throws {DKIM_TempError}
+ * @throws {import("../error.mjs.js").DKIM_TempError}
  */
 async function getDMARCPolicy(fromAddress, queryDnsTxt) {
 	let dmarcRecord;
@@ -208,22 +208,14 @@ async function getDMARCPolicy(fromAddress, queryDnsTxt) {
  * @param {queryDnsTxtCallback} queryDnsTxt
  * @returns {Promise<DMARCRecord|null>}
  * @throws {DKIM_Error}
- * @throws {DKIM_TempError}
+ * @throws {import("../error.mjs.js").DKIM_TempError}
  */
 async function getDMARCRecord(domain, queryDnsTxt) {
 	let dmarcRecord = null;
 
 	// get the DMARC Record
 	const result = await queryDnsTxt(`_dmarc.${domain}`);
-
-	// throw error on bogus result or DNS error
-	if (result.bogus) {
-		throw new DKIM_TempError("DKIM_DNSERROR_DNSSEC_BOGUS");
-	}
-	if (result.rcode !== DNS.RCODE.NoError && result.rcode !== DNS.RCODE.NXDomain) {
-		log.info("DNS query failed with result:", result);
-		throw new DKIM_TempError("DKIM_DNSERROR_SERVER_ERROR");
-	}
+	DNS.checkForErrors(result);
 
 	// try to parse DMARC Record if record was found in DNS Server
 	if (result.data !== null && result.data[0]) {

diff --git a/modules/dkim/keyStore.mjs.js b/modules/dkim/keyStore.mjs.js
@@ -2,7 +2,7 @@
  * Abstract DKIM key store for key retrieval.
  * Will get the key either from DNS or an internal cache.
  *
- * Copyright (c) 2013-2018;2021-2024 Philippe Lieser
+ * Copyright (c) 2013-2018;2021-2025 Philippe Lieser
  *
  * This software is licensed under the terms of the MIT License.
  *
@@ -14,8 +14,8 @@
 ///<reference path="../../RuntimeMessage.d.ts" />
 ///<reference path="../dns.d.ts" />
 
-import { DKIM_SigError, DKIM_TempError } from "../error.mjs.js";
 import { Deferred, dateToString } from "../utils.mjs.js";
+import { DKIM_SigError } from "../error.mjs.js";
 import DNS from "../dns.mjs.js";
 import Logging from "../logging.mjs.js";
 import prefs from "../preferences.mjs.js";
@@ -319,7 +319,7 @@ export default class KeyStore {
 	 * @param {string} selector
 	 * @returns {Promise<DkimKeyResult>}
 	 * @throws {DKIM_SigError}
-	 * @throws {DKIM_TempError}
+	 * @throws {import("../error.mjs.js").DKIM_TempError}
 	 */
 	async fetchKey(sdid, selector) {
 		switch (prefs["key.storing"]) {
@@ -364,19 +364,13 @@ export default class KeyStore {
 	 * @param {string} selector
 	 * @returns {Promise<DkimKeyResult>}
 	 * @throws {DKIM_SigError}
-	 * @throws {DKIM_TempError}
+	 * @throws {import("../error.mjs.js").DKIM_TempError}
 	 */
 	async #getKeyFromDNS(sdid, selector) {
 		const dnsRes = await this._queryDnsTxt(`${selector}._domainkey.${sdid}`);
 		log.debug("dns result", dnsRes);
+		DNS.checkForErrors(dnsRes);
 
-		if (dnsRes.bogus) {
-			throw new DKIM_TempError("DKIM_DNSERROR_DNSSEC_BOGUS");
-		}
-		if (dnsRes.rcode !== DNS.RCODE.NoError && dnsRes.rcode !== DNS.RCODE.NXDomain) {
-			log.info("DNS query failed with result:", dnsRes);
-			throw new DKIM_TempError("DKIM_DNSERROR_SERVER_ERROR");
-		}
 		if (dnsRes.data === null || !dnsRes.data[0]) {
 			throw new DKIM_SigError("DKIM_SIGERROR_NOKEY");
 		}

diff --git a/modules/dns.mjs.js b/modules/dns.mjs.js
@@ -3,7 +3,7 @@
  *  - JSDNS
  *  - libunbound
  *
- * Copyright (c) 2020-2023 Philippe Lieser
+ * Copyright (c) 2020-2023;2025 Philippe Lieser
  *
  * This software is licensed under the terms of the MIT License.
  *
@@ -181,4 +181,21 @@ export default class DNS {
 				throw new Error("invalid resolver preference");
 		}
 	}
+
+	/**
+	 * Throws error on bogus result or if result contains a DNS error.
+	 *
+	 * @param {DnsTxtResult} dnsResult
+	 * @throws {DKIM_SigError}
+	 * @throws {DKIM_TempError}
+	 */
+	static checkForErrors(dnsResult) {
+		if (dnsResult.bogus) {
+			throw new DKIM_TempError("DKIM_DNSERROR_DNSSEC_BOGUS");
+		}
+		if (dnsResult.rcode !== DNS.RCODE.NoError && dnsResult.rcode !== DNS.RCODE.NXDomain) {
+			log.info("DNS query failed with result:", dnsResult);
+			throw new DKIM_TempError("DKIM_DNSERROR_SERVER_ERROR");
+		}
+	}
 }
diff --git a/test/helpers/chaiUtils.mjs.js b/test/helpers/chaiUtils.mjs.js
@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2020-2022 Philippe Lieser
+ * Copyright (c) 2020-2022;2025 Philippe Lieser
  *
  * This software is licensed under the terms of the MIT License.
  *
@@ -14,7 +14,7 @@
 const expect = globalThis.expect;
 export default expect;
 
-import { DKIM_SigError } from "../../modules/error.mjs.js";
+import { DKIM_SigError, DKIM_TempError } from "../../modules/error.mjs.js";
 import Logging from "../../modules/logging.mjs.js";
 
 // disable logging in tests
@@ -59,6 +59,45 @@ export function expectAsyncDkimSigError(promise, errorType) {
 	});
 }
 
+/**
+ * Assert that the given promise is rejected with a certain type of DKIM_TempError.
+ *
+ * @param {Promise<any>} promise
+ * @param {string} errorType - expected error type
+ * @returns {Promise<void>}
+ */
+export function expectAsyncDkimTempError(promise, errorType) {
+	return new Promise((resolve, reject) => {
+		// TODO: Use Chai Plugin Utilities instead of expect.fail
+		promise.then(
+			value => {
+				try {
+					expect.fail(`${value}`, errorType, "expected a DKIM_TempError to be thrown, got a value instead");
+				} catch (e) {
+					// @ts-expect-error
+					e.showDiff = true;
+					reject(e);
+				}
+			},
+			reason => {
+				try {
+					if (reason instanceof DKIM_TempError) {
+						if (reason.errorType !== errorType) {
+							expect.fail(`${reason}`, errorType, "expected a different error type of DKIM_TempError");
+						}
+						resolve();
+					}
+					expect.fail(`${reason}`, errorType, "expected a DKIM_TempError to be thrown, got a different Error instead");
+				} catch (e) {
+					// @ts-expect-error
+					e.showDiff = true;
+					reject(e);
+				}
+			}
+		);
+	});
+}
+
 /**
  * Assert that the given promise is rejected.
  *

diff --git a/test/unittest/keyStoreSpec.mjs.js b/test/unittest/keyStoreSpec.mjs.js
@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2021;2023 Philippe Lieser
+ * Copyright (c) 2021;2023;2025 Philippe Lieser
  *
  * This software is licensed under the terms of the MIT License.
  *
@@ -11,7 +11,7 @@
 
 import "../helpers/initWebExtensions.mjs.js";
 import KeyStore, { KeyDb } from "../../modules/dkim/keyStore.mjs.js";
-import expect, { expectAsyncDkimSigError } from "../helpers/chaiUtils.mjs.js";
+import expect, { expectAsyncDkimSigError, expectAsyncDkimTempError } from "../helpers/chaiUtils.mjs.js";
 import DNS from "../../modules/dns.mjs.js";
 import prefs from "../../modules/preferences.mjs.js";
 import sinon from "../helpers/sinonUtils.mjs.js";
@@ -167,5 +167,41 @@ describe("Key store [unittest]", function () {
 
 			expect(fakeQueryDnsTxt.calledThrice).is.true;
 		});
+
+		it("Bogus DNS result", async function () {
+			const keyStore = new KeyStore(sinon.fake.resolves({
+				data: null,
+				rcode: DNS.RCODE.NoError,
+				secure: true,
+				bogus: true,
+			}));
+
+			const dnsResult = keyStore.fetchKey("example.com", "selector1");
+			await expectAsyncDkimTempError(dnsResult, "DKIM_DNSERROR_DNSSEC_BOGUS");
+		});
+
+		it("DNS result with error code", async function () {
+			const keyStore = new KeyStore(sinon.fake.resolves({
+				data: null,
+				rcode: DNS.RCODE.ServFail,
+				secure: false,
+				bogus: false,
+			}));
+
+			const dnsResult = keyStore.fetchKey("example.com", "selector1");
+			await expectAsyncDkimTempError(dnsResult, "DKIM_DNSERROR_SERVER_ERROR");
+		});
+
+		it("DNS result Non-Existent Domain", async function () {
+			const keyStore = new KeyStore(sinon.fake.resolves({
+				data: null,
+				rcode: DNS.RCODE.NXDomain,
+				secure: false,
+				bogus: false,
+			}));
+
+			const dnsResult = keyStore.fetchKey("example.com", "selector1");
+			await expectAsyncDkimSigError(dnsResult, "DKIM_SIGERROR_NOKEY");
+		});
 	});
 });