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 };