fix: enforce full SAN set, reject punycode labels, normalize IPv6 classification, rate-limit name updates

Author: samgbafa

src/ip.test.ts

@@ -30,3 +30,31 @@ test("rejects public IPv6 and malformed addresses", () => {
   assert.equal(isPrivateAddress("not-an-ip"), false);
   assert.equal(isPrivateAddress(""), false);
 });
+
+test("accepts expanded IPv6 spellings of private addresses", () => {
+  assert.equal(isPrivateAddress("0:0:0:0:0:0:0:1"), true); // expanded loopback
+  assert.equal(isPrivateAddress("fe80:0:0:0:0:0:0:1"), true);
+  assert.equal(isPrivateAddress("fd00:0000:0000:0000:0000:0000:0000:0001"), true);
+});
+
+test("classifies IPv4-mapped IPv6 by the embedded IPv4", () => {
+  assert.equal(isPrivateAddress("::ffff:192.168.1.1"), true);
+  assert.equal(isPrivateAddress("::ffff:10.0.0.5"), true);
+  assert.equal(isPrivateAddress("::ffff:8.8.8.8"), false);
+});
+
+test("classifies NAT64-embedded IPv6 by the embedded IPv4", () => {
+  assert.equal(isPrivateAddress("64:ff9b::192.168.1.1"), true);
+  assert.equal(isPrivateAddress("64:ff9b::c0a8:101"), true); // 192.168.1.1 in hex groups
+  assert.equal(isPrivateAddress("64:ff9b::8.8.8.8"), false);
+});
+
+test("classifies 6to4 IPv6 by the embedded IPv4", () => {
+  assert.equal(isPrivateAddress("2002:c0a8:101::1"), true); // embeds 192.168.1.1
+  assert.equal(isPrivateAddress("2002:808:808::1"), false); // embeds 8.8.8.8
+});
+
+test("fails closed on ambiguous IPv6 forms", () => {
+  assert.equal(isPrivateAddress("::"), false); // unspecified
+  assert.equal(isPrivateAddress("::ffff:0:8.8.8.8"), false); // not the mapped /96
+});

src/ip.ts

@@ -23,9 +23,64 @@ function isPrivateIPv4(address: string): boolean {
 }
 
 function isPrivateIPv6(address: string): boolean {
-  const normalized = address.toLowerCase();
-  if (normalized === "::1") return true; // loopback
-  if (normalized.startsWith("fe80:")) return true; // fe80::/10 link-local
-  if (/^f[cd][0-9a-f]{2}:/.test(normalized)) return true; // fc00::/7 unique local
+  // Normalize to the full 8x16-bit group form so shorthand ("::"), expanded,
+  // and IPv4-embedded spellings all classify identically. Fail closed on
+  // anything that does not normalize cleanly.
+  const groups = expandIPv6(address);
+  if (!groups) return false;
+
+  if (groups.slice(0, 7).every((g) => g === 0) && groups[7] === 1) return true; // ::1 loopback
+  if (groups.slice(0, 5).every((g) => g === 0) && groups[5] === 0xffff) {
+    // ::ffff:a.b.c.d IPv4-mapped -- classify by the embedded IPv4
+    return isPrivateIPv4(embeddedIPv4(groups[6], groups[7]));
+  }
+  if (
+    groups[0] === 0x0064 &&
+    groups[1] === 0xff9b &&
+    groups.slice(2, 6).every((g) => g === 0)
+  ) {
+    // 64:ff9b::/96 NAT64 -- classify by the embedded IPv4
+    return isPrivateIPv4(embeddedIPv4(groups[6], groups[7]));
+  }
+  if (groups[0] === 0x2002) {
+    // 2002::/16 6to4 -- classify by the embedded IPv4
+    return isPrivateIPv4(embeddedIPv4(groups[1], groups[2]));
+  }
+  if ((groups[0] & 0xffc0) === 0xfe80) return true; // fe80::/10 link-local
+  if ((groups[0] & 0xfe00) === 0xfc00) return true; // fc00::/7 unique local
   return false;
 }
