Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion src/dns.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1695,6 +1695,36 @@ export function proxyProbeFromArgs(args = [], claimed = []) {
return { name: claimed[0] ? `a.${claimed[0]}` : null, invalid: false };
}

/**
* A Moshpit name to test this machine's setup with.
*
* Synthetic on purpose — the proxy mints a certificate for any name inside the
* namespace, so the probe does not have to name something anybody registered.
* What it does have to be is a hostname the rest of the program can parse.
*
* `a.${tlds[0]}` was not. The first ending the registry returns is `00`, and
* `https://a.00/` is rejected outright by the WHATWG URL parser: a final label
* of digits makes the whole host a candidate IPv4 literal, and `a.00` is not a
* valid one. So proxy detection asked about a name that could not be looked up,
* concluded there was no proxy on a machine that had one running and holding
* 443, and started the bridge without proxy mode — leaving every name to answer
* its origin with a certificate no CA has signed.
*
* An all-numeric ending is legal and wanted (`.420`, `.2600`); it is unusable
* only here, which is why this skips them for the probe rather than the registry
* refusing to sell them. `--proxy-probe <name>` overrides, for a machine where
* the choice has to be a specific real name.
*/
export function probeName(tlds = [], args = []) {
const at = args.indexOf("--proxy-probe");
if (at >= 0) {
const given = args[at + 1];
if (given && !given.startsWith("--")) return given;
}
const usable = tlds.find((t) => !/^\d+$/.test(String(t)));
return usable ? `a.${usable}` : null;
}

export function upstreamsFromArgs(args = []) {
const servers = [];
const invalid = [];
Expand Down Expand Up @@ -3095,7 +3125,7 @@ export async function dnsCommand(args = [], out = console.log, deps = {}) {

// The name whose resolution proves the bridge is answering, alongside the
// clearnet one that proves it is forwarding.
const moshpitProbe = tlds[0] ? `a.${tlds[0]}` : null;
const moshpitProbe = probeName(tlds, rest);
// Restoring files is not a rollback on Windows: NRPT rules are not files,
// so the undo is the disable plan's commands rather than the enable plan's.
const undoSteps = platform === "windows" ? disablePlan({ platform, tlds, linuxBackend }).steps : undefined;
Expand Down
43 changes: 42 additions & 1 deletion test/dns-service.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { tmpdir } from "node:os";
import { join } from "node:path";

import { installService, removeService, serviceUnit, servicePaths, UNIT_NAME } from "../src/dns-service.mjs";
import { proxyProbeFromArgs, captureRestorePoint } from "../src/dns.mjs";
import { proxyProbeFromArgs, captureRestorePoint, probeName } from "../src/dns.mjs";
import { proxyServiceUnit, proxyServicePaths, PROXY_UNIT_NAME } from "../src/dns-service.mjs";
import { rootIsNarrow, ensureProxyService } from "../src/dns-service.mjs";

Expand Down Expand Up @@ -347,3 +347,44 @@ test("a narrow root is reminted as part of bringing the proxy up", async () => {
"and the remint is reported, because it invalidates the trust the machine already has",
);
});

/* -------------------------------------- the name `dns enable` tests itself with */

// `a.${tlds[0]}` looked harmless for as long as the first ending happened to be
// a word. It is `00` now, and `https://a.00/` is rejected by the WHATWG URL
// parser — a final label of digits makes the host a candidate IPv4 literal, and
// `a.00` is not a valid one.
//
// The consequence was not a bad error message. Proxy detection asked about a
// name that could not be looked up, concluded there was no proxy on a machine
// that had one running and holding 443, and started the bridge without proxy
// mode — so every name answered its origin with a certificate no CA had signed,
// on a machine whose setup had otherwise completed perfectly.

test("an all-numeric ending is skipped — it cannot be a URL host", () => {
assert.equal(probeName(["00", "01", "2600", "hacker"]), "a.hacker");
});

test("the chosen probe actually parses as a URL", () => {
// The assertion the old code would have failed, and the reason this matters.
assert.doesNotThrow(() => new URL(`https://${probeName(["00", "01", "hacker"])}/`));
assert.throws(() => new URL("https://a.00/"), { code: "ERR_INVALID_URL" });
});

test("numeric endings stay sellable — they are only unusable as a probe", () => {
// `.420` and `.2600` are names people want. Nothing here refuses them; this
// picks a different one to *test with*.
assert.equal(probeName(["2600", "eggs"]), "a.eggs");
});

test("a registry of only numeric endings yields no probe rather than a broken one", () => {
assert.equal(probeName(["00", "2600", "420"]), null);
assert.equal(probeName([]), null);
});

test("--proxy-probe wins, for a machine that needs a specific real name", () => {
assert.equal(probeName(["00", "hacker"], ["--proxy-probe", "chovy.hacker"]), "chovy.hacker");
// A bare flag is a typo, not a request to probe with the next flag.
assert.equal(probeName(["hacker"], ["--proxy-probe"]), "a.hacker");
assert.equal(probeName(["hacker"], ["--proxy-probe", "--write"]), "a.hacker");
});
Loading