From 634cfb9446f59fd1482b656d0a1ea11be5b90a27 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Thu, 8 Oct 2026 20:24:08 +0900 Subject: [PATCH] Handle remote key context failures Treat malformed JSON-LD contexts and context transport failures as unavailable keys instead of allowing them to abort verification. Cover actor, standalone key, owner, and fallback decoding while preserving unexpected loader errors and the existing negative cache behavior. Add regression coverage for both key formats, nested context errors, and Deno, Node.js, and Bun transport error shapes. Fixes https://github.com/fedify-dev/fedify/issues/1267 Assisted-by: OpenCode:deepseek-flash Assisted-by: Codex:gpt-6.1-sol Assisted-by: Claude Code:claude-fable-5-1 Assisted-by: Claude Code:claude-opus-5-5 --- CHANGES.md | 9 + changes.d/fedify/remote-key-context-errors.md | 8 + packages/fedify/src/sig/key.test.ts | 216 +++++++++++++++++- packages/fedify/src/sig/key.ts | 106 ++++++++- 4 files changed, 330 insertions(+), 9 deletions(-) create mode 100644 changes.d/fedify/remote-key-context-errors.md diff --git a/CHANGES.md b/CHANGES.md index fb7ebb560..19d6194b2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,6 +8,15 @@ Version 2.0.32 To be released. +### @fedify/fedify + + - Fixed signature verification throwing an exception when a remote actor + supplied a malformed JSON-LD context or its context could not be fetched. + These requesters are now treated as unverifiable. [[#1267], [#1270]] + +[#1267]: https://github.com/fedify-dev/fedify/issues/1267 +[#1270]: https://github.com/fedify-dev/fedify/pull/1270 + ### @fedify/cfworkers - Fixed npm refusing to install `@fedify/cfworkers` alongside diff --git a/changes.d/fedify/remote-key-context-errors.md b/changes.d/fedify/remote-key-context-errors.md new file mode 100644 index 000000000..277bd932a --- /dev/null +++ b/changes.d/fedify/remote-key-context-errors.md @@ -0,0 +1,8 @@ +--- +links: + '#1267': https://github.com/fedify-dev/fedify/issues/1267 + '#1270': https://github.com/fedify-dev/fedify/pull/1270 +--- + - Fixed signature verification throwing an exception when a remote actor + supplied a malformed JSON-LD context or its context could not be fetched. + These requesters are now treated as unverifiable. [[#1267], [#1270]] diff --git a/packages/fedify/src/sig/key.test.ts b/packages/fedify/src/sig/key.test.ts index 5120de5be..5f9734f10 100644 --- a/packages/fedify/src/sig/key.test.ts +++ b/packages/fedify/src/sig/key.test.ts @@ -1,6 +1,13 @@ import { mockDocumentLoader, test } from "@fedify/fixture"; -import { CryptographicKey, Multikey } from "@fedify/vocab"; -import { assert, assertEquals, assertRejects, assertThrows } from "@std/assert"; +import { CryptographicKey, Multikey, Person } from "@fedify/vocab"; +import { FetchError, getDocumentLoader, UrlError } from "@fedify/vocab-runtime"; +import { + assert, + assertEquals, + assertRejects, + assertStrictEquals, + assertThrows, +} from "@std/assert"; import { ed25519Multikey, rsaPrivateKey2, @@ -8,6 +15,7 @@ import { rsaPublicKey2, rsaPublicKey3, } from "../testing/keys.ts"; +import { wrapContextLoaderForJsonLd } from "./ld.ts"; import { exportJwk, fetchKey, @@ -470,6 +478,210 @@ test("fetchKey() returns null for a malformed actor publicKey", async () => { }); }); +// Exercise the JSON-LD boundary before key iteration, including the separate +// owner document fetched for a standalone key. +for (const cls of [CryptographicKey, Multikey]) { + for (const location of ["actor", "key", "owner"] as const) { + test(`fetchKey() handles JSON-LD context failures in ${cls.name} ${location} documents`, async (t) => { + const actorId = new URL("https://example.com/context-actor"); + const keyId = new URL(`${actorId.href}#key`); + const key = cls === CryptographicKey + ? rsaPublicKey1.clone({ id: keyId, owner: actorId }) + : ed25519Multikey.clone({ id: keyId, controller: actorId }); + const actor = new Person({ + id: actorId, + ...(key instanceof CryptographicKey + ? { publicKeys: [key] } + : { assertionMethods: [key] }), + }); + const actorDocument = await actor.toJsonLd({ + contextLoader: mockDocumentLoader, + }) as Record; + const keyDocument = await key.toJsonLd({ + contextLoader: mockDocumentLoader, + }) as Record; + const contextUrl = `https://example.com/context/${cls.name}/${location}`; + const transportErrors = [ + new TypeError("fetch failed"), + new TypeError("error reading a body from connection"), + globalThis.Object.assign( + new TypeError("The socket connection was closed unexpectedly."), + { code: "ECONNRESET" }, + ), + globalThis.Object.assign(new TypeError("Network request failed"), { + code: "ECONNRESET", + }), + new TypeError("Network request failed", { + cause: globalThis.Object.assign(new Error("connection refused"), { + code: "ECONNREFUSED", + }), + }), + new FetchError(contextUrl, "HTTP 503"), + new UrlError("DNS lookup failed", { reason: "dns" }), + globalThis.Object.assign(new Error("request aborted"), { + name: "AbortError", + }), + ]; + const programmingErrors = [ + new Error("loader bug"), + new ReferenceError("loader bug"), + new TypeError("loader bug"), + "loader bug", + null, + undefined, + ]; + const cases: ( + | { kind: "inline" } + | { kind: "remote"; document: unknown } + | { kind: "url"; wrapped: boolean } + | { kind: "wrapped"; error: Error } + | { + kind: "transport" | "programming"; + error: unknown; + fallback?: boolean; + } + )[] = [ + { kind: "inline" }, + { kind: "url", wrapped: false }, + { kind: "url", wrapped: true }, + { kind: "wrapped", error: new TypeError("Invalid URL string.") }, + { kind: "remote", document: 123 }, + { kind: "remote", document: "not JSON" }, + { kind: "remote", document: { "@context": contextUrl } }, + ...transportErrors.map((error) => ({ + kind: "transport" as const, + error, + })), + ...programmingErrors.map((error) => ({ + kind: "programming" as const, + error, + })), + ...(location === "key" + ? [ + { + kind: "transport" as const, + error: new TypeError("fetch failed"), + fallback: true, + }, + { + kind: "programming" as const, + error: new ReferenceError("fallback bug"), + fallback: true, + }, + ] + : []), + ]; + for (const scenario of cases) { + const description = "error" in scenario + ? scenario.error instanceof Error + ? `${scenario.error.name}: ${scenario.error.message}` + : String(scenario.error) + : "document" in scenario + ? JSON.stringify(scenario.document) + : "invalid type mapping"; + await t.step( + `${scenario.kind}: ${description}${ + "fallback" in scenario ? " (fallback)" : "" + }`, + async () => { + const cache: Record = + {}; + let recovered = false; + let contextLoads = 0; + const options: FetchKeyOptions = { + documentLoader(resource) { + const document = structuredClone( + resource === keyId.href && location !== "actor" + ? keyDocument + : actorDocument, + ); + if ( + location === "actor" || + (location === "key" && resource === keyId.href) || + (location === "owner" && resource === actorId.href) + ) { + const context = scenario.kind === "inline" + ? { + broken: { + "@id": "https://example.com/ns#broken", + "@type": "not-an-absolute-iri", + }, + } + : scenario.kind === "url" + ? "http://[" + : contextUrl; + document["@context"] = [document["@context"], context].flat(); + } + return Promise.resolve({ + contextUrl: null, + documentUrl: resource, + document, + }); + }, + async contextLoader(resource) { + if (scenario.kind === "url" && resource === "http://[") { + const loader = getDocumentLoader(); + return await (scenario.wrapped + ? wrapContextLoaderForJsonLd(loader) + : loader)(resource); + } + if (scenario.kind === "wrapped" && resource === contextUrl) { + return await wrapContextLoaderForJsonLd(() => + Promise.reject(scenario.error) + )(resource); + } + if (resource !== contextUrl) { + return await mockDocumentLoader(resource); + } + contextLoads++; + if ( + !recovered && "error" in scenario && + (!("fallback" in scenario) || contextLoads > 1) + ) throw scenario.error; + return { + contextUrl: null, + documentUrl: resource, + document: !recovered && "document" in scenario + ? scenario.document + : { "@context": {} }, + }; + }, + keyCache: { + get: (id) => Promise.resolve(cache[id.href]), + set(id, value) { + cache[id.href] = value; + return Promise.resolve(); + }, + }, + }; + const lookup = () => + fetchKey(keyId, cls, options); + if (scenario.kind === "programming" && "error" in scenario) { + assertStrictEquals(await assertRejects(lookup), scenario.error); + assertEquals(cache, {}); + return; + } + assertEquals(await lookup(), { key: null, cached: false }); + assertEquals(cache, { [keyId.href]: null }); + const previousLoads = contextLoads; + assertEquals(await lookup(), { key: null, cached: true }); + assertEquals(contextLoads, previousLoads); + if (scenario.kind === "transport") { + // An expired negative cache entry must allow the loader to recover. + delete cache[keyId.href]; + recovered = true; + const result = await lookup(); + assertEquals(result.cached, false); + assertEquals(result.key?.id, keyId); + assert(result.key?.publicKey != null); + } + }, + ); + } + }); + } +} + test("fetchKey() rejects a key whose owner does not link back", async () => { // Both sides of the `owner` claim are written by the same host, so the // claim is only worth what the named owner's own document says. Neither diff --git a/packages/fedify/src/sig/key.ts b/packages/fedify/src/sig/key.ts index e630b6ff1..26752b5aa 100644 --- a/packages/fedify/src/sig/key.ts +++ b/packages/fedify/src/sig/key.ts @@ -5,7 +5,12 @@ import { type Multikey, Object, } from "@fedify/vocab"; -import { type DocumentLoader, getDocumentLoader } from "@fedify/vocab-runtime"; +import { + type DocumentLoader, + FetchError, + getDocumentLoader, + UrlError, +} from "@fedify/vocab-runtime"; import { getLogger } from "@logtape/logtape"; import { SpanKind, @@ -15,6 +20,82 @@ import { } from "@opentelemetry/api"; import metadata from "../../deno.json" with { type: "json" }; +/** Classifies decoding failures without hiding unexpected loader errors. */ +function classifyKeyJsonLdError( + error: unknown, + seen = new Set(), +): "invalid" | "fetch" | null { + if (!(error instanceof Error) || !error.name.startsWith("jsonld.")) { + return null; + } + if (seen.has(error)) throw error; + seen.add(error); + const details = (error as Error & { + details?: { code?: unknown; cause?: unknown; url?: unknown }; + }).details; + if (typeof details?.code !== "string") return null; + if ( + details.code !== "loading remote context failed" || + (!("cause" in details) && !("cause" in error)) + ) return "invalid"; + + // jsonld wraps both loader errors and JSON.parse failures in InvalidUrl. + // A bad context body is invalid data; a failed fetch is a lookup failure. + const cause = "cause" in details ? details.cause : error.cause; + const nestedFailure = classifyKeyJsonLdError(cause, seen); + if (nestedFailure != null) return nestedFailure; + if (cause instanceof SyntaxError) return "invalid"; + if (cause instanceof FetchError || cause instanceof UrlError) return "fetch"; + if ( + cause instanceof Error && + ["AbortError", "TimeoutError", "NetworkError"].includes(cause.name) + ) return "fetch"; + if (cause instanceof TypeError) { + // The context loader can reject a malformed URL before any fetch occurs. + // Match the same URL errors as the LD-signature context-loading boundary. + const urlCode = (cause as TypeError & { code?: unknown }).code; + if (cause.name === "InvalidContextReferenceError") return "invalid"; + if ( + urlCode === "ERR_INVALID_URL" || + cause.message === "Invalid URL string." || + /^Invalid URL(?::|$)/.test(cause.message) || + / cannot be parsed as a URL\.?$/.test(cause.message) + ) { + // A valid reference can still fail in a loader's URL handling. Like + // wrapContextLoaderForJsonLd(), keep that a fetch failure rather than + // attributing it to malformed remote data. + return typeof details.url === "string" && + /^[A-Za-z][A-Za-z0-9+.-]*:/.test(details.url) && + !URL.canParse(details.url) + ? "invalid" + : "fetch"; + } + const networkCause = cause.cause as { code?: unknown } | undefined; + const code = (cause as TypeError & { code?: unknown }).code ?? + networkCause?.code; + if ( + typeof code === "string" && [ + "ECONNREFUSED", + "ECONNRESET", + "ECONNABORTED", + "ETIMEDOUT", + "EHOSTUNREACH", + "ENETUNREACH", + "ENOTFOUND", + "EAI_AGAIN", + "UND_ERR_CONNECT_TIMEOUT", + "UND_ERR_HEADERS_TIMEOUT", + "UND_ERR_BODY_TIMEOUT", + "UND_ERR_SOCKET", + ].includes(code) || + /^(fetch failed|Failed to fetch|error sending request|error reading a body from connection|Unable to connect|ConnectionRefused|Network connection lost|The socket connection was closed unexpectedly)/ + .test(cause.message) + ) return "fetch"; + } + // Keep the original error and stack for application/loader programming bugs. + throw cause; +} + /** * Checks if the given key is valid and supported. No-op if the key is valid, * otherwise throws an error. @@ -245,10 +326,11 @@ export async function fetchActorDocument( baseUrl: documentUrl, }); } catch (error) { - if (!(error instanceof TypeError)) throw error; + const failure = classifyKeyJsonLdError(error); + if (failure == null && !(error instanceof TypeError)) throw error; logger.debug( - "The document served at {documentUrl} is not a valid object: {error}", - { documentUrl: documentUrl.href, error }, + "Failed to decode the actor document at {documentUrl}: {error}", + { documentUrl: documentUrl.href, failure: failure ?? "invalid", error }, ); return null; } @@ -458,6 +540,15 @@ async function fetchKeyInternal( tracerProvider, }); } catch (e) { + const failure = classifyKeyJsonLdError(e); + if (failure != null) { + logger.debug( + "Failed to decode key {keyId}: {error}", + { keyId, failure, error: e }, + ); + await keyCache?.set(cacheKey, null); + return { key: null, cached: false }; + } if (!(e instanceof TypeError)) throw e; try { object = await cls.fromJsonLd(document, { @@ -466,10 +557,11 @@ async function fetchKeyInternal( tracerProvider, }); } catch (e) { - if (e instanceof TypeError) { + const failure = classifyKeyJsonLdError(e); + if (failure != null || e instanceof TypeError) { logger.debug( - "Failed to verify; key {keyId} returned an invalid object.", - { keyId }, + "Failed to decode key {keyId}: {error}", + { keyId, failure: failure ?? "invalid", error: e }, ); await keyCache?.set(cacheKey, null); return { key: null, cached: false };