Skip to content

Commit

Permalink
Upstream to two doh resolvers at once on cf
Browse files Browse the repository at this point in the history
Query two doh resolvers at once and return answer from which ever is fastest,
to ensure that a slower responding resolver does not aggravate latencies. This
is done only on cloudflare workers since egress is free. On other platforms,
there's just the one upstream resolver is queried to.

Other clean-up include removing an unused env var, isAggCacheReq; fixing
util.js:emptyArray impl; adding a fn defferedOp to util.js:timedSafeAsyncOp;
returning a properly constructed servfail even on timeouts and exceptions
in doh.js (and not just responding with 4xx or 5xx); patching up http2 error
handling semantics for nodejs runtime.
  • Loading branch information
ignoramous committed Jan 19, 2022
1 parent 65a3509 commit 33e9107
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 105 deletions.
2 changes: 1 addition & 1 deletion src/commons/dnsutil.js
Expand Up @@ -22,7 +22,7 @@ export const maxDNSPacketSize = 4096;
const _dnsCloudflareSec = "1.1.1.2";
const _dnsCacheSize = 10000;

const _minRequestTimeout = 5000; // 5s
const _minRequestTimeout = 4000; // 4s
const _maxRequestTimeout = 30000; // 30s

export function dnsIpv4() {
Expand Down
6 changes: 6 additions & 0 deletions src/commons/envutil.js
Expand Up @@ -5,6 +5,7 @@
* 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/.
*/

// musn't import /depend on anything.
export function onFly() {
return env && env.cloudPlatform === "fly";
Expand Down Expand Up @@ -59,3 +60,8 @@ export function dohResolver() {
if (!env) return null;
return env.dnsResolverUrl;
}

export function secondaryDohResolver() {
if (!env) return null;
return env.secondaryDohResolver;
}
19 changes: 15 additions & 4 deletions src/commons/util.js
Expand Up @@ -151,17 +151,27 @@ export function timedOp(op, ms, cleanup = () => {}) {
});
}

export function timedSafeAsyncOp(promisedOp, ms, defaultOut) {
export function timedSafeAsyncOp(promisedOp, ms, defaultOp) {
// aggregating promises is a valid use-case for the otherwise
// "deferred promise anti-pattern". That is, using promise
// constructs (async, await, catch, then etc) within a
// "new Promise()" is an anti-pattern and hard to get right:
// stackoverflow.com/a/23803744 and stackoverflow.com/a/25569299
return new Promise((resolve, _) => {
let timedout = false;

const defferedOp = () => {
defaultOp()
.then((v) => {
resolve(v);
})
.catch((e) => {
reject(e);
});
};
const tid = timeout(ms, () => {
timedout = true;
resolve(defaultOut);
defferedOp();
});

promisedOp()
Expand All @@ -172,7 +182,8 @@ export function timedSafeAsyncOp(promisedOp, ms, defaultOut) {
}
})
.catch((ignored) => {
if (!timedout) resolve(defaultOut);
if (!timedout) defferedOp();
// else: handled by timeout
});
});
}
Expand Down Expand Up @@ -279,7 +290,7 @@ export function emptyArray(a) {
// obj v arr: stackoverflow.com/a/2462810
if (typeof a !== "object") return false;
// len(a) === 0 is empty
return !!a.length && a.length <= 0;
return a.length <= 0;
}

export function concatObj(...args) {
Expand Down
5 changes: 2 additions & 3 deletions src/core/currentRequest.js
Expand Up @@ -64,7 +64,7 @@ export default class CurrentRequest {
this.headers(),
this.additionalHeader(JSON.stringify(ex))
),
status: servfail ? 200 : 500, // rfc8484 section-4.2.1
status: servfail ? 200 : 408, // rfc8484 section-4.2.1
});
}

Expand All @@ -85,8 +85,7 @@ export default class CurrentRequest {
}

this.stopProcessing = true;
// TODO: decode arrayBuffer if dnsPacket missing?
this.decodedDnsPacket = dnsPacket || this.decodedDnsPacket;
this.decodedDnsPacket = dnsPacket || dnsutil.decode(arrayBuffer);
this.httpResponse = new Response(arrayBuffer, { headers: this.headers() });
}

Expand Down
52 changes: 17 additions & 35 deletions src/core/doh.js
Expand Up @@ -12,11 +12,7 @@ import * as util from "../commons/util.js";
import * as dnsutil from "../commons/dnsutil.js";

export function handleRequest(event) {
return util.timedSafeAsyncOp(
async () => proxyRequest(event),
dnsutil.requestTimeout(),
servfail()
);
return proxyRequest(event);
}

