diff --git a/src/proxy/control-plane-signature.api-contract.test.ts b/src/proxy/control-plane-signature.api-contract.test.ts new file mode 100644 index 0000000000..fa790319b9 --- /dev/null +++ b/src/proxy/control-plane-signature.api-contract.test.ts @@ -0,0 +1,300 @@ +import "#veryfront/schemas/_test-setup.ts"; +import { assertEquals, assertRejects } from "#veryfront/testing/assert.ts"; +import { afterEach, describe, it } from "#veryfront/testing/bdd.ts"; +import { + isAuthenticInternalControlPlaneCandidate, + isVerifiedInternalControlPlaneRequest, + resolveVerifiedControlPlaneBranchBinding, +} from "./control-plane-signature.ts"; + +/** + * Cross-repo contract: control-plane-signature.test.ts mints its own compliant + * JWS, so it never exercises what veryfront-api actually sends. This mints the + * payload exactly as veryfront-api's createControlPlaneRequestSignature does. + */ + +const PUBLIC_KEY_ENV = "CHANNEL_DISPATCH_SIGNING_PUBLIC_KEY"; +const RUN_STREAM_URL = "http://protected.preview.veryfront.com/api/control-plane/runs/r_1/stream"; +const encoder = new TextEncoder(); + +function base64url(data: string): string { + return btoa(data).replace(/\+/g, "-").replace(/\//g, "_").replace(/=+$/, ""); +} + +function base64urlBytes(bytes: Uint8Array): string { + return base64url(String.fromCharCode(...bytes)); +} + +async function sha256Base64url(value: string): Promise { + const digest = await crypto.subtle.digest("SHA-256", encoder.encode(value)); + return base64urlBytes(new Uint8Array(digest)); +} + +function encodePem(label: string, der: ArrayBuffer): string { + const base64 = btoa(String.fromCharCode(...new Uint8Array(der))); + const lines = base64.match(/.{1,64}/g) ?? [base64]; + return `-----BEGIN ${label}-----\n${lines.join("\n")}\n-----END ${label}-----`; +} + +/** Mint a control-plane JWS with exactly the claim set veryfront-api signs. */ +async function mintApiStyleJws( + body: string, + claimOverrides: Record = {}, +): Promise<{ jws: string; publicKeyPem: string }> { + const keyPair = await crypto.subtle.generateKey( + "Ed25519", + true, + ["sign", "verify"], + ) as CryptoKeyPair; + const publicKeyPem = encodePem( + "PUBLIC KEY", + await crypto.subtle.exportKey("spki", keyPair.publicKey), + ); + + const now = Math.floor(Date.now() / 1000); + const payload: Record = { + iss: "veryfront-api", + aud: "protected", + sub: "r_1", + surface: "studio", + project_id: "proj-1", + request_hash: await sha256Base64url(body), + request_method: "POST", + request_path: "/api/control-plane/runs/r_1/stream", + iat: now, + exp: now + 300, + ...claimOverrides, + }; + const encodedHeader = base64url(JSON.stringify({ alg: "EdDSA", typ: "JWT" })); + const encodedPayload = base64url(JSON.stringify(payload)); + const signature = await crypto.subtle.sign( + "Ed25519", + keyPair.privateKey, + encoder.encode(`${encodedHeader}.${encodedPayload}`), + ); + + return { + publicKeyPem, + jws: `${encodedHeader}.${encodedPayload}.${base64urlBytes(new Uint8Array(signature))}`, + }; +} + +/** + * Verify through the BOUND entry point. The unbound candidate check leaves aud, + * project_id and request_hash unverified, so asserting through it would let + * those three claims drift silently. + * + * Two claims stay deliberately unbound here. `sub` is never compared on this + * path — the run id is already pinned through request_path — and `surface` is + * only checked for membership of CONTROL_PLANE_SURFACES, because the proxy has + * no business asserting which surface a caller speaks for. + */ +async function verifyApiStyleRequest( + claimOverrides: Record = {}, +): Promise { + const body = JSON.stringify({ messages: [{ role: "user", content: "hi" }] }); + const { jws, publicKeyPem } = await mintApiStyleJws(body, claimOverrides); + Deno.env.set(PUBLIC_KEY_ENV, publicKeyPem); + + const req = new Request(RUN_STREAM_URL, { + method: "POST", + headers: { + "x-token": "user-token", + "x-veryfront-control-plane-jws": jws, + }, + body, + }); + + return await isVerifiedInternalControlPlaneRequest(req, new URL(RUN_STREAM_URL), { + audience: "protected", + expectedProjectId: "proj-1", + }); +} + +describe("control-plane signature: veryfront-api contract", () => { + afterEach(() => { + Deno.env.delete(PUBLIC_KEY_ENV); + }); + + it("accepts the JWS veryfront-api actually mints", async () => { + assertEquals(await verifyApiStyleRequest(), true); + }); + + // 0.1.1189 (#3251) added these claims; veryfront-api kept minting the old set + // and every run against a protected environment was 302'd to sign-in. + it("rejects a JWS missing request_method", async () => { + assertEquals(await verifyApiStyleRequest({ request_method: undefined }), false); + }); + + it("rejects a JWS missing request_path", async () => { + assertEquals(await verifyApiStyleRequest({ request_path: undefined }), false); + }); + + it("rejects a JWS whose request_method does not match the request", async () => { + assertEquals(await verifyApiStyleRequest({ request_method: "DELETE" }), false); + }); + + // Verified through the bound entry point, so a drifted audience or project id + // is caught. The unbound candidate check leaves both unverified. + it("rejects a JWS whose aud is not the bound project", async () => { + assertEquals(await verifyApiStyleRequest({ aud: "another-project" }), false); + }); + + it("rejects a JWS whose project_id is not the bound project", async () => { + assertEquals(await verifyApiStyleRequest({ project_id: "another-project-id" }), false); + }); + + it("rejects a JWS whose request_path does not match the request", async () => { + assertEquals( + await verifyApiStyleRequest({ request_path: "/api/control-plane/runs/r_1" }), + false, + ); + }); +}); + +describe("control-plane signature: body binding", () => { + afterEach(() => { + Deno.env.delete(PUBLIC_KEY_ENV); + }); + + async function resolveBinding(signedBody: string, sentBody: string) { + const { jws, publicKeyPem } = await mintApiStyleJws(signedBody); + Deno.env.set(PUBLIC_KEY_ENV, publicKeyPem); + const req = new Request(RUN_STREAM_URL, { + method: "POST", + headers: { "x-token": "t", "x-veryfront-control-plane-jws": jws }, + body: sentBody, + }); + return await resolveVerifiedControlPlaneBranchBinding(req, new URL(RUN_STREAM_URL), { + audience: "protected", + expectedProjectId: "proj-1", + }); + } + + const RUN_BODY = JSON.stringify({ + run: { project: {} }, + agentSource: { type: "release" }, + }); + + it("accepts a body matching the signed request_hash", async () => { + assertEquals(await resolveBinding(RUN_BODY, RUN_BODY), {}); + }); + + it("rejects a body that does not match the signed request_hash", async () => { + await assertRejects(() => + resolveBinding( + RUN_BODY, + JSON.stringify({ run: { project: {} }, agentSource: { type: "release" }, tampered: true }), + ) + ); + }); +}); + +describe("control-plane signature: rejection reasons", () => { + afterEach(() => { + Deno.env.delete(PUBLIC_KEY_ENV); + }); + + async function reasonsFor( + build: (jws: string) => { headers: Record; publicKeyPem?: string }, + url = RUN_STREAM_URL, + ): Promise<{ reasons: string[]; pathnames: string[] }> { + const body = JSON.stringify({ messages: [] }); + const { jws, publicKeyPem } = await mintApiStyleJws(body); + const built = build(jws); + Deno.env.delete(PUBLIC_KEY_ENV); + const key = built.publicKeyPem ?? publicKeyPem; + if (key !== "") Deno.env.set(PUBLIC_KEY_ENV, key); + + const reasons: string[] = []; + const pathnames: string[] = []; + const req = new Request(url, { method: "POST", headers: built.headers, body }); + await isAuthenticInternalControlPlaneCandidate(req, new URL(url), { + warn: (_msg, extra) => { + if (typeof extra?.reason === "string") reasons.push(extra.reason); + if (typeof extra?.pathname === "string") pathnames.push(extra.pathname); + }, + }); + return { reasons, pathnames }; + } + + /** Exactly one warn, carrying the expected reason. */ + async function reasonFor( + build: (jws: string) => { headers: Record; publicKeyPem?: string }, + ): Promise { + const { reasons } = await reasonsFor(build); + assertEquals(reasons.length, 1); + return reasons[0]; + } + + it("reports a missing x-token", async () => { + assertEquals( + await reasonFor((jws) => ({ headers: { "x-veryfront-control-plane-jws": jws } })), + "missing_x_token", + ); + }); + + it("reports an unconfigured verification key", async () => { + assertEquals( + await reasonFor((jws) => ({ + headers: { "x-token": "t", "x-veryfront-control-plane-jws": jws }, + publicKeyPem: "", + })), + "verification_key_not_configured", + ); + }); + + it("reports a missing control-plane header when a dispatch signature was presented", async () => { + assertEquals( + await reasonFor((jws) => ({ headers: { "x-token": "t", "x-veryfront-dispatch-jws": jws } })), + "missing_signature_header", + ); + }); + + // An unauthenticated client picks the runId segment, so logging one line per + // request would be a remote write into log ingest. + it("stays silent for a caller that presented no signature header", async () => { + const { reasons } = await reasonsFor(() => ({ headers: { "x-token": "t" } })); + assertEquals(reasons, []); + }); + + it("stays silent for an unauthenticated request with a huge path", async () => { + const huge = `http://protected.preview.veryfront.com/api/control-plane/runs/${ + "A".repeat(8000) + }/stream`; + const { reasons } = await reasonsFor(() => ({ headers: {} }), huge); + assertEquals(reasons, []); + }); + + it("bounds the logged pathname", async () => { + const huge = `http://protected.preview.veryfront.com/api/control-plane/runs/${ + "A".repeat(8000) + }/stream`; + const { pathnames } = await reasonsFor( + (jws) => ({ headers: { "x-veryfront-control-plane-jws": jws } }), + huge, + ); + assertEquals(pathnames.length, 1); + assertEquals(pathnames[0]?.length, 256); + }); + + it("reports a rejected signature", async () => { + assertEquals( + await reasonFor((jws) => ({ + headers: { "x-token": "t", "x-veryfront-control-plane-jws": `${jws}tampered` }, + })), + "signature_rejected", + ); + }); + + it("stays silent for ordinary non-internal routes", async () => { + const reasons: string[] = []; + const pageUrl = "http://protected.preview.veryfront.com/"; + await isAuthenticInternalControlPlaneCandidate( + new Request(pageUrl, { method: "GET" }), + new URL(pageUrl), + { warn: (_msg, extra) => reasons.push(String(extra?.reason)) }, + ); + assertEquals(reasons.length, 0); + }); +}); diff --git a/src/proxy/control-plane-signature.ts b/src/proxy/control-plane-signature.ts index 57f9be769e..cda6fd38e1 100644 --- a/src/proxy/control-plane-signature.ts +++ b/src/proxy/control-plane-signature.ts @@ -19,6 +19,10 @@ * not consume the body: authoritative body-hash verification still runs in the * renderer. Signature headers remain available to that downstream verifier. * + * Rejections are logged with a reason so a turned-away internal caller can be + * diagnosed from one line. The reason is never returned to the client, and + * anonymous traffic that presented no signature header is not logged at all. + * * @module proxy/control-plane-signature */ @@ -33,6 +37,10 @@ import { DEFAULT_MAX_BODY_SIZE_BYTES } from "#veryfront/utils/constants/index.ts import { isWellFormedString } from "#veryfront/utils/is-well-formed-string.ts"; import { isCanonicalOpaqueProjectIdentifier } from "#veryfront/utils/project-identity.ts"; +export interface InternalControlPlaneSignatureLogger { + warn: (msg: string, extra?: Record) => void; +} + const CONTROL_PLANE_JWS_HEADER = "x-veryfront-control-plane-jws"; const DISPATCH_JWS_HEADER = "x-veryfront-dispatch-jws"; @@ -42,6 +50,7 @@ export const INTERNAL_CONTROL_PLANE_SIGNATURE_HEADERS = [ DISPATCH_JWS_HEADER, ] as const; +const MAX_LOGGED_PATHNAME_CODE_UNITS = 256; const PUBLIC_KEY_ENV_VAR = "CHANNEL_DISPATCH_SIGNING_PUBLIC_KEY"; const MAX_SIGNATURE_AGE_SECONDS = 60; const MAX_BRANCH_NAME_CODE_UNITS = 255; @@ -250,25 +259,38 @@ export async function resolveVerifiedControlPlaneBranchBinding( return parseVerifiedBranchBinding(rawBody); } -async function verifyInternalControlPlaneSignature( +/** + * Why a signed-internal check did not admit the request. + * + * "reserved" routes are internal but never admissible, so they share the silent + * `route_not_admissible` reason with ordinary public traffic. + */ +export type InternalControlPlaneRejection = + | "route_not_admissible" + | "missing_x_token" + | "verification_key_not_configured" + | "missing_signature_header" + | "signature_rejected"; + +async function checkInternalControlPlaneSignature( req: Request, url: URL, binding?: InternalControlPlaneProjectBinding, -): Promise { +): Promise { const routeKind = classifyInternalControlPlaneRequest(req.method, url.pathname); - if (routeKind === "public" || routeKind === "reserved") return false; + if (routeKind === "public" || routeKind === "reserved") return "route_not_admissible"; // The candidate only matters when there is an x-token to use for metadata // lookup or forward after the resolved project binding succeeds. - if (!req.headers.get("x-token")) return false; + if (!req.headers.get("x-token")) return "missing_x_token"; const publicKeyPem = getHostEnv(PUBLIC_KEY_ENV_VAR); - if (!publicKeyPem) return false; + if (!publicKeyPem) return "verification_key_not_configured"; if (routeKind === "dispatch") { const dispatchJws = req.headers.get(DISPATCH_JWS_HEADER); - if (!dispatchJws) return false; - return await verifyDispatchJwsSignature(dispatchJws, { + if (!dispatchJws) return "missing_signature_header"; + const verified = await verifyDispatchJwsSignature(dispatchJws, { publicKeyPem, maxAgeSeconds: MAX_SIGNATURE_AGE_SECONDS, ...(binding @@ -278,11 +300,12 @@ async function verifyInternalControlPlaneSignature( } : {}), }); + return verified ? null : "signature_rejected"; } const controlPlaneJws = req.headers.get(CONTROL_PLANE_JWS_HEADER); - if (!controlPlaneJws) return false; - return await verifyControlPlaneJwsSignature(controlPlaneJws, { + if (!controlPlaneJws) return "missing_signature_header"; + const verified = await verifyControlPlaneJwsSignature(controlPlaneJws, { publicKeyPem, maxAgeSeconds: MAX_SIGNATURE_AGE_SECONDS, requestMethod: req.method, @@ -294,6 +317,45 @@ async function verifyInternalControlPlaneSignature( } : {}), }); + return verified ? null : "signature_rejected"; +} + +async function verifyInternalControlPlaneSignature( + req: Request, + url: URL, + binding?: InternalControlPlaneProjectBinding, + logger?: InternalControlPlaneSignatureLogger, +): Promise { + const rejection = await checkInternalControlPlaneSignature(req, url, binding); + if (rejection === null) return true; + + if (shouldLogRejection(req, rejection)) { + logger?.warn("Internal control-plane signature not accepted", { + reason: rejection, + method: req.method, + // Two admissible route patterns carry an unbounded runId segment, and any + // unauthenticated client can choose it. Logging it whole is a remote + // write into log ingest, so bound it. + pathname: url.pathname.slice(0, MAX_LOGGED_PATHNAME_CODE_UNITS), + ...(binding?.audience ? { audience: binding.audience } : {}), + }); + } + + return false; +} + +/** + * Log only rejections that describe a caller which tried to authenticate. + * + * A request carrying no signature header at all is anonymous internet traffic + * that chose its own path; one line per request is amplification, and it buries + * the rejections that describe a real internal caller. Every reason still logs + * once a signature header is present. + */ +function shouldLogRejection(req: Request, rejection: InternalControlPlaneRejection): boolean { + if (rejection === "route_not_admissible") return false; + if (rejection === "verification_key_not_configured") return true; + return INTERNAL_CONTROL_PLANE_SIGNATURE_HEADERS.some((header) => req.headers.has(header)); } /** @@ -308,8 +370,9 @@ async function verifyInternalControlPlaneSignature( export async function isAuthenticInternalControlPlaneCandidate( req: Request, url: URL, + logger?: InternalControlPlaneSignatureLogger, ): Promise { - return await verifyInternalControlPlaneSignature(req, url); + return await verifyInternalControlPlaneSignature(req, url, undefined, logger); } /** @@ -323,7 +386,8 @@ export async function isVerifiedInternalControlPlaneRequest( req: Request, url: URL, binding: InternalControlPlaneProjectBinding, + logger?: InternalControlPlaneSignatureLogger, ): Promise { if (!binding.audience) return false; - return await verifyInternalControlPlaneSignature(req, url, binding); + return await verifyInternalControlPlaneSignature(req, url, binding, logger); } diff --git a/src/proxy/handler.ts b/src/proxy/handler.ts index edcd7babbe..51dc77dfa7 100644 --- a/src/proxy/handler.ts +++ b/src/proxy/handler.ts @@ -794,11 +794,13 @@ export function createProxyHandler(options: ProxyHandlerOptions) { req, url, { audience: projectSlug }, + logger, ); } else if (isCustomDomain) { signedInternalControlPlaneCandidate = await isAuthenticInternalControlPlaneCandidate( req, url, + logger, ); } let signedInternalControlPlaneRequest = false; @@ -810,7 +812,7 @@ export function createProxyHandler(options: ProxyHandlerOptions) { await isVerifiedInternalControlPlaneRequest(req, url, { audience: resolvedProjectSlug, expectedProjectId: resolvedProjectId, - }); + }, logger); if (!projectSlug && parsedDomain.isVeryfrontDomain) { return { @@ -1268,7 +1270,7 @@ export function createProxyHandler(options: ProxyHandlerOptions) { const scope = getScope(parsedDomain.environment); const projectSlug = parsedDomain.slug ?? undefined; const signedInternalControlPlaneRequest = projectSlug !== undefined && - await isVerifiedInternalControlPlaneRequest(req, url, { audience: projectSlug }); + await isVerifiedInternalControlPlaneRequest(req, url, { audience: projectSlug }, logger); const { token } = await resolveProxyRequestToken({ req, url,