Skip to content

Commit 58fffe1

Browse files
ralyodioclaude
andauthored
fix(dns): a proxy this run started is a proxy it can trust (#481)
Proxy detection asked the proxy for a certificate under `a.<ending>`, and the proxy will not present one for a name with no key published in the registry. That refusal is deliberate — it produces a TLS failure a browser can explain rather than a reset mid-request — and `dns enable` has no way to invent a name somebody has registered, because the registry lists endings and not names. `a.<ending>` is the best it could construct, and that is precisely the shape the proxy refuses. So the check could not pass on a correctly configured machine. Three lines apart, both true as written: ok proxy holds 127.0.0.1:443 -- no pinned-TLS proxy on this machine The bridge was then started without proxy mode, so every Moshpit name answered its origin directly and a stock client got a certificate no CA had signed — on a machine that had, in the same run, installed the proxy, started it, watched it take the port, and installed the local root into the system trust store. v0.88.0 made the probe name parseable, which moved the failure from ERR_INVALID_URL to ERR_TLS_CERT_ALTNAME_INVALID and no further: a name that parses is still a name with no pin. The probe was the wrong instrument. It exists for a proxy that something else installed, where the question is genuinely open. It is not the question here: this run wrote the unit, restarted it, and confirmed the port held. That is stronger evidence of whose proxy it is than any handshake, so it is used, and the probe is kept for the case it was written for. Three tests, driving `dns enable` rather than the helper: a proxy this run started turns proxy mode on and the bridge is checked to have actually been told (`proxy: "127.0.0.1"` — without that names still answer their origin); a proxy it did not start is still probed; and no proxy at all still brings DNS up, because an optional component must never cost a machine its resolver. Suite: 2768 tests, 0 failures. Known and not fixed here: a bridge that is already running keeps the mode it started with, so on a machine where one is up this takes effect on the next restart of it rather than immediately. Claude-Session: https://claude.ai/code/session_01ThnQwoieWt8VR6N7gtgnhp Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4125b03 commit 58fffe1

2 files changed

Lines changed: 91 additions & 0 deletions

File tree

src/dns.mjs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3294,6 +3294,7 @@ export async function dnsCommand(args = [], out = console.log, deps = {}) {
32943294
// Moshpit names and cannot verify them, which is bad; a machine whose DNS
32953295
// was refused because an optional component would not start is worse.
32963296
let proxyUnitPath = null;
3297+
let ours = false;
32973298
if (!rest.includes("--no-proxy") && platform === "linux") {
32983299
if (!proxyWrapper()) {
32993300
out(" -- no pinned-TLS proxy installed — https:// on a name will not verify");
@@ -3306,6 +3307,10 @@ export async function dnsCommand(args = [], out = console.log, deps = {}) {
33063307
out(` ${step.ok ? "ok " : "-- "} ${step.step}${step.error ? ` — ${step.error}` : ""}`);
33073308
}
33083309
if (!ensured.ok) out(` -- the proxy is not serving (${ensured.reason}) — https:// will not verify`);
3310+
// Started by this run, and confirmed holding the port. That is the
3311+
// strongest evidence available that the proxy on 443 is ours, and it is
3312+
// strictly better than the handshake below.
3313+
ours = ensured.ok;
33093314
}
33103315
}
33113316

@@ -3363,6 +3368,26 @@ export async function dnsCommand(args = [], out = console.log, deps = {}) {
33633368
// proxy and retracting it two lines later is worse than not looking.
33643369
out(" -- the bridge already running was not started by this run, so it keeps its own");
33653370
out(" mode — to pick up proxy mode: moshcode dns disable && moshcode dns enable");
3371+
} else if (ours) {
3372+
// No handshake needed, and none that would work.
3373+
//
3374+
// The probe asks the proxy for a certificate under some name. The proxy
3375+
// will not present one for a name with no key published in the registry —
3376+
// deliberately, so a browser gets a TLS failure it can explain rather than
3377+
// a reset mid-request — and this command has no way to invent a name that
3378+
// someone has registered. There is no endpoint that lists names, only
3379+
// endings, so `a.<ending>` is the best it could ever do and that is
3380+
// exactly the name the proxy refuses.
3381+
//
3382+
// Which made the check impossible to pass on a correctly configured
3383+
// machine: `proxy holds 127.0.0.1:443` and `no pinned-TLS proxy on this
3384+
// machine`, three lines apart, both true as written.
3385+
//
3386+
// So when this run installed the unit, started it, and watched it take the
3387+
// port, that is the answer. The probe below stays for a proxy this run did
3388+
// not start, where the question is genuinely open.
3389+
proxyAddress = { v4: DEFAULT_HOST, v6: null };
3390+
out(` ok pinned-TLS proxy on ${DEFAULT_HOST}:${PROXY_PORT} — started by this run, so every live name will answer there`);
33663391
} else if (!rest.includes("--no-proxy")) {
33673392
const probeName = moshpitProbe || "";
33683393
if (!probeName) {

test/dns-enable-rollback.test.mjs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,3 +472,69 @@ function noSystem() {
472472
uid: 0,
473473
};
474474
}
475+
476+
/* --------------------------- a proxy this run started is a proxy it can trust */
477+
478+
// Detection used to ask the proxy for a certificate under `a.<ending>`. The
479+
// proxy will not present one for a name with no key published in the registry —
480+
// deliberately, so a browser gets a TLS failure it can explain — and nothing
481+
// here can invent a name someone has registered, because the registry lists
482+
// endings and not names.
483+
//
484+
// So on a correctly configured machine the check could not pass. Three lines
485+
// apart, both true as written:
486+
//
487+
// ok proxy holds 127.0.0.1:443
488+
// -- no pinned-TLS proxy on this machine
489+
//
490+
// and the bridge was then started without proxy mode, leaving every name to
491+
// answer its origin with a certificate no CA had signed.
492+
493+
test("a proxy started by this run turns proxy mode on without a handshake", async () => {
494+
const lines = [];
495+
let startedWith = null;
496+
const code = await dnsCommand(["enable"], (l) => lines.push(String(l)), {
497+
...noSystem(),
498+
proxyWrapper: () => "/home/x/.local/bin/moshpit-proxy",
499+
ensureProxy: async () => ({ ok: true, steps: [{ step: "proxy holds 127.0.0.1:443", ok: true }] }),
500+
applyWith: async () => ({ saved: { ok: true }, applied: { ok: true, results: [] }, verified: { ok: true, checks: [] }, rolledBack: null, backups: [] }),
501+
// If this is consulted at all the fix has not worked: the whole point is
502+
// that no name is invented and no handshake is attempted.
503+
findLocalProxyImpl: async () => { throw new Error("the probe must not run for a proxy we started"); },
504+
startBridge: async (opts) => { startedWith = opts; return { started: true, pid: 1, alreadyRunning: false }; },
505+
});
506+
507+
assert.equal(code, 0);
508+
assert.match(lines.join("\n"), /started by this run/);
509+
assert.equal(startedWith?.proxy, "127.0.0.1", "the bridge has to be told, or names still answer their origin");
510+
});
511+
512+
test("a proxy this run did not start is still probed", async () => {
513+
// The question is genuinely open there, so the handshake stays.
514+
const lines = [];
515+
let probed = false;
516+
await dnsCommand(["enable"], (l) => lines.push(String(l)), {
517+
...noSystem(),
518+
proxyWrapper: () => "/home/x/.local/bin/moshpit-proxy",
519+
ensureProxy: async () => ({ ok: false, reason: "not-listening", steps: [] }),
520+
applyWith: async () => ({ saved: { ok: true }, applied: { ok: true, results: [] }, verified: { ok: true, checks: [] }, rolledBack: null, backups: [] }),
521+
findLocalProxyImpl: async () => { probed = true; return { found: false, why: null, address: { v4: null, v6: null } }; },
522+
});
523+
524+
assert.equal(probed, true);
525+
});
526+
527+
test("with no proxy installed, nothing is claimed and DNS still comes up", async () => {
528+
const lines = [];
529+
let startedWith = null;
530+
const code = await dnsCommand(["enable"], (l) => lines.push(String(l)), {
531+
...noSystem(),
532+
proxyWrapper: () => null,
533+
applyWith: async () => ({ saved: { ok: true }, applied: { ok: true, results: [] }, verified: { ok: true, checks: [] }, rolledBack: null, backups: [] }),
534+
startBridge: async (opts) => { startedWith = opts; return { started: true, pid: 1, alreadyRunning: false }; },
535+
});
536+
537+
assert.equal(code, 0, "a missing optional component must never refuse the machine its DNS");
538+
assert.equal(startedWith?.proxy, null);
539+
assert.match(lines.join("\n"), /no pinned-TLS proxy installed/);
540+
});

0 commit comments

Comments
 (0)