Skip to content

Commit adbaa90

Browse files
committed
merge ca-trust-guard: match the ca-trust guard to node's CA loader (upstream PR cnighswonger#296, draft)
2 parents 23346ac + b012691 commit adbaa90

7 files changed

Lines changed: 634 additions & 145 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,15 @@
44

55
### Fixed
66

7-
- **`--remote-control` no longer clobbers another component's `NODE_EXTRA_CA_CERTS`.** That variable takes exactly one file, so on a host where something else also MITMs `api.anthropic.com` (a corporate agent, an account-pinning proxy) the last writer won and every other CA was silently untrusted — measured breaking Remote Control inbound. The launcher now publishes its own CA to `${CLAUDE_CONFIG_DIR:-~/.claude}/ca-trust.d/ccf.pem` (own filename only, never a sibling's, rewritten every launch, atomically via temp + `rename`) and reads a merged `ca-trust.pem` if one exists. It never writes the merged bundle: merging needs ambient corporate-root discovery, which is environment-specific and belongs outside this repo. The bundle is used only when every PEM block in it parses **and** one of them is our own CA (compared by DER) — a bundle that is torn or predates our publish is worse than none, since it makes the client distrust the very proxy it is routed through. On a host with no other MITM and no bundle, behavior is byte-identical to before. Both paths are fixed names under the config dir with no env override: they are two halves of one rendezvous, so a knob on either half alone would let a participant drop out of the contract while appearing to implement it. See [Coexisting with another MITM](README.md#coexisting-with-another-mitm-on-the-same-machine-ca-trustd).
7+
- **`--remote-control` no longer clobbers another component's `NODE_EXTRA_CA_CERTS`.** That variable takes exactly one file, so on a host where something else also MITMs `api.anthropic.com` (a corporate agent, an account-pinning proxy) the last writer won and every other CA was silently untrusted — measured breaking Remote Control inbound. The launcher now publishes its own CA to `${CLAUDE_CONFIG_DIR:-~/.claude}/ca-trust.d/ccf.pem` (own filename only, never a sibling's, rewritten every launch, atomically via temp + `rename`) and reads a merged `ca-trust.pem` if one exists. It never writes the merged bundle: merging needs ambient corporate-root discovery, which is environment-specific and belongs outside this repo. The bundle is used only when every PEM block in it is terminated **and** one of its `CERTIFICATE` blocks is our own CA (compared by DER) — a bundle that is torn or predates our publish is worse than none, since it makes the client distrust the very proxy it is routed through. On a host with no other MITM and no bundle, behavior is byte-identical to before. Both paths are fixed names under the config dir with no env override: they are two halves of one rendezvous, so a knob on either half alone would let a participant drop out of the contract while appearing to implement it. See [Coexisting with another MITM](README.md#coexisting-with-another-mitm-on-the-same-machine-ca-trustd).
8+
9+
- **The `ca-trust.pem` guard now matches what Node's CA loader actually accepts.** As shipped it disagreed with a real handshake on 8 of 20 measured bundle shapes (node v24.11.1 / openssl 3.6.1). Seven were needless refusals of healthy bundles — any non-certificate block (a CRL, a public key, key material), or a provenance comment that merely mentioned `-----BEGIN `, voided the whole file — and refusing is not the safe direction: it silently drops every sibling and corporate CA for that session, which is the failure this contract exists to prevent. The eighth was the dangerous direction: our own CA relabelled `TRUSTED CERTIFICATE` parses to byte-identical DER, so the guard reported "carries our CA" while Node's loader skipped the block entirely and every request failed TLS. The guard now anchors markers to line starts (tolerating trailing whitespace, which openssl still reacts to), bounds each block's `END` search at the next `BEGIN` so a torn entry cannot borrow a later block's terminator, requires every block to decode — X509 for `CERTIFICATE`, valid base64 armor for everything else, since Node aborts the whole load on any block it cannot read regardless of label — and requires the DER match to land on a `CERTIFICATE` block. Where it cannot tell (a block damaged *after* ours, whose truncated body may still decode) it refuses — conservative, never permissive. The decision moved to `bin/ca-trust.mjs` so the tests drive the shipped code: it was previously inline in a top-level script with a hand-copied twin in the test file, and mutating the real one left the entire suite green.
10+
11+
- **Orphaned publish temps are reaped even when publishing fails.** The reaper shared the publish `try`, so `rename` throwing skipped it — meaning that on exactly the hosts where publishing is persistently broken (a root-owned `ccf.pem`, a read-only mount, `ENOSPC`) each launch abandoned one full-CA temp and collected none, growing without bound in the directory a bundle builder globs.
12+
13+
- **A corrupt `ca.pem` no longer blames the merged bundle, and is no longer handed to the client.** Parsing our own CA sat inside the bundle `try`, so an unparseable `ca.pem` was reported as `ignoring <ca-trust.pem> (...)` — naming a file that may be perfectly healthy — and then `NODE_EXTRA_CA_CERTS` was pointed at the very file that had just failed to parse. It is now parsed in its own step and named in its own message, and when it does not parse the variable is left unset: node falls back to its built-in store, which is the honest state, rather than to a file we vouch for and cannot read.
14+
15+
- **The launcher no longer prints the wiring recipe that would undo its own coexistence.** The spawned proxy's `export NODE_EXTRA_CA_CERTS=<our ca.pem>` banner is correct standalone advice and exactly wrong under `--remote-control`, which relays that stderr: it appeared right after the launcher had published to `ca-trust.d` and adopted the merged bundle, telling the operator to pin the variable to our CA alone. The server now prints the recipe only when the operator is the one wiring: `process.channel` is set exactly when our launcher `fork()`ed it, and the launcher is the only `fork()` site (the `server` subcommand uses `spawn`, and a service manager runs it bare). The mode line still prints either way, so forward-proxy mode stays visible. Standalone, the recipe carries a same-host-MITM caveat, as do the README's manual-wiring recipes.
816

917
### Documentation
1018

README.md

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,14 @@ The generated systemd unit / launchd agent carries `CACHE_FIX_FORWARD_PROXY=on`,
7777
- `HTTPS_PROXY` — where the proxy listens: `http://127.0.0.1:<port>` (default port `9801`, or your `CACHE_FIX_PROXY_PORT`).
7878
- `NODE_EXTRA_CA_CERTS` — the CA the proxy generated on first start: `~/.claude/cache-fix-ca/ca.pem` (or `$CACHE_FIX_CA_DIR/ca.pem`).
7979

80-
Three ways to wire it, depending on how broadly you want the vars to apply:
80+
Three ways to wire it, depending on how broadly you want the vars to apply.
81+
82+
> **If anything else on this host also MITMs `api.anthropic.com`** — a corporate
83+
> TLS-inspecting agent, an account-switching pin proxy — do not use these
84+
> recipes. `NODE_EXTRA_CA_CERTS` takes one file, so pinning it to our CA alone
85+
> silently untrusts every other component. Use `--remote-control`, which
86+
> publishes into `ca-trust.d/` and consumes the merged bundle instead. See
87+
> [Coexisting with another MITM](#coexisting-with-another-mitm-on-the-same-machine-ca-trustd).
8188
8289
```bash
8390
# a) per-invocation — scoped to just this claude run
@@ -124,13 +131,44 @@ environment-specific (a Linux host may keep them outside the bundle a shell
124131
points at; a Mac keeps them in the keychain), and two components both rebuilding
125132
it would race one output.
126133

127-
The bundle is used only if it is intact (balanced `BEGIN`/`END` markers) **and**
128-
carries our own CA. A bundle failing either check is worse than no bundle — it
129-
would make the client distrust the very proxy it is being routed through, so
130-
every request fails TLS rather than merely losing some other component's CA. In
131-
that case, and when no bundle exists at all, the launcher falls back to our own
132-
CA and behaves exactly as it did before any of this existed. **A host with no
133-
other MITM and no bundle builder sees no change.**
134+
The bundle is used only if every PEM block in it is terminated **and** one of its
135+
`CERTIFICATE` blocks is our own CA. A bundle failing either check is worse than
136+
no bundle — it would make the client distrust the very proxy it is being routed
137+
through, so every request fails TLS rather than merely losing some other
138+
component's CA. In that case, and when no bundle exists at all, the launcher
139+
falls back to our own CA and behaves exactly as it did before any of this
140+
existed. **A host with no other MITM and no bundle builder sees no change.**
141+
142+
The check mirrors what Node's `NODE_EXTRA_CA_CERTS` loader does, which is
143+
narrower than "is this valid PEM" in one direction and wider in another, and both
144+
were measured against a real handshake rather than read off the spec:
145+
146+
- **Well-formed non-certificate blocks are ignored, not fatal.** A merged bundle
147+
legitimately carries CRLs, public keys and key material next to the roots; Node
148+
skips them and verifies fine. Parsing every block as a certificate rejected
149+
those bundles outright, and rejecting is not the safe direction here — it drops
150+
every sibling and corporate CA for the whole session. A *damaged* one is a
151+
different matter: Node aborts the whole load on any block it cannot decode,
152+
whatever the label, so every block must decode even though only certificates
153+
are compared.
154+
- **The `CERTIFICATE` label is load-bearing.** Our own CA relabelled
155+
`TRUSTED CERTIFICATE` parses to byte-identical DER, so a label-blind comparison
156+
reports "carries us" — while Node's loader skips the block entirely and the
157+
session then fails every request. A DER match on a non-`CERTIFICATE` block does
158+
not count.
159+
- **Markers are line-anchored, but tolerate trailing whitespace.** A provenance
160+
header that merely mentions `-----BEGIN ` is prose, not a block. A real marker
161+
wearing a trailing space still is one — openssl reacts to it, so a strict
162+
end-of-line anchor would hide that block from the guard while Node still tried
163+
to load it.
164+
- **Each block ends at its own `END`.** The terminator is searched for only up to
165+
the next `BEGIN`, so an unterminated entry cannot borrow the `END` line of a
166+
later one and pass as intact.
167+
168+
Where the guard cannot tell (a block damaged *after* ours, whose truncated body
169+
may or may not still decode) it refuses. Refusing costs the other components'
170+
CAs for one session; accepting a bundle Node cannot load costs the session
171+
entirely, so the guard is allowed to be conservative and never permissive.
134172

135173
Both paths are fixed names under `<config>`, deliberately with no env override
136174
of their own. They are two halves of one rendezvous: a knob on either half alone

bin/ca-trust.mjs

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
import { X509Certificate } from "node:crypto";
2+
3+
// Does a non-certificate block's armor decode? Node only needs that much from a
4+
// CRL or key block, so this is deliberately weaker than parsing it as whatever
5+
// it claims to be — the guard's job is to predict node's loader, not to
6+
// validate the block's contents.
7+
//
8+
// Base64 is checked as whole 4-character quanta, not merely as an alphabet. An
9+
// alphabet-only test accepted a one-character body: measured, `A` in a
10+
// PUBLIC KEY block ahead of our CA gave guard=accept while node reported
11+
// `bad base64 decode` and loaded zero extra CAs. Padding is equally positional —
12+
// `AAA=` and `AA==` load, `A===`, `=AAA` and `AA=A` do not. Measured 16/16
13+
// agreement with a real handshake on the rule below.
14+
//
15+
// A body containing `-` reads as damaged here even though node accepts it
16+
// (openssl stops at the dash), which is the conservative direction and the only
17+
// measured disagreement.
18+
function isBase64Body(body) {
19+
const b = body.replace(/\s+/g, "");
20+
return b.length > 0 && b.length % 4 === 0 && /^[A-Za-z0-9+/]+={0,2}$/.test(b);
21+
}
22+
23+
// Is this merged CA bundle safe to hand claude as NODE_EXTRA_CA_CERTS?
24+
//
25+
// Lives in its own module for one reason: the launcher is a top-level script,
26+
// so a test could not import this decision and the previous version kept a
27+
// hand-copied duplicate in test/proxy-forward-ca.test.mjs under a "change one,
28+
// change both" comment. Measured: mutating the launcher's copy left the whole
29+
// suite green, so the trust path had no regression cover at all. Two call sites
30+
// is below this repo's bar for a new abstraction; the justification here is not
31+
// reuse, it is that the test must drive the shipped code rather than an
32+
// adjacent copy of it.
33+
//
34+
// The rule this implements is node's, not openssl's-in-general, and every
35+
// clause below was measured against a real TLS handshake (node v24.11.1,
36+
// openssl 3.6.1) rather than reasoned from the spec. See
37+
// test/proxy-forward-ca.test.mjs, which re-runs that comparison on each shape.
38+
//
39+
// It stays a pre-flight guard, never proof: it establishes the file parses and
40+
// carries us, never that node will verify a given leaf with it.
41+
export function bundleCarriesOurCA(text, ourCaPem) {
42+
const ourDer = new X509Certificate(ourCaPem).raw;
43+
// Line-anchored. openssl only honours a marker that begins a line, so a
44+
// marker quoted inside prose is not a block. The previous count-based check
45+
// read the raw file and rejected any bundle whose provenance header happened
46+
// to name the marker — measured: `# see -----BEGIN CERTIFICATE-----` ahead of
47+
// a healthy CA was refused while node authorized the same bytes.
48+
//
49+
// No CRLF normalization: `$` in a /m regex matches before a `\r`, and the END
50+
// search below is anchored on the leading `\n`, so both halves already read a
51+
// CRLF file the same as an LF one. Measured across 102 shapes (34 bundle
52+
// layouts x LF/CRLF/mixed): identical verdicts with and without the replace.
53+
// Trailing whitespace is tolerated on the marker line. openssl still reacts
54+
// to `-----BEGIN CERTIFICATE----- ` (one trailing space), so a `$`-anchored
55+
// pattern made that block invisible to us while node still tried to load it
56+
// — measured: a corrupt block wearing a trailing space was skipped by the
57+
// guard and failed the handshake.
58+
// The label pattern is permissive on purpose. Restricting it to uppercase,
59+
// digits and spaces made every other legal label invisible to us while
60+
// openssl still treated the block as real — measured: a malformed `X-FOO`
61+
// block ahead of our CA gave guard=accept while node loaded zero extra CAs.
62+
// Every label tried behaved as a real block (hyphenated, lowercase,
63+
// underscored, dotted, punctuated, even empty), so the label decides only
64+
// WHICH check a block gets, never whether it is one.
65+
// `-` is legal INSIDE a label, so the stop condition is the `-----` run, not
66+
// the first hyphen: `[^-]*` failed to match `X-FOO` at all, which is the same
67+
// blind spot in a new place.
68+
const marker = /^-----BEGIN ((?:(?!-----).)*)-----[ \t]*$/gm;
69+
let carriesUs = false;
70+
for (let m; (m = marker.exec(text)); ) {
71+
const label = m[1];
72+
// Bounded by the NEXT marker, not by a search to end-of-file. An unbounded
73+
// indexOf lets a torn block borrow the END line of a later one, so the
74+
// unterminated check never fires and the slice spans two entries.
75+
// The END marker must also END ITS LINE, bar trailing whitespace. indexOf
76+
// alone ignored whatever followed it, so `-----END CERTIFICATE-----garbage`
77+
// and `-----END CERTIFICATE-------` both read as terminators here while
78+
// openssl rejected the block and node loaded zero CAs — measured, both as
79+
// false accepts on an otherwise healthy bundle. Whitespace is fine (13/13
80+
// agreement with a real handshake on what may follow), anything else is not.
81+
const endMarker = `\n-----END ${label}-----`;
82+
const nextBegin = text.indexOf("\n-----BEGIN ", m.index + 1);
83+
let end = -1;
84+
for (let at = text.indexOf(endMarker, m.index); at !== -1;
85+
at = text.indexOf(endMarker, at + 1)) {
86+
const lineEnd = text.indexOf("\n", at + 1);
87+
const tail = text.slice(at + endMarker.length, lineEnd === -1 ? undefined : lineEnd);
88+
if (/^[ \t\r]*$/.test(tail)) { end = at; break; }
89+
}
90+
if (end !== -1 && nextBegin !== -1 && end > nextBegin) end = -1;
91+
// Unterminated, or closed by a different label. Fatal whatever the label
92+
// is, and deliberately not analyzed further: openssl's base64 decoder
93+
// treats the next '-' as end-of-data instead of an error, so a torn block
94+
// yields a valid entry when its truncated body happens to be a complete
95+
// DER and garbage when it does not. Measured both outcomes from the same
96+
// tear position with only the body length changed. Since the result is not
97+
// knowable from out here, a damaged file is refused rather than guessed at.
98+
if (end === -1) return { ok: false, reason: `unterminated ${label} block` };
99+
const block = text.slice(m.index, end + endMarker.length);
100+
// EVERY block must decode, whatever its label. Node's PEM reader aborts the
101+
// whole extras load on any block it cannot decode — a truncated CRL or key
102+
// block ahead of our CA takes the entire file down with it, our own entry
103+
// included. Skipping non-certificate blocks outright (as this did) waved
104+
// those bundles through: measured, a corrupt PUBLIC KEY and a corrupt
105+
// X509 CRL each gave guard=accept while the handshake failed
106+
// UNABLE_TO_VERIFY_LEAF_SIGNATURE.
107+
//
108+
// What "decodes" means differs by label, and both halves were measured
109+
// against a real handshake rather than reasoned from the spec. For a
110+
// CERTIFICATE, base64 validity is not enough — a well-formed base64 body
111+
// that is not a certificate still kills the load, so it must parse as X509.
112+
// For everything else node only needs the armor to decode, so valid base64
113+
// is the whole bar; demanding more would reject the CRLs and key blocks a
114+
// real corporate bundle legitimately carries, and rejecting does not fail
115+
// safe here — it drops every sibling and corporate CA for the session,
116+
// which is the failure this contract exists to prevent.
117+
if (label === "CERTIFICATE") {
118+
let der;
119+
try { der = new X509Certificate(block).raw; }
120+
catch { return { ok: false, reason: "undecodable CERTIFICATE block" }; }
121+
if (der.equals(ourDer)) carriesUs = true;
122+
} else if (!isBase64Body(text.slice(m.index + m[0].length, end))) {
123+
return { ok: false, reason: `undecodable ${label} block` };
124+
}
125+
}
126+
// A bundle that exists but predates our publish is WORSE than no bundle:
127+
// handing it to claude makes the client distrust the very proxy it is routed
128+
// through, so every request fails TLS rather than merely losing some other
129+
// component's CA.
130+
//
131+
// Matched on DER, and only on a CERTIFICATE block, because neither weaker
132+
// check is sound. Measured: relabelling our own CA to TRUSTED CERTIFICATE
133+
// leaves X509Certificate parsing it into byte-identical DER while node's CA
134+
// loader skips it entirely — the old guard accepted that bundle and the
135+
// handshake then failed with UNABLE_TO_VERIFY_LEAF_SIGNATURE. That is the
136+
// exact outcome this check exists to prevent, so the label is load-bearing.
137+
return carriesUs ? { ok: true } : { ok: false, reason: "bundle does not carry our CA" };
138+
}

0 commit comments

Comments
 (0)