From 8f635c60cdd1015f981c77e6632d51b7655085e3 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 9 Mar 2026 11:22:32 +0900 Subject: [PATCH 1/8] Handle unverified inbound activities Add Federatable.onUnverifiedActivity() so applications can inspect inbound activities whose signatures could not be verified without weakening the existing guarantee that inbox listeners only receive verified activities. Expose detailed HTTP signature failure reasons and key-fetch results, preserve key-cache and fetch-key tracing behavior, and surface the new verification details through OpenTelemetry span data and FedifySpanExporter records. Update the manual and changelog entries and add regression tests covering malformed signatures, deleted-key retries, cached unavailable keys, and telemetry output. Fixes https://github.com/fedify-dev/fedify/issues/472 Co-Authored-By: OpenAI Codex --- CHANGES.md | 28 ++ docs/manual/inbox.md | 68 ++++- docs/manual/opentelemetry.md | 17 ++ .../fedify/src/federation/builder.test.ts | 22 ++ packages/fedify/src/federation/builder.ts | 9 + packages/fedify/src/federation/callback.ts | 30 ++ packages/fedify/src/federation/federation.ts | 31 ++ .../fedify/src/federation/handler.test.ts | 110 ++++++- packages/fedify/src/federation/handler.ts | 92 +++++- .../fedify/src/federation/middleware.test.ts | 235 ++++++++++++++ packages/fedify/src/federation/middleware.ts | 1 + packages/fedify/src/otel/exporter.test.ts | 59 ++++ packages/fedify/src/otel/exporter.ts | 32 +- packages/fedify/src/sig/http.test.ts | 119 +++++++- packages/fedify/src/sig/http.ts | 286 ++++++++++++++---- packages/fedify/src/sig/key.test.ts | 57 +++- packages/fedify/src/sig/key.ts | 264 ++++++++++++++-- packages/testing/src/mock.ts | 5 + packages/vocab-runtime/src/docloader.test.ts | 12 +- packages/vocab-runtime/src/docloader.ts | 1 + packages/vocab-runtime/src/request.ts | 9 +- 21 files changed, 1376 insertions(+), 111 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a72ca2356..57eab4317 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,14 +10,42 @@ To be released. ### @fedify/fedify + - Added `Federatable.onUnverifiedActivity()` so applications can inspect + inbound activities whose signatures could not be verified and optionally + return a custom response instead of the default `401 Unauthorized`. + This is useful for cases like `Delete` deliveries from actors whose + signing keys now return `404 Not Found` or `410 Gone`. Added the + supporting public types `UnverifiedActivityHandler` and + `UnverifiedActivityReason`. [[#472], [#611]] + + - Added `verifyRequestDetailed()` plus the public types + `VerifyRequestDetailedResult`, `VerifyRequestFailureReason`, and + `FetchKeyErrorResult` so applications can distinguish unsigned requests, + invalid signatures, and key-fetch failures during HTTP signature + verification. [[#611]] + + - OpenTelemetry spans/events and `FedifySpanExporter` signature details now + expose HTTP signature failure reasons and key-fetch failure details for + inbound activities. [[#611]] + - Fixed `RequestContext.getSignedKeyOwner()` to return `null` instead of throwing an error when the remote server requires authorized fetch and returns `401 Unauthorized` for the key owner lookup. Previously, this caused a `500 Internal Server Error` when interoperating with servers like GoToSocial that have authorized fetch enabled. [[#473], [#589]] +[#472]: https://github.com/fedify-dev/fedify/issues/472 [#473]: https://github.com/fedify-dev/fedify/issues/473 [#589]: https://github.com/fedify-dev/fedify/pull/589 +[#611]: https://github.com/fedify-dev/fedify/pull/611 + +### @fedify/vocab-runtime + + - Added optional `FetchError.response` so callers can inspect the original + failed HTTP response when remote document or key fetches return an HTTP + error (such as `404 Not Found` or `410 Gone`). This enables higher-level + APIs to distinguish transport failures from specific HTTP fetch failures. + [[#611]] ### @fedify/cli diff --git a/docs/manual/inbox.md b/docs/manual/inbox.md index 66916f0f5..515b9c8f5 100644 --- a/docs/manual/inbox.md +++ b/docs/manual/inbox.md @@ -26,9 +26,10 @@ activities with various specifications, such as: - [Linked Data Signatures] - Object Integrity Proofs ([FEP-8b32]) -You don't need to worry about the signature verification at all—unsigned -activities and invalid signatures are silently ignored. If you want to see why -some activities are ignored, you can turn on [logging](./log.md) for +You don't need to worry about the signature verification at all. By default, +activities whose signatures/proofs cannot be verified are rejected with +`401 Unauthorized` and are not passed to inbox listeners. If you want to see +why some activities are rejected, you can turn on [logging](./log.md) for `["fedify", "sig"]` category. [HTTP Signatures]: https://datatracker.ietf.org/doc/html/draft-cavage-http-signatures-12 @@ -37,6 +38,58 @@ some activities are ignored, you can turn on [logging](./log.md) for [FEP-8b32]: https://w3id.org/fep/8b32 +Handling unverified activities +------------------------------ + +*This API is available since Fedify 2.1.0.* + +Most applications can keep the default behavior and ignore unverified inbound +activities. However, some applications need finer control. Typical examples +include: + + - remote actor deletions where the `Delete` activity can still be parsed, + but the signing key now returns `410 Gone` + - noisy redelivery loops from remote servers that keep retrying activities + you have decided not to process + - custom logging, metrics, moderation, or quarantine flows for suspicious + inbound traffic + +For these cases, you can register `~Federatable.onUnverifiedActivity()`. The +callback receives the `RequestContext`, the parsed activity, and a reason +object whose `type` is one of `"noSignature"`, `"invalidSignature"`, or +`"keyFetchError"`. + +If the callback returns a `Response`, Fedify uses it as-is. If it returns +nothing (`void`), Fedify falls back to the default `401 Unauthorized` +response. + +~~~~ typescript twoslash +import { type Federation } from "@fedify/fedify"; +import { Delete } from "@fedify/vocab"; +const federation = null as unknown as Federation; +// ---cut-before--- +federation.onUnverifiedActivity((ctx, activity, reason) => { + if ( + activity instanceof Delete && + reason.type === "keyFetchError" && + "status" in reason.result && + reason.result.status === 410 + ) { + // For example, stop redelivery of a Delete from a permanently gone actor. + return new Response(null, { status: 202 }); + } +}); +~~~~ + +Returning a custom response does not pass the activity to the inbox listeners +registered through `~InboxListenerSetters.on()`. Verified activities continue +to flow to those listeners as usual; unverified activities remain opt-in. + +The request context includes the original `Request` object, so you can inspect +details such as the `Host` header through `RequestContext.request` when making +policy decisions. + + Registering an inbox listener ----------------------------- @@ -374,7 +427,9 @@ its own retry logic and rely on the backend to handle retries. This avoids duplicate retry mechanisms and leverages the backend's optimized retry features. > [!NOTE] -> Activities with invalid signatures/proofs are silently ignored and not queued. +> Activities with invalid signatures/proofs are not queued and are not passed +> to inbox listeners. If `~Federatable.onUnverifiedActivity()` is configured, +> the hook runs before the default `401 Unauthorized` response is returned. > [!TIP] > If your inbox listeners are mostly I/O-bound, consider parallelizing @@ -505,8 +560,9 @@ federation ~~~~ > [!NOTE] -> Activities with invalid signatures/proofs are silently ignored and not passed -> to the error handler. +> Activities with invalid signatures/proofs are not passed to the error +> handler. If you need to inspect them, use +> `~Federatable.onUnverifiedActivity()` instead. Forwarding activities to another server diff --git a/docs/manual/opentelemetry.md b/docs/manual/opentelemetry.md index f87800eae..39e4e684a 100644 --- a/docs/manual/opentelemetry.md +++ b/docs/manual/opentelemetry.md @@ -222,6 +222,13 @@ Each span event includes attributes with detailed information: - `http_signatures.verified`: Whether HTTP Signatures were verified (`true`/`false`) - `http_signatures.key_id`: The key ID used for HTTP signature verification + - `http_signatures.failure_reason` (optional): Why HTTP signature + verification failed (`noSignature`, `invalidSignature`, or + `keyFetchError`) + - `http_signatures.key_fetch_status` (optional): The HTTP status code when + fetching the signing key failed with an HTTP response + - `http_signatures.key_fetch_error` (optional): The error type when fetching + the signing key failed without an HTTP response **`activitypub.activity.sent` event attributes:** @@ -279,6 +286,10 @@ for ActivityPub: | `http_signatures.signature` | string | The signature of the HTTP request in hexadecimal. | `"73a74c990beabe6e59cc68f9c6db7811b59cbb22fd12dcffb3565b651540efe9"` | | `http_signatures.algorithm` | string | The algorithm of the HTTP request signature. | `"rsa-sha256"` | | `http_signatures.key_id` | string | The public key ID of the HTTP request signature. | `"https://example.com/actor/1#main-key"` | +| `http_signatures.verified` | boolean | Whether the HTTP request signature was verified successfully. | `false` | +| `http_signatures.failure_reason` | string | Why HTTP signature verification failed (`noSignature`, `invalidSignature`, or `keyFetchError`). | `"keyFetchError"` | +| `http_signatures.key_fetch_status` | int | The HTTP status code from a failed signing-key fetch, when available. | `410` | +| `http_signatures.key_fetch_error` | string | The error type from a non-HTTP signing-key fetch failure, when available. | `"TypeError"` | | `http_signatures.digest.{algorithm}` | string | The digest of the HTTP request body in hexadecimal. The `{algorithm}` is the digest algorithm (e.g., `sha`, `sha-256`). | `"d41d8cd98f00b204e9800998ecf8427e"` | | `ld_signatures.key_id` | string | The public key ID of the Linked Data signature. | `"https://example.com/actor/1#main-key"` | | `ld_signatures.signature` | string | The signature of the Linked Data in hexadecimal. | `"73a74c990beabe6e59cc68f9c6db7811b59cbb22fd12dcffb3565b651540efe9"` | @@ -534,6 +545,12 @@ Each `TraceActivityRecord` contains: - `httpSignaturesVerified`: Whether HTTP Signatures were verified - `httpSignaturesKeyId` (optional): The key ID used for HTTP signature verification, if available + - `httpSignaturesFailureReason` (optional): Why HTTP signature + verification failed, if available + - `httpSignaturesKeyFetchStatus` (optional): The HTTP status code from a + failed key fetch, if available + - `httpSignaturesKeyFetchError` (optional): The error type from a + non-HTTP key fetch failure, if available - `ldSignaturesVerified`: Whether Linked Data Signatures were verified - `timestamp`: ISO 8601 timestamp - `inboxUrl`: The target inbox URL (for outbound activities) diff --git a/packages/fedify/src/federation/builder.test.ts b/packages/fedify/src/federation/builder.test.ts index 6be1353c8..931b1f924 100644 --- a/packages/fedify/src/federation/builder.test.ts +++ b/packages/fedify/src/federation/builder.test.ts @@ -8,6 +8,7 @@ import type { InboxListener, NodeInfoDispatcher, ObjectDispatcher, + UnverifiedActivityReason, } from "./callback.ts"; import { MemoryKvStore } from "./kv.ts"; import type { FederationImpl } from "./middleware.ts"; @@ -166,6 +167,27 @@ test("FederationBuilder", async (t) => { assertEquals(implRfc.firstKnock, "rfc9421"); }); + await t.step( + "should copy unverified activity handler into built federation", + async () => { + const builder = createFederationBuilder(); + const kv = new MemoryKvStore(); + const handler = ( + _ctx: unknown, + _activity: Activity, + _reason: UnverifiedActivityReason, + ) => { + return; + }; + + builder.onUnverifiedActivity(handler); + + const federation = await builder.build({ kv }); + const impl = federation as FederationImpl; + assertEquals(impl.unverifiedActivityHandler, handler); + }, + ); + await t.step( "should register multiple object dispatchers and verify them", async () => { diff --git a/packages/fedify/src/federation/builder.ts b/packages/fedify/src/federation/builder.ts index 094dcb47c..223eded21 100644 --- a/packages/fedify/src/federation/builder.ts +++ b/packages/fedify/src/federation/builder.ts @@ -30,6 +30,7 @@ import type { ObjectDispatcher, OutboxPermanentFailureHandler, SharedInboxKeyDispatcher, + UnverifiedActivityHandler, WebFingerLinksDispatcher, } from "./callback.ts"; import type { Context, RequestContext } from "./context.ts"; @@ -108,6 +109,7 @@ export class FederationBuilderImpl inboxListeners?: InboxListenerSet; inboxErrorHandler?: InboxErrorHandler; sharedInboxKeyDispatcher?: SharedInboxKeyDispatcher; + unverifiedActivityHandler?: UnverifiedActivityHandler; outboxPermanentFailureHandler?: OutboxPermanentFailureHandler; idempotencyStrategy?: | IdempotencyStrategy @@ -188,6 +190,7 @@ export class FederationBuilderImpl f.inboxListeners = this.inboxListeners?.clone(); f.inboxErrorHandler = this.inboxErrorHandler; f.sharedInboxKeyDispatcher = this.sharedInboxKeyDispatcher; + f.unverifiedActivityHandler = this.unverifiedActivityHandler; f.outboxPermanentFailureHandler = this.outboxPermanentFailureHandler; f.idempotencyStrategy = this.idempotencyStrategy; return f; @@ -1166,6 +1169,12 @@ export class FederationBuilderImpl return setters; } + onUnverifiedActivity( + handler: UnverifiedActivityHandler, + ): void { + this.unverifiedActivityHandler = handler; + } + setCollectionDispatcher< TObject extends Object, TParam extends string, diff --git a/packages/fedify/src/federation/callback.ts b/packages/fedify/src/federation/callback.ts index 94bc12f75..fbfa12f14 100644 --- a/packages/fedify/src/federation/callback.ts +++ b/packages/fedify/src/federation/callback.ts @@ -1,5 +1,6 @@ import type { Activity, Actor, Object } from "@fedify/vocab"; import type { Link } from "@fedify/webfinger"; +import type { VerifyRequestFailureReason } from "../sig/http.ts"; import type { NodeInfo } from "../nodeinfo/types.ts"; import type { PageItems } from "./collection.ts"; import type { Context, InboxContext, RequestContext } from "./context.ts"; @@ -180,6 +181,35 @@ export type InboxListener = ( activity: TActivity, ) => void | Promise; +/** + * The reason why an incoming activity could not be verified. + * + * Unlike inbox listeners registered through {@link InboxListenerSetters.on}, + * unverified activity handlers are called only when the activity payload could + * be parsed but its HTTP signatures could not be verified. + * + * @since 2.1.0 + */ +export type UnverifiedActivityReason = VerifyRequestFailureReason; + +/** + * A callback that handles activities whose signatures could not be verified. + * + * Returning a {@link Response} overrides Fedify's default `401 Unauthorized` + * response. Returning `void` keeps the default behavior. + * + * @template TContextData The context data to pass to the {@link Context}. + * @param context The request context. + * @param activity The incoming activity that could be parsed. + * @param reason The reason why signature verification failed. + * @since 2.1.0 + */ +export type UnverifiedActivityHandler = ( + context: RequestContext, + activity: Activity, + reason: UnverifiedActivityReason, +) => void | Response | Promise; + /** * A callback that handles errors in an inbox. * diff --git a/packages/fedify/src/federation/federation.ts b/packages/fedify/src/federation/federation.ts index cc2c39cb2..0be49957e 100644 --- a/packages/fedify/src/federation/federation.ts +++ b/packages/fedify/src/federation/federation.ts @@ -33,6 +33,7 @@ import type { OutboxErrorHandler, OutboxPermanentFailureHandler, SharedInboxKeyDispatcher, + UnverifiedActivityHandler, WebFingerLinksDispatcher, } from "./callback.ts"; import type { Context, InboxContext, RequestContext } from "./context.ts"; @@ -460,6 +461,36 @@ export interface Federatable { sharedInboxPath?: string, ): InboxListenerSetters; + /** + * Registers a callback for incoming activities whose HTTP signatures could + * not be verified. + * + * The regular inbox listeners registered through + * {@link InboxListenerSetters.on} continue to receive only verified + * activities. This hook is an opt-in escape hatch for applications that + * need to inspect unverified deliveries and optionally override the default + * `401 Unauthorized` response. + * + * @example + * ``` typescript + * federation.onUnverifiedActivity((ctx, activity, reason) => { + * if ( + * reason.type === "keyFetchError" && + * "status" in reason.result && + * reason.result.status === 410 + * ) { + * return new Response(null, { status: 202 }); + * } + * }); + * ``` + * + * @param handler A callback to handle an unverified activity. + * @since 2.1.0 + */ + onUnverifiedActivity( + handler: UnverifiedActivityHandler, + ): void; + /** * Registers a collection of objects dispatcher. * diff --git a/packages/fedify/src/federation/handler.test.ts b/packages/fedify/src/federation/handler.test.ts index 1add1f6d2..9ff081886 100644 --- a/packages/fedify/src/federation/handler.test.ts +++ b/packages/fedify/src/federation/handler.test.ts @@ -10,6 +10,7 @@ import { type Object, Person, } from "@fedify/vocab"; +import { FetchError } from "@fedify/vocab-runtime"; import { assert, assertEquals } from "@std/assert"; import { signRequest } from "../sig/http.ts"; import { @@ -1914,7 +1915,7 @@ test("handleInbox() records OpenTelemetry span events", async () => { // Verify event attributes assert(event.attributes != null); - assertEquals(event.attributes["activitypub.activity.verified"], true); + assertEquals(event.attributes["activitypub.activity.verified"], false); assertEquals(event.attributes["http_signatures.verified"], false); assert(typeof event.attributes["activitypub.activity.json"] === "string"); @@ -1925,3 +1926,110 @@ test("handleInbox() records OpenTelemetry span events", async () => { assertEquals(recordedActivity.id, "https://example.com/activity"); assertEquals(recordedActivity.type, "Create"); }); + +test("handleInbox() records unverified HTTP signature details", async () => { + const [tracerProvider, exporter] = createTestTracerProvider(); + const kv = new MemoryKvStore(); + const federation = createFederation({ kv, tracerProvider }); + const keyId = new URL("https://gone.example/users/someone#main-key"); + + const activity = new Create({ + id: new URL("https://example.com/activity"), + actor: new URL("https://gone.example/users/someone"), + object: new Note({ + id: new URL("https://example.com/note"), + content: "Hello, world!", + }), + }); + + const request = new Request("https://example.com/users/someone/inbox", { + method: "POST", + headers: { + "Content-Type": "application/activity+json", + }, + body: JSON.stringify(await activity.toJsonLd()), + }); + const signed = await signRequest(request, rsaPrivateKey3, keyId); + + const documentLoader = (url: string) => { + if (url === keyId.href) { + throw new FetchError( + keyId, + `HTTP 410: ${keyId.href}`, + new Response(null, { status: 410 }), + ); + } + return mockDocumentLoader(url); + }; + + const context = createRequestContext({ + federation, + request: signed, + url: new URL(signed.url), + data: undefined, + documentLoader, + contextLoader: mockDocumentLoader, + getActorUri(identifier: string) { + return new URL(`https://example.com/users/${identifier}`); + }, + }); + + const actorDispatcher: ActorDispatcher = (ctx, identifier) => { + if (identifier !== "someone") return null; + return new Person({ + id: ctx.getActorUri(identifier), + name: "Someone", + inbox: new URL("https://example.com/users/someone/inbox"), + publicKey: rsaPublicKey2, + }); + }; + + const response = await handleInbox(signed, { + recipient: "someone", + context, + inboxContextFactory(_activity) { + return createInboxContext({ ...context, clone: undefined }); + }, + kv, + kvPrefixes: { + activityIdempotence: ["activityIdempotence"], + publicKey: ["publicKey"], + }, + actorDispatcher, + inboxListeners: new InboxListenerSet(), + inboxErrorHandler: undefined, + unverifiedActivityHandler() { + return new Response("", { status: 202 }); + }, + onNotFound: (_request) => new Response("Not found", { status: 404 }), + signatureTimeWindow: false, + skipSignatureVerification: false, + tracerProvider, + }); + + assertEquals(response.status, 202); + + const verifySpans = exporter.getSpans("http_signatures.verify"); + assertEquals(verifySpans.length, 1); + assertEquals( + verifySpans[0].attributes["http_signatures.failure_reason"], + "keyFetchError", + ); + assertEquals( + verifySpans[0].attributes["http_signatures.key_fetch_status"], + 410, + ); + + const events = exporter.getEvents( + "activitypub.inbox", + "activitypub.activity.received", + ); + assertEquals(events.length, 1); + const event = events[0]; + assert(event.attributes != null); + assertEquals( + event.attributes["http_signatures.failure_reason"], + "keyFetchError", + ); + assertEquals(event.attributes["http_signatures.key_fetch_status"], 410); +}); diff --git a/packages/fedify/src/federation/handler.ts b/packages/fedify/src/federation/handler.ts index cb09e4774..a0fe51b85 100644 --- a/packages/fedify/src/federation/handler.ts +++ b/packages/fedify/src/federation/handler.ts @@ -20,7 +20,7 @@ import type { } from "@opentelemetry/api"; import { SpanKind, SpanStatusCode, trace } from "@opentelemetry/api"; import metadata from "../../deno.json" with { type: "json" }; -import { verifyRequest } from "../sig/http.ts"; +import { verifyRequestDetailed } from "../sig/http.ts"; import { detachSignature, verifyJsonLd } from "../sig/ld.ts"; import { doesActorOwnKey } from "../sig/owner.ts"; import { verifyObject } from "../sig/proof.ts"; @@ -36,6 +36,7 @@ import type { InboxErrorHandler, ObjectAuthorizePredicate, ObjectDispatcher, + UnverifiedActivityHandler, } from "./callback.ts"; import type { PageItems } from "./collection.ts"; import type { Context, InboxContext, RequestContext } from "./context.ts"; @@ -465,6 +466,7 @@ export interface InboxHandlerParameters { actorDispatcher?: ActorDispatcher; inboxListeners?: InboxListenerSet; inboxErrorHandler?: InboxErrorHandler; + unverifiedActivityHandler?: UnverifiedActivityHandler; onNotFound(request: Request): Response | Promise; signatureTimeWindow: Temporal.Duration | Temporal.DurationLike | false; skipSignatureVerification: boolean; @@ -532,6 +534,7 @@ async function handleInboxInternal( actorDispatcher, inboxListeners, inboxErrorHandler, + unverifiedActivityHandler, onNotFound, signatureTimeWindow, skipSignatureVerification, @@ -620,9 +623,11 @@ async function handleInboxInternal( } const jsonWithoutSig = detachSignature(json); let activity: Activity | null = null; + let activityVerified = false; if (ldSigVerified) { logger.debug("Linked Data Signatures are verified.", { recipient, json }); activity = await Activity.fromJsonLd(jsonWithoutSig, ctx); + activityVerified = true; } else { logger.debug( "Linked Data Signatures are not verified.", @@ -668,39 +673,110 @@ async function handleInboxInternal( "Object Integrity Proofs are verified.", { recipient, activity: json }, ); + activityVerified = true; } } let httpSigKey: CryptographicKey | null = null; if (activity == null) { if (!skipSignatureVerification) { - const key = await verifyRequest(request, { + const verification = await verifyRequestDetailed(request, { contextLoader: ctx.contextLoader, documentLoader: ctx.documentLoader, timeWindow: signatureTimeWindow, keyCache, tracerProvider, }); - if (key == null) { + if (verification.verified === false) { + const reason = verification.reason; logger.error( "Failed to verify the request's HTTP Signatures.", - { recipient }, + { + recipient, + reason: reason.type, + keyId: "keyId" in reason ? reason.keyId?.href : undefined, + }, ); span.setStatus({ code: SpanStatusCode.ERROR, message: `Failed to verify the request's HTTP Signatures.`, }); - const response = new Response( + if (unverifiedActivityHandler == null) { + return new Response( + "Failed to verify the request signature.", + { + status: 401, + headers: { "Content-Type": "text/plain; charset=utf-8" }, + }, + ); + } + try { + activity = await Activity.fromJsonLd(jsonWithoutSig, ctx); + } catch (error) { + logger.error("Failed to parse activity:\n{error}", { + recipient, + activity: json, + error, + }); + try { + await inboxErrorHandler?.(ctx, error as Error); + } catch (error) { + logger.error( + "An unexpected error occurred in inbox error handler:\n{error}", + { error, activity: json, recipient }, + ); + } + return new Response("Invalid activity.", { + status: 400, + headers: { "Content-Type": "text/plain; charset=utf-8" }, + }); + } + if (activity.id != null) { + span.setAttribute("activitypub.activity.id", activity.id.href); + } + span.setAttribute( + "activitypub.activity.type", + getTypeId(activity).href, + ); + const eventAttributes: Record = { + "activitypub.activity.json": JSON.stringify(json), + "activitypub.activity.verified": false, + "ld_signatures.verified": ldSigVerified, + "http_signatures.verified": false, + "http_signatures.key_id": "keyId" in reason + ? (reason.keyId?.href ?? "") + : "", + "http_signatures.failure_reason": reason.type, + }; + if (reason.type === "keyFetchError") { + if ("status" in reason.result) { + eventAttributes["http_signatures.key_fetch_status"] = + reason.result.status; + } else { + eventAttributes["http_signatures.key_fetch_error"] = + reason.result.error.name || + reason.result.error.constructor.name || + "Error"; + } + } + span.addEvent("activitypub.activity.received", eventAttributes); + const response = await unverifiedActivityHandler( + ctx, + activity, + reason, + ); + if (response instanceof Response) return response; + return new Response( "Failed to verify the request signature.", { status: 401, headers: { "Content-Type": "text/plain; charset=utf-8" }, }, ); - return response; } else { logger.debug("HTTP Signatures are verified.", { recipient }); + activityVerified = true; } - httpSigKey = key; + httpSigKey = verification.key; } activity = await Activity.fromJsonLd(jsonWithoutSig, ctx); } @@ -712,7 +788,7 @@ async function handleInboxInternal( // Record the received activity with verification details span.addEvent("activitypub.activity.received", { "activitypub.activity.json": JSON.stringify(json), - "activitypub.activity.verified": activity != null, + "activitypub.activity.verified": activityVerified, "ld_signatures.verified": ldSigVerified, "http_signatures.verified": httpSigKey != null, "http_signatures.key_id": httpSigKey?.id?.href ?? "", diff --git a/packages/fedify/src/federation/middleware.test.ts b/packages/fedify/src/federation/middleware.test.ts index 7f0656738..5a116ac07 100644 --- a/packages/fedify/src/federation/middleware.test.ts +++ b/packages/fedify/src/federation/middleware.test.ts @@ -1527,6 +1527,241 @@ test("Federation.setInboxListeners()", async (t) => { ]); }); + await t.step("onUnverifiedActivity()", async (t) => { + const options = { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + }; + + async function createInboxRequest( + activity: vocab.Create, + signature?: { privateKey: CryptoKey; keyId: URL }, + ): Promise { + let request = new Request("https://example.com/inbox", { + method: "POST", + headers: { + "Content-Type": "application/activity+json", + accept: "application/ld+json", + }, + body: JSON.stringify(await activity.toJsonLd(options)), + }); + if (signature != null) { + request = await signRequest( + request, + signature.privateKey, + signature.keyId, + ); + } + return request; + } + + function createFederationWithLoader( + documentLoader: typeof mockDocumentLoader, + ) { + const federation = createFederation({ + kv: new MemoryKvStore(), + documentLoaderFactory: () => documentLoader, + contextLoaderFactory: () => mockDocumentLoader, + }); + const verified: vocab.Create[] = []; + federation.setActorDispatcher("/users/{identifier}", () => { + return new vocab.Person({}); + }); + federation.setInboxListeners("/users/{identifier}/inbox", "/inbox") + .on(vocab.Create, (_ctx, activity) => { + verified.push(activity); + }); + return { federation, verified }; + } + + await t.step("receives noSignature reason", async () => { + const { federation, verified } = createFederationWithLoader( + mockDocumentLoader, + ); + let receivedReason: unknown = null; + federation.onUnverifiedActivity((_ctx, _activity, reason) => { + receivedReason = reason; + return new Response(null, { status: 202 }); + }); + + const response = await federation.fetch( + await createInboxRequest( + new vocab.Create({ + id: new URL("https://remote.example/activities/no-signature"), + actor: new URL("https://remote.example/actors/alice"), + }), + ), + { contextData: undefined }, + ); + + assertEquals(response.status, 202); + assertEquals(receivedReason, { type: "noSignature" }); + assertEquals(verified, []); + }); + + await t.step("receives keyFetchError for 410 responses", async () => { + const goneKeyId = new URL("https://gone.example/actors/alice#main-key"); + const goneLoader = async (url: string) => { + if (url === goneKeyId.href) { + throw new FetchError( + goneKeyId, + `HTTP 410: ${goneKeyId.href}`, + new Response(null, { status: 410 }), + ); + } + return await mockDocumentLoader(url); + }; + const { federation, verified } = createFederationWithLoader(goneLoader); + let receivedReason: unknown = null; + federation.onUnverifiedActivity((_ctx, _activity, reason) => { + receivedReason = reason; + return new Response(null, { status: 202 }); + }); + + const response = await federation.fetch( + await createInboxRequest( + new vocab.Create({ + id: new URL("https://gone.example/activities/delete"), + actor: new URL("https://gone.example/actors/alice"), + }), + { privateKey: rsaPrivateKey3, keyId: goneKeyId }, + ), + { contextData: undefined }, + ); + + assertEquals(response.status, 202); + assertEquals(verified, []); + assertEquals( + (receivedReason as { type: string }).type, + "keyFetchError", + ); + assertEquals( + (receivedReason as { keyId: URL }).keyId.href, + goneKeyId.href, + ); + assertEquals( + ( + receivedReason as { + result: { status: number; response: Response }; + } + ).result.status, + 410, + ); + }); + + await t.step("falls back to 401 when handler returns void", async () => { + const missingKeyId = new URL( + "https://missing.example/actors/alice#main-key", + ); + const missingLoader = async (url: string) => { + if (url === missingKeyId.href) { + throw new FetchError( + missingKeyId, + `HTTP 404: ${missingKeyId.href}`, + new Response(null, { status: 404 }), + ); + } + return await mockDocumentLoader(url); + }; + const { federation, verified } = createFederationWithLoader( + missingLoader, + ); + let receivedReason: unknown = null; + federation.onUnverifiedActivity((_ctx, _activity, reason) => { + receivedReason = reason; + }); + + const response = await federation.fetch( + await createInboxRequest( + new vocab.Create({ + id: new URL("https://missing.example/activities/delete"), + actor: new URL("https://missing.example/actors/alice"), + }), + { privateKey: rsaPrivateKey3, keyId: missingKeyId }, + ), + { contextData: undefined }, + ); + + assertEquals(response.status, 401); + assertEquals(verified, []); + assertEquals( + (receivedReason as { type: string }).type, + "keyFetchError", + ); + assertEquals( + ( + receivedReason as { + result: { status: number; response: Response }; + } + ).result.status, + 404, + ); + }); + + await t.step("receives invalidSignature reason", async () => { + const { federation, verified } = createFederationWithLoader( + mockDocumentLoader, + ); + let receivedReason: unknown = null; + federation.onUnverifiedActivity((_ctx, _activity, reason) => { + receivedReason = reason; + return new Response(null, { status: 202 }); + }); + + const keyId = new URL("https://example.com/person2#key3"); + const response = await federation.fetch( + await createInboxRequest( + new vocab.Create({ + id: new URL("https://example.com/activities/invalid-signature"), + actor: new URL("https://example.com/person2"), + }), + { privateKey: rsaPrivateKey2, keyId }, + ), + { contextData: undefined }, + ); + + assertEquals(response.status, 202); + assertEquals(verified, []); + assertEquals( + (receivedReason as { type: string }).type, + "invalidSignature", + ); + assertEquals( + (receivedReason as { keyId: URL }).keyId.href, + keyId.href, + ); + }); + + await t.step("does not run for verified activities", async () => { + const { federation, verified } = createFederationWithLoader( + mockDocumentLoader, + ); + let unverifiedCalls = 0; + federation.onUnverifiedActivity(() => { + unverifiedCalls++; + return new Response(null, { status: 202 }); + }); + + const response = await federation.fetch( + await createInboxRequest( + new vocab.Create({ + id: new URL("https://example.com/activities/verified"), + actor: new URL("https://example.com/person2"), + }), + { + privateKey: rsaPrivateKey3, + keyId: new URL("https://example.com/person2#key3"), + }, + ), + { contextData: undefined }, + ); + + assertEquals(response.status, 202); + assertEquals(unverifiedCalls, 0); + assertEquals(verified.length, 1); + }); + }); + await t.step("onError()", async () => { const federation = createFederation({ kv, diff --git a/packages/fedify/src/federation/middleware.ts b/packages/fedify/src/federation/middleware.ts index 476f53be6..7fe1b91ab 100644 --- a/packages/fedify/src/federation/middleware.ts +++ b/packages/fedify/src/federation/middleware.ts @@ -1481,6 +1481,7 @@ export class FederationImpl actorDispatcher: this.actorCallbacks?.dispatcher, inboxListeners: this.inboxListeners, inboxErrorHandler: this.inboxErrorHandler, + unverifiedActivityHandler: this.unverifiedActivityHandler, onNotFound, signatureTimeWindow: this.signatureTimeWindow, skipSignatureVerification: this.skipSignatureVerification, diff --git a/packages/fedify/src/otel/exporter.test.ts b/packages/fedify/src/otel/exporter.test.ts index 4f5e8e5bc..2b583d108 100644 --- a/packages/fedify/src/otel/exporter.test.ts +++ b/packages/fedify/src/otel/exporter.test.ts @@ -70,6 +70,9 @@ function createActivityReceivedEvent(options: { ldSigVerified?: boolean; httpSigVerified?: boolean; httpSigKeyId?: string; + httpSigFailureReason?: string; + httpSigKeyFetchStatus?: number; + httpSigKeyFetchError?: string; }): TimedEvent { return { name: "activitypub.activity.received", @@ -80,6 +83,9 @@ function createActivityReceivedEvent(options: { "ld_signatures.verified": options.ldSigVerified ?? false, "http_signatures.verified": options.httpSigVerified ?? true, "http_signatures.key_id": options.httpSigKeyId ?? "", + "http_signatures.failure_reason": options.httpSigFailureReason ?? "", + "http_signatures.key_fetch_status": options.httpSigKeyFetchStatus, + "http_signatures.key_fetch_error": options.httpSigKeyFetchError ?? "", }, }; } @@ -833,6 +839,59 @@ test("FedifySpanExporter", async (t) => { }, ); + await t.step( + "extracts HTTP signature failure details for inbound activity", + async () => { + const kv = new MemoryKvStore(); + const exporter = new FedifySpanExporter(kv); + + const activityJson = JSON.stringify({ + "@context": "https://www.w3.org/ns/activitystreams", + type: "Delete", + id: "https://example.com/activities/unverified", + actor: "https://example.com/users/alice", + }); + + const span = createMockSpan({ + traceId: "sig-failure-trace", + spanId: "span1", + name: "activitypub.inbox", + events: [ + createActivityReceivedEvent({ + activityJson, + verified: false, + httpSigVerified: false, + httpSigKeyId: "https://example.com/users/alice#main-key", + httpSigFailureReason: "keyFetchError", + httpSigKeyFetchStatus: 410, + ldSigVerified: false, + }), + ], + }); + + await new Promise((resolve) => { + exporter.export([span], () => resolve()); + }); + + const activities = await exporter.getActivitiesByTraceId( + "sig-failure-trace", + ); + assertEquals(activities.length, 1); + assertEquals( + activities[0].signatureDetails?.httpSignaturesFailureReason, + "keyFetchError", + ); + assertEquals( + activities[0].signatureDetails?.httpSignaturesKeyFetchStatus, + 410, + ); + assertEquals( + activities[0].signatureDetails?.httpSignaturesKeyFetchError, + undefined, + ); + }, + ); + await t.step( "handles activity without actor field", async () => { diff --git a/packages/fedify/src/otel/exporter.ts b/packages/fedify/src/otel/exporter.ts index a59e44a35..685ff0940 100644 --- a/packages/fedify/src/otel/exporter.ts +++ b/packages/fedify/src/otel/exporter.ts @@ -26,6 +26,21 @@ export interface SignatureVerificationDetails { */ readonly httpSignaturesKeyId?: string; + /** + * The reason why HTTP signature verification failed, if available. + */ + readonly httpSignaturesFailureReason?: string; + + /** + * The HTTP status code from a failed key fetch, if available. + */ + readonly httpSignaturesKeyFetchStatus?: number; + + /** + * The error type from a non-HTTP key fetch failure, if available. + */ + readonly httpSignaturesKeyFetchError?: string; + /** * Whether Linked Data Signatures were verified. */ @@ -332,12 +347,16 @@ export class FedifySpanExporter implements SpanExporter { // Extract signature verification details const httpSigVerified = attrs["http_signatures.verified"]; const httpSigKeyId = attrs["http_signatures.key_id"]; + const httpSigFailureReason = attrs["http_signatures.failure_reason"]; + const httpSigKeyFetchStatus = attrs["http_signatures.key_fetch_status"]; + const httpSigKeyFetchError = attrs["http_signatures.key_fetch_error"]; const ldSigVerified = attrs["ld_signatures.verified"]; let signatureDetails: SignatureVerificationDetails | undefined; if ( typeof httpSigVerified === "boolean" || - typeof ldSigVerified === "boolean" + typeof ldSigVerified === "boolean" || + typeof httpSigFailureReason === "string" ) { signatureDetails = { httpSignaturesVerified: httpSigVerified === true, @@ -345,6 +364,17 @@ export class FedifySpanExporter implements SpanExporter { typeof httpSigKeyId === "string" && httpSigKeyId !== "" ? httpSigKeyId : undefined, + httpSignaturesFailureReason: typeof httpSigFailureReason === "string" && + httpSigFailureReason !== "" + ? httpSigFailureReason + : undefined, + httpSignaturesKeyFetchStatus: typeof httpSigKeyFetchStatus === "number" + ? httpSigKeyFetchStatus + : undefined, + httpSignaturesKeyFetchError: typeof httpSigKeyFetchError === "string" && + httpSigKeyFetchError !== "" + ? httpSigKeyFetchError + : undefined, ldSignaturesVerified: ldSigVerified === true, }; } diff --git a/packages/fedify/src/sig/http.test.ts b/packages/fedify/src/sig/http.test.ts index 9a68efc5c..2c95b18b2 100644 --- a/packages/fedify/src/sig/http.test.ts +++ b/packages/fedify/src/sig/http.test.ts @@ -1,6 +1,10 @@ -import { mockDocumentLoader, test } from "@fedify/fixture"; +import { + createTestTracerProvider, + mockDocumentLoader, + test, +} from "@fedify/fixture"; import type { CryptographicKey, Multikey } from "@fedify/vocab"; -import { exportSpki } from "@fedify/vocab-runtime"; +import { exportSpki, FetchError } from "@fedify/vocab-runtime"; import { assert, assertEquals, @@ -29,6 +33,7 @@ import { signRequest, timingSafeEqual, verifyRequest, + verifyRequestDetailed, type VerifyRequestOptions, } from "./http.ts"; import { exportJwk, type KeyCache } from "./key.ts"; @@ -252,6 +257,116 @@ test("verifyRequest() [draft-cavage]", async () => { assert(await verifyRequest(request2, options2) != null); }); +test("verifyRequestDetailed() classifies malformed signatures as invalid", async () => { + const draftMissingKeyId = await verifyRequestDetailed( + new Request("https://example.com/", { + method: "POST", + headers: { + Date: "Tue, 05 Mar 2024 07:49:44 GMT", + Digest: "sha-256=47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=", + Signature: 'headers="(request-target) date digest",signature="AAAA"', + }, + body: "", + }), + { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + }, + ); + assertFalse(draftMissingKeyId.verified); + assertEquals(draftMissingKeyId.reason.type, "invalidSignature"); + assertFalse("keyId" in draftMissingKeyId.reason); + + const draftInvalidKeyId = await verifyRequestDetailed( + new Request("https://example.com/", { + method: "POST", + headers: { + Date: "Tue, 05 Mar 2024 07:49:44 GMT", + Digest: "sha-256=47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=", + Signature: 'keyId="not a url",headers="(request-target) date digest",' + + 'signature="AAAA"', + }, + body: "", + }), + { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + }, + ); + assertFalse(draftInvalidKeyId.verified); + assertEquals(draftInvalidKeyId.reason.type, "invalidSignature"); + assertFalse("keyId" in draftInvalidKeyId.reason); + + const rfcMissingKeyId = await verifyRequestDetailed( + new Request("https://example.com/api/resource", { + method: "GET", + headers: { + Host: "example.com", + Date: "Tue, 05 Mar 2024 07:49:44 GMT", + "Signature-Input": + 'sig1=("@method" "@target-uri" "@authority");created=1709626184', + Signature: "sig1=:AAAA:", + }, + }), + { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + spec: "rfc9421", + }, + ); + assertFalse(rfcMissingKeyId.verified); + assertEquals(rfcMissingKeyId.reason.type, "invalidSignature"); + assertFalse("keyId" in rfcMissingKeyId.reason); +}); + +test("verifyRequestDetailed() records failure details on span", async () => { + const [tracerProvider, exporter] = createTestTracerProvider(); + const keyId = new URL("https://gone.example/actors/alice#main-key"); + const request = await signRequest( + new Request("https://example.com/inbox", { + method: "POST", + headers: { + "Content-Type": "application/activity+json", + accept: "application/ld+json", + }, + body: JSON.stringify({ + "@context": "https://www.w3.org/ns/activitystreams", + type: "Create", + actor: "https://gone.example/actors/alice", + }), + }), + rsaPrivateKey2, + keyId, + ); + + const result = await verifyRequestDetailed(request, { + tracerProvider, + contextLoader: mockDocumentLoader, + documentLoader(url) { + if (url === keyId.href) { + throw new FetchError( + keyId, + `HTTP 410: ${keyId.href}`, + new Response(null, { status: 410 }), + ); + } + return mockDocumentLoader(url); + }, + }); + + assertFalse(result.verified); + const spans = exporter.getSpans("http_signatures.verify"); + assertEquals(spans.length, 1); + const span = spans[0]; + assertEquals(span.attributes["http_signatures.verified"], false); + assertEquals( + span.attributes["http_signatures.failure_reason"], + "keyFetchError", + ); + assertEquals(span.attributes["http_signatures.key_id"], keyId.href); + assertEquals(span.attributes["http_signatures.key_fetch_status"], 410); +}); + test("signRequest() and verifyRequest() [rfc9421] implementation", async () => { // Create a fixed timestamp and content for consistent testing const currentTimestamp = 1709626184; diff --git a/packages/fedify/src/sig/http.ts b/packages/fedify/src/sig/http.ts index 4fd962ea8..0c7c7129f 100644 --- a/packages/fedify/src/sig/http.ts +++ b/packages/fedify/src/sig/http.ts @@ -21,7 +21,12 @@ import { Item, } from "structured-field-values"; import metadata from "../../deno.json" with { type: "json" }; -import { fetchKey, type KeyCache, validateCryptoKey } from "./key.ts"; +import { + fetchKeyDetailed, + type FetchKeyErrorResult, + type KeyCache, + validateCryptoKey, +} from "./key.ts"; /** * The standard to use for signing and verifying HTTP signatures. @@ -546,6 +551,107 @@ export interface VerifyRequestOptions { tracerProvider?: TracerProvider; } +/** + * The reason why {@link verifyRequestDetailed} could not verify a request. + * @since 2.1.0 + */ +export type VerifyRequestFailureReason = + | { + readonly type: "keyFetchError"; + readonly keyId: URL; + readonly result: FetchKeyErrorResult; + } + | { + readonly type: "invalidSignature"; + readonly keyId?: URL; + } + | { + readonly type: "noSignature"; + }; + +/** + * The detailed result of {@link verifyRequestDetailed}. + * @since 2.1.0 + */ +export type VerifyRequestDetailedResult = + | { + readonly verified: true; + readonly key: CryptographicKey; + } + | { + readonly verified: false; + readonly reason: VerifyRequestFailureReason; + }; + +function noSignatureResult(): VerifyRequestDetailedResult { + return { + verified: false, + reason: { type: "noSignature" }, + }; +} + +function invalidSignatureResult( + keyId: URL | null, +): VerifyRequestDetailedResult { + return keyId == null + ? { + verified: false, + reason: { type: "invalidSignature" }, + } + : { + verified: false, + reason: { type: "invalidSignature", keyId }, + }; +} + +function keyFetchErrorResult( + keyId: URL, + result: FetchKeyErrorResult, +): VerifyRequestDetailedResult { + return { + verified: false, + reason: { type: "keyFetchError", keyId, result }, + }; +} + +function parseKeyId(value: string | undefined): URL | null { + if (value == null) return null; + try { + return new URL(value); + } catch { + return null; + } +} + +function getKeyFetchErrorName(error: Error): string { + return error.name || error.constructor.name || "Error"; +} + +function recordVerificationResult( + span: Span, + result: VerifyRequestDetailedResult, +): void { + span.setAttribute("http_signatures.verified", result.verified); + if (result.verified === true) return; + const reason = result.reason; + span.setAttribute("http_signatures.failure_reason", reason.type); + if ("keyId" in reason && reason.keyId != null) { + span.setAttribute("http_signatures.key_id", reason.keyId.href); + } + if (reason.type !== "keyFetchError") return; + if ("status" in reason.result) { + span.setAttribute( + "http_signatures.key_fetch_status", + reason.result.status, + ); + } else { + span.setAttribute( + "http_signatures.key_fetch_error", + getKeyFetchErrorName(reason.result.error), + ); + } +} + /** * Verifies the signature of a request. * @@ -563,6 +669,23 @@ export async function verifyRequest( request: Request, options: VerifyRequestOptions = {}, ): Promise { + const result = await verifyRequestDetailed(request, options); + return result.verified ? result.key : null; +} + +/** + * Verifies the signature of a request and returns a structured failure reason + * when verification does not succeed. + * + * @param request The request to verify. + * @param options Options for verifying the request. + * @returns The verified public key, or a structured verification failure. + * @since 2.1.0 + */ +export async function verifyRequestDetailed( + request: Request, + options: VerifyRequestOptions = {}, +): Promise { const tracerProvider = options.tracerProvider ?? trace.getTracerProvider(); const tracer = tracerProvider.getTracer( metadata.name, @@ -587,15 +710,18 @@ export async function verifyRequest( : "draft-cavage-http-signatures-12"; } - let key: CryptographicKey | null; + let result: VerifyRequestDetailedResult; if (spec === "rfc9421") { - key = await verifyRequestRfc9421(request, span, options); + result = await verifyRequestRfc9421(request, span, options); } else { - key = await verifyRequestDraft(request, span, options); + result = await verifyRequestDraft(request, span, options); } - if (key == null) span.setStatus({ code: SpanStatusCode.ERROR }); - return key; + recordVerificationResult(span, result); + if (!result.verified) { + span.setStatus({ code: SpanStatusCode.ERROR }); + } + return result; } catch (error) { span.setStatus({ code: SpanStatusCode.ERROR, @@ -620,38 +746,46 @@ async function verifyRequestDraft( keyCache, tracerProvider, }: VerifyRequestOptions = {}, -): Promise { +): Promise { const logger = getLogger(["fedify", "sig", "http"]); if (request.bodyUsed) { logger.error( "Failed to verify; the request body is already consumed.", { url: request.url }, ); - return null; + return noSignatureResult(); } else if (request.body?.locked) { logger.error( "Failed to verify; the request body is locked.", { url: request.url }, ); - return null; + return noSignatureResult(); } const originalRequest = request; request = request.clone() as Request; - const dateHeader = request.headers.get("Date"); - if (dateHeader == null) { + const sigHeader = request.headers.get("Signature"); + if (sigHeader == null) { logger.debug( - "Failed to verify; no Date header found.", + "Failed to verify; no Signature header found.", { headers: Object.fromEntries(request.headers.entries()) }, ); - return null; + return noSignatureResult(); } - const sigHeader = request.headers.get("Signature"); - if (sigHeader == null) { + const sigValues = Object.fromEntries( + sigHeader.split(",").map((pair) => + pair.match(/^\s*([A-Za-z]+)=(?:"([^"]*)"|(\d+))\s*$/) + ).filter((m) => m != null).map((m) => + [m![1], m![2] ?? m![3]] as [string, string] + ), + ); + const parsedKeyId = parseKeyId(sigValues.keyId); + const dateHeader = request.headers.get("Date"); + if (dateHeader == null) { logger.debug( - "Failed to verify; no Signature header found.", + "Failed to verify; no Date header found.", { headers: Object.fromEntries(request.headers.entries()) }, ); - return null; + return invalidSignatureResult(parsedKeyId); } const digestHeader = request.headers.get("Digest"); if ( @@ -662,7 +796,7 @@ async function verifyRequestDraft( "Failed to verify; no Digest header found.", { headers: Object.fromEntries(request.headers.entries()) }, ); - return null; + return invalidSignatureResult(parsedKeyId); } let body: ArrayBuffer | null = null; if (digestHeader != null) { @@ -682,7 +816,7 @@ async function verifyRequestDraft( digest: digestBase64, error, }); - return null; + return invalidSignatureResult(parsedKeyId); } if (span.isRecording()) { span.setAttribute(`http_signatures.digest.${algo}`, encodeHex(digest)); @@ -701,7 +835,7 @@ async function verifyRequestDraft( expectedDigest: encodeBase64(expectedDigest), }, ); - return null; + return invalidSignatureResult(parsedKeyId); } matched = true; } @@ -714,7 +848,7 @@ async function verifyRequestDraft( algorithms: digests.map(([algo]) => algo), }, ); - return null; + return invalidSignatureResult(parsedKeyId); } } const date = Temporal.Instant.from(new Date(dateHeader).toISOString()); @@ -727,40 +861,33 @@ async function verifyRequestDraft( "Failed to verify; Date is too far in the future.", { date: date.toString(), now: now.toString() }, ); - return null; + return invalidSignatureResult(parsedKeyId); } else if (Temporal.Instant.compare(date, now.subtract(tw)) < 0) { logger.debug( "Failed to verify; Date is too far in the past.", { date: date.toString(), now: now.toString() }, ); - return null; + return invalidSignatureResult(parsedKeyId); } } - const sigValues = Object.fromEntries( - sigHeader.split(",").map((pair) => - pair.match(/^\s*([A-Za-z]+)=(?:"([^"]*)"|(\d+))\s*$/) - ).filter((m) => m != null).map((m) => - [m![1], m![2] ?? m![3]] as [string, string] - ), - ); if (!("keyId" in sigValues)) { logger.debug( "Failed to verify; no keyId field found in the Signature header.", { signature: sigHeader }, ); - return null; + return invalidSignatureResult(null); } else if (!("headers" in sigValues)) { logger.debug( "Failed to verify; no headers field found in the Signature header.", { signature: sigHeader }, ); - return null; + return invalidSignatureResult(parsedKeyId); } else if (!("signature" in sigValues)) { logger.debug( "Failed to verify; no signature field found in the Signature header.", { signature: sigHeader }, ); - return null; + return invalidSignatureResult(parsedKeyId); } if ("expires" in sigValues) { const expiresSeconds = parseInt(sigValues.expires); @@ -769,7 +896,7 @@ async function verifyRequestDraft( "Failed to verify; invalid expires field in the Signature header: {expires}.", { expires: sigValues.expires, signature: sigHeader }, ); - return null; + return invalidSignatureResult(parsedKeyId); } const expires = Temporal.Instant.fromEpochMilliseconds( expiresSeconds * 1000, @@ -783,7 +910,7 @@ async function verifyRequestDraft( signature: sigHeader, }, ); - return null; + return invalidSignatureResult(parsedKeyId); } } if ("created" in sigValues) { @@ -793,7 +920,7 @@ async function verifyRequestDraft( "Failed to verify; invalid created field in the Signature header: {created}.", { created: sigValues.created, signature: sigHeader }, ); - return null; + return invalidSignatureResult(parsedKeyId); } if (timeWindow !== false) { const created = Temporal.Instant.fromEpochMilliseconds( @@ -805,28 +932,37 @@ async function verifyRequestDraft( "Failed to verify; created is too far in the future.", { created: created.toString(), now: now.toString() }, ); - return null; + return invalidSignatureResult(parsedKeyId); } else if (Temporal.Instant.compare(created, now.subtract(tw)) < 0) { logger.debug( "Failed to verify; created is too far in the past.", { created: created.toString(), now: now.toString() }, ); - return null; + return invalidSignatureResult(parsedKeyId); } } } const { keyId, headers, signature } = sigValues; + const keyIdUrl = parseKeyId(keyId); + if (keyIdUrl == null) return invalidSignatureResult(null); span?.setAttribute("http_signatures.key_id", keyId); if ("algorithm" in sigValues) { span?.setAttribute("http_signatures.algorithm", sigValues.algorithm); } - const { key, cached } = await fetchKey(new URL(keyId), CryptographicKey, { - documentLoader, - contextLoader, - keyCache, - tracerProvider, - }); - if (key == null) return null; + const { key, cached, fetchError } = await fetchKeyDetailed( + keyIdUrl, + CryptographicKey, + { + documentLoader, + contextLoader, + keyCache, + tracerProvider, + }, + ); + if (fetchError != null) { + return keyFetchErrorResult(keyIdUrl, fetchError); + } + if (key == null) return invalidSignatureResult(keyIdUrl); const headerNames = headers.split(/\s+/g); if ( !headerNames.includes("(request-target)") || !headerNames.includes("date") @@ -836,7 +972,7 @@ async function verifyRequestDraft( "{headers}.", { headers }, ); - return null; + return invalidSignatureResult(keyIdUrl); } if (body != null && !headerNames.includes("digest")) { logger.debug( @@ -844,7 +980,7 @@ async function verifyRequestDraft( "{headers}.", { headers }, ); - return null; + return invalidSignatureResult(keyIdUrl); } const message = headerNames.map((name) => `${name}: ` + @@ -874,7 +1010,7 @@ async function verifyRequestDraft( "is invalid. Retrying with the freshly fetched key...", { keyId, signature, message }, ); - return await verifyRequest( + return await verifyRequestDetailed( originalRequest, { documentLoader, @@ -894,9 +1030,9 @@ async function verifyRequestDraft( "is correct. The message to sign is:\n{message}", { keyId, signature, message }, ); - return null; + return invalidSignatureResult(keyIdUrl); } - return key; + return { verified: true, key }; } /** @@ -977,20 +1113,20 @@ async function verifyRequestRfc9421( keyCache, tracerProvider, }: VerifyRequestOptions = {}, -): Promise { +): Promise { const logger = getLogger(["fedify", "sig", "http"]); if (request.bodyUsed) { logger.error( "Failed to verify; the request body is already consumed.", { url: request.url }, ); - return null; + return noSignatureResult(); } else if (request.body?.locked) { logger.error( "Failed to verify; the request body is locked.", { url: request.url }, ); - return null; + return noSignatureResult(); } const originalRequest = request; @@ -1003,7 +1139,7 @@ async function verifyRequestRfc9421( "Failed to verify; no Signature-Input header found.", { headers: Object.fromEntries(request.headers.entries()) }, ); - return null; + return noSignatureResult(); } const signatureHeader = request.headers.get("Signature"); @@ -1012,7 +1148,7 @@ async function verifyRequestRfc9421( "Failed to verify; no Signature header found.", { headers: Object.fromEntries(request.headers.entries()) }, ); - return null; + return noSignatureResult(); } // Parse the Signature-Input and Signature headers @@ -1030,21 +1166,23 @@ async function verifyRequestRfc9421( "Failed to verify; no valid signatures found in Signature-Input header.", { header: signatureInputHeader }, ); - return null; + return invalidSignatureResult(null); } - // Verify the first signature we can find - // In practice, we could implement signature selection logic here - let validKey: CryptographicKey | null = null; + let failure: VerifyRequestDetailedResult = noSignatureResult(); for (const sigName of signatureNames) { // Skip if we don't have the signature bytes if (!signatures[sigName]) { + failure = invalidSignatureResult( + parseKeyId(signatureInputs[sigName]?.keyId), + ); continue; } const sigInput = signatureInputs[sigName]; const sigBytes = signatures[sigName]; + const keyId = parseKeyId(sigInput.keyId); // Validate signature input parameters if (!sigInput.keyId) { @@ -1052,6 +1190,7 @@ async function verifyRequestRfc9421( "Failed to verify; missing keyId in signature {signatureName}.", { signatureName: sigName, signatureInput: signatureInputHeader }, ); + failure = invalidSignatureResult(null); continue; } @@ -1060,6 +1199,7 @@ async function verifyRequestRfc9421( "Failed to verify; missing created timestamp in signature {signatureName}.", { signatureName: sigName, signatureInput: signatureInputHeader }, ); + failure = invalidSignatureResult(keyId); continue; } @@ -1077,6 +1217,7 @@ async function verifyRequestRfc9421( "Failed to verify; signature created time is too far in the future.", { created: signatureCreated.toString(), now: now.toString() }, ); + failure = invalidSignatureResult(keyId); continue; } else if ( Temporal.Instant.compare(signatureCreated, now.subtract(tw)) < 0 @@ -1085,6 +1226,7 @@ async function verifyRequestRfc9421( "Failed to verify; signature created time is too far in the past.", { created: signatureCreated.toString(), now: now.toString() }, ); + failure = invalidSignatureResult(keyId); continue; } } @@ -1101,6 +1243,7 @@ async function verifyRequestRfc9421( "Failed to verify; Content-Digest header required but not found.", { components: sigInput.components }, ); + failure = invalidSignatureResult(keyId); continue; } @@ -1115,6 +1258,7 @@ async function verifyRequestRfc9421( "Failed to verify; Content-Digest verification failed.", { contentDigest: contentDigestHeader }, ); + failure = invalidSignatureResult(keyId); continue; } } @@ -1122,9 +1266,13 @@ async function verifyRequestRfc9421( // Fetch the public key span?.setAttribute("http_signatures.key_id", sigInput.keyId); span?.setAttribute("http_signatures.created", sigInput.created.toString()); + if (keyId == null) { + failure = invalidSignatureResult(null); + continue; + } - const { key, cached } = await fetchKey( - new URL(sigInput.keyId), + const { key, cached, fetchError } = await fetchKeyDetailed( + keyId, CryptographicKey, { documentLoader, @@ -1134,8 +1282,13 @@ async function verifyRequestRfc9421( }, ); + if (fetchError != null) { + failure = keyFetchErrorResult(keyId, fetchError); + continue; + } if (!key) { logger.debug("Failed to fetch key: {keyId}", { keyId: sigInput.keyId }); + failure = invalidSignatureResult(keyId); continue; } @@ -1169,6 +1322,7 @@ async function verifyRequestRfc9421( supported: Object.keys(rfc9421AlgorithmMap), }, ); + failure = invalidSignatureResult(keyId); continue; } @@ -1185,6 +1339,7 @@ async function verifyRequestRfc9421( "Failed to create signature base for verification: {error}", { error, signatureInput: sigInput }, ); + failure = invalidSignatureResult(keyId); continue; } const signatureBaseBytes = new TextEncoder().encode(signatureBase); @@ -1201,8 +1356,7 @@ async function verifyRequestRfc9421( ); if (verified) { - validKey = key; - break; + return { verified: true, key }; } else if (cached) { // If we used a cached key and verification failed, try fetching fresh key logger.debug( @@ -1210,7 +1364,7 @@ async function verifyRequestRfc9421( { keyId: sigInput.keyId }, ); - return await verifyRequest( + return await verifyRequestDetailed( originalRequest, { documentLoader, @@ -1229,16 +1383,18 @@ async function verifyRequestRfc9421( "Failed to verify signature with fetched key {keyId}; signature invalid.", { keyId: sigInput.keyId, signatureBase }, ); + failure = invalidSignatureResult(keyId); } } catch (error) { logger.debug( "Error during signature verification: {error}", { error, keyId: sigInput.keyId, algorithm: sigInput.alg }, ); + failure = invalidSignatureResult(keyId); } } - return validKey; + return failure; } /** diff --git a/packages/fedify/src/sig/key.test.ts b/packages/fedify/src/sig/key.test.ts index 6f52f8d0a..0f2802619 100644 --- a/packages/fedify/src/sig/key.test.ts +++ b/packages/fedify/src/sig/key.test.ts @@ -1,4 +1,8 @@ -import { mockDocumentLoader, test } from "@fedify/fixture"; +import { + createTestTracerProvider, + mockDocumentLoader, + test, +} from "@fedify/fixture"; import { CryptographicKey, Multikey } from "@fedify/vocab"; import { assertEquals, assertRejects, assertThrows } from "@std/assert"; import { @@ -11,6 +15,7 @@ import { import { exportJwk, fetchKey, + fetchKeyDetailed, type FetchKeyOptions, generateCryptoKeyPair, importJwk, @@ -351,3 +356,53 @@ test("fetchKey()", async () => { }, ); }); + +test("fetchKeyDetailed()", async () => { + const cache: Record = { + "https://example.com/nothing": null, + }; + let documentLoaderCalls = 0; + const [tracerProvider, exporter] = createTestTracerProvider(); + const options: FetchKeyOptions = { + documentLoader(url) { + documentLoaderCalls++; + return mockDocumentLoader(url); + }, + contextLoader: mockDocumentLoader, + tracerProvider, + keyCache: { + get(keyId) { + return Promise.resolve(cache[keyId.href]); + }, + set(keyId, key) { + cache[keyId.href] = key; + return Promise.resolve(); + }, + } satisfies KeyCache, + }; + + assertEquals( + await fetchKeyDetailed( + "https://example.com/nothing", + CryptographicKey, + options, + ), + { key: null, cached: true }, + ); + assertEquals(documentLoaderCalls, 0); + + assertEquals( + await fetchKeyDetailed( + "https://example.com/key", + CryptographicKey, + options, + ), + { key: rsaPublicKey1, cached: false }, + ); + assertEquals(documentLoaderCalls, 1); + + const spans = exporter.getSpans("activitypub.fetch_key"); + assertEquals(spans.length, 2); + assertEquals(spans[0].attributes["activitypub.actor.key.cached"], true); + assertEquals(spans[1].attributes["activitypub.actor.key.cached"], false); +}); diff --git a/packages/fedify/src/sig/key.ts b/packages/fedify/src/sig/key.ts index 016703743..6ee967260 100644 --- a/packages/fedify/src/sig/key.ts +++ b/packages/fedify/src/sig/key.ts @@ -4,7 +4,11 @@ import { type Multikey, Object, } from "@fedify/vocab"; -import { type DocumentLoader, getDocumentLoader } from "@fedify/vocab-runtime"; +import { + type DocumentLoader, + FetchError, + getDocumentLoader, +} from "@fedify/vocab-runtime"; import { getLogger } from "@logtape/logtape"; import { SpanKind, @@ -190,6 +194,68 @@ export interface FetchKeyResult { readonly cached: boolean; } +/** + * Detailed fetch failure information from {@link fetchKeyDetailed}. + * @since 2.1.0 + */ +export type FetchKeyErrorResult = + | { + readonly status: number; + readonly response: Response; + } + | { + readonly error: Error; + }; + +/** + * The result of {@link fetchKeyDetailed}. + * @since 2.1.0 + */ +export interface FetchKeyDetailedResult + extends FetchKeyResult { + /** + * The error that occurred while fetching the key, if fetching failed before + * a document could be parsed. + */ + readonly fetchError?: FetchKeyErrorResult; +} + +async function withFetchKeySpan( + keyId: URL, + tracerProvider: TracerProvider | undefined, + fetcher: () => Promise, +): Promise { + tracerProvider ??= trace.getTracerProvider(); + const tracer = tracerProvider.getTracer(metadata.name, metadata.version); + return await tracer.startActiveSpan( + "activitypub.fetch_key", + { + kind: SpanKind.CLIENT, + attributes: { + "http.method": "GET", + "url.full": keyId.href, + "url.scheme": keyId.protocol.replace(/:$/, ""), + "url.domain": keyId.hostname, + "url.path": keyId.pathname, + "url.query": keyId.search.replace(/^\?/, ""), + "url.fragment": keyId.hash.replace(/^#/, ""), + }, + }, + async (span) => { + try { + const result = await fetcher(); + span.setAttribute("activitypub.actor.key.cached", result.cached); + return result; + } catch (e) { + span.setStatus({ code: SpanStatusCode.ERROR, message: String(e) }); + throw e; + } finally { + span.end(); + } + }, + ); +} + /** * Fetches a {@link CryptographicKey} or {@link Multikey} from the given URL. * If the given URL contains an {@link Actor} object, it tries to find @@ -218,34 +284,184 @@ export function fetchKey( }, options: FetchKeyOptions = {}, ): Promise> { - const tracerProvider = options.tracerProvider ?? trace.getTracerProvider(); - const tracer = tracerProvider.getTracer(metadata.name, metadata.version); keyId = typeof keyId === "string" ? new URL(keyId) : keyId; - return tracer.startActiveSpan( - "activitypub.fetch_key", - { - kind: SpanKind.CLIENT, - attributes: { - "http.method": "GET", - "url.full": keyId.href, - "url.scheme": keyId.protocol.replace(/:$/, ""), - "url.domain": keyId.hostname, - "url.path": keyId.pathname, - "url.query": keyId.search.replace(/^\?/, ""), - "url.fragment": keyId.hash.replace(/^#/, ""), + return withFetchKeySpan( + keyId, + options.tracerProvider, + () => fetchKeyInternal(keyId, cls, options), + ); +} + +/** + * Fetches a {@link CryptographicKey} or {@link Multikey} from the given URL, + * preserving transport-level fetch failures for callers that need to inspect + * why the key could not be loaded. + * + * @template T The type of the key to fetch. Either {@link CryptographicKey} + * or {@link Multikey}. + * @param keyId The URL of the key. + * @param cls The class of the key to fetch. Either {@link CryptographicKey} + * or {@link Multikey}. + * @param options Options for fetching the key. + * @returns The fetched key, or detailed fetch failure information. + * @since 2.1.0 + */ +export async function fetchKeyDetailed( + keyId: URL | string, + // deno-lint-ignore no-explicit-any + cls: (new (...args: any[]) => T) & { + fromJsonLd( + jsonLd: unknown, + options: { + documentLoader?: DocumentLoader; + contextLoader?: DocumentLoader; + tracerProvider?: TracerProvider; }, - }, - async (span) => { + ): Promise; + }, + { documentLoader, contextLoader, keyCache, tracerProvider }: FetchKeyOptions = + {}, +): Promise> { + const cacheKey = typeof keyId === "string" ? new URL(keyId) : keyId; + return await withFetchKeySpan( + cacheKey, + tracerProvider, + async () => { + const logger = getLogger(["fedify", "sig", "key"]); + keyId = typeof keyId === "string" ? keyId : keyId.href; + if (keyCache != null) { + const cachedKey = await keyCache.get(cacheKey); + if (cachedKey instanceof cls && cachedKey.publicKey != null) { + logger.debug("Key {keyId} found in cache.", { keyId }); + return { + key: cachedKey as T & { publicKey: CryptoKey }, + cached: true, + }; + } else if (cachedKey === null) { + logger.debug( + "Entry {keyId} found in cache, but it is unavailable.", + { keyId }, + ); + return { key: null, cached: true }; + } + } + logger.debug("Fetching key {keyId} to verify signature...", { keyId }); + let document: unknown; try { - const result = await fetchKeyInternal(keyId, cls, options); - span.setAttribute("activitypub.actor.key.cached", result.cached); - return result; + const remoteDocument = await (documentLoader ?? getDocumentLoader())( + keyId, + ); + document = remoteDocument.document; + } catch (error) { + logger.debug("Failed to fetch key {keyId}.", { keyId, error }); + await keyCache?.set(cacheKey, null); + if (error instanceof FetchError && error.response != null) { + return { + key: null, + cached: false, + fetchError: { + status: error.response.status, + response: error.response.clone(), + }, + }; + } + return { + key: null, + cached: false, + fetchError: { + error: error instanceof Error ? error : new Error(String(error)), + }, + }; + } + let object: Object | T; + try { + object = await Object.fromJsonLd(document, { + documentLoader, + contextLoader, + tracerProvider, + }); } catch (e) { - span.setStatus({ code: SpanStatusCode.ERROR, message: String(e) }); - throw e; - } finally { - span.end(); + if (!(e instanceof TypeError)) throw e; + try { + object = await cls.fromJsonLd(document, { + documentLoader, + contextLoader, + tracerProvider, + }); + } catch (e) { + if (e instanceof TypeError) { + logger.debug( + "Failed to verify; key {keyId} returned an invalid object.", + { keyId }, + ); + await keyCache?.set(cacheKey, null); + return { key: null, cached: false }; + } + throw e; + } + } + let key: T | null = null; + if (object instanceof cls) key = object; + else if (isActor(object)) { + // @ts-ignore: cls is either CryptographicKey or Multikey + const keys = cls === CryptographicKey + ? object.getPublicKeys({ + documentLoader, + contextLoader, + tracerProvider, + }) + : object.getAssertionMethods({ + documentLoader, + contextLoader, + tracerProvider, + }); + let length = 0; + let lastKey: T | null = null; + for await (const k of keys) { + length++; + lastKey = k as T; + if (k.id?.href === keyId) { + key = k as T; + break; + } + } + const keyIdUrl = new URL(keyId); + if (key == null && keyIdUrl.hash === "" && length === 1) { + key = lastKey; + } + if (key == null) { + logger.debug( + "Failed to verify; object {keyId} returned an {actorType}, " + + "but has no key matching {keyId}.", + { keyId, actorType: object.constructor.name }, + ); + await keyCache?.set(cacheKey, null); + return { key: null, cached: false }; + } + } else { + logger.debug( + "Failed to verify; key {keyId} returned an invalid object.", + { keyId }, + ); + await keyCache?.set(cacheKey, null); + return { key: null, cached: false }; } + if (key.publicKey == null) { + logger.debug( + "Failed to verify; key {keyId} has no publicKeyPem field.", + { keyId }, + ); + await keyCache?.set(cacheKey, null); + return { key: null, cached: false }; + } + if (keyCache != null) { + await keyCache.set(cacheKey, key); + logger.debug("Key {keyId} cached.", { keyId }); + } + return { + key: key as T & { publicKey: CryptoKey }, + cached: false, + }; }, ); } diff --git a/packages/testing/src/mock.ts b/packages/testing/src/mock.ts index a77fedcb5..021188049 100644 --- a/packages/testing/src/mock.ts +++ b/packages/testing/src/mock.ts @@ -201,6 +201,7 @@ class MockFederation implements Federation { private featuredDispatcher?: any; private featuredTagsDispatcher?: any; private inboxListeners: Map = new Map(); + private unverifiedActivityHandler?: any; private contextData?: TContextData; private receivedActivities: Activity[] = []; @@ -249,6 +250,10 @@ class MockFederation implements Federation { }; } + onUnverifiedActivity(handler: any): void { + this.unverifiedActivityHandler = handler; + } + setInboxDispatcher(_path: any, dispatcher: any): any { this.inboxDispatcher = dispatcher; // Note: inboxPath is set in setInboxListeners diff --git a/packages/vocab-runtime/src/docloader.test.ts b/packages/vocab-runtime/src/docloader.test.ts index 41ba8b267..6aba684f9 100644 --- a/packages/vocab-runtime/src/docloader.test.ts +++ b/packages/vocab-runtime/src/docloader.test.ts @@ -11,10 +11,18 @@ test("new FetchError()", () => { deepStrictEqual(e.name, "FetchError"); deepStrictEqual(e.url, new URL("https://example.com/")); deepStrictEqual(e.message, "https://example.com/: An error message."); - - const e2 = new FetchError(new URL("https://example.org/")); + deepStrictEqual(e.response, undefined); + + const response = new Response(null, { status: 410 }); + const e2 = new FetchError( + new URL("https://example.org/"), + undefined, + response, + ); deepStrictEqual(e2.url, new URL("https://example.org/")); deepStrictEqual(e2.message, "https://example.org/"); + ok(e2.response != null); + deepStrictEqual(e2.response.status, 410); }); test("getDocumentLoader()", async (t) => { diff --git a/packages/vocab-runtime/src/docloader.ts b/packages/vocab-runtime/src/docloader.ts index 165fd5ba7..d5fbc2153 100644 --- a/packages/vocab-runtime/src/docloader.ts +++ b/packages/vocab-runtime/src/docloader.ts @@ -135,6 +135,7 @@ export async function getRemoteDocument( throw new FetchError( documentUrl, `HTTP ${response.status}: ${documentUrl}`, + response.clone(), ); } const contentType = response.headers.get("Content-Type"); diff --git a/packages/vocab-runtime/src/request.ts b/packages/vocab-runtime/src/request.ts index 3276264f0..649118b9b 100644 --- a/packages/vocab-runtime/src/request.ts +++ b/packages/vocab-runtime/src/request.ts @@ -11,16 +11,23 @@ export class FetchError extends Error { */ url: URL; + /** + * The HTTP response that failed, if available. + */ + response?: Response; + /** * Constructs a new `FetchError`. * * @param url The URL that failed to fetch. * @param message Error message. + * @param response The failed HTTP response, if available. */ - constructor(url: URL | string, message?: string) { + constructor(url: URL | string, message?: string, response?: Response) { super(message == null ? url.toString() : `${url}: ${message}`); this.name = "FetchError"; this.url = typeof url === "string" ? new URL(url) : url; + this.response = response; } } From 192e8cbca249414bb85dba2b2dd4a0f79c764f3d Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 9 Mar 2026 11:35:49 +0900 Subject: [PATCH 2/8] Move unverified activity hook to inbox setters Register the unverified-activity hook from InboxListenerSetters instead of Federatable so the API stays grouped with inbox listener configuration and reads naturally in the chaining flow. Update the builder, mock federation, tests, documentation, and changelog text to reflect the new registration point without changing the underlying inbox verification behavior. https://github.com/fedify-dev/fedify/issues/472 Co-Authored-By: OpenAI Codex --- CHANGES.md | 12 ++-- docs/manual/inbox.md | 39 ++++++------ .../fedify/src/federation/builder.test.ts | 4 +- packages/fedify/src/federation/builder.ts | 12 ++-- packages/fedify/src/federation/federation.ts | 62 ++++++++++--------- .../fedify/src/federation/middleware.test.ts | 50 +++++++++------ packages/testing/src/mock.ts | 8 +-- 7 files changed, 101 insertions(+), 86 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 57eab4317..6d1aeb448 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,12 +10,12 @@ To be released. ### @fedify/fedify - - Added `Federatable.onUnverifiedActivity()` so applications can inspect - inbound activities whose signatures could not be verified and optionally - return a custom response instead of the default `401 Unauthorized`. - This is useful for cases like `Delete` deliveries from actors whose - signing keys now return `404 Not Found` or `410 Gone`. Added the - supporting public types `UnverifiedActivityHandler` and + - Added `InboxListenerSetters.onUnverifiedActivity()` so applications can + inspect inbound activities whose signatures could not be verified and + optionally return a custom response instead of the default + `401 Unauthorized`. This is useful for cases like `Delete` deliveries + from actors whose signing keys now return `404 Not Found` or `410 Gone`. + Added the supporting public types `UnverifiedActivityHandler` and `UnverifiedActivityReason`. [[#472], [#611]] - Added `verifyRequestDetailed()` plus the public types diff --git a/docs/manual/inbox.md b/docs/manual/inbox.md index 515b9c8f5..37201c7c5 100644 --- a/docs/manual/inbox.md +++ b/docs/manual/inbox.md @@ -54,10 +54,10 @@ include: - custom logging, metrics, moderation, or quarantine flows for suspicious inbound traffic -For these cases, you can register `~Federatable.onUnverifiedActivity()`. The -callback receives the `RequestContext`, the parsed activity, and a reason -object whose `type` is one of `"noSignature"`, `"invalidSignature"`, or -`"keyFetchError"`. +For these cases, you can register +`~InboxListenerSetters.onUnverifiedActivity()`. The callback receives the +`RequestContext`, the parsed activity, and a reason object whose `type` is one +of `"noSignature"`, `"invalidSignature"`, or `"keyFetchError"`. If the callback returns a `Response`, Fedify uses it as-is. If it returns nothing (`void`), Fedify falls back to the default `401 Unauthorized` @@ -68,17 +68,19 @@ import { type Federation } from "@fedify/fedify"; import { Delete } from "@fedify/vocab"; const federation = null as unknown as Federation; // ---cut-before--- -federation.onUnverifiedActivity((ctx, activity, reason) => { - if ( - activity instanceof Delete && - reason.type === "keyFetchError" && - "status" in reason.result && - reason.result.status === 410 - ) { - // For example, stop redelivery of a Delete from a permanently gone actor. - return new Response(null, { status: 202 }); - } -}); +federation + .setInboxListeners("/users/{identifier}/inbox", "/inbox") + .onUnverifiedActivity((ctx, activity, reason) => { + if ( + activity instanceof Delete && + reason.type === "keyFetchError" && + "status" in reason.result && + reason.result.status === 410 + ) { + // For example, stop redelivery of a Delete from a permanently gone actor. + return new Response(null, { status: 202 }); + } + }); ~~~~ Returning a custom response does not pass the activity to the inbox listeners @@ -428,8 +430,9 @@ duplicate retry mechanisms and leverages the backend's optimized retry features. > [!NOTE] > Activities with invalid signatures/proofs are not queued and are not passed -> to inbox listeners. If `~Federatable.onUnverifiedActivity()` is configured, -> the hook runs before the default `401 Unauthorized` response is returned. +> to inbox listeners. If +> `~InboxListenerSetters.onUnverifiedActivity()` is configured, the hook runs +> before the default `401 Unauthorized` response is returned. > [!TIP] > If your inbox listeners are mostly I/O-bound, consider parallelizing @@ -562,7 +565,7 @@ federation > [!NOTE] > Activities with invalid signatures/proofs are not passed to the error > handler. If you need to inspect them, use -> `~Federatable.onUnverifiedActivity()` instead. +> `~InboxListenerSetters.onUnverifiedActivity()` instead. Forwarding activities to another server diff --git a/packages/fedify/src/federation/builder.test.ts b/packages/fedify/src/federation/builder.test.ts index 931b1f924..fbd9756c0 100644 --- a/packages/fedify/src/federation/builder.test.ts +++ b/packages/fedify/src/federation/builder.test.ts @@ -180,7 +180,9 @@ test("FederationBuilder", async (t) => { return; }; - builder.onUnverifiedActivity(handler); + builder + .setInboxListeners("/users/{identifier}/inbox") + .onUnverifiedActivity(handler); const federation = await builder.build({ kv }); const impl = federation as FederationImpl; diff --git a/packages/fedify/src/federation/builder.ts b/packages/fedify/src/federation/builder.ts index 223eded21..850114944 100644 --- a/packages/fedify/src/federation/builder.ts +++ b/packages/fedify/src/federation/builder.ts @@ -1153,6 +1153,12 @@ export class FederationBuilderImpl this.inboxErrorHandler = handler; return setters; }, + onUnverifiedActivity: ( + handler: UnverifiedActivityHandler, + ): InboxListenerSetters => { + this.unverifiedActivityHandler = handler; + return setters; + }, setSharedKeyDispatcher: ( dispatcher: SharedInboxKeyDispatcher, ): InboxListenerSetters => { @@ -1169,12 +1175,6 @@ export class FederationBuilderImpl return setters; } - onUnverifiedActivity( - handler: UnverifiedActivityHandler, - ): void { - this.unverifiedActivityHandler = handler; - } - setCollectionDispatcher< TObject extends Object, TParam extends string, diff --git a/packages/fedify/src/federation/federation.ts b/packages/fedify/src/federation/federation.ts index 0be49957e..d45678fa7 100644 --- a/packages/fedify/src/federation/federation.ts +++ b/packages/fedify/src/federation/federation.ts @@ -461,36 +461,6 @@ export interface Federatable { sharedInboxPath?: string, ): InboxListenerSetters; - /** - * Registers a callback for incoming activities whose HTTP signatures could - * not be verified. - * - * The regular inbox listeners registered through - * {@link InboxListenerSetters.on} continue to receive only verified - * activities. This hook is an opt-in escape hatch for applications that - * need to inspect unverified deliveries and optionally override the default - * `401 Unauthorized` response. - * - * @example - * ``` typescript - * federation.onUnverifiedActivity((ctx, activity, reason) => { - * if ( - * reason.type === "keyFetchError" && - * "status" in reason.result && - * reason.result.status === 410 - * ) { - * return new Response(null, { status: 202 }); - * } - * }); - * ``` - * - * @param handler A callback to handle an unverified activity. - * @since 2.1.0 - */ - onUnverifiedActivity( - handler: UnverifiedActivityHandler, - ): void; - /** * Registers a collection of objects dispatcher. * @@ -1201,6 +1171,38 @@ export interface InboxListenerSetters { handler: InboxErrorHandler, ): InboxListenerSetters; + /** + * Registers a callback for incoming activities whose HTTP signatures could + * not be verified. + * + * The regular inbox listeners registered through {@link on} continue to + * receive only verified activities. This hook is an opt-in escape hatch for + * applications that need to inspect unverified deliveries and optionally + * override the default `401 Unauthorized` response. + * + * @example + * ``` typescript + * federation + * .setInboxListeners("/users/{identifier}/inbox", "/inbox") + * .onUnverifiedActivity((ctx, activity, reason) => { + * if ( + * reason.type === "keyFetchError" && + * "status" in reason.result && + * reason.result.status === 410 + * ) { + * return new Response(null, { status: 202 }); + * } + * }); + * ``` + * + * @param handler A callback to handle an unverified activity. + * @returns The setters object so that settings can be chained. + * @since 2.1.0 + */ + onUnverifiedActivity( + handler: UnverifiedActivityHandler, + ): InboxListenerSetters; + /** * Configures a callback to dispatch the key pair for the authenticated * document loader of the {@link Context} passed to the shared inbox listener. diff --git a/packages/fedify/src/federation/middleware.test.ts b/packages/fedify/src/federation/middleware.test.ts index 5a116ac07..d716b67ca 100644 --- a/packages/fedify/src/federation/middleware.test.ts +++ b/packages/fedify/src/federation/middleware.test.ts @@ -1567,19 +1567,23 @@ test("Federation.setInboxListeners()", async (t) => { federation.setActorDispatcher("/users/{identifier}", () => { return new vocab.Person({}); }); - federation.setInboxListeners("/users/{identifier}/inbox", "/inbox") + const inboxListeners = federation.setInboxListeners( + "/users/{identifier}/inbox", + "/inbox", + ) .on(vocab.Create, (_ctx, activity) => { verified.push(activity); }); - return { federation, verified }; + return { federation, verified, inboxListeners }; } await t.step("receives noSignature reason", async () => { - const { federation, verified } = createFederationWithLoader( - mockDocumentLoader, - ); + const { federation, verified, inboxListeners } = + createFederationWithLoader( + mockDocumentLoader, + ); let receivedReason: unknown = null; - federation.onUnverifiedActivity((_ctx, _activity, reason) => { + inboxListeners.onUnverifiedActivity((_ctx, _activity, reason) => { receivedReason = reason; return new Response(null, { status: 202 }); }); @@ -1611,9 +1615,12 @@ test("Federation.setInboxListeners()", async (t) => { } return await mockDocumentLoader(url); }; - const { federation, verified } = createFederationWithLoader(goneLoader); + const { federation, verified, inboxListeners } = + createFederationWithLoader( + goneLoader, + ); let receivedReason: unknown = null; - federation.onUnverifiedActivity((_ctx, _activity, reason) => { + inboxListeners.onUnverifiedActivity((_ctx, _activity, reason) => { receivedReason = reason; return new Response(null, { status: 202 }); }); @@ -1663,11 +1670,12 @@ test("Federation.setInboxListeners()", async (t) => { } return await mockDocumentLoader(url); }; - const { federation, verified } = createFederationWithLoader( - missingLoader, - ); + const { federation, verified, inboxListeners } = + createFederationWithLoader( + missingLoader, + ); let receivedReason: unknown = null; - federation.onUnverifiedActivity((_ctx, _activity, reason) => { + inboxListeners.onUnverifiedActivity((_ctx, _activity, reason) => { receivedReason = reason; }); @@ -1699,11 +1707,12 @@ test("Federation.setInboxListeners()", async (t) => { }); await t.step("receives invalidSignature reason", async () => { - const { federation, verified } = createFederationWithLoader( - mockDocumentLoader, - ); + const { federation, verified, inboxListeners } = + createFederationWithLoader( + mockDocumentLoader, + ); let receivedReason: unknown = null; - federation.onUnverifiedActivity((_ctx, _activity, reason) => { + inboxListeners.onUnverifiedActivity((_ctx, _activity, reason) => { receivedReason = reason; return new Response(null, { status: 202 }); }); @@ -1733,11 +1742,12 @@ test("Federation.setInboxListeners()", async (t) => { }); await t.step("does not run for verified activities", async () => { - const { federation, verified } = createFederationWithLoader( - mockDocumentLoader, - ); + const { federation, verified, inboxListeners } = + createFederationWithLoader( + mockDocumentLoader, + ); let unverifiedCalls = 0; - federation.onUnverifiedActivity(() => { + inboxListeners.onUnverifiedActivity(() => { unverifiedCalls++; return new Response(null, { status: 202 }); }); diff --git a/packages/testing/src/mock.ts b/packages/testing/src/mock.ts index 021188049..63efec738 100644 --- a/packages/testing/src/mock.ts +++ b/packages/testing/src/mock.ts @@ -201,7 +201,6 @@ class MockFederation implements Federation { private featuredDispatcher?: any; private featuredTagsDispatcher?: any; private inboxListeners: Map = new Map(); - private unverifiedActivityHandler?: any; private contextData?: TContextData; private receivedActivities: Activity[] = []; @@ -250,10 +249,6 @@ class MockFederation implements Federation { }; } - onUnverifiedActivity(handler: any): void { - this.unverifiedActivityHandler = handler; - } - setInboxDispatcher(_path: any, dispatcher: any): any { this.inboxDispatcher = dispatcher; // Note: inboxPath is set in setInboxListeners @@ -348,6 +343,9 @@ class MockFederation implements Federation { onError(): any { return this; }, + onUnverifiedActivity(): any { + return this; + }, setSharedKeyDispatcher(): any { return this; }, From 8a8f557aa3d10fd2e3abb2a0d2a8c0b807063696 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 9 Mar 2026 11:53:43 +0900 Subject: [PATCH 3/8] Re-export detailed signature APIs Expose the new detailed HTTP signature and key-fetch APIs from @fedify/fedify/sig so the documented public surface is available to supported consumers instead of only internal source-path imports. https://github.com/fedify-dev/fedify/pull/611#discussion_r2902912278 https://github.com/fedify-dev/fedify/pull/611#discussion_r2902912281 https://github.com/fedify-dev/fedify/pull/611#discussion_r2902921993 Co-Authored-By: OpenAI Codex --- packages/fedify/src/sig/mod.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/fedify/src/sig/mod.ts b/packages/fedify/src/sig/mod.ts index d28ce0e6d..8f7342f9c 100644 --- a/packages/fedify/src/sig/mod.ts +++ b/packages/fedify/src/sig/mod.ts @@ -9,11 +9,17 @@ export { signRequest, type SignRequestOptions, verifyRequest, + verifyRequestDetailed, + type VerifyRequestDetailedResult, + type VerifyRequestFailureReason, type VerifyRequestOptions, } from "./http.ts"; export { exportJwk, fetchKey, + fetchKeyDetailed, + type FetchKeyDetailedResult, + type FetchKeyErrorResult, type FetchKeyOptions, type FetchKeyResult, generateCryptoKeyPair, From 653cfb942bd2721dc9d3291b7f2c6f0fe05e81e5 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 9 Mar 2026 11:54:43 +0900 Subject: [PATCH 4/8] Cover detailed key fetch error results Add regression tests for fetchKeyDetailed() so both fetchError variants are exercised: HTTP failures that carry a Response and non-HTTP failures that only expose the thrown Error. https://github.com/fedify-dev/fedify/pull/611#discussion_r2902921986 Co-Authored-By: OpenAI Codex --- packages/fedify/src/sig/key.test.ts | 51 +++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/packages/fedify/src/sig/key.test.ts b/packages/fedify/src/sig/key.test.ts index 0f2802619..f80def098 100644 --- a/packages/fedify/src/sig/key.test.ts +++ b/packages/fedify/src/sig/key.test.ts @@ -4,6 +4,7 @@ import { test, } from "@fedify/fixture"; import { CryptographicKey, Multikey } from "@fedify/vocab"; +import { FetchError } from "@fedify/vocab-runtime"; import { assertEquals, assertRejects, assertThrows } from "@std/assert"; import { ed25519Multikey, @@ -406,3 +407,53 @@ test("fetchKeyDetailed()", async () => { assertEquals(spans[0].attributes["activitypub.actor.key.cached"], true); assertEquals(spans[1].attributes["activitypub.actor.key.cached"], false); }); + +test("fetchKeyDetailed() returns detailed fetch errors", async () => { + const goneKeyId = new URL("https://example.com/gone-key"); + const goneResult = await fetchKeyDetailed( + goneKeyId, + CryptographicKey, + { + documentLoader(url) { + if (url === goneKeyId.href) { + throw new FetchError( + goneKeyId, + `HTTP 410: ${goneKeyId.href}`, + new Response(null, { status: 410 }), + ); + } + return mockDocumentLoader(url); + }, + contextLoader: mockDocumentLoader, + }, + ); + assertEquals(goneResult.key, null); + assertEquals(goneResult.cached, false); + const goneError = goneResult.fetchError; + assertEquals(goneError != null && "status" in goneError, true); + if (goneError == null || !("status" in goneError)) { + throw new Error("Expected HTTP fetch error details."); + } + assertEquals(goneError.status, 410); + assertEquals(goneError.response.status, 410); + + const failure = new TypeError("boom"); + const errorResult = await fetchKeyDetailed( + "https://example.com/error-key", + CryptographicKey, + { + documentLoader() { + throw failure; + }, + contextLoader: mockDocumentLoader, + }, + ); + assertEquals(errorResult.key, null); + assertEquals(errorResult.cached, false); + const detailedError = errorResult.fetchError; + assertEquals(detailedError != null && "error" in detailedError, true); + if (detailedError == null || !("error" in detailedError)) { + throw new Error("Expected non-HTTP fetch error details."); + } + assertEquals(detailedError.error, failure); +}); From 342e5d0521880058bbaf94198400a73a9dbf5b31 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 9 Mar 2026 11:56:55 +0900 Subject: [PATCH 5/8] Deduplicate detailed key fetch handling Refactor fetchKeyDetailed() and fetchKeyInternal() to share the cache lookup, remote document fetch, and fetched-document resolution paths so key-fetch behavior is kept in one place. This keeps the detailed fetch-error reporting added in this PR while reducing the risk that later bug fixes or behavior changes would diverge between the detailed and legacy code paths. https://github.com/fedify-dev/fedify/pull/611#discussion_r2902921995 Co-Authored-By: OpenAI Codex --- packages/fedify/src/sig/key.ts | 337 ++++++++++++++------------------- 1 file changed, 139 insertions(+), 198 deletions(-) diff --git a/packages/fedify/src/sig/key.ts b/packages/fedify/src/sig/key.ts index 6ee967260..cef70790c 100644 --- a/packages/fedify/src/sig/key.ts +++ b/packages/fedify/src/sig/key.ts @@ -220,6 +220,19 @@ export interface FetchKeyDetailedResult readonly fetchError?: FetchKeyErrorResult; } +type FetchableKeyClass = + // deno-lint-ignore no-explicit-any + (new (...args: any[]) => T) & { + fromJsonLd( + jsonLd: unknown, + options: { + documentLoader?: DocumentLoader; + contextLoader?: DocumentLoader; + tracerProvider?: TracerProvider; + }, + ): Promise; + }; + async function withFetchKeySpan( keyId: URL, tracerProvider: TracerProvider | undefined, @@ -271,17 +284,7 @@ async function withFetchKeySpan( */ export function fetchKey( keyId: URL | string, - // deno-lint-ignore no-explicit-any - cls: (new (...args: any[]) => T) & { - fromJsonLd( - jsonLd: unknown, - options: { - documentLoader?: DocumentLoader; - contextLoader?: DocumentLoader; - tracerProvider?: TracerProvider; - }, - ): Promise; - }, + cls: FetchableKeyClass, options: FetchKeyOptions = {}, ): Promise> { keyId = typeof keyId === "string" ? new URL(keyId) : keyId; @@ -308,209 +311,77 @@ export function fetchKey( */ export async function fetchKeyDetailed( keyId: URL | string, - // deno-lint-ignore no-explicit-any - cls: (new (...args: any[]) => T) & { - fromJsonLd( - jsonLd: unknown, - options: { - documentLoader?: DocumentLoader; - contextLoader?: DocumentLoader; - tracerProvider?: TracerProvider; - }, - ): Promise; - }, - { documentLoader, contextLoader, keyCache, tracerProvider }: FetchKeyOptions = - {}, + cls: FetchableKeyClass, + options: FetchKeyOptions = {}, ): Promise> { const cacheKey = typeof keyId === "string" ? new URL(keyId) : keyId; return await withFetchKeySpan( cacheKey, - tracerProvider, + options.tracerProvider, async () => { - const logger = getLogger(["fedify", "sig", "key"]); - keyId = typeof keyId === "string" ? keyId : keyId.href; - if (keyCache != null) { - const cachedKey = await keyCache.get(cacheKey); - if (cachedKey instanceof cls && cachedKey.publicKey != null) { - logger.debug("Key {keyId} found in cache.", { keyId }); - return { - key: cachedKey as T & { publicKey: CryptoKey }, - cached: true, - }; - } else if (cachedKey === null) { - logger.debug( - "Entry {keyId} found in cache, but it is unavailable.", - { keyId }, - ); - return { key: null, cached: true }; - } - } - logger.debug("Fetching key {keyId} to verify signature...", { keyId }); - let document: unknown; - try { - const remoteDocument = await (documentLoader ?? getDocumentLoader())( - keyId, - ); - document = remoteDocument.document; - } catch (error) { - logger.debug("Failed to fetch key {keyId}.", { keyId, error }); - await keyCache?.set(cacheKey, null); - if (error instanceof FetchError && error.response != null) { + return await fetchKeyWithResult( + cacheKey, + cls, + options, + async (error, cacheKey, keyId, keyCache, logger) => { + logger.debug("Failed to fetch key {keyId}.", { keyId, error }); + await keyCache?.set(cacheKey, null); + if (error instanceof FetchError && error.response != null) { + return { + key: null, + cached: false, + fetchError: { + status: error.response.status, + response: error.response.clone(), + }, + }; + } return { key: null, cached: false, fetchError: { - status: error.response.status, - response: error.response.clone(), + error: error instanceof Error ? error : new Error(String(error)), }, }; - } - return { - key: null, - cached: false, - fetchError: { - error: error instanceof Error ? error : new Error(String(error)), - }, - }; - } - let object: Object | T; - try { - object = await Object.fromJsonLd(document, { - documentLoader, - contextLoader, - tracerProvider, - }); - } catch (e) { - if (!(e instanceof TypeError)) throw e; - try { - object = await cls.fromJsonLd(document, { - documentLoader, - contextLoader, - tracerProvider, - }); - } catch (e) { - if (e instanceof TypeError) { - logger.debug( - "Failed to verify; key {keyId} returned an invalid object.", - { keyId }, - ); - await keyCache?.set(cacheKey, null); - return { key: null, cached: false }; - } - throw e; - } - } - let key: T | null = null; - if (object instanceof cls) key = object; - else if (isActor(object)) { - // @ts-ignore: cls is either CryptographicKey or Multikey - const keys = cls === CryptographicKey - ? object.getPublicKeys({ - documentLoader, - contextLoader, - tracerProvider, - }) - : object.getAssertionMethods({ - documentLoader, - contextLoader, - tracerProvider, - }); - let length = 0; - let lastKey: T | null = null; - for await (const k of keys) { - length++; - lastKey = k as T; - if (k.id?.href === keyId) { - key = k as T; - break; - } - } - const keyIdUrl = new URL(keyId); - if (key == null && keyIdUrl.hash === "" && length === 1) { - key = lastKey; - } - if (key == null) { - logger.debug( - "Failed to verify; object {keyId} returned an {actorType}, " + - "but has no key matching {keyId}.", - { keyId, actorType: object.constructor.name }, - ); - await keyCache?.set(cacheKey, null); - return { key: null, cached: false }; - } - } else { - logger.debug( - "Failed to verify; key {keyId} returned an invalid object.", - { keyId }, - ); - await keyCache?.set(cacheKey, null); - return { key: null, cached: false }; - } - if (key.publicKey == null) { - logger.debug( - "Failed to verify; key {keyId} has no publicKeyPem field.", - { keyId }, - ); - await keyCache?.set(cacheKey, null); - return { key: null, cached: false }; - } - if (keyCache != null) { - await keyCache.set(cacheKey, key); - logger.debug("Key {keyId} cached.", { keyId }); - } - return { - key: key as T & { publicKey: CryptoKey }, - cached: false, - }; + }, + ); }, ); } -async function fetchKeyInternal( - keyId: URL | string, - // deno-lint-ignore no-explicit-any - cls: (new (...args: any[]) => T) & { - fromJsonLd( - jsonLd: unknown, - options: { - documentLoader?: DocumentLoader; - contextLoader?: DocumentLoader; - tracerProvider?: TracerProvider; - }, - ): Promise; - }, - { documentLoader, contextLoader, keyCache, tracerProvider }: FetchKeyOptions = - {}, -): Promise> { - const logger = getLogger(["fedify", "sig", "key"]); - const cacheKey = typeof keyId === "string" ? new URL(keyId) : keyId; - keyId = typeof keyId === "string" ? keyId : keyId.href; - if (keyCache != null) { - const cachedKey = await keyCache.get(cacheKey); - if (cachedKey instanceof cls && cachedKey.publicKey != null) { - logger.debug("Key {keyId} found in cache.", { keyId }); - return { - key: cachedKey as T & { publicKey: CryptoKey }, - cached: true, - }; - } else if (cachedKey === null) { - logger.debug( - "Entry {keyId} found in cache, but it is unavailable.", - { keyId }, - ); - return { key: null, cached: true }; - } - } - logger.debug("Fetching key {keyId} to verify signature...", { keyId }); - let document: unknown; - try { - const remoteDocument = await (documentLoader ?? getDocumentLoader())(keyId); - document = remoteDocument.document; - } catch (_) { - logger.debug("Failed to fetch key {keyId}.", { keyId }); - await keyCache?.set(cacheKey, null); - return { key: null, cached: false }; +async function getCachedFetchKey( + cacheKey: URL, + keyId: string, + cls: FetchableKeyClass, + keyCache: KeyCache | undefined, + logger: ReturnType, +): Promise | null> { + if (keyCache == null) return null; + const cachedKey = await keyCache.get(cacheKey); + if (cachedKey instanceof cls && cachedKey.publicKey != null) { + logger.debug("Key {keyId} found in cache.", { keyId }); + return { + key: cachedKey as T & { publicKey: CryptoKey }, + cached: true, + }; + } else if (cachedKey === null) { + logger.debug( + "Entry {keyId} found in cache, but it is unavailable.", + { keyId }, + ); + return { key: null, cached: true }; } + return null; +} + +async function resolveFetchedKey( + document: unknown, + cacheKey: URL, + keyId: string, + cls: FetchableKeyClass, + { documentLoader, contextLoader, keyCache, tracerProvider }: FetchKeyOptions, + logger: ReturnType, +): Promise> { let object: Object | T; try { object = await Object.fromJsonLd(document, { @@ -598,6 +469,76 @@ async function fetchKeyInternal( }; } +async function fetchKeyWithResult< + T extends CryptographicKey | Multikey, + TResult extends FetchKeyResult, +>( + cacheKey: URL, + cls: FetchableKeyClass, + options: FetchKeyOptions, + onFetchError: ( + error: unknown, + cacheKey: URL, + keyId: string, + keyCache: KeyCache | undefined, + logger: ReturnType, + ) => Promise | TResult, +): Promise { + const logger = getLogger(["fedify", "sig", "key"]); + const keyId = cacheKey.href; + const cached = await getCachedFetchKey( + cacheKey, + keyId, + cls, + options.keyCache, + logger, + ); + if (cached != null) return cached as TResult; + logger.debug("Fetching key {keyId} to verify signature...", { keyId }); + let document: unknown; + try { + const remoteDocument = + await (options.documentLoader ?? getDocumentLoader())( + keyId, + ); + document = remoteDocument.document; + } catch (error) { + return await onFetchError( + error, + cacheKey, + keyId, + options.keyCache, + logger, + ); + } + return await resolveFetchedKey( + document, + cacheKey, + keyId, + cls, + options, + logger, + ) as TResult; +} + +async function fetchKeyInternal( + keyId: URL | string, + cls: FetchableKeyClass, + options: FetchKeyOptions = {}, +): Promise> { + const cacheKey = typeof keyId === "string" ? new URL(keyId) : keyId; + return await fetchKeyWithResult( + cacheKey, + cls, + options, + async (_error, cacheKey, keyId, keyCache, logger) => { + logger.debug("Failed to fetch key {keyId}.", { keyId }); + await keyCache?.set(cacheKey, null); + return { key: null, cached: false }; + }, + ); +} + /** * A cache for storing cryptographic keys. * @since 0.12.0 From 991f18e02a1715c1160a1a2acf108c78e2744f38 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 9 Mar 2026 12:23:18 +0900 Subject: [PATCH 6/8] Preserve cached key fetch failure reasons Persist negative key-cache entries and their fetch failure metadata through KvKeyCache so repeated verification attempts can keep reporting keyFetchError details instead of degrading to invalidSignature after the first miss. Also add regression coverage for cached-null persistence, fetch-error metadata round-tripping, and inbox retries that should continue seeing the original 410 result. https://github.com/fedify-dev/fedify/pull/611#discussion_r2902956697 Co-Authored-By: OpenAI Codex --- .../fedify/src/federation/keycache.test.ts | 42 ++++++++ packages/fedify/src/federation/keycache.ts | 71 ++++++++++++- .../fedify/src/federation/middleware.test.ts | 59 +++++++++++ packages/fedify/src/sig/key.ts | 99 ++++++++++++++++--- 4 files changed, 254 insertions(+), 17 deletions(-) diff --git a/packages/fedify/src/federation/keycache.test.ts b/packages/fedify/src/federation/keycache.test.ts index 501fe946e..efac46b7f 100644 --- a/packages/fedify/src/federation/keycache.test.ts +++ b/packages/fedify/src/federation/keycache.test.ts @@ -38,6 +38,7 @@ test("KvKeyCache.set()", async () => { await cache.set(new URL("https://example.com/null"), null); assert(cache.nullKeys.has("https://example.com/null")); + assertEquals(await kv.get(["pk", "https://example.com/null"]), null); }); test("KvKeyCache.get()", async () => { @@ -64,4 +65,45 @@ test("KvKeyCache.get()", async () => { cache.nullKeys.add("https://example.com/null"); assertEquals(await cache.get(new URL("https://example.com/null")), null); + + await kv.set(["pk", "https://example.com/null2"], null); + const cache2 = new KvKeyCache(kv, ["pk"]); + assertEquals(await cache2.get(new URL("https://example.com/null2")), null); +}); + +test("KvKeyCache fetch error metadata", async () => { + const kv = new MemoryKvStore(); + const cache = new KvKeyCache(kv, ["pk"]); + const keyId = new URL("https://example.com/key"); + + await cache.setFetchError(keyId, { + status: 410, + response: new Response("gone", { + status: 410, + statusText: "Gone", + headers: { "content-type": "text/plain" }, + }), + }); + const httpError = await cache.getFetchError(keyId); + assert(httpError != null && "status" in httpError); + if (httpError == null || !("status" in httpError)) { + throw new Error("Expected HTTP fetch error metadata."); + } + assertEquals(httpError.status, 410); + assertEquals(httpError.response.status, 410); + assertEquals(await httpError.response.text(), "gone"); + + await cache.setFetchError(keyId, { + error: Object.assign(new Error("boom"), { name: "TypeError" }), + }); + const nonHttpError = await cache.getFetchError(keyId); + assert(nonHttpError != null && "error" in nonHttpError); + if (nonHttpError == null || !("error" in nonHttpError)) { + throw new Error("Expected non-HTTP fetch error metadata."); + } + assertEquals(nonHttpError.error.name, "TypeError"); + assertEquals(nonHttpError.error.message, "boom"); + + await cache.setFetchError(keyId, null); + assertEquals(await cache.getFetchError(keyId), undefined); }); diff --git a/packages/fedify/src/federation/keycache.ts b/packages/fedify/src/federation/keycache.ts index 2ddd46715..d04f35a67 100644 --- a/packages/fedify/src/federation/keycache.ts +++ b/packages/fedify/src/federation/keycache.ts @@ -1,6 +1,6 @@ import { CryptographicKey, Multikey } from "@fedify/vocab"; import type { DocumentLoader } from "@fedify/vocab-runtime"; -import type { KeyCache } from "../sig/key.ts"; +import type { FetchKeyErrorResult, KeyCache } from "../sig/key.ts"; import type { KvKey, KvStore } from "./kv.ts"; export interface KvKeyCacheOptions { @@ -21,12 +21,20 @@ export class KvKeyCache implements KeyCache { this.options = options; } + #getFetchErrorKey(keyId: URL): KvKey { + return [...this.prefix, "__fetchError", keyId.href]; + } + async get( keyId: URL, ): Promise { if (this.nullKeys.has(keyId.href)) return null; const serialized = await this.kv.get([...this.prefix, keyId.href]); - if (serialized == null) return undefined; + if (serialized === undefined) return undefined; + if (serialized === null) { + this.nullKeys.add(keyId.href); + return null; + } try { return await CryptographicKey.fromJsonLd(serialized, this.options); } catch { @@ -45,11 +53,68 @@ export class KvKeyCache implements KeyCache { ): Promise { if (key == null) { this.nullKeys.add(keyId.href); - await this.kv.delete([...this.prefix, keyId.href]); + await this.kv.set([...this.prefix, keyId.href], null); return; } this.nullKeys.delete(keyId.href); const serialized = await key.toJsonLd(this.options); await this.kv.set([...this.prefix, keyId.href], serialized); } + + async getFetchError(keyId: URL): Promise { + const cached = await this.kv.get(this.#getFetchErrorKey(keyId)); + if (cached == null || typeof cached !== "object") return undefined; + if ( + "status" in cached && typeof cached.status === "number" && + "statusText" in cached && typeof cached.statusText === "string" && + "headers" in cached && Array.isArray(cached.headers) && + "body" in cached && typeof cached.body === "string" + ) { + return { + status: cached.status, + response: new Response(cached.body, { + status: cached.status, + statusText: cached.statusText, + headers: cached.headers, + }), + }; + } else if ( + "errorName" in cached && typeof cached.errorName === "string" && + "errorMessage" in cached && typeof cached.errorMessage === "string" + ) { + const error = new Error(cached.errorMessage); + error.name = cached.errorName; + return { error }; + } + return undefined; + } + + async setFetchError( + keyId: URL, + error: FetchKeyErrorResult | null, + ): Promise { + if (error == null) { + await this.kv.delete(this.#getFetchErrorKey(keyId)); + return; + } + if ("status" in error) { + await this.kv.set( + this.#getFetchErrorKey(keyId), + { + status: error.status, + statusText: error.response.statusText, + headers: Array.from(error.response.headers.entries()), + body: await error.response.clone().text(), + }, + ); + return; + } + await this.kv.set( + this.#getFetchErrorKey(keyId), + { + errorName: error.error.name, + errorMessage: error.error.message, + }, + ); + } } diff --git a/packages/fedify/src/federation/middleware.test.ts b/packages/fedify/src/federation/middleware.test.ts index d716b67ca..b99542007 100644 --- a/packages/fedify/src/federation/middleware.test.ts +++ b/packages/fedify/src/federation/middleware.test.ts @@ -1656,6 +1656,65 @@ test("Federation.setInboxListeners()", async (t) => { ); }); + await t.step("preserves keyFetchError details across retries", async () => { + const keyId = new URL("https://gone.example/actors/alice#main-key"); + let keyFetches = 0; + const goneLoader = async (url: string) => { + if (url === keyId.href) { + keyFetches++; + throw new FetchError( + keyId, + `HTTP 410: ${keyId.href}`, + new Response(null, { status: 410 }), + ); + } + return await mockDocumentLoader(url); + }; + const { federation, inboxListeners } = createFederationWithLoader( + goneLoader, + ); + const reasons: unknown[] = []; + inboxListeners.onUnverifiedActivity((_ctx, _activity, reason) => { + reasons.push(reason); + return new Response(null, { status: 202 }); + }); + + const request = await createInboxRequest( + new vocab.Create({ + id: new URL("https://gone.example/activities/retry"), + actor: new URL("https://gone.example/actors/alice"), + }), + { privateKey: rsaPrivateKey3, keyId }, + ); + + const first = await federation.fetch(request.clone() as Request, { + contextData: undefined, + }); + const second = await federation.fetch(request.clone() as Request, { + contextData: undefined, + }); + + assertEquals(first.status, 202); + assertEquals(second.status, 202); + assertEquals(keyFetches, 1); + assertEquals( + (reasons[0] as { type: string }).type, + "keyFetchError", + ); + assertEquals( + (reasons[1] as { type: string }).type, + "keyFetchError", + ); + assertEquals( + ( + reasons[1] as { + result: { status: number; response: Response }; + } + ).result.status, + 410, + ); + }); + await t.step("falls back to 401 when handler returns void", async () => { const missingKeyId = new URL( "https://missing.example/actors/alice#main-key", diff --git a/packages/fedify/src/sig/key.ts b/packages/fedify/src/sig/key.ts index cef70790c..4d97dfdda 100644 --- a/packages/fedify/src/sig/key.ts +++ b/packages/fedify/src/sig/key.ts @@ -220,6 +220,14 @@ export interface FetchKeyDetailedResult readonly fetchError?: FetchKeyErrorResult; } +interface FetchErrorMetadataCache extends KeyCache { + getFetchError?(keyId: URL): Promise; + setFetchError?( + keyId: URL, + error: FetchKeyErrorResult | null, + ): Promise; +} + type FetchableKeyClass = // deno-lint-ignore no-explicit-any (new (...args: any[]) => T) & { @@ -319,29 +327,54 @@ export async function fetchKeyDetailed( cacheKey, options.tracerProvider, async () => { - return await fetchKeyWithResult( + return await fetchKeyWithResult>( cacheKey, cls, options, + async (cacheKey, keyId, keyCache, logger) => { + const fetchError = await keyCache?.getFetchError?.(cacheKey); + if (fetchError != null) { + logger.debug( + "Entry {keyId} found in cache with preserved fetch failure " + + "details.", + { keyId }, + ); + return { + key: null, + cached: true, + fetchError, + }; + } + logger.debug( + "Entry {keyId} found in cache, but no fetch failure details " + + "are available.", + { keyId }, + ); + return { key: null, cached: true }; + }, async (error, cacheKey, keyId, keyCache, logger) => { logger.debug("Failed to fetch key {keyId}.", { keyId, error }); await keyCache?.set(cacheKey, null); if (error instanceof FetchError && error.response != null) { + const fetchError = { + status: error.response.status, + response: error.response.clone(), + } satisfies FetchKeyErrorResult; + await keyCache?.setFetchError?.(cacheKey, fetchError); return { key: null, cached: false, - fetchError: { - status: error.response.status, - response: error.response.clone(), - }, + fetchError, }; } + const fetchError = { + error: error instanceof Error ? error : new Error(String(error)), + } satisfies FetchKeyErrorResult; + await keyCache?.setFetchError?.(cacheKey, fetchError); return { key: null, cached: false, - fetchError: { - error: error instanceof Error ? error : new Error(String(error)), - }, + fetchError, }; }, ); @@ -374,6 +407,16 @@ async function getCachedFetchKey( return null; } +async function clearFetchErrorMetadata( + keyId: URL, + keyCache: KeyCache | undefined, +): Promise { + await (keyCache as FetchErrorMetadataCache | undefined)?.setFetchError?.( + keyId, + null, + ); +} + async function resolveFetchedKey( document: unknown, cacheKey: URL, @@ -404,6 +447,7 @@ async function resolveFetchedKey( { keyId }, ); await keyCache?.set(cacheKey, null); + await clearFetchErrorMetadata(cacheKey, keyCache); return { key: null, cached: false }; } throw e; @@ -441,6 +485,7 @@ async function resolveFetchedKey( { keyId, actorType: object.constructor.name }, ); await keyCache?.set(cacheKey, null); + await clearFetchErrorMetadata(cacheKey, keyCache); return { key: null, cached: false }; } } else { @@ -449,6 +494,7 @@ async function resolveFetchedKey( { keyId }, ); await keyCache?.set(cacheKey, null); + await clearFetchErrorMetadata(cacheKey, keyCache); return { key: null, cached: false }; } if (key.publicKey == null) { @@ -457,12 +503,14 @@ async function resolveFetchedKey( { keyId }, ); await keyCache?.set(cacheKey, null); + await clearFetchErrorMetadata(cacheKey, keyCache); return { key: null, cached: false }; } if (keyCache != null) { await keyCache.set(cacheKey, key); logger.debug("Key {keyId} cached.", { keyId }); } + await clearFetchErrorMetadata(cacheKey, keyCache); return { key: key as T & { publicKey: CryptoKey }, cached: false, @@ -476,23 +524,33 @@ async function fetchKeyWithResult< cacheKey: URL, cls: FetchableKeyClass, options: FetchKeyOptions, + onCachedUnavailable: ( + cacheKey: URL, + keyId: string, + keyCache: FetchErrorMetadataCache | undefined, + logger: ReturnType, + ) => Promise | TResult, onFetchError: ( error: unknown, cacheKey: URL, keyId: string, - keyCache: KeyCache | undefined, + keyCache: FetchErrorMetadataCache | undefined, logger: ReturnType, ) => Promise | TResult, ): Promise { const logger = getLogger(["fedify", "sig", "key"]); const keyId = cacheKey.href; + const keyCache = options.keyCache as FetchErrorMetadataCache | undefined; const cached = await getCachedFetchKey( cacheKey, keyId, cls, - options.keyCache, + keyCache, logger, ); + if (cached?.key === null && cached.cached) { + return await onCachedUnavailable(cacheKey, keyId, keyCache, logger); + } if (cached != null) return cached as TResult; logger.debug("Fetching key {keyId} to verify signature...", { keyId }); let document: unknown; @@ -507,7 +565,7 @@ async function fetchKeyWithResult< error, cacheKey, keyId, - options.keyCache, + keyCache, logger, ); } @@ -527,13 +585,26 @@ async function fetchKeyInternal( options: FetchKeyOptions = {}, ): Promise> { const cacheKey = typeof keyId === "string" ? new URL(keyId) : keyId; - return await fetchKeyWithResult( + return await fetchKeyWithResult>( cacheKey, cls, options, - async (_error, cacheKey, keyId, keyCache, logger) => { - logger.debug("Failed to fetch key {keyId}.", { keyId }); + (_cacheKey, _keyId, _keyCache, _logger) => { + return { key: null, cached: true }; + }, + async (error, cacheKey, keyId, keyCache, logger) => { + logger.debug("Failed to fetch key {keyId}.", { keyId, error }); await keyCache?.set(cacheKey, null); + if (error instanceof FetchError && error.response != null) { + await keyCache?.setFetchError?.(cacheKey, { + status: error.response.status, + response: error.response.clone(), + }); + } else { + await keyCache?.setFetchError?.(cacheKey, { + error: error instanceof Error ? error : new Error(String(error)), + }); + } return { key: null, cached: false }; }, ); From abcdfc254b959bccbca6fa3cce1b2ed4c343e440 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 9 Mar 2026 13:11:01 +0900 Subject: [PATCH 7/8] Expire negative key cache entries Give KvKeyCache a short TTL for unavailable keys and persisted fetch-failure metadata so temporary network errors or 5xx responses do not poison signature verification indefinitely. Also add regression coverage for expiring unavailable entries and for round-tripping stored fetch-failure metadata. https://github.com/fedify-dev/fedify/pull/611#discussion_r2903013243 Co-Authored-By: OpenAI Codex --- .../fedify/src/federation/keycache.test.ts | 23 ++++++++++++- packages/fedify/src/federation/keycache.ts | 32 +++++++++++++++---- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/packages/fedify/src/federation/keycache.test.ts b/packages/fedify/src/federation/keycache.test.ts index efac46b7f..92a7de8bd 100644 --- a/packages/fedify/src/federation/keycache.test.ts +++ b/packages/fedify/src/federation/keycache.test.ts @@ -63,7 +63,10 @@ test("KvKeyCache.get()", async () => { assertInstanceOf(multikey, Multikey); assertEquals(multikey?.id?.href, "https://example.com/key2"); - cache.nullKeys.add("https://example.com/null"); + cache.nullKeys.set( + "https://example.com/null", + Temporal.Now.instant().add(Temporal.Duration.from({ minutes: 10 })), + ); assertEquals(await cache.get(new URL("https://example.com/null")), null); await kv.set(["pk", "https://example.com/null2"], null); @@ -107,3 +110,21 @@ test("KvKeyCache fetch error metadata", async () => { await cache.setFetchError(keyId, null); assertEquals(await cache.getFetchError(keyId), undefined); }); + +test("KvKeyCache unavailable entries expire", async () => { + const kv = new MemoryKvStore(); + const cache = new KvKeyCache(kv, ["pk"], { + unavailableKeyTtl: Temporal.Duration.from({ milliseconds: 1 }), + }); + const keyId = new URL("https://example.com/expired"); + + await cache.set(keyId, null); + await cache.setFetchError(keyId, { + status: 410, + response: new Response(null, { status: 410 }), + }); + await new Promise((resolve) => setTimeout(resolve, 10)); + + assertEquals(await cache.get(keyId), undefined); + assertEquals(await cache.getFetchError(keyId), undefined); +}); diff --git a/packages/fedify/src/federation/keycache.ts b/packages/fedify/src/federation/keycache.ts index d04f35a67..0dacbc3d2 100644 --- a/packages/fedify/src/federation/keycache.ts +++ b/packages/fedify/src/federation/keycache.ts @@ -6,19 +6,23 @@ import type { KvKey, KvStore } from "./kv.ts"; export interface KvKeyCacheOptions { documentLoader?: DocumentLoader; contextLoader?: DocumentLoader; + unavailableKeyTtl?: Temporal.Duration; } export class KvKeyCache implements KeyCache { readonly kv: KvStore; readonly prefix: KvKey; readonly options: KvKeyCacheOptions; - readonly nullKeys: Set; + readonly unavailableKeyTtl: Temporal.Duration; + readonly nullKeys: Map; constructor(kv: KvStore, prefix: KvKey, options: KvKeyCacheOptions = {}) { this.kv = kv; this.prefix = prefix; - this.nullKeys = new Set(); this.options = options; + this.unavailableKeyTtl = options.unavailableKeyTtl ?? + Temporal.Duration.from({ minutes: 10 }); + this.nullKeys = new Map(); } #getFetchErrorKey(keyId: URL): KvKey { @@ -28,11 +32,20 @@ export class KvKeyCache implements KeyCache { async get( keyId: URL, ): Promise { - if (this.nullKeys.has(keyId.href)) return null; + const negativeExpiration = this.nullKeys.get(keyId.href); + if (negativeExpiration != null) { + if (Temporal.Now.instant().until(negativeExpiration).sign >= 0) { + return null; + } + this.nullKeys.delete(keyId.href); + } const serialized = await this.kv.get([...this.prefix, keyId.href]); if (serialized === undefined) return undefined; if (serialized === null) { - this.nullKeys.add(keyId.href); + this.nullKeys.set( + keyId.href, + Temporal.Now.instant().add(this.unavailableKeyTtl), + ); return null; } try { @@ -52,8 +65,13 @@ export class KvKeyCache implements KeyCache { key: CryptographicKey | Multikey | null, ): Promise { if (key == null) { - this.nullKeys.add(keyId.href); - await this.kv.set([...this.prefix, keyId.href], null); + this.nullKeys.set( + keyId.href, + Temporal.Now.instant().add(this.unavailableKeyTtl), + ); + await this.kv.set([...this.prefix, keyId.href], null, { + ttl: this.unavailableKeyTtl, + }); return; } this.nullKeys.delete(keyId.href); @@ -106,6 +124,7 @@ export class KvKeyCache implements KeyCache { headers: Array.from(error.response.headers.entries()), body: await error.response.clone().text(), }, + { ttl: this.unavailableKeyTtl }, ); return; } @@ -115,6 +134,7 @@ export class KvKeyCache implements KeyCache { errorName: error.error.name, errorMessage: error.error.message, }, + { ttl: this.unavailableKeyTtl }, ); } } From 12243f498ad141fb9e2caa50b846064636f53312 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 9 Mar 2026 13:12:35 +0900 Subject: [PATCH 8/8] Handle unverified hook errors gracefully Catch exceptions thrown by onUnverifiedActivity(), forward them to inboxErrorHandler, and fall back to the default 401 response instead of letting unverified deliveries escape as uncaught 500s. Add regression coverage to ensure hook failures are reported to the inbox error handler and still produce the default unauthorized response. https://github.com/fedify-dev/fedify/pull/611#discussion_r2903013246 Co-Authored-By: OpenAI Codex --- packages/fedify/src/federation/handler.ts | 34 +++++++++++-- .../fedify/src/federation/middleware.test.ts | 49 +++++++++++++++++++ 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/packages/fedify/src/federation/handler.ts b/packages/fedify/src/federation/handler.ts index a0fe51b85..18703eaba 100644 --- a/packages/fedify/src/federation/handler.ts +++ b/packages/fedify/src/federation/handler.ts @@ -759,11 +759,35 @@ async function handleInboxInternal( } } span.addEvent("activitypub.activity.received", eventAttributes); - const response = await unverifiedActivityHandler( - ctx, - activity, - reason, - ); + let response: void | Response; + try { + response = await unverifiedActivityHandler( + ctx, + activity, + reason, + ); + } catch (error) { + logger.error( + "An unexpected error occurred in unverified activity handler:\n" + + "{error}", + { error, activity: json, recipient }, + ); + try { + await inboxErrorHandler?.(ctx, error as Error); + } catch (error) { + logger.error( + "An unexpected error occurred in inbox error handler:\n{error}", + { error, activity: json, recipient }, + ); + } + return new Response( + "Failed to verify the request signature.", + { + status: 401, + headers: { "Content-Type": "text/plain; charset=utf-8" }, + }, + ); + } if (response instanceof Response) return response; return new Response( "Failed to verify the request signature.", diff --git a/packages/fedify/src/federation/middleware.test.ts b/packages/fedify/src/federation/middleware.test.ts index b99542007..a2d15b10f 100644 --- a/packages/fedify/src/federation/middleware.test.ts +++ b/packages/fedify/src/federation/middleware.test.ts @@ -1765,6 +1765,55 @@ test("Federation.setInboxListeners()", async (t) => { ); }); + await t.step( + "falls back to 401 and reports hook errors", + async () => { + const missingKeyId = new URL( + "https://missing.example/actors/alice#main-key", + ); + const missingLoader = async (url: string) => { + if (url === missingKeyId.href) { + throw new FetchError( + missingKeyId, + `HTTP 404: ${missingKeyId.href}`, + new Response(null, { status: 404 }), + ); + } + return await mockDocumentLoader(url); + }; + const { federation, verified, inboxListeners } = + createFederationWithLoader( + missingLoader, + ); + let receivedErrorMessage: string | null = null; + inboxListeners + .onUnverifiedActivity(() => { + throw new Error("Intended unverified hook failure"); + }) + .onError((_ctx, error) => { + receivedErrorMessage = error.message; + }); + + const response = await federation.fetch( + await createInboxRequest( + new vocab.Create({ + id: new URL("https://missing.example/activities/error"), + actor: new URL("https://missing.example/actors/alice"), + }), + { privateKey: rsaPrivateKey3, keyId: missingKeyId }, + ), + { contextData: undefined }, + ); + + assertEquals(response.status, 401); + assertEquals(verified, []); + assertEquals( + receivedErrorMessage, + "Intended unverified hook failure", + ); + }, + ); + await t.step("receives invalidSignature reason", async () => { const { federation, verified, inboxListeners } = createFederationWithLoader(