fix: Reject JWT tokens minted for a different registry

README.md

@@ -66,6 +66,19 @@ Set the USERNAME and PASSWORD as secrets with `npx wrangler secret put USERNAME
 You can add a base64 encoded JWT public key to verify passwords (or token) that are signed by the private key.
 `npx wrangler secret put JWT_REGISTRY_TOKENS_PUBLIC_KEY --env production`
 
+Tokens are bound to a single registry. Every token must carry an `aud` claim naming
+the registry it is for, and a request is rejected with `401` unless `aud` matches the
+host it arrived on. `createToken()` sets this from its `registryUrl` argument. Only
+host and port are compared, so `https://registry.example`, `http://registry.example`
+and `registry.example` are equivalent, but `registry.example:8787` is a different
+registry from `registry.example`.
+
+**Give each deployment its own key pair.** This registry has no per-account or
+per-repository scoping: any token that verifies grants the full extent of its
+capabilities over the whole registry. Deployments sharing a
+`JWT_REGISTRY_TOKENS_PUBLIC_KEY` therefore form one trust domain, and the `aud` check
+is all that separates them.
+
 ### Using with Docker
 
 You can use this registry with Docker to push and pull images.

src/token.ts

@@ -7,6 +7,53 @@ import {
 } from "./auth";
 import { base64UrlDecode } from "./utils";
 
