fix(reconcile): repair a CA:TRUE certificate, which a pin check cannot see

scripts/moshpit-reconcile.sh

@@ -23,10 +23,15 @@
 # discovery would silently serve a subset and look like it had covered
 # everything. A box that serves three endings should say so in one line.
 #
-# Idempotent, and quiet when there is nothing to do: a name is reconfigured
-# only when its nginx block is missing or the pin the registry publishes is not
-# the key this box actually serves. Without that check every run would reload
+# Idempotent, and quiet when there is nothing to do: a name is reconfigured only
+# when its nginx block is missing, the pin the registry publishes is not the key
+# this box actually serves, or the certificate it serves is marked CA:TRUE and
+# so cannot be trusted by anything. Without those checks every run would reload
 # nginx, and a timer would reload it forever.
+#
+# The CA:TRUE trigger converges in one pass rather than firing forever: the
+# repair reuses the key, so the pin does not move and the next pass finds
+# nothing to do.
 set -eu
 
 CONF="${MOSHPIT_CONF:-/etc/moshpit/reconcile.conf}"
@@ -38,6 +43,10 @@ ENDINGS="${MOSHPIT_ENDINGS:-}"
 HOST="${MOSHPIT_HOST:-}"
 ENABLEDIR="${MOSHPIT_ENABLEDIR:-/etc/nginx/sites-enabled}"
 SETUP="${MOSHPIT_SETUP:-$(dirname "$0")/setup-origin.sh}"
+# Where to ask what this box is actually serving. Always the local nginx in
+# production; overridable so the certificate checks can be exercised against a
+# fixture server instead of requiring port 443 and root to test.
+ORIGIN_ADDR="${MOSHPIT_ORIGIN_ADDR:-127.0.0.1:443}"
 DRY_RUN=0
 FAILURES=0
 
