The image proxy accepts URLs signed with MASTER_KEY_PREV while MASTER_KEY rotates (#210)
* The image proxy accepts URLs signed with MASTER_KEY_PREV while MASTER_KEY rotates
4 files changed+40−15
Files (4)
modifiedapps/diffinery/docs/self-hosting/configuration.md+2−2View file
| @@ -200,12 +200,12 @@ | ||
| 200 | 200 | |
| 201 | 201 | - New and replaced secrets (repository, organization and environment secrets, model provider keys, webhook secrets, import tokens) are encrypted with the new key. Each stored value names the key it was encrypted with (`v2.<key id>.…`, where the key id is the first 8 hex characters of the key's SHA-256: `printf %s "$NEW_KEY" | base64 -d | sha256sum | cut -c1-8`). Values stored before key ids existed (`v1.…`) are tried with both keys. |
| 202 | 202 | - Stored values move to the new key by themselves as they are used: repository, organization and environment secrets the next time a workflow job uses them, model provider keys the next time an agent task does, webhook secrets the next time the webhook delivers, and the GitHub token of an import when the import is retried. A value nothing uses stays encrypted with the old key until someone saves it again. |
| 203 | -- Unsubscribe links, invitation and verification emails, sign-up codes, GitHub linking codes and OAuth state, runtime tokens, and cache and artifact URLs signed with the old key keep working; new ones are signed with the new key. | |
| 203 | +- Unsubscribe links, invitation and verification emails, sign-up codes, GitHub linking codes and OAuth state, runtime tokens, cache and artifact URLs, and image proxy URLs in pages already rendered signed with the old key keep working; new ones are signed with the new key. | |
| 204 | 204 | |
| 205 | 205 | Remove `MASTER_KEY_PREV` (`pnpm exec wrangler secret delete MASTER_KEY_PREV -c wrangler.deploy.jsonc`) only when nothing you still need depends on the old key: |
| 206 | 206 | |
| 207 | 207 | - Every repository, organization and environment secret and every model provider key you still need has been used or saved again since the rotation. Secrets your workflows use every day move over by themselves; one that only a rare workflow uses (a release, a yearly job) may not have, and its value can't be read back, so whoever manages it re-enters it. One left behind can't be decrypted once `MASTER_KEY_PREV` is gone, and runs that use it fail. |
| 208 | -- Links already sent have expired, or you accept that they stop working: runtime tokens and cache and artifact URLs last about as long as their job, sign-up codes 10 minutes, verification links 24 hours, invitation emails until the invitation expires, and unsubscribe links up to a year. Someone with an old unsubscribe link can still change their email settings after signing in. | |
| 208 | +- Links already sent have expired, or you accept that they stop working: runtime tokens and cache and artifact URLs last about as long as their job, sign-up codes 10 minutes, image proxy URLs until pages and browsers cache them no more (at most 7 days), verification links 24 hours, invitation emails until the invitation expires, and unsubscribe links up to a year. Someone with an old unsubscribe link can still change their email settings after signing in. | |
| 209 | 209 | |
| 210 | 210 | If the old key leaked, treat the secrets as exposed: replace their values at the source (new API tokens, new passwords) and save the new values, which are sealed with the new key. Then remove `MASTER_KEY_PREV` right away: until then, anyone holding the old key can still forge what it signs. |
| 211 | 211 |
modifiedapps/diffinery/src/http/camo.ts+2−3View file
| @@ -1,7 +1,7 @@ | ||
| 1 | 1 | import { Hono } from "hono"; |
| 2 | 2 | import type { Env } from "../env.js"; |
| 3 | 3 | import type { AppVars } from "../auth/principal.js"; |
| 4 | -import { camoHeaders, camoKey, fetchCamoImage, verifyCamo } from "../lib/camo.js"; | |
| 4 | +import { camoHeaders, camoKeys, fetchCamoImage, verifyCamo } from "../lib/camo.js"; | |
| 5 | 5 | |
| 6 | 6 | /** |
| 7 | 7 | * GET /-/camo/<signature>/<url, base64url>: an image from another site, fetched by Diffinery so the |
| @@ -25,8 +25,7 @@ | ||
| 25 | 25 | }); |
| 26 | 26 | |
| 27 | 27 | camo.get("/-/camo/:sig/:url", async (c) => { |
| 28 | - const key = camoKey(c.env); | |
| 29 | - const url = key ? verifyCamo(key, c.req.param("sig"), c.req.param("url")) : null; | |
| 28 | + const url = verifyCamo(camoKeys(c.env), c.req.param("sig"), c.req.param("url")); | |
| 30 | 29 | if (!url) return notFound(false); |
| 31 | 30 | |
| 32 | 31 | const cache = typeof caches === "undefined" ? null : (caches as unknown as { default: Cache }).default; |
modifiedapps/diffinery/src/lib/camo.ts+17−9View file
| @@ -1,6 +1,7 @@ | ||
| 1 | 1 | import type { Env } from "../env.js"; |
| 2 | 2 | import { localOnly } from "../auth/dev.js"; |
| 3 | 3 | import { checkPublicUrl } from "../imports/options.js"; |
| 4 | +import { masterKeys } from "./crypto.js"; | |
| 4 | 5 | import { Sha256 } from "./sha256.js"; |
| 5 | 6 | import { b64url, fromB64url, safeEqual } from "./util.js"; |
| 6 | 7 | |
| @@ -16,18 +17,25 @@ | ||
| 16 | 17 | * Only URLs Diffinery signed are fetched, so the route isn't an open proxy. |
| 17 | 18 | */ |
| 18 | 19 | |
| 19 | -type KeyEnv = Pick<Env, "MASTER_KEY" | "ACCESS_AUD" | "PUBLIC_URL">; | |
| 20 | +type KeyEnv = Pick<Env, "MASTER_KEY" | "MASTER_KEY_PREV" | "ACCESS_AUD" | "PUBLIC_URL">; | |
| 20 | 21 | |
| 21 | 22 | /** |
| 22 | - * The signing key, from MASTER_KEY under its own label. Kept in this one function so it can move | |
| 23 | - * to a shared derivation helper (and accept a previous key while MASTER_KEY rotates) in one place. | |
| 23 | + * The keys proxy URLs are checked with, from the master keys under their own label: the current | |
| 24 | + * one first (the one new URLs are signed with), then MASTER_KEY_PREV while MASTER_KEY rotates, so | |
| 25 | + * pages rendered and cached before a rotation keep their images (lib/crypto.ts masterKeys). | |
| 24 | 26 | */ |
| 25 | -export function camoKey(env: KeyEnv): Uint8Array | null { | |
| 27 | +export function camoKeys(env: KeyEnv): Uint8Array[] { | |
| 26 | 28 | // Local development has no MASTER_KEY unless one is passed; production always does. |
| 27 | - const secret = env.MASTER_KEY ?? (localOnly(env) ? "diffinery-dev-camo" : null); | |
| 28 | - return secret ? new TextEncoder().encode(`diffinery-camo-v1:${secret}`) : null; | |
| 29 | + const secrets = masterKeys(env); | |
| 30 | + if (!secrets.length && localOnly(env)) secrets.push("diffinery-dev-camo"); | |
| 31 | + return secrets.map((s) => new TextEncoder().encode(`diffinery-camo-v1:${s}`)); | |
| 29 | 32 | } |
| 30 | 33 | |
| 34 | +/** The key new proxy URLs are signed with. */ | |
| 35 | +export function camoKey(env: KeyEnv): Uint8Array | null { | |
| 36 | + return camoKeys(env)[0] ?? null; | |
| 37 | +} | |
| 38 | + | |
| 31 | 39 | const hexBytes = (hex: string): Uint8Array => Uint8Array.from(hex.match(/../g)!, (h) => parseInt(h, 16)); |
| 32 | 40 | |
| 33 | 41 | /** |
| @@ -59,8 +67,8 @@ | ||
| 59 | 67 | return `${CAMO_PREFIX}${sign(key, url)}/${b64url(new TextEncoder().encode(url))}`; |
| 60 | 68 | } |
| 61 | 69 | |
| 62 | -/** The URL a proxy path's two segments stand for, or null when the signature doesn't match. */ | |
| 63 | -export function verifyCamo(key: Uint8Array, sig: string, encoded: string): string | null { | |
| 70 | +/** The URL a proxy path's two segments stand for, or null when the signature matches none of `keys`. */ | |
| 71 | +export function verifyCamo(keys: Uint8Array | readonly Uint8Array[], sig: string, encoded: string): string | null { | |
| 64 | 72 | if (sig.length > 64 || encoded.length > Math.ceil((MAX_CAMO_URL * 4) / 3) + 4) return null; |
| 65 | 73 | if (!/^[\w-]+$/.test(sig) || !/^[\w-]+$/.test(encoded)) return null; |
| 66 | 74 | let url: string; |
| @@ -69,7 +77,7 @@ | ||
| 69 | 77 | } catch { |
| 70 | 78 | return null; |
| 71 | 79 | } |
| 72 | - return safeEqual(sig, sign(key, url)) ? url : null; | |
| 80 | + return (keys instanceof Uint8Array ? [keys] : keys).some((key) => safeEqual(sig, sign(key, url))) ? url : null; | |
| 73 | 81 | } |
| 74 | 82 | |
| 75 | 83 | /** |
modifiedapps/diffinery/test/unit/camo.test.ts+19−1View file
| @@ -1,5 +1,5 @@ | ||
| 1 | 1 | import { describe, expect, it } from "vitest"; |
| 2 | -import { cacheTtl, camoHeaders, camoKey, camoPath, checkImageUrl, fetchCamoImage, hmacSha256, imageProxy, MAX_CAMO_URL, verifyCamo } from "../../src/lib/camo.js"; | |
| 2 | +import { cacheTtl, camoHeaders, camoKey, camoKeys, camoPath, checkImageUrl, fetchCamoImage, hmacSha256, imageProxy, MAX_CAMO_URL, verifyCamo } from "../../src/lib/camo.js"; | |
| 3 | 3 | import { renderMarkdown } from "../../src/web/markdown.js"; |
| 4 | 4 | import { b64url } from "../../src/lib/util.js"; |
| 5 | 5 | |
| @@ -27,6 +27,24 @@ | ||
| 27 | 27 | expect(verifyCamo(key, sig!, enc!)).toBe(url); |
| 28 | 28 | }); |
| 29 | 29 | |
| 30 | + it("keeps verifying paths signed with the previous key while MASTER_KEY rotates", () => { | |
| 31 | + const url = "https://img.example.com/a.png"; | |
| 32 | + const [, , , oldSig, enc] = camoPath(key, url).split("/"); | |
| 33 | + const rotated = { ...env, MASTER_KEY: "bmV3LW1hc3Rlci1rZXktZm9yLXRlc3RzLW9ubHktMzI=", MASTER_KEY_PREV: env.MASTER_KEY }; | |
| 34 | + const keys = camoKeys(rotated); | |
| 35 | + expect(keys).toHaveLength(2); | |
| 36 | + // New paths are signed with the new key; old ones still verify. | |
| 37 | + expect(camoKey(rotated)).toEqual(keys[0]); | |
| 38 | + expect(camoPath(keys[0]!, url)).not.toBe(camoPath(key, url)); | |
| 39 | + expect(verifyCamo(keys, oldSig!, enc!)).toBe(url); | |
| 40 | + const [, , , newSig] = camoPath(keys[0]!, url).split("/"); | |
| 41 | + expect(verifyCamo(keys, newSig!, enc!)).toBe(url); | |
| 42 | + // Once MASTER_KEY_PREV is removed, the old signature is refused. | |
| 43 | + expect(verifyCamo(camoKeys({ ...rotated, MASTER_KEY_PREV: undefined }), oldSig!, enc!)).toBeNull(); | |
| 44 | + // MASTER_KEY_PREV alone signs nothing. | |
| 45 | + expect(camoKeys({ ...env, MASTER_KEY: undefined, MASTER_KEY_PREV: env.MASTER_KEY })).toHaveLength(0); | |
| 46 | + }); | |
| 47 | + | |
| 30 | 48 | it("refuses unsigned, tampered, re-signed and malformed paths", () => { |
| 31 | 49 | const [, , , sig, enc] = camoPath(key, "https://img.example.com/a.png").split("/"); |
| 32 | 50 | const other = b64url(new TextEncoder().encode("https://evil.example/a.png")); |