From 884097a5d1080c1cb5b17b7047433fdc0dbcd567 Mon Sep 17 00:00:00 2001 From: Murtaza Aliakbar Date: Thu, 31 Jul 2025 02:54:47 +0530 Subject: [PATCH 1/5] all: catch err on dns decode --- src/plugins/command-control/cc.js | 8 +++++++- src/plugins/dns-op/cache.js | 18 +++++++++++++++--- src/plugins/dns-op/resolver.js | 11 +++++++++-- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/plugins/command-control/cc.js b/src/plugins/command-control/cc.js index 801a2223ad..f20dddff6a 100644 --- a/src/plugins/command-control/cc.js +++ b/src/plugins/command-control/cc.js @@ -323,7 +323,13 @@ async function domainNameToList( querypacket ); const ans = await res.arrayBuffer(); - const anspacket = dnsutil.decode(ans); + let anspacket; + try { + anspacket = dnsutil.decode(ans); + } catch (e) { + log.w(rxid, "malformed dns response in command-control:", e.message); + return r; // empty response + } const ansdomains = dnsutil.extractDomains(anspacket); for (const d of ansdomains) { diff --git a/src/plugins/dns-op/cache.js b/src/plugins/dns-op/cache.js index a9faf37d3e..d19c85eaf1 100644 --- a/src/plugins/dns-op/cache.js +++ b/src/plugins/dns-op/cache.js @@ -7,12 +7,12 @@ */ import { LfuCache } from "@serverless-dns/lfu-cache"; -import { CacheApi } from "./cache-api.js"; import * as bufutil from "../../commons/bufutil.js"; import * as dnsutil from "../../commons/dnsutil.js"; import * as envutil from "../../commons/envutil.js"; import * as util from "../../commons/util.js"; import * as cacheutil from "../cache-util.js"; +import { CacheApi } from "./cache-api.js"; export class DnsCache { constructor(size) { @@ -136,7 +136,13 @@ export class DnsCache { if (util.emptyObj(res)) return null; const b = res.dnsBuffer; - const p = dnsutil.decode(b); + let p; + try { + p = dnsutil.decode(b); + } catch (e) { + this.log.w("malformed dns packet in local cache:", e.message); + return null; // return null to indicate cache miss + } const m = res.metadata; const cr = cacheutil.makeCacheValue(p, b, m); @@ -173,7 +179,13 @@ export class DnsCache { // 'b' shouldn't be null; but a dns question or a dns answer const b = await response.arrayBuffer(); // when 'b' is less than dns-packet header-size, decode errs out - const p = dnsutil.decode(b); + let p; + try { + p = dnsutil.decode(b); + } catch (e) { + this.log.w("malformed dns packet in http cache:", e.message); + return null; // return null to indicate cache miss + } // though 'm' is never empty const m = metadata; diff --git a/src/plugins/dns-op/resolver.js b/src/plugins/dns-op/resolver.js index f5ba2a72af..f90b606094 100644 --- a/src/plugins/dns-op/resolver.js +++ b/src/plugins/dns-op/resolver.js @@ -265,7 +265,14 @@ export default class DNSResolver { const ans = await res.arrayBuffer(); - const r = this.makeRdnsResponse(rxid, ans, blf, stamps); + let r; + try { + r = this.makeRdnsResponse(rxid, ans, blf, stamps); + } catch (e) { + this.log.w(rxid, "upstream returned malformed dns response:", e.message); + const pkt = dnsutil.servfail(decodedpacket.id, decodedpacket.questions); + r = pres.dnsResponse(dnsutil.decode(pkt), pkt, stamps); + } // blockAnswer is a no-op if the ans is already quad0 // check outgoing cached dns-packet against blocklists @@ -292,7 +299,7 @@ export default class DNSResolver { makeRdnsResponse(rxid, raw, blf, stamps = null) { if (!raw) throw new Error(rxid + " mk-res no upstream result"); - const dnsPacket = dnsutil.decode(raw); + const dnsPacket = dnsutil.decode(raw); // may throw if malformed // stamps are empty for domains that are not in any blocklist // but there's no way to know if that was indeed the case as // stamps are sent here by cache-resolver, which may or may not From 0c5da67ee36e761d486d9bfc21cac529cb5b8344 Mon Sep 17 00:00:00 2001 From: Murtaza Aliakbar Date: Thu, 31 Jul 2025 04:12:59 +0530 Subject: [PATCH 2/5] all: handle err on decoding b32 flag --- src/commons/bufutil.js | 20 ++++++++++++++------ src/plugins/rdns-util.js | 22 ++++++++++++++++------ src/plugins/users/user-op.js | 8 ++++---- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/commons/bufutil.js b/src/commons/bufutil.js index e7de6eb069..7d37327a2e 100644 --- a/src/commons/bufutil.js +++ b/src/commons/bufutil.js @@ -98,13 +98,21 @@ export function base64ToBytes(b64uri) { } export function decodeFromBinary(b, u8) { - // if b is a u8 array, simply u16 it - if (u8) return new Uint16Array(raw(b)); - // if b is a binary-string, convert it to u8 - const bytes = binaryStringToBytes(b); - // ...and then to u16 - return new Uint16Array(raw(bytes)); + const conv = u8 ? b : binaryStringToBytes(b); + + // Ensure the byte array has even length for Uint16Array + // Uint16Array requires byte length to be a multiple of 2 + const ab = raw(conv); + if (ab.byteLength % 2 !== 0) { + // Pad with an extra zero byte if odd length + const padded = new Uint8Array(ab.byteLength + 1); + padded.set(new Uint8Array(ab)); + padded[ab.byteLength] = 0; + return new Uint16Array(padded.buffer); + } + + return new Uint16Array(ab); } export function decodeFromBinaryArray(b) { diff --git a/src/plugins/rdns-util.js b/src/plugins/rdns-util.js index 2c80bfe48d..48b1456d27 100644 --- a/src/plugins/rdns-util.js +++ b/src/plugins/rdns-util.js @@ -423,12 +423,22 @@ export function base64ToUintV1(b64Flag) { /** * @param {string} b64Flag - * @returns {Uint16Array} + * @returns {Uint16Array?} */ export function base32ToUintV1(flag) { - // TODO: check for empty flag - const b32 = decodeURI(flag); - return bufutil.decodeFromBinaryArray(rbase32(b32)); + try { + if (util.emptyString(flag)) return null; + const b32 = decodeURI(flag); + if (util.emptyString(b32)) return null; + + const decoded = rbase32(b32); + if (util.emptyString(decoded)) return null; + + return bufutil.decodeFromBinaryArray(decoded); + } catch (e) { + log.w("Rdns:base32ToUintV1", "error decoding b32 flag", e); + } + return null; } /** @@ -475,7 +485,7 @@ export function stampVersion(s) { // TODO: The logic to parse stamps must be kept in sync with: // github.com/celzero/website-dns/blob/8e6056bb/src/js/flag.js#L260-L425 /** - * + * May return empty BlockstampInfo if flag is invalid or empty. * @param {string} flag * @returns {pres.BlockstampInfo} */ @@ -499,7 +509,7 @@ export function unstamp(flag) { } else if (v === "1") { const convertor = isFlagB32 ? base32ToUintV1 : base64ToUintV1; const f = s[1]; - r.userBlocklistFlagUint = convertor(f) || null; + r.userBlocklistFlagUint = convertor(f) || null; // convertor may return null } else { log.w("Rdns:unstamp", "unknown blocklist stamp version in " + s); } diff --git a/src/plugins/users/user-op.js b/src/plugins/users/user-op.js index 3247072af0..3bf907b0ba 100644 --- a/src/plugins/users/user-op.js +++ b/src/plugins/users/user-op.js @@ -5,13 +5,13 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { UserCache } from "./user-cache.js"; -import * as pres from "../plugin-response.js"; -import * as util from "../../commons/util.js"; +import * as dnsutil from "../../commons/dnsutil.js"; import * as envutil from "../../commons/envutil.js"; +import * as util from "../../commons/util.js"; +import * as pres from "../plugin-response.js"; import * as rdnsutil from "../rdns-util.js"; import * as token from "./auth-token.js"; -import * as dnsutil from "../../commons/dnsutil.js"; +import { UserCache } from "./user-cache.js"; // TODO: determine an approp cache-size const cacheSize = 20000; From fecb2071590d3267b9f2f30ab7f6a33179f15422 Mon Sep 17 00:00:00 2001 From: Murtaza Aliakbar Date: Thu, 31 Jul 2025 19:45:13 +0530 Subject: [PATCH 3/5] io: never throw from dnsExceptionResponse Called from plugin entrypoint when plugins fail to generate a response and so it must always set a http response to be sent. --- src/core/doh.js | 1 + src/core/io-state.js | 64 +++++++++++++++++++++++++++++++------------- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/src/core/doh.js b/src/core/doh.js index f3ab3481d7..77293270c2 100644 --- a/src/core/doh.js +++ b/src/core/doh.js @@ -62,6 +62,7 @@ function optionsRequest(request) { } /** + * Must not throw! * @param {IOState} io * @param {Error} err */ diff --git a/src/core/io-state.js b/src/core/io-state.js index bd33e9f7a2..7ff728793e 100644 --- a/src/core/io-state.js +++ b/src/core/io-state.js @@ -70,7 +70,9 @@ export default class IOState { initDecodedDnsPacketIfNeeded() { if (!this.decodedDnsPacket) { this.decodedDnsPacket = this.emptyDecodedDnsPacket(); + return true; } + return false; } dnsExceptionResponse(res) { @@ -87,23 +89,47 @@ export default class IOState { this.exceptionFrom = res.exceptionFrom || "no-origin"; } - const qid = this.decodedDnsPacket.id; - const questions = this.decodedDnsPacket.questions; - const servfail = dnsutil.servfail(qid, questions); - const ex = { - exceptionFrom: this.exceptionFrom, - exceptionStack: this.exceptionStack, - }; - this.decodedDnsPacket = dnsutil.decode(servfail); - - this.logDnsPkt(); - this.httpResponse = new Response(servfail, { - headers: util.concatHeaders( - this.headers(servfail), - this.debugHeaders(JSON.stringify(ex)) - ), - status: servfail ? 200 : 408, // rfc8484 section-4.2.1 - }); + try { + const qid = this.decodedDnsPacket.id; // may be null + const questions = this.decodedDnsPacket.questions; // may be null + const servfail = dnsutil.servfail(qid, questions); // may be empty + const hasServfail = !bufutil.emptyBuf(servfail); + const ex = { + exceptionFrom: this.exceptionFrom, + exceptionStack: this.exceptionStack, + }; + + if (hasServfail) { + // TODO: try-catch as decode may throw? + this.decodedDnsPacket = dnsutil.decode(servfail); + } + + this.logDnsPkt(); + this.httpResponse = new Response(servfail, { + headers: util.concatHeaders( + this.headers(servfail), + this.debugHeaders(JSON.stringify(ex)) + ), + status: hasServfail ? 200 : 408, // rfc8484 section-4.2.1 + }); + } catch (e) { + const pktjson = JSON.stringify(this.decodedDnsPacket || {}); + this.log.e("dnsExceptionResponse", pktjson, e.stack); + if ( + this.exceptionStack === "no-res" || + this.exceptionStack === "no-stack" + ) { + this.exceptionStack = e.stack; + this.exceptionFrom = "IOState:errorResponse"; + } + this.httpResponse = new Response(null, { + headers: util.concatHeaders( + this.headers(), + this.debugHeaders(JSON.stringify(this.exceptionStack)) + ), + status: 503, + }); + } } hResponse(r) { @@ -158,14 +184,14 @@ export default class IOState { } dnsBlockResponse(blockflag) { - this.initDecodedDnsPacketIfNeeded(); + this.initDecodedDnsPacketIfNeeded(); // initializes to empty obj this.stopProcessing = true; this.isDnsBlock = true; this.flag = blockflag; try { this.assignBlockResponse(); - const b = dnsutil.encode(this.decodedDnsPacket); + const b = dnsutil.encode(this.decodedDnsPacket); // may throw if empty or malformed this.httpResponse = new Response(b, { headers: this.headers(b), }); From d08d1a59e588658c7dcac9f6f289a203e56149f9 Mon Sep 17 00:00:00 2001 From: Murtaza Aliakbar Date: Fri, 1 Aug 2025 06:16:55 +0530 Subject: [PATCH 4/5] hmac: add salt --- src/commons/crypto.js | 11 +++++++++-- src/core/plugin.js | 11 +++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/commons/crypto.js b/src/commons/crypto.js index 2cbe461747..b79c29acd0 100644 --- a/src/commons/crypto.js +++ b/src/commons/crypto.js @@ -14,6 +14,11 @@ import { emptyString } from "./util.js"; const tktsz = 48; const hkdfalgkeysz = 32; // sha256 +// hex: 9f34ba3c3c9097fef97e97effbb4bda4b9afa17dbb9b02f091a25d119ac91c5f +const fixedsalt = new Uint8Array([ + 159, 52, 186, 60, 60, 144, 151, 254, 249, 126, 151, 239, 251, 180, 189, 164, + 185, 175, 161, 125, 187, 155, 2, 240, 145, 162, 93, 17, 154, 201, 28, 95, +]); export async function tkt48(seed, ctx) { if (!emptyBuf(seed) && !emptyString(ctx)) { @@ -29,8 +34,10 @@ export async function tkt48(seed, ctx) { return t; } -// salt for hkdf can be zero: stackoverflow.com/a/64403302 -export async function gen(secret, info, salt = new Uint8Array()) { +// salt for hkdf can be zero if secret is pseudorandom +// but a fixed salt is needed for high-entropy +// but non uniform keys like outputs of DHKE +export async function gen(secret, info, salt = fixedsalt) { if (emptyBuf(secret) || emptyBuf(info)) { throw new Error("empty secret/info"); } diff --git a/src/core/plugin.js b/src/core/plugin.js index a64d05dd8e..f193da3701 100644 --- a/src/core/plugin.js +++ b/src/core/plugin.js @@ -6,18 +6,17 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { services } from "./svc.js"; import * as bufutil from "../commons/bufutil.js"; import * as dnsutil from "../commons/dnsutil.js"; import * as envutil from "../commons/envutil.js"; -import * as rdnsutil from "../plugins/rdns-util.js"; import * as util from "../commons/util.js"; -import IOState from "./io-state.js"; import { RResp } from "../plugins/plugin-response.js"; +import * as rdnsutil from "../plugins/rdns-util.js"; +import IOState from "./io-state.js"; +import { services } from "./svc.js"; export default class RethinkPlugin { /** - * * @param {{request: Request, waitUntil: Function, respondWith: Function}} event */ constructor(event) { @@ -161,7 +160,7 @@ export default class RethinkPlugin { /** * @param {RResp} response - * @param {IOState} io + * @param {Promise} io */ async commandControlCallback(response, io) { const rxid = this.ctx.get("rxid"); @@ -178,7 +177,7 @@ export default class RethinkPlugin { * Adds "userBlocklistInfo", "userBlocklistInfo", and "dnsResolverUrl" * to RethinkPlugin ctx. * @param {RResp} response - * @param {IOState} io + * @param {Promise} io */ async userOpCallback(response, io) { const rxid = this.ctx.get("rxid"); From 1705cef94fa70f420c88c4daac824f2581e73f62 Mon Sep 17 00:00:00 2001 From: Murtaza Aliakbar Date: Fri, 1 Aug 2025 06:17:26 +0530 Subject: [PATCH 5/5] node: missing await on handleTCPData --- src/server-node.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server-node.js b/src/server-node.js index ad5fa6af79..5fca8f0780 100644 --- a/src/server-node.js +++ b/src/server-node.js @@ -978,7 +978,7 @@ async function handleTCPData(socket, chunk, sb, host, flag) { // if there is any out of band data, handle it if (!bufutil.emptyBuf(oob)) { log.d(`tcp: pipelined, handle oob: ${oob.byteLength}`); - n += handleTCPData(socket, oob, sb, host, flag); + n += await handleTCPData(socket, oob, sb, host, flag); } } // continue reading from socket return n;