@@ -80,13 +89,33 @@ points_here() {
 # The pin this box serves for a name, computed the same way the registry's
 # publisher computes it. Empty when nothing is listening for that name.
 served_pin() {
-  echo | openssl s_client -connect "127.0.0.1:443" -servername "$1" 2>/dev/null \
+  echo | openssl s_client -connect "$ORIGIN_ADDR" -servername "$1" 2>/dev/null \
     | openssl x509 -pubkey -noout 2>/dev/null \
     | openssl pkey -pubin -outform der 2>/dev/null \
     | openssl dgst -sha256 -binary 2>/dev/null \
     | openssl enc -base64 2>/dev/null
 }
 
+# Is the certificate this box serves for a name marked as a certificate
+# authority?
+#
+# The second repair trigger, and one the pin comparison structurally cannot see.
+# CA:TRUE is what openssl's `req -x509` produces by default, so every origin set
+# up before that default was overridden is serving one — and such a certificate
+# cannot be trusted directly, because an anchor marked CA:TRUE may issue for any
+# name rather than the one printed on it. A stock client is left with
+# "self-signed certificate" and no way forward.
+#
+# Re-issuing fixes it and reuses the key, so the pin does not move. That is what
+# makes the repair free, and it is also exactly why the check above stays silent
+# about it: serving and published agree, so a box would pull the fix and then
+# reconcile contentedly forever without ever applying it.
+served_is_ca() {
+  echo | openssl s_client -connect "$ORIGIN_ADDR" -servername "$1" 2>/dev/null \
+    | openssl x509 -noout -ext basicConstraints 2>/dev/null \
+    | grep -q 'CA:TRUE'
+}
+
 published_pin() {
   curl -fsS --max-time 15 "$REGISTRY/api/moshpit/tlds/$2/pins?label=$1" 2>/dev/null \
     | grep -o '"pin":"[^"]*"' | head -1 | cut -d'"' -f4
@@ -113,6 +142,10 @@ reconcile_name() {
       # name outright. Worth naming both, because the usual cause is a
       # certificate regenerated without republishing.
       need="published pin does not match what is served ($publish vs $serving)"
+    elif served_is_ca "$name"; then
+      # Checked last because it is the expensive one and the rarest, and because
+      # a name failing any check above is going to be re-issued anyway.
+      need="the certificate it serves is marked CA:TRUE, which no client can safely trust"
     fi
   fi
 

tests/reconcile-ca-trigger.test.ts

@@ -0,0 +1,128 @@
+// The repair trigger a pin comparison cannot see.
+//
+// `moshpit-reconcile.sh` re-runs setup-origin.sh when a name's nginx block is
+// missing or the served key is not the published one. Neither fires for the
+// case that actually left every stock client refusing these names: a
+// certificate marked CA:TRUE. It is the right key — the pin matches — and it is
+// unusable, because an anchor marked CA:TRUE may issue for any name and so
+// cannot be trusted directly.
+//
+// The repair reuses the key, so the pin does not move. That is what makes it
+// free, and exactly why the existing checks stay silent: a box pulls the fix
+// and then reconciles contentedly forever without applying it.
+//
+// The function is lifted out of the script and run against a real TLS server,
+// rather than restated here — a restated pipeline would pass while the script
+// probed something else entirely.
+
+import { describe, test } from "node:test";
+import assert from "node:assert/strict";
+import { execFile } from "node:child_process";
+import { readFile, writeFile } from "node:fs/promises";
+import { createServer } from "node:tls";
+import { join } from "node:path";
+import { fileURLToPath } from "node:url";
+import { promisify } from "node:util";
+import { tempDir } from "./helpers.ts";
+
+const run = promisify(execFile);
+const script = fileURLToPath(new URL("../scripts/moshpit-reconcile.sh", import.meta.url));
+
+/** A self-signed certificate for `name`, as a CA or as a plain leaf. */
+async function certificate(dir: string, name: string, { ca }: { ca: boolean }) {
+  const certPath = join(dir, `${name}-${ca}.crt`);
+  const keyPath = join(dir, `${name}-${ca}.key`);
+  await run("openssl", [
+    "req", "-x509", "-new", "-nodes",
+    "-newkey", "ec", "-pkeyopt", "ec_paramgen_curve:prime256v1",
+    "-sha256", "-days", "1", "-subj", `/CN=${name}`,
+    "-addext", `subjectAltName=DNS:${name}`,
+    "-addext", `basicConstraints=critical,CA:${ca ? "TRUE" : "FALSE"}`,
+    "-keyout", keyPath, "-out", certPath,
+  ]);
+  return { cert: await readFile(certPath), key: await readFile(keyPath) };
+}
+
+/**
+ * Serve a certificate on an ephemeral loopback port for the duration of a test.
+ *
+ * The socket is destroyed rather than ended, and every connection is dropped
+ * before close. `openssl s_client` does not hang up when the server half-closes
+ * — it waits for a close_notify it is never sent — so `server.close()` sits
+ * waiting for a connection that is waiting for it, and the test run hangs with
+ * no failure and no output.
+ */
+async function origin(t: any, creds: { cert: Buffer; key: Buffer }) {
+  const server = createServer({ cert: creds.cert, key: creds.key }, (socket) => socket.destroy());
+  await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", () => resolve()));
+  t.after(() => new Promise<void>((resolve) => {
+    server.closeAllConnections?.();
+    server.close(() => resolve());
+  }));
+  return `127.0.0.1:${(server.address() as { port: number }).port}`;
+}
+
+/**
+ * Run the script's own `served_is_ca` against an address.
+ *
+ * Extracted from the file so the test cannot drift from what runs in
+ * production: if the probe changes, this runs the changed one.
+ */
+async function servedIsCa(addr: string, name: string): Promise<boolean> {
+  const body = await readFile(script, "utf8");
+  const fn = /^served_is_ca\(\) \{\n[\s\S]*?^\}/m.exec(body);
+  assert.ok(fn, "served_is_ca is no longer defined in moshpit-reconcile.sh");
+
+  const dir = await tempDir("reconcile-probe-");
+  const runner = join(dir, "probe.sh");
+  await writeFile(runner, `#!/bin/sh\nset -eu\nORIGIN_ADDR="$1"\n${fn[0]}\nserved_is_ca "$2"\n`);
+
+  try {
+    await run("sh", [runner, addr, name]);
+    return true;
+  } catch {
+    return false;   // grep -q found no CA:TRUE, so the function exited non-zero
+  }
+}
+
+describe("reconcile — the CA:TRUE repair trigger", () => {
+  test("a certificate marked CA:TRUE is spotted", async (t) => {
+    const dir = await tempDir();
+    const addr = await origin(t, await certificate(dir, "chovy.hacker", { ca: true }));
+    assert.equal(await servedIsCa(addr, "chovy.hacker"), true);
+  });
+
+  test("the certificate the fix issues is not", async (t) => {
+    // The other half: if this said yes, reconcile would re-issue on every pass
+    // and reload nginx once a minute forever — the failure the script's own
+    // header warns about.
+    const dir = await tempDir();
+    const addr = await origin(t, await certificate(dir, "chovy.hacker", { ca: false }));
+    assert.equal(await servedIsCa(addr, "chovy.hacker"), false);
+  });
+
+  test("nothing listening is not a CA, so an unreachable origin is not re-issued forever", async () => {
+    // Port 1 on loopback: reserved, never bound. A probe that cannot connect
+    // must not report the dangerous shape — "no answer" is already covered by
+    // the served-pin check above it, which produces a better message.
+    assert.equal(await servedIsCa("127.0.0.1:1", "chovy.hacker"), false);
+  });
+});
+
+describe("reconcile — the trigger is actually wired in", () => {
+  test("served_is_ca is consulted in the decision chain, not merely defined", async () => {
+    // A helper nothing calls is the most plausible way this regresses: the
+    // tests above would still pass with the elif deleted.
+    const body = await readFile(script, "utf8");
+    assert.match(body, /elif served_is_ca "\$name"; then/);
+    assert.match(body, /need="the certificate it serves is marked CA:TRUE/);
+  });
+
+  test("the probe address is what both certificate checks use", async () => {
+    const body = await readFile(script, "utf8");
+    // Hardcoding 127.0.0.1:443 back into either probe would make them
+    // untestable and silently un-run here.
+    assert.doesNotMatch(body, /-connect "127\.0\.0\.1:443"/);
+    assert.equal((body.match(/-connect "\$ORIGIN_ADDR"/g) || []).length, 2);
+  });
+});