async function proxyRequest(event) {
Expand All @@ -26,44 +22,30 @@ async function proxyRequest(event) {

try {
const plugin = new RethinkPlugin(event);
await plugin.executePlugin(cr);

// TODO: cors-headers are also set in server-node.js
// centralize setting these in just one place, if possible
const ua = event.request.headers.get("User-Agent");
if (util.fromBrowser(ua)) currentRequest.setCorsHeadersIfNeeded();

return cr.httpResponse;
await util.timedSafeAsyncOp(
/* op*/ async () => plugin.executePlugin(cr),
/* waitMs*/ dnsutil.requestTimeout(),
/* onTimeout*/ async () => errorResponse(cr)
);
} catch (err) {
log.e("doh", "proxy-request error", err);
return errorOrServfail(event.request, err, cr);
errorResponse(cr, err);
}

// TODO: cors-headers are also set in server-node.js
// centralize setting these in just one place, if possible
const ua = event.request.headers.get("User-Agent");
if (util.fromBrowser(ua)) cr.setCorsHeadersIfNeeded();

return cr.httpResponse;
}

function optionsRequest(request) {
return request.method === "OPTIONS";
}

function errorOrServfail(request, err, currentRequest) {
const ua = request.headers.get("User-Agent");
if (!util.fromBrowser(ua)) return servfail(currentRequest);

const res = new Response(JSON.stringify(err.stack), {
status: 503, // unavailable
headers: util.browserHeaders(),
});
return res;
}

function servfail(currentRequest) {
if (
util.emptyObj(currentRequest) ||
util.emptyObj(currentRequest.decodedDnsPacket)
) {
return util.respond408();
}

const qid = currentRequest.decodedDnsPacket.id;
const qs = currentRequest.decodedDnsPacket.questions;
return dnsutil.servfail(qid, qs);
function errorResponse(currentRequest, err = null) {
const eres = util.emptyObj(err) ? null : util.errResponse("doh.js", err);
currentRequest.dnsExceptionResponse(eres);
}
20 changes: 7 additions & 13 deletions src/core/env.js
Expand Up @@ -66,6 +66,11 @@ const _ENV_VAR_MAPPINGS = {
type: "string",
default: "https://cloudflare-dns.com/dns-query",
},
secondaryDohResolver: {
name: "CF_DNS_RESOLVER_URL_2",
type: "string",
default: "https://dns.google/dns-query",
},
onInvalidFlagStopProcessing: {
name: "CF_ON_INVALID_FLAG_STOPPROCESSING",
type: "boolean",
Expand All @@ -91,17 +96,6 @@ const _ENV_VAR_MAPPINGS = {
type: "number",
default: "2",
},

// set to on - off aggressive cache plugin
// as of now Cache-api is available only on worker
// so load() will set this to false for other runtime.
isAggCacheReq: {
name: {
worker: "IS_AGGRESSIVE_CACHE_REQ",
},
type: "boolean",
default: "false",
},
};

/**
Expand Down Expand Up @@ -145,7 +139,7 @@ function _getRuntimeEnv(runtime) {
else throw new Error(`unsupported runtime: ${runtime}`);

// assign default value when user-defined value is missing
if (env[key] === null || env[key] === undefined) {
if (env[key] == null) {
console.warn(key, "env[key] default value:", val);
env[key] = val;
}
Expand All @@ -156,7 +150,7 @@ function _getRuntimeEnv(runtime) {
else if (type === "string") env[key] = env[key] || "";
else throw new Error(`unsupported type: ${type}`);

console.debug("Added", key, mappedKey, env[key]);
console.debug("Added", key, env[key]);
}

return env;
Expand Down
4 changes: 3 additions & 1 deletion src/core/plugin.js
Expand Up @@ -103,6 +103,8 @@ export default class RethinkPlugin {
"request",
"requestDecodedDnsPacket",
"event",
// resolver-url overriden by user-op
"userDnsResolverUrl",
"blocklistFilter",
"dnsCache",
],
Expand Down Expand Up @@ -233,7 +235,7 @@ export default class RethinkPlugin {
} else if (!util.emptyObj(r)) {
// r.userBlocklistInfo and r.dnsResolverUrl are never "null"
this.registerParameter("userBlocklistInfo", r.userBlocklistInfo);
this.registerParameter("dnsResolverUrl", r.dnsResolverUrl);
this.registerParameter("userDnsResolverUrl", r.dnsResolverUrl);
} else {
this.log.i(rxid, "user-op is a no-op, possibly a command-control req");
}
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/basic/userOperation.js
Expand Up @@ -64,7 +64,8 @@ export class UserOperation {
}

response.data.userBlocklistInfo = currentUser;
response.data.dnsResolverUrl = envutil.dohResolver();
// sets user-preferred doh upstream
response.data.dnsResolverUrl = null;
} catch (e) {
this.log.e(param.rxid, "loadUser", e);
response = util.errResponse("UserOp:loadUser", e);
Expand Down

0 comments on commit 33e9107

Please sign in to comment.