fix(site): one key per name, not one per ending (#253)

`keyPaths` keyed certificates off the TLD, justified as "that is the
granularity the registry stores". It is the opposite of what the registry
stores. Migration 009 says so, and says why:

  Per name rather than per TLD, and that is forced by 008: names under a
  TLD are sold, so `blue.eggs` can belong to someone who does not own
  `.eggs`. Hanging keys off the TLD would let its operator publish a key
  for a name they already sold.

A shared per-ending key is that hole in private-key form. The ending's
operator holds the key for every name they sold, and every buyer holds a
key that signs for every other buyer. 009 closed it at the pin layer;
this reintroduced it one layer down, where it is worse — a pin can be
withdrawn, a distributed private key cannot.

The certificate carried `DNS:*.<tld>` for the same reason, so each
buyer's certificate asserted authority over every other name in a
namespace they merely bought into. Now `DNS:<name>` alone.

Found because it broke nginx on a live box: `moshcode site` wrote
conf.d blocks pointing at /etc/ssl/moshpit/hacker.crt while the
certificates on disk were per-name, so `nginx -t` failed with three
missing files and the box could not reload. Per-name paths agree with
what setup-origin.sh has written all along.

Path sanitising now keeps dots, so `alt.2600` stays `alt.2600.crt`.
Runs of dots collapse to one, so no `..` survives to mean "parent":
`../../etc/passwd` becomes `etcpasswd`, a harmless filename inside the
key directory.

841 tests pass.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: ralyodio

src/pins.mjs

@@ -49,15 +49,36 @@ export function pinFromCertificate(certPem) {
 }
 
 /**
- * Where a TLD's key lives.
+ * Where a name's key lives.
  *
- * One key per ending, not per name, because that is the granularity the
- * registry stores. Giving each name its own key would mean a new pin published
- * per site, and every name under the ending would then accept all of them —
- * strictly more keys able to impersonate each other, for no isolation gained.
+ * One key per name, not per ending. This used to key off the TLD, on the
+ * belief that "that is the granularity the registry stores" — which is the
+ * opposite of true. Migration 009 is explicit about it, and about why:
+ *
+ *   Per name rather than per TLD, and that is forced by 008: names under a TLD
+ *   are sold, so `blue.eggs` can belong to someone who does not own `.eggs`.
+ *   Hanging keys off the TLD would let its operator publish a key for a name
+ *   they already sold — impersonating a buyer inside the namespace they bought
+ *   into.
+ *
+ * A shared per-ending key is that hole in private-key form: the ending's
+ * operator holds the key for every name they have sold, and every buyer holds
+ * a key that signs for every other buyer. Per-name keys make a compromise stop
+ * at one site.
+ *
+ * The dot is kept — `chovy.hacker.crt`, not `chovyhacker.crt` — which also
+ * matches the certificates setup-origin.sh has been writing all along.
+ * Separators that could climb out of `dir` are dropped rather than escaped:
+ * `../../etc/passwd` collapses to `etcpasswd`, which is a harmless filename
+ * inside the directory rather than a path anywhere else.
  */
-export function keyPaths(tld, dir = "/etc/ssl/moshpit") {
-  const safe = String(tld ?? "").toLowerCase().replace(/[^a-z0-9]/g, "");
+export function keyPaths(name, dir = "/etc/ssl/moshpit") {
+  const safe = String(name ?? "")
+    .toLowerCase()
+    .replace(/[^a-z0-9.-]/g, "")
+    // Any run of dots becomes one, so no `..` survives to mean "parent".
+    .replace(/\.{2,}/g, ".")
+    .replace(/^[.-]+|[.-]+$/g, "");
   if (!safe) return null;
   return { key: `${dir}/${safe}.key`, cert: `${dir}/${safe}.crt`, dir };
 }
@@ -80,9 +101,12 @@ export function certificateCommand({ name, tld, paths, days = 3650 }) {
       "-newkey", "ec", "-pkeyopt", "ec_paramgen_curve:prime256v1",
       "-keyout", paths.key, "-out", paths.cert,
       "-days", String(days), "-subj", subject,
-      // Every name under the ending, since they share this key. Browsers and
+      // This name and nothing else. It used to carry `DNS:*.${tld}` as well,
+      // on the assumption that every name under the ending shared one key —
+      // which would have each buyer's certificate assert authority over every
+      // other name in a namespace they merely bought into. Browsers and
       // pin-checking clients both read SAN, not CN.
-      "-addext", `subjectAltName=DNS:${name},DNS:*.${tld}`,
+      "-addext", `subjectAltName=DNS:${name}`,
     ],
   };
 }