+/**
+ * Reduces an audience value to a canonical `host[:port]` string for comparison.
+ *
+ * Accepts the two spellings that are in use:
+ *   - a full origin, e.g. "https://registry.example", which is what
+ *     `createToken()` documents and what its `registryUrl` argument implies
+ *   - a bare host, e.g. "registry.example" or "registry.example:8787"
+ *
+ * The scheme is deliberately ignored. Two registries are distinguished by host,
+ * not by whether a given request arrived over http or https, so comparing hosts
+ * is what the cross-registry threat model actually calls for and it keeps local
+ * http development working against tokens minted with an https audience.
+ *
+ * Returns null when the value is empty or cannot be read as either spelling.
+ * Callers must treat null as a verification failure.
+ */
+function normalizeAudience(value: string): string | null {
+  const trimmed = value.trim();
+  if (trimmed === "") {
+    return null;
+  }
+
+  // Absolute URL form.
+  if (/^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed)) {
+    try {
+      const { host } = new URL(trimmed);
+      return host === "" ? null : host.toLowerCase();
+    } catch {
+      return null;
+    }
+  }
+
+  // Bare host form. Reject anything carrying a path, query, fragment, or
+  // userinfo so that a malformed audience can never normalize onto another
+  // host (e.g. "registry-a.example@registry-b.example").
+  if (/[/?#@\\]/.test(trimmed)) {
+    return null;
+  }
+
+  try {
+    const { host } = new URL(`https://${trimmed}`);
+    return host === "" ? null : host.toLowerCase();
+  } catch {
+    return null;
+  }
+}
+
 export function importKeyFromBase64(key: string): JsonWebKeyWithKid {
   // Decodes the base64 value and performs unicode normalization.
   // The library's `JsonWebKeyWithKid` type requires `kid`, but ES256/HS256 sign
@@ -52,6 +99,12 @@ export class RegistryTokens implements Authenticator {
     return [exportedPrivateKey, exportedPublicKey];
   }
 
+  /**
+   * @param registryUrl The registry this token may be used against, recorded as
+   * the `aud` claim and enforced by {@link RegistryTokens.verifyAudience}. Accepts
+   * an origin ("https://registry.example") or a bare host ("registry.example:8787");
+   * only host and port are compared, scheme and path are ignored.
+   */
   async createToken(
     accountID: string,
     caps: RegistryTokenCapability[],
@@ -97,6 +150,14 @@ export class RegistryTokens implements Authenticator {
       // the JWT signature is valid, decode it now
       const decoded = jwt.decode(token);
       const payload = decoded.payload as RegistryAuthProtocolTokenPayload;
+
+      // A valid signature only proves the token came from a trusted issuer. It
+      // says nothing about which registry the issuer minted it for, so bind the
+      // token to this registry before honouring any of its capabilities.
+      if (!RegistryTokens.verifyAudience(request, payload)) {
+        return { verified: false, payload: null };
+      }
+
       return RegistryTokens.verifyPayload(request, payload);
     } catch (error) {
       // If the verification fails (e.g., due to token expiration or signature mismatch),
@@ -109,6 +170,47 @@ export class RegistryTokens implements Authenticator {
     }
   }
 
+  /**
+   * Checks that this token was minted for this registry.
+   *
+   * `createToken()` records the target registry in the `aud` claim. Without
+   * this check, every registry that trusts the same `JWT_REGISTRY_TOKENS_PUBLIC_KEY`
+   * accepts tokens minted for any of the others, so a token holder on one
+   * registry can cross into another (for example dev into prod). This registry
+   * applies no per-account or per-repository scoping, so such a token grants
+   * the full extent of its capabilities against the whole target registry.
+   *
+   * Tokens without a usable `aud` are rejected: `aud` is a required field of
+   * RegistryAuthProtocolTokenPayload and is always set by `createToken()`, so
+   * accepting tokens that omit it would leave the bypass permanently open.
+   */
+  static verifyAudience(request: Request, payload: RegistryAuthProtocolTokenPayload): boolean {
+    // Guard the type at runtime: RFC 7519 also permits `aud` to be an array,
+    // which this registry does not issue and does not accept.
+    if (typeof payload.aud !== "string") {
+      console.warn("verifyToken: failed jwt verification: token is missing a string 'aud' claim");
+      return false;
+    }
+
+    const audience = normalizeAudience(payload.aud);
+    if (audience === null) {
+      console.warn("verifyToken: failed jwt verification: token 'aud' claim is not a usable registry host");
+      return false;
+    }
+
+    const expected = new URL(request.url).host.toLowerCase();
+    if (audience !== expected) {
+      // Neither value is secret, and a mismatch is usually a misconfigured
+      // issuer, so log both to make that diagnosable.
+      console.warn(
+        `verifyToken: failed jwt verification: token audience "${audience}" does not match this registry "${expected}"`,
+      );
+      return false;
+    }
+
+    return true;
+  }
+
   static verifyPayload(request: Request, payload: RegistryAuthProtocolTokenPayload) {
     // Check if token has expired
     const now = Math.floor(Date.now() / 1000);

test/index.test.ts

@@ -2,8 +2,8 @@ import { afterEach, describe, expect, test, vi } from "vitest";
 import { SHA256_PREFIX_LEN, getSHA256 } from "../src/user";
 import { TagsList } from "../src/router";
 import { Env } from "..";
-import { RegistryTokens } from "../src/token";
-import { RegistryAuthProtocolTokenPayload } from "../src/auth";
+import { newRegistryTokens, RegistryTokens } from "../src/token";
+import { RegistryAuthProtocolTokenPayload, RegistryTokenCapability } from "../src/auth";
 import { registries } from "../src/registry/registry";
 import type { ReferrerDescriptor } from "../src/registry/registry";
 import { isDockerDotIO, RegistryHTTPClient } from "../src/registry/http";
@@ -1282,6 +1282,154 @@ describe("tokens", async () => {
   });
 });
 
+// A token carries the registry it was minted for in its `aud` claim. Two
+// deployments that trust the same JWT_REGISTRY_TOKENS_PUBLIC_KEY must not
+// accept each other's tokens, otherwise a token holder on one registry can
+// read (or with "push", overwrite) content on the other.
+describe("token audience binding", async () => {
+  const registryA = "registry-a.example";
+  const registryB = "registry-b.example";
+
+  async function mintToken(
+    audience: string,
+    capabilities: RegistryTokenCapability[] = ["pull"],
+  ): Promise<{ token: string; publicKey: string }> {
+    const [privateKey, publicKey] = await RegistryTokens.createPrivateAndPublicKey();
+    const tokens = await newRegistryTokens(publicKey);
+    const token = await tokens.createToken("some-account-id", capabilities, 30, privateKey, audience);
+    return { token, publicKey };
+  }
+
+  // authenticationMethodFromEnv prefers the JWT authenticator, but clearing the
+  // basic-auth vars keeps the intent of these tests unambiguous.
+  function jwtEnv(publicKey: string): Env {
+    return {
+      ...(env as Env),
+      JWT_REGISTRY_TOKENS_PUBLIC_KEY: publicKey,
+      USERNAME: undefined,
+      PASSWORD: undefined,
+      READONLY_USERNAME: undefined,
+      READONLY_PASSWORD: undefined,
+    };
+  }
+
+  // Docker sends the token in the Basic-auth password field, which is exactly
+  // how this registry expects to receive it.
+  async function fetchWithToken(host: string, path: string, token: string, publicKey: string): Promise<Response> {
+    const request = new Request(new URL(`https://${host}${path}`), {
+      method: "GET",
+      headers: { Authorization: usernamePasswordToAuth("v0", token) },
+    });
+    const ctx = createExecutionContext();
+    const res = await worker.fetch(request, jwtEnv(publicKey), ctx);
+    await waitOnExecutionContext(ctx);
+    return res as Response;
+  }
+
+  test("a token replayed against another registry cannot read a manifest", async () => {
+    // This registry applies no per-account or per-host key prefixing, so the
+    // manifest seeded here is reachable from any host that serves this bucket.
+    // The audience claim is the only thing standing between the two registries.
+    const name = "victim/app";
+    await createManifest(name, await generateManifest(name), "latest");
+
+    const { token, publicKey } = await mintToken(`https://${registryA}`);
+
+    // Control: the token is otherwise entirely valid, and the manifest is
+    // readable, on the registry the token names.
+    const authorized = await fetchWithToken(registryA, `/v2/${name}/manifests/latest`, token, publicKey);
+    expect(authorized.status).toBe(200);
+
+    // Replay: the same token against a second registry trusting the same key.
+    // Before the audience check this returned 200 and leaked the manifest.
+    const replayed = await fetchWithToken(registryB, `/v2/${name}/manifests/latest`, token, publicKey);
+    expect(replayed.status).toBe(401);
+  });
+
+  test("a token replayed against another registry cannot reach the API root", async () => {
+    const { token, publicKey } = await mintToken(`https://${registryA}`);
+
+    const authorized = await fetchWithToken(registryA, "/v2/", token, publicKey);
+    expect(authorized.status).toBe(200);
+
+    // /v2/ short-circuits the capability checks, so it needs its own coverage:
+    // the audience must be enforced before that short-circuit is reached.
+    const replayed = await fetchWithToken(registryB, "/v2/", token, publicKey);
+    expect(replayed.status).toBe(401);
+  });
+
+  test("a push token replayed against another registry cannot write", async () => {
+    const { token, publicKey } = await mintToken(`https://${registryA}`, ["pull", "push"]);
+
+    const request = new Request(new URL(`https://${registryB}/v2/victim/app/blobs/uploads/`), {
+      method: "POST",
+      headers: { Authorization: usernamePasswordToAuth("v0", token) },
+    });
+    const ctx = createExecutionContext();
+    const res = (await worker.fetch(request, jwtEnv(publicKey), ctx)) as Response;
+    await waitOnExecutionContext(ctx);
+
+    expect(res.status).toBe(401);
+  });
+
+  test("a token is rejected when the audience host matches but the port does not", async () => {
+    const { token, publicKey } = await mintToken(`https://${registryA}:8787`);
+
+    const matching = await fetchWithToken(`${registryA}:8787`, "/v2/", token, publicKey);
+    expect(matching.status).toBe(200);
+
+    const mismatched = await fetchWithToken(`${registryA}:9999`, "/v2/", token, publicKey);
+    expect(mismatched.status).toBe(401);
+  });
+
+  test("verifyAudience accepts the spellings an issuer may reasonably use", async () => {
+    const request = new Request("https://registry.example/v2/");
+    for (const audience of [
+      "https://registry.example",
+      "https://registry.example/",
+      "http://registry.example",
+      "registry.example",
+      "  https://registry.example  ",
+      "https://REGISTRY.example",
+    ]) {
+      expect(
+        RegistryTokens.verifyAudience(request, { aud: audience } as RegistryAuthProtocolTokenPayload),
+        `expected audience ${JSON.stringify(audience)} to be accepted`,
+      ).toBe(true);
+    }
+  });
+
+  test("verifyAudience rejects a missing, unusable, or foreign audience", async () => {
+    const request = new Request("https://registry.example/v2/");
+    for (const audience of [
+      undefined,
+      "",
+      "   ",
+      // An array audience is legal per RFC 7519 but is not issued or accepted here.
+      ["https://registry.example"],
+      "https://registry-b.example",
+      "registry-b.example",
+      // Must not be fooled into reading a foreign host as the audience.
+      "registry.example@registry-b.example",
+      "https://registry-b.example/registry.example",
+      "https://registry.example.evil.test",
+      "registry.example:8787",
+    ]) {
+      expect(
+        RegistryTokens.verifyAudience(request, { aud: audience } as unknown as RegistryAuthProtocolTokenPayload),
+        `expected audience ${JSON.stringify(audience)} to be rejected`,
+      ).toBe(false);
+    }
+  });
+
+  test("username and password authentication is unaffected", async () => {
+    // user.ts reuses verifyPayload for its capability checks and has no
+    // audience of its own, so the new check must not reach it.
+    const res = await fetch(createRequest("GET", "/v2/", null));
+    expect(res.status).toBe(200);
+  });
+});
+
 test("registries configuration", async () => {
   const testCases = [
     {