diff --git a/README.md b/README.md index b468ea0..d6759c8 100644 --- a/README.md +++ b/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. diff --git a/src/token.ts b/src/token.ts index a0f82c3..0571756 100644 --- a/src/token.ts +++ b/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); diff --git a/test/index.test.ts b/test/index.test.ts index fffa61d..b191639 100644 --- a/test/index.test.ts +++ b/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 { + 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 = [ {