+
+/** Expands a valid IPv6 address into its 8 16-bit groups, or null if it cannot. */
+function expandIPv6(address: string): number[] | null {
+  let addr = address.toLowerCase();
+
+  // Rewrite a trailing dotted-quad (e.g. "::ffff:192.168.1.1") as two hex groups.
+  const v4Match = addr.match(/^(.*:)(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/);
+  if (v4Match) {
+    const octets = v4Match[2].split(".").map(Number);
+    if (octets.some((o) => o > 255)) return null;
+    const hi = ((octets[0] << 8) | octets[1]).toString(16);
+    const lo = ((octets[2] << 8) | octets[3]).toString(16);
+    addr = `${v4Match[1]}${hi}:${lo}`;
+  }
+
+  const halves = addr.split("::");
+  if (halves.length > 2) return null;
+  const head = halves[0] ? halves[0].split(":") : [];
+  const tail = halves.length === 2 && halves[1] ? halves[1].split(":") : [];
+  const missing = 8 - head.length - tail.length;
+  if (halves.length === 2 ? missing < 0 : head.length !== 8) return null;
+  const parts = halves.length === 2 ? [...head, ...Array(missing).fill("0"), ...tail] : head;
+
+  const groups: number[] = [];
+  for (const part of parts) {
+    if (!/^[0-9a-f]{1,4}$/.test(part)) return null;
+    groups.push(Number.parseInt(part, 16));
+  }
+  return groups.length === 8 ? groups : null;
+}
+
+function embeddedIPv4(hi: number, lo: number): string {
+  return `${hi >> 8}.${hi & 0xff}.${lo >> 8}.${lo & 0xff}`;
+}

src/names.test.ts

@@ -10,6 +10,7 @@ import {
   validateCertRequest,
   validateNameClaim,
   validateNameDelete,
+  validateNameLabel,
   verifyCertRequest,
   verifyNameClaim,
   verifyNameDelete,
@@ -157,6 +158,23 @@ test("rejects names with invalid dns characters", () => {
   );
 });
 
+test("rejects punycode (xn--) labels", () => {
+  assert.throws(() => validateNameLabel("xn--nxasmq6b"), /punycode/);
+  assert.throws(
+    () =>
+      validateNameClaim({
+        version: 1,
+        action: "claim",
+        name: "xn--nxasmq6b",
+        subject: "did:key:z6MkiFake",
+        lanIps: ["10.0.0.1"],
+        sequence: 1,
+        signature: "sig",
+      }),
+    /punycode/
+  );
+});
+
 test("rejects reserved names", () => {
   for (const reserved of ["api", "www", "admin", "acme"]) {
     assert.throws(
@@ -259,3 +277,9 @@ test("csr domain check rejects a csr with extra SAN entries", () => {
   const csr = createTestCsr(domain, [domain, "evil.example.com"]);
   assert.throws(() => assertCsrMatchesDomain(csr, domain), NameError);
 });
+
+test("csr domain check rejects a csr with a non-dNSName SAN entry", () => {
+  const domain = fqdnForName("mynode");
+  const csr = createTestCsr(domain, [domain, { type: 7, ip: "8.8.8.8" }]); // iPAddress
+  assert.throws(() => assertCsrMatchesDomain(csr, domain), /only a dNSName entry/);
+});

src/names.ts

@@ -61,6 +61,9 @@ export function validateNameLabel(name: unknown): string {
       "name must be a dns-safe label (lowercase letters, digits, hyphens; cannot start or end with a hyphen)"
     );
   }
+  if (lower.startsWith("xn--")) {
+    throw new NameError('name must not be a punycode ("xn--") label');
+  }
   if (RESERVED_NAMES.has(lower)) {
     throw new NameError(`name "${lower}" is reserved`);
   }
@@ -263,8 +266,9 @@ export async function verifyCertRequest(record: CertRequestRecord): Promise<bool
 }
 
 /**
- * Enforces that the CSR's CN and every SAN dNSName entry is exactly the
- * expected <name>.local.tinycloud.link domain -- nothing broader, nothing else.
+ * Enforces that the CSR's CN and its complete SAN set is exactly the
+ * expected <name>.local.tinycloud.link domain as a single dNSName entry --
+ * nothing broader, nothing else, no SAN entries of any other type.
  */
 export function assertCsrMatchesDomain(csrPem: string, expectedDomain: string): void {
   let csr: forge.pki.CertificateSigningRequest;
@@ -281,9 +285,16 @@ export function assertCsrMatchesDomain(csrPem: string, expectedDomain: string):
   const sanExtension = (extensionRequest?.extensions ?? []).find(
     (ext: { name?: string }) => ext.name === "subjectAltName"
   ) as { altNames?: Array<{ type: number; value: string }> } | undefined;
-  const sanNames = (sanExtension?.altNames ?? [])
-    .filter((entry) => entry.type === 2) // dNSName
-    .map((entry) => entry.value);
+  const altNames = sanExtension?.altNames ?? [];
+  for (const entry of altNames) {
+    if (entry.type !== 2) {
+      // 2 = dNSName
+      throw new NameError(
+        `csr subjectAltName must contain only a dNSName entry for ${expectedDomain} (found an entry of type ${entry.type})`
+      );
+    }
+  }
+  const sanNames = altNames.map((entry) => entry.value);
 
   const names = new Set<string>([...(cn ? [cn] : []), ...sanNames]);
 

src/server.test.ts

@@ -15,7 +15,7 @@ import { FakeAcmeClient } from "./test-support/fake-acme-client.js";
 import { InMemoryCertRateLimiter, InMemoryNameStore } from "./test-support/memory-stores.js";
 import { didKeySigner, type Signer } from "./test-support/signing.js";
 
-function buildApp() {
+function buildApp(options: { nameUpdateRateLimitPerDay?: number } = {}) {
   const nameStore = new InMemoryNameStore();
   const dnsProvider = new InMemoryDnsProvider();
   const rateLimiter = new InMemoryCertRateLimiter();
@@ -33,6 +33,7 @@ function buildApp() {
     acmeIssuer,
     rateLimiter,
     certRateLimitPerDay: 2,
+    nameUpdateRateLimitPerDay: options.nameUpdateRateLimitPerDay,
   });
   return { app, dnsProvider };
 }
@@ -109,6 +110,19 @@ test("same subject can update lanIps with a higher sequence", async () => {
   assert.deepEqual(dnsProvider.addressRecords.get(fqdnForName("office")), ["192.168.1.77"]);
 });
 
+test("enforces the daily name update rate limit per name", async () => {
+  const { app, dnsProvider } = buildApp({ nameUpdateRateLimitPerDay: 2 });
+  const signer = didKeySigner(38);
+  const first = await claim(app, "hammered", signer, ["192.168.1.19"], 1);
+  const second = await claim(app, "hammered", signer, ["192.168.1.20"], 2);
+  const third = await claim(app, "hammered", signer, ["192.168.1.21"], 3);
+  assert.equal(first.status, 201);
+  assert.equal(second.status, 200);
+  assert.equal(third.status, 429);
+  // The DNS provider was not touched by the rate-limited update.
+  assert.deepEqual(dnsProvider.addressRecords.get(fqdnForName("hammered")), ["192.168.1.20"]);
+});
+
 test("rejects a stale sequence update", async () => {
   const { app } = buildApp();
   const signer = didKeySigner(24);

src/server.ts

@@ -24,11 +24,19 @@ export interface ServerConfig {
   rateLimiter: CertRateLimiter;
   attestationDocument?: string;
   certRateLimitPerDay?: number;
+  nameUpdateRateLimitPerDay?: number;
+}
+
+// Prefixes name-update entries so they share the cert rate-limit store without
+// colliding with cert issuances (claimed names can never contain a colon).
+function nameUpdateRateLimitKey(name: string): string {
+  return `name-update:${name}`;
 }
 
 export function createServer(config: ServerConfig): Hono {
   const app = new Hono();
   const rateLimit = config.certRateLimitPerDay ?? 5;
+  const nameUpdateRateLimit = config.nameUpdateRateLimitPerDay ?? 30;
 
   app.get("/health", (c) => c.json({ ok: true }));
 
@@ -80,6 +88,14 @@ export function createServer(config: ServerConfig): Hono {
       return c.json({ error: "invalid record signature" }, 401);
     }
 
+    const recentUpdates = await config.rateLimiter.countRecentIssuances(
+      nameUpdateRateLimitKey(claim.name),
+      DAY_MS
+    );
+    if (recentUpdates >= nameUpdateRateLimit) {
+      return c.json({ error: "name update rate limit exceeded for this name" }, 429);
+    }
+
     const status = await config.nameStore.put({
       name: claim.name,
       subject: claim.subject,
@@ -92,6 +108,7 @@ export function createServer(config: ServerConfig): Hono {
     }
 
     await config.dnsProvider.upsertAddressRecords(fqdnForName(claim.name), claim.lanIps);
+    await config.rateLimiter.recordIssuance(nameUpdateRateLimitKey(claim.name));
 
     return c.json(
       { name: claim.name, subject: claim.subject, lanIps: claim.lanIps, status },

src/test-support/csr.ts

@@ -1,7 +1,10 @@
 import forge from "node-forge";
 
+/** A SAN entry: a bare string becomes a dNSName; an object passes through to forge as-is. */
+export type TestAltName = string | { type: number; value?: string; ip?: string };
+
 /** Builds a real PKCS#10 CSR PEM for tests, without shelling out to openssl. */
-export function createTestCsr(commonName: string, altNames: string[] = [commonName]): string {
+export function createTestCsr(commonName: string, altNames: TestAltName[] = [commonName]): string {
   const keys = forge.pki.rsa.generateKeyPair(2048);
   const csr = forge.pki.createCertificationRequest();
   csr.publicKey = keys.publicKey;
@@ -12,7 +15,9 @@ export function createTestCsr(commonName: string, altNames: string[] = [commonNa
       extensions: [
         {
           name: "subjectAltName",
-          altNames: altNames.map((value) => ({ type: 2, value })),
+          altNames: altNames.map((entry) =>
+            typeof entry === "string" ? { type: 2, value: entry } : entry
+          ),
         },
       ],
     },