src/serve.mjs

@@ -253,7 +253,7 @@ export function caddySite({ name, root, proxy }) {
 export async function tlsFor(name, { dir = "/etc/ssl/moshpit", readFile = fs.readFile } = {}) {
   const tld = tldOf(name);
   if (!tld) return null;
-  const paths = keyPaths(tld, dir);
+  const paths = keyPaths(name, dir);
   if (!paths) return null;
 
   try {

test/pins.test.mjs

@@ -41,32 +41,48 @@ test("pins hang off the ending, not the name", () => {
   assert.equal(tldOf("SEO.RANK"), "rank", "case is not part of the identity");
 });
 
-test("one key per ending, because that is the granularity the registry stores", () => {
-  const paths = keyPaths("hacker");
-  assert.match(paths.key, /hacker\.key$/);
-  assert.match(paths.cert, /hacker\.crt$/);
-
-  // Per-name keys would mean a pin published per site, and every name under
-  // the ending would then accept all of them — more keys able to impersonate
-  // each other, for no isolation gained.
-  assert.deepEqual(keyPaths("hacker"), keyPaths("hacker"));
+test("one key per name, because a name can belong to someone who does not own the ending", () => {
+  const paths = keyPaths("chovy.hacker");
+  assert.match(paths.key, /\/chovy\.hacker\.key$/);
+  assert.match(paths.cert, /\/chovy\.hacker\.crt$/);
+  assert.deepEqual(keyPaths("chovy.hacker"), keyPaths("chovy.hacker"));
 });
 
-test("a path traversal in the ending cannot escape the key directory", () => {
-  // The ending reaches this from a registry response, so it is not trusted
+test("two names under one ending do not share a key", () => {
+  // This is the whole point. Names under a TLD are sold, so a shared key means
+  // the ending's operator holds the private key for every name they sold, and
+  // every buyer holds a key that signs for every other buyer. Migration 009
+  // exists to stop exactly that at the pin layer; keying certificates off the
+  // TLD reintroduced it one layer down.
+  assert.notEqual(keyPaths("chovy.hacker").key, keyPaths("auto.hacker").key);
+  assert.notEqual(keyPaths("chovy.hacker").cert, keyPaths("auto.hacker").cert);
+});
+
+test("a numeric ending keeps its digits", () => {
+  // `.2600` is a registered ending. Stripping to alphanumerics used to be
+  // harmless here; now that the label is included it must not eat the dot.
+  assert.match(keyPaths("alt.2600").cert, /\/alt\.2600\.crt$/);
+});
+
+test("a path traversal in the name cannot escape the key directory", () => {
+  // The name reaches this from a registry response, so it is not trusted
   // input. Writing a key through `../../` would be a very bad day.
   assert.match(keyPaths("../../etc/passwd").key, /\/etcpasswd\.key$/);
+  assert.match(keyPaths("a/../../b.hacker").key, /\/a\.b\.hacker\.key$/);
   assert.equal(keyPaths("..."), null, "nothing usable left after stripping");
   assert.equal(keyPaths(""), null);
 });
 
-test("the certificate covers every name under the ending, since they share the key", () => {
-  const paths = keyPaths("hacker");
+test("the certificate covers this name and nothing else", () => {
+  const paths = keyPaths("chovy.hacker");
   const { args } = certificateCommand({ name: "chovy.hacker", tld: "hacker", paths });
   const san = args[args.indexOf("-addext") + 1];
 
   assert.match(san, /DNS:chovy\.hacker/);
-  assert.match(san, /DNS:\*\.hacker/, "the other names under this ending present the same key");
+  // A wildcard here would have each buyer's certificate assert authority over
+  // every other name under an ending they merely bought into — the same hole
+  // migration 009 closed at the pin layer.
+  assert.doesNotMatch(san, /\*/, "no name may vouch for its neighbours");
   assert.match(args.join(" "), /prime256v1/, "P-256, matching what is already deployed");
 });