From e2eb16dae717f40c37616cc196115f8a7723a32a Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 00:41:33 +0900 Subject: [PATCH 01/14] Plumb meterProvider option into signature verifiers Add an optional meterProvider?: MeterProvider field to the three OpenTelemetry-aware verifier option types, mirroring the existing tracerProvider: - VerifyRequestOptions in sig/http.ts - VerifySignatureOptions in sig/ld.ts (inherited by VerifyJsonLdOptions) - VerifyProofOptions in sig/proof.ts (inherited by VerifyObjectOptions) Thread the option from the existing inbox/outbox call sites: - handler.ts's handleInboxInternal forwards parameters.meterProvider to verifyJsonLd, verifyObject, and verifyRequestDetailed. - middleware.ts's InboxContext.handleObject and getSignedKey forward this.meterProvider. verifyRequestDraft and verifyRequestRfc9421 now also destructure meterProvider, and the recursive verifyRequestDetailed calls inside the HTTP double-knock fallback thread both meterProvider and tracerProvider through (previously they silently fell back to the global tracer provider on retry, which would have split spans across providers; this fixes that drop alongside the new meterProvider). No metrics are recorded yet. Subsequent commits will add the activitypub.signature.verification.duration and activitypub.signature.key_fetch.duration histograms that consume this option. https://github.com/fedify-dev/fedify/issues/316 https://github.com/fedify-dev/fedify/issues/737 Assisted-by: Claude Code:claude-opus-4-7 --- packages/fedify/src/federation/handler.ts | 4 ++++ packages/fedify/src/federation/middleware.ts | 2 ++ packages/fedify/src/sig/http.ts | 14 ++++++++++++++ packages/fedify/src/sig/ld.ts | 14 +++++++++++++- packages/fedify/src/sig/proof.ts | 14 +++++++++++++- 5 files changed, 46 insertions(+), 2 deletions(-) diff --git a/packages/fedify/src/federation/handler.ts b/packages/fedify/src/federation/handler.ts index 5f6dc0ed9..ddca7e8cb 100644 --- a/packages/fedify/src/federation/handler.ts +++ b/packages/fedify/src/federation/handler.ts @@ -842,6 +842,7 @@ async function handleInboxInternal( signatureTimeWindow, skipSignatureVerification, inboxChallengePolicy, + meterProvider, tracerProvider, } = parameters; const logger = getLogger(["fedify", "federation", "inbox"]); @@ -913,6 +914,7 @@ async function handleInboxInternal( contextLoader: ctx.contextLoader, documentLoader: ctx.documentLoader, keyCache, + meterProvider, tracerProvider, }); } catch (error) { @@ -942,6 +944,7 @@ async function handleInboxInternal( contextLoader: ctx.contextLoader, documentLoader: ctx.documentLoader, keyCache, + meterProvider, tracerProvider, }); } catch (error) { @@ -991,6 +994,7 @@ async function handleInboxInternal( documentLoader: ctx.documentLoader, timeWindow: signatureTimeWindow, keyCache, + meterProvider, tracerProvider, }); if (verification.verified === false) { diff --git a/packages/fedify/src/federation/middleware.ts b/packages/fedify/src/federation/middleware.ts index b75237bb8..f52e00318 100644 --- a/packages/fedify/src/federation/middleware.ts +++ b/packages/fedify/src/federation/middleware.ts @@ -2862,6 +2862,7 @@ export class ContextImpl implements Context { { contextLoader, documentLoader: options.documentLoader ?? this.documentLoader, + meterProvider: this.meterProvider, tracerProvider: options.tracerProvider ?? this.tracerProvider, keyCache, }, @@ -3089,6 +3090,7 @@ class RequestContextImpl extends ContextImpl contextLoader: options.contextLoader ?? this.contextLoader, documentLoader: options.documentLoader ?? this.documentLoader, timeWindow: this.federation.signatureTimeWindow, + meterProvider: this.meterProvider, tracerProvider: options.tracerProvider ?? this.tracerProvider, }); } diff --git a/packages/fedify/src/sig/http.ts b/packages/fedify/src/sig/http.ts index e5457afb8..833376787 100644 --- a/packages/fedify/src/sig/http.ts +++ b/packages/fedify/src/sig/http.ts @@ -2,6 +2,7 @@ import { CryptographicKey } from "@fedify/vocab"; import { type DocumentLoader, FetchError } from "@fedify/vocab-runtime"; import { getLogger } from "@logtape/logtape"; import { + type MeterProvider, type Span, SpanStatusCode, trace, @@ -653,6 +654,13 @@ export interface VerifyRequestOptions { * @since 1.3.0 */ tracerProvider?: TracerProvider; + + /** + * The OpenTelemetry meter provider. If omitted, the global meter provider + * is used. + * @since 2.3.0 + */ + meterProvider?: MeterProvider; } /** @@ -849,6 +857,7 @@ async function verifyRequestDraft( timeWindow, currentTime, keyCache, + meterProvider, tracerProvider, }: VerifyRequestOptions = {}, ): Promise { @@ -1126,6 +1135,8 @@ async function verifyRequestDraft( get: () => Promise.resolve(undefined), set: async (keyId, key) => await keyCache?.set(keyId, key), }, + meterProvider, + tracerProvider, }, ); } @@ -1216,6 +1227,7 @@ async function verifyRequestRfc9421( timeWindow, currentTime, keyCache, + meterProvider, tracerProvider, }: VerifyRequestOptions = {}, ): Promise { @@ -1481,6 +1493,8 @@ async function verifyRequestRfc9421( set: async (keyId, key) => await keyCache?.set(keyId, key), }, spec: "rfc9421", + meterProvider, + tracerProvider, }, ); } else { diff --git a/packages/fedify/src/sig/ld.ts b/packages/fedify/src/sig/ld.ts index 3a6c01554..3234fcb00 100644 --- a/packages/fedify/src/sig/ld.ts +++ b/packages/fedify/src/sig/ld.ts @@ -2,7 +2,12 @@ import { Activity, CryptographicKey, getTypeId, Object } from "@fedify/vocab"; import { type DocumentLoader, getDocumentLoader } from "@fedify/vocab-runtime"; import jsonld from "@fedify/vocab-runtime/jsonld"; import { getLogger } from "@logtape/logtape"; -import { SpanStatusCode, trace, type TracerProvider } from "@opentelemetry/api"; +import { + type MeterProvider, + SpanStatusCode, + trace, + type TracerProvider, +} from "@opentelemetry/api"; import { decodeBase64, encodeBase64 } from "byte-encodings/base64"; import { encodeHex } from "byte-encodings/hex"; import metadata from "../../deno.json" with { type: "json" }; @@ -275,6 +280,13 @@ export interface VerifySignatureOptions { * @since 1.3.0 */ tracerProvider?: TracerProvider; + + /** + * The OpenTelemetry meter provider. If omitted, the global meter provider + * is used. + * @since 2.3.0 + */ + meterProvider?: MeterProvider; } /** diff --git a/packages/fedify/src/sig/proof.ts b/packages/fedify/src/sig/proof.ts index cb7885631..ebe2febd3 100644 --- a/packages/fedify/src/sig/proof.ts +++ b/packages/fedify/src/sig/proof.ts @@ -7,7 +7,12 @@ import { } from "@fedify/vocab"; import type { DocumentLoader } from "@fedify/vocab-runtime"; import { getLogger } from "@logtape/logtape"; -import { SpanStatusCode, trace, type TracerProvider } from "@opentelemetry/api"; +import { + type MeterProvider, + SpanStatusCode, + trace, + type TracerProvider, +} from "@opentelemetry/api"; import { encodeHex } from "byte-encodings/hex"; import serialize from "json-canon"; import metadata from "../../deno.json" with { type: "json" }; @@ -276,6 +281,13 @@ export interface VerifyProofOptions { * @since 1.3.0 */ tracerProvider?: TracerProvider; + + /** + * The OpenTelemetry meter provider. If omitted, the global meter provider + * is used. + * @since 2.3.0 + */ + meterProvider?: MeterProvider; } /** From a120a20b6f31463cb21a740bed158843ae159deb Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 01:09:48 +0900 Subject: [PATCH 02/14] Record HTTP Signature verification duration as a metric Add the `activitypub.signature.verification.duration` OpenTelemetry histogram (unit `ms`) and instrument the HTTP Signatures verification path so each public call to `verifyRequestDetailed()` (and its `verifyRequest()` wrapper) emits exactly one measurement. Each measurement carries: - `activitypub.signature.kind` = `http` - `activitypub.signature.result` = `verified`, `rejected`, `missing`, or `error` - `http_signatures.algorithm`, only when the parsed algorithm value comes from a bounded set. For draft-cavage signatures the value is gated by a `DRAFT_KNOWN_ALGORITHMS` whitelist; for RFC 9421 it is recorded only after the algorithm matches `rfc9421AlgorithmMap`. Both gates keep the metric attribute on a small spec-bounded set so attacker-supplied algorithm strings cannot inflate cardinality. - `http_signatures.failure_reason`, only on `rejected` rows, taken from the `VerifyRequestFailureReason.type` discriminator. The new `FederationMetrics.recordSignatureVerificationDuration()` helper writes the measurement through the existing `getFederationMetrics(meterProvider)` cache, mirroring the shape of the existing delivery and queue-task metric helpers. Implementation notes: - `verifyRequestDraft()` and `verifyRequestRfc9421()` now accept a mutable `HttpSignatureMetricsContext` so the outer `verifyRequestDetailed()` can read back the bounded algorithm of the candidate whose result was actually returned, without surfacing it on the public result type. - `verifyRequestRfc9421()` tracks the bounded algorithm per candidate via a `setFailure(result, algorithm?)` helper, so a later candidate cannot inherit an earlier candidate's algorithm when iterating multi-signature inputs. - The cached-key retry paths in both spec implementations now call `verifyRequestDraft()` / `verifyRequestRfc9421()` directly with the outer span and metrics context, instead of recursing through `verifyRequestDetailed()`. This collapses the retry into a single observed verification (one `http_signatures.verify` span and one histogram measurement per public call), and is regression- guarded by a new `cached-key retry emits one measurement, not two` test step. New tests cover the verified, missing, invalid-signature, key-fetch failure, double-count (cached-key retry and `verifyRequest()` wrapper), and unknown-algorithm cardinality paths. https://github.com/fedify-dev/fedify/issues/316 https://github.com/fedify-dev/fedify/issues/737 Assisted-by: Claude Code:claude-opus-4-7 Assisted-by: Codex:gpt-5.5 --- packages/fedify/src/federation/metrics.ts | 88 +++++++ packages/fedify/src/sig/http.test.ts | 276 ++++++++++++++++++++++ packages/fedify/src/sig/http.ts | 151 ++++++++++-- 3 files changed, 494 insertions(+), 21 deletions(-) diff --git a/packages/fedify/src/federation/metrics.ts b/packages/fedify/src/federation/metrics.ts index 34169bcf2..ee127bfbe 100644 --- a/packages/fedify/src/federation/metrics.ts +++ b/packages/fedify/src/federation/metrics.ts @@ -31,10 +31,64 @@ export interface QueueTaskCommonAttributes { activityType?: string; } +/** + * The kind of ActivityPub signature verified, used as the + * `activitypub.signature.kind` metric attribute. + * @since 2.3.0 + */ +export type SignatureVerificationKind = + | "http" + | "linked_data" + | "object_integrity"; + +/** + * The terminal classification of a signature verification attempt, used as + * the `activitypub.signature.result` metric attribute. + * + * - `verified`: the signature was checked and accepted. + * - `rejected`: the signature was checked and refused (bad signature, key + * fetch failure, owner mismatch, etc.). + * - `missing`: no signature was present. Only HTTP Signatures and Linked + * Data Signatures distinguish this from `rejected`; Object Integrity + * Proofs never carry this value because callers decide whether to invoke + * {@link import("../sig/proof.ts").verifyProof} at all. + * - `error`: verification threw an unexpected error. + * @since 2.3.0 + */ +export type SignatureVerificationResult = + | "verified" + | "rejected" + | "missing" + | "error"; + +/** + * Optional attributes recorded alongside an + * `activitypub.signature.verification.duration` measurement. Each field is + * scoped to the matching signature kind and is omitted when its value is not + * available; values are expected to come from small, spec-bounded sets so + * they do not inflate metric cardinality. + * @since 2.3.0 + */ +export interface SignatureVerificationExtraAttributes { + /** `http_signatures.algorithm` (HTTP Signatures only). */ + algorithm?: string; + /** `ld_signatures.type` (Linked Data Signatures only). */ + ldType?: string; + /** `object_integrity_proofs.cryptosuite` (Object Integrity Proofs only). */ + cryptosuite?: string; + /** + * `http_signatures.failure_reason`, recorded only on HTTP Signature + * failures so the histogram can be sliced by reason without exploding + * cardinality on success rows. + */ + failureReason?: string; +} + class FederationMetrics { readonly deliverySent: Counter; readonly deliveryPermanentFailure: Counter; readonly signatureVerificationFailure: Counter; + readonly signatureVerificationDuration: Histogram; readonly deliveryDuration: Histogram; readonly inboxProcessingDuration: Histogram; readonly httpServerRequestCount: Counter; @@ -66,6 +120,15 @@ class FederationMetrics { unit: "{failure}", }, ); + this.signatureVerificationDuration = meter.createHistogram( + "activitypub.signature.verification.duration", + { + description: + "Duration of ActivityPub signature verification, including local " + + "key lookup and remote key fetches.", + unit: "ms", + }, + ); this.deliveryDuration = meter.createHistogram( "activitypub.delivery.duration", { @@ -208,6 +271,31 @@ class FederationMetrics { this.signatureVerificationFailure.add(1, attributes); } + recordSignatureVerificationDuration( + durationMs: number, + kind: SignatureVerificationKind, + result: SignatureVerificationResult, + extra: SignatureVerificationExtraAttributes = {}, + ): void { + const attributes: Attributes = { + "activitypub.signature.kind": kind, + "activitypub.signature.result": result, + }; + if (extra.algorithm != null) { + attributes["http_signatures.algorithm"] = extra.algorithm; + } + if (extra.failureReason != null) { + attributes["http_signatures.failure_reason"] = extra.failureReason; + } + if (extra.ldType != null) { + attributes["ld_signatures.type"] = extra.ldType; + } + if (extra.cryptosuite != null) { + attributes["object_integrity_proofs.cryptosuite"] = extra.cryptosuite; + } + this.signatureVerificationDuration.record(durationMs, attributes); + } + recordInboxProcessingDuration( activityType: string, durationMs: number, diff --git a/packages/fedify/src/sig/http.test.ts b/packages/fedify/src/sig/http.test.ts index 1f61dfd2b..153472d2e 100644 --- a/packages/fedify/src/sig/http.test.ts +++ b/packages/fedify/src/sig/http.test.ts @@ -1,4 +1,5 @@ import { + createTestMeterProvider, createTestTracerProvider, mockDocumentLoader, test, @@ -10,6 +11,7 @@ import { assertEquals, assertExists, assertFalse, + assertGreaterOrEqual, assertRejects, assertStringIncludes, assertThrows, @@ -367,6 +369,280 @@ test("verifyRequestDetailed() records failure details on span", async () => { assertEquals(span.attributes["http_signatures.key_fetch_status"], 410); }); +test("verifyRequestDetailed() records verification duration metric", async (t) => { + const buildSignedRequest = (): Promise => + 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://example.com/key2", + }), + }), + rsaPrivateKey2, + new URL("https://example.com/key2"), + ); + + await t.step("verified path emits one measurement", async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const request = await buildSignedRequest(); + + const result = await verifyRequestDetailed(request, { + contextLoader: mockDocumentLoader, + documentLoader: mockDocumentLoader, + meterProvider, + }); + assert(result.verified); + + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + const measurement = measurements[0]; + assertEquals(measurement.type, "histogram"); + assertGreaterOrEqual(measurement.value, 0); + assertEquals( + measurement.attributes["activitypub.signature.kind"], + "http", + ); + assertEquals( + measurement.attributes["activitypub.signature.result"], + "verified", + ); + assertEquals( + measurement.attributes["http_signatures.algorithm"], + "rsa-sha256", + ); + assertFalse( + "http_signatures.failure_reason" in measurement.attributes, + ); + }); + + await t.step("missing signature is recorded as result=missing", async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const result = await verifyRequestDetailed( + new Request("https://example.com/inbox", { + method: "POST", + headers: { "Content-Type": "application/activity+json" }, + body: "{}", + }), + { + contextLoader: mockDocumentLoader, + documentLoader: mockDocumentLoader, + meterProvider, + }, + ); + assertFalse(result.verified); + assertEquals(result.reason.type, "noSignature"); + + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + assertEquals( + measurements[0].attributes["activitypub.signature.kind"], + "http", + ); + assertEquals( + measurements[0].attributes["activitypub.signature.result"], + "missing", + ); + assertFalse( + "http_signatures.failure_reason" in measurements[0].attributes, + ); + }); + + await t.step( + "invalid signature is recorded as result=rejected with failure_reason", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const result = 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="https://example.com/key2",' + + 'headers="(request-target) date digest",signature="AAAA"', + }, + body: "", + }), + { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }, + ); + assertFalse(result.verified); + assertEquals(result.reason.type, "invalidSignature"); + + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + assertEquals( + measurements[0].attributes["activitypub.signature.kind"], + "http", + ); + assertEquals( + measurements[0].attributes["activitypub.signature.result"], + "rejected", + ); + assertEquals( + measurements[0].attributes["http_signatures.failure_reason"], + "invalidSignature", + ); + }, + ); + + await t.step( + "key fetch failure is recorded as result=rejected with failure_reason=keyFetchError", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + 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, { + contextLoader: mockDocumentLoader, + documentLoader(url) { + if (url === keyId.href) { + throw new FetchError( + keyId, + `HTTP 410: ${keyId.href}`, + new Response(null, { status: 410 }), + ); + } + return mockDocumentLoader(url); + }, + meterProvider, + }); + assertFalse(result.verified); + assertEquals(result.reason.type, "keyFetchError"); + + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + assertEquals( + measurements[0].attributes["activitypub.signature.result"], + "rejected", + ); + assertEquals( + measurements[0].attributes["http_signatures.failure_reason"], + "keyFetchError", + ); + }, + ); + + await t.step( + "verifyRequest() wrapper emits exactly one measurement, not two", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const request = await buildSignedRequest(); + const key = await verifyRequest(request, { + contextLoader: mockDocumentLoader, + documentLoader: mockDocumentLoader, + meterProvider, + }); + assertExists(key); + assertEquals( + recorder.getMeasurements( + "activitypub.signature.verification.duration", + ).length, + 1, + ); + }, + ); + + await t.step( + "cached-key retry emits one measurement, not two", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + // Prime the cache with a wrong key so the verifier fails with the cached + // key and falls through to the fresh-fetch retry path; both attempts + // must collapse to a single measurement. + const cache: Record = { + "https://example.com/key2": rsaPublicKey1, + }; + const request = await buildSignedRequest(); + const key = await verifyRequest(request, { + contextLoader: mockDocumentLoader, + documentLoader: mockDocumentLoader, + meterProvider, + keyCache: { + get(keyId) { + return Promise.resolve(cache[keyId.href]); + }, + set(keyId, k) { + cache[keyId.href] = k; + return Promise.resolve(); + }, + } satisfies KeyCache, + }); + assertExists(key); + assertEquals( + recorder.getMeasurements( + "activitypub.signature.verification.duration", + ).length, + 1, + ); + }, + ); + + await t.step( + "draft-cavage with unknown algorithm omits the algorithm metric attribute", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const result = 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="https://example.com/key2",algorithm="x-attacker-supplied",' + + 'headers="(request-target) date digest",signature="AAAA"', + }, + body: "", + }), + { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }, + ); + assertFalse(result.verified); + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + assertFalse( + "http_signatures.algorithm" in measurements[0].attributes, + ); + }, + ); +}); + 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 833376787..236c976a6 100644 --- a/packages/fedify/src/sig/http.ts +++ b/packages/fedify/src/sig/http.ts @@ -22,6 +22,11 @@ import { Item, } from "structured-field-values"; import metadata from "../../deno.json" with { type: "json" }; +import { + getDurationMs, + getFederationMetrics, + type SignatureVerificationResult, +} from "../federation/metrics.ts"; import { type AcceptSignatureComponent, fulfillAcceptSignature, @@ -740,6 +745,35 @@ function getKeyFetchErrorName(error: Error): string { return error.name || error.constructor.name || "Error"; } +interface HttpSignatureMetricsContext { + algorithm?: string; +} + +/** + * Known draft-cavage `algorithm` parameter values, used to keep the + * `http_signatures.algorithm` metric attribute on a bounded set. The header + * field is attacker-controlled and not used to select the verification + * algorithm, so unknown values are dropped from the metric to prevent + * cardinality blow-up. + */ +const DRAFT_KNOWN_ALGORITHMS: ReadonlySet = new Set([ + "ecdsa-sha256", + "ecdsa-sha384", + "ecdsa-sha512", + "ed25519", + "hs2019", + "rsa-sha1", + "rsa-sha256", + "rsa-sha512", +]); + +function classifyHttpVerifyResult( + result: VerifyRequestDetailedResult, +): SignatureVerificationResult { + if (result.verified) return "verified"; + return result.reason.type === "noSignature" ? "missing" : "rejected"; +} + function recordVerificationResult( span: Span, result: VerifyRequestDetailedResult, @@ -814,6 +848,10 @@ export async function verifyRequestDetailed( span.setAttribute(ATTR_HTTP_REQUEST_HEADER(name), value); } } + const start = performance.now(); + const metricsContext: HttpSignatureMetricsContext = {}; + let result: VerifyRequestDetailedResult | undefined; + let threw = false; try { // Choose implementation based on spec option let spec = options.spec; @@ -823,11 +861,20 @@ export async function verifyRequestDetailed( : "draft-cavage-http-signatures-12"; } - let result: VerifyRequestDetailedResult; if (spec === "rfc9421") { - result = await verifyRequestRfc9421(request, span, options); + result = await verifyRequestRfc9421( + request, + span, + metricsContext, + options, + ); } else { - result = await verifyRequestDraft(request, span, options); + result = await verifyRequestDraft( + request, + span, + metricsContext, + options, + ); } recordVerificationResult(span, result); @@ -836,12 +883,30 @@ export async function verifyRequestDetailed( } return result; } catch (error) { + threw = true; span.setStatus({ code: SpanStatusCode.ERROR, message: String(error), }); throw error; } finally { + const classified: SignatureVerificationResult = threw + ? "error" + : classifyHttpVerifyResult(result!); + const failureReason = classified === "rejected" && result != null && + !result.verified + ? result.reason.type + : undefined; + getFederationMetrics(options.meterProvider) + .recordSignatureVerificationDuration( + getDurationMs(start), + "http", + classified, + { + algorithm: metricsContext.algorithm, + failureReason, + }, + ); span.end(); } }, @@ -851,6 +916,7 @@ export async function verifyRequestDetailed( async function verifyRequestDraft( request: Request, span: Span, + metricsContext: HttpSignatureMetricsContext, { documentLoader, contextLoader, @@ -1062,6 +1128,10 @@ async function verifyRequestDraft( span?.setAttribute("http_signatures.key_id", keyId); if ("algorithm" in sigValues) { span?.setAttribute("http_signatures.algorithm", sigValues.algorithm); + const normalizedAlgorithm = sigValues.algorithm.toLowerCase(); + if (DRAFT_KNOWN_ALGORITHMS.has(normalizedAlgorithm)) { + metricsContext.algorithm = normalizedAlgorithm; + } } const { key, cached, fetchError } = await fetchKeyDetailed( keyIdUrl, @@ -1124,8 +1194,14 @@ async function verifyRequestDraft( "is invalid. Retrying with the freshly fetched key...", { keyId, signature, message }, ); - return await verifyRequestDetailed( + // Reuse the outer span and metricsContext so the cached-key retry stays + // a single observed verification operation: one `http_signatures.verify` + // span and one `activitypub.signature.verification.duration` measurement + // per public call. + return await verifyRequestDraft( originalRequest, + span, + metricsContext, { documentLoader, contextLoader, @@ -1221,6 +1297,7 @@ async function verifyRfc9421ContentDigest( async function verifyRequestRfc9421( request: Request, span: Span, + metricsContext: HttpSignatureMetricsContext, { documentLoader, contextLoader, @@ -1287,12 +1364,29 @@ async function verifyRequestRfc9421( } let failure: VerifyRequestDetailedResult = noSignatureResult(); + // Tracks the bounded algorithm of the candidate that ended up as the final + // `failure`, so a histogram measurement records the algorithm of the + // signature actually returned rather than an earlier candidate that + // happened to carry a bounded algorithm but was overwritten by a later + // unsupported candidate. + let failureAlgorithm: string | undefined; + + // `setFailure` keeps the failure result and its bounded algorithm in sync; + // every continue-out-of-iteration path goes through it so a later candidate + // cannot inherit an earlier candidate's algorithm. + const setFailure = ( + result: VerifyRequestDetailedResult, + algorithm?: string, + ): void => { + failure = result; + failureAlgorithm = algorithm; + }; for (const sigName of signatureNames) { // Skip if we don't have the signature bytes if (!signatures[sigName]) { - failure = invalidSignatureResult( - parseKeyId(signatureInputs[sigName]?.keyId), + setFailure( + invalidSignatureResult(parseKeyId(signatureInputs[sigName]?.keyId)), ); continue; } @@ -1307,7 +1401,7 @@ async function verifyRequestRfc9421( "Failed to verify; missing keyId in signature {signatureName}.", { signatureName: sigName, signatureInput: signatureInputHeader }, ); - failure = invalidSignatureResult(null); + setFailure(invalidSignatureResult(null)); continue; } @@ -1316,7 +1410,7 @@ async function verifyRequestRfc9421( "Failed to verify; missing created timestamp in signature {signatureName}.", { signatureName: sigName, signatureInput: signatureInputHeader }, ); - failure = invalidSignatureResult(keyId); + setFailure(invalidSignatureResult(keyId)); continue; } @@ -1334,7 +1428,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); + setFailure(invalidSignatureResult(keyId)); continue; } else if ( Temporal.Instant.compare(signatureCreated, now.subtract(tw)) < 0 @@ -1343,7 +1437,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); + setFailure(invalidSignatureResult(keyId)); continue; } } @@ -1360,7 +1454,7 @@ async function verifyRequestRfc9421( "Failed to verify; Content-Digest header required but not found.", { components: sigInput.components }, ); - failure = invalidSignatureResult(keyId); + setFailure(invalidSignatureResult(keyId)); continue; } @@ -1375,7 +1469,7 @@ async function verifyRequestRfc9421( "Failed to verify; Content-Digest verification failed.", { contentDigest: contentDigestHeader }, ); - failure = invalidSignatureResult(keyId); + setFailure(invalidSignatureResult(keyId)); continue; } } @@ -1384,7 +1478,7 @@ async function verifyRequestRfc9421( span?.setAttribute("http_signatures.key_id", sigInput.keyId); span?.setAttribute("http_signatures.created", sigInput.created.toString()); if (keyId == null) { - failure = invalidSignatureResult(null); + setFailure(invalidSignatureResult(null)); continue; } @@ -1400,12 +1494,12 @@ async function verifyRequestRfc9421( ); if (fetchError != null) { - failure = keyFetchErrorResult(keyId, fetchError); + setFailure(keyFetchErrorResult(keyId, fetchError)); continue; } if (!key) { logger.debug("Failed to fetch key: {keyId}", { keyId: sigInput.keyId }); - failure = invalidSignatureResult(keyId); + setFailure(invalidSignatureResult(keyId)); continue; } @@ -1429,8 +1523,14 @@ async function verifyRequestRfc9421( alg = "ed25519"; } } - if (alg) span?.setAttribute("http_signatures.algorithm", alg); + if (alg) { + span?.setAttribute("http_signatures.algorithm", alg); + } const algorithm = alg && rfc9421AlgorithmMap[alg]; + // Only record the algorithm metric attribute after the value matches the + // RFC 9421 algorithm map, so attacker-supplied `alg` strings cannot + // inflate `http_signatures.algorithm` cardinality. + const candidateAlgorithm = algorithm ? alg : undefined; if (!algorithm) { logger.debug( "Failed to verify; unsupported algorithm: {algorithm}", @@ -1439,7 +1539,7 @@ async function verifyRequestRfc9421( supported: Object.keys(rfc9421AlgorithmMap), }, ); - failure = invalidSignatureResult(keyId); + setFailure(invalidSignatureResult(keyId)); continue; } @@ -1456,7 +1556,7 @@ async function verifyRequestRfc9421( "Failed to create signature base for verification: {error}", { error, signatureInput: sigInput }, ); - failure = invalidSignatureResult(keyId); + setFailure(invalidSignatureResult(keyId), candidateAlgorithm); continue; } const signatureBaseBytes = new TextEncoder().encode(signatureBase); @@ -1473,6 +1573,7 @@ async function verifyRequestRfc9421( ); if (verified) { + metricsContext.algorithm = candidateAlgorithm; return { verified: true, key, signatureLabel: sigName }; } else if (cached) { // If we used a cached key and verification failed, try fetching fresh key @@ -1481,8 +1582,15 @@ async function verifyRequestRfc9421( { keyId: sigInput.keyId }, ); - return await verifyRequestDetailed( + // Reuse the outer span and metricsContext so the cached-key retry + // stays a single observed verification operation: one + // `http_signatures.verify` span and one + // `activitypub.signature.verification.duration` measurement per + // public call. + return await verifyRequestRfc9421( originalRequest, + span, + metricsContext, { documentLoader, contextLoader, @@ -1502,17 +1610,18 @@ async function verifyRequestRfc9421( "Failed to verify signature with fetched key {keyId}; signature invalid.", { keyId: sigInput.keyId, signatureBase }, ); - failure = invalidSignatureResult(keyId); + setFailure(invalidSignatureResult(keyId), candidateAlgorithm); } } catch (error) { logger.debug( "Error during signature verification: {error}", { error, keyId: sigInput.keyId, algorithm: sigInput.alg }, ); - failure = invalidSignatureResult(keyId); + setFailure(invalidSignatureResult(keyId), candidateAlgorithm); } } + metricsContext.algorithm = failureAlgorithm; return failure; } From be2d9606824aaaf33a0129818effde7ceb667372 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 01:19:09 +0900 Subject: [PATCH 03/14] Record Linked Data Signature verification duration as a metric MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the `activitypub.signature.verification.duration` histogram to cover Linked Data Signatures. `verifyJsonLd()` now emits exactly one measurement per call, carrying: - `activitypub.signature.kind` = `linked_data` - `activitypub.signature.result` = `verified`, `rejected`, `missing`, or `error` - `ld_signatures.type`, gated by a new `LD_KNOWN_SIGNATURE_TYPES` whitelist (currently `RsaSignature2017`). Unknown type strings, including attacker-supplied ones, keep their span attribute but are dropped from the metric so cardinality stays bounded. Result classification: - thrown error → `error` - returned `true` → `verified` - returned `false` with a `signature` property present (even when it is `null` or otherwise malformed) → `rejected` - returned `false` with no `signature` property at all → `missing` Two small helpers tidy up the existing inline checks: - `hasLdSignatureProperty()` does the loose presence check used for classification, so malformed signatures are still reported as `rejected` rather than `missing`. - `getLdSignatureObject()` extracts the inner signature record only when it is a plain object, driving both the existing span attributes and the new bounded-type lookup. New test cases in *packages/fedify/src/sig/ld.test.ts* cover verified, missing, invalid, malformed-null-signature, and unknown-type cardinality paths. https://github.com/fedify-dev/fedify/issues/316 https://github.com/fedify-dev/fedify/issues/737 Assisted-by: Claude Code:claude-opus-4-7 Assisted-by: Codex:gpt-5.5 --- packages/fedify/src/sig/ld.test.ts | 141 ++++++++++++++++++++++++++++- packages/fedify/src/sig/ld.ts | 97 ++++++++++++++------ 2 files changed, 211 insertions(+), 27 deletions(-) diff --git a/packages/fedify/src/sig/ld.test.ts b/packages/fedify/src/sig/ld.test.ts index b6db88171..93cfe5a3f 100644 --- a/packages/fedify/src/sig/ld.test.ts +++ b/packages/fedify/src/sig/ld.test.ts @@ -1,8 +1,13 @@ -import { mockDocumentLoader, test } from "@fedify/fixture"; +import { + createTestMeterProvider, + mockDocumentLoader, + test, +} from "@fedify/fixture"; import { CryptographicKey } from "@fedify/vocab"; import { assert } from "@std/assert/assert"; import { assertEquals } from "@std/assert/assert-equals"; import { assertFalse } from "@std/assert/assert-false"; +import { assertGreaterOrEqual } from "@std/assert/assert-greater-or-equal"; import { assertRejects } from "@std/assert/assert-rejects"; import { assertThrows } from "@std/assert/assert-throws"; import { encodeBase64 } from "byte-encodings/base64"; @@ -338,4 +343,138 @@ test("verifyJsonLd()", async () => { assertFalse(verified2); }); +test("verifyJsonLd() records verification duration metric", async (t) => { + await t.step( + "verified path records result=verified with bounded type", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const verified = await verifyJsonLd(testVector, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }); + assert(verified); + + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + const m = measurements[0]; + assertEquals(m.type, "histogram"); + assertGreaterOrEqual(m.value, 0); + assertEquals(m.attributes["activitypub.signature.kind"], "linked_data"); + assertEquals(m.attributes["activitypub.signature.result"], "verified"); + assertEquals(m.attributes["ld_signatures.type"], "RsaSignature2017"); + }, + ); + + await t.step( + "missing signature records result=missing without type", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const verified = await verifyJsonLd(document, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }); + assertFalse(verified); + + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + assertEquals( + measurements[0].attributes["activitypub.signature.kind"], + "linked_data", + ); + assertEquals( + measurements[0].attributes["activitypub.signature.result"], + "missing", + ); + assertFalse("ld_signatures.type" in measurements[0].attributes); + }, + ); + + await t.step("invalid signature records result=rejected", async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const tampered = { + ...testVector, + signature: { + ...testVector.signature, + signatureValue: encodeBase64(new Uint8Array([1, 2, 3, 4])), + }, + }; + const verified = await verifyJsonLd(tampered, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }); + assertFalse(verified); + + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + assertEquals( + measurements[0].attributes["activitypub.signature.result"], + "rejected", + ); + assertEquals( + measurements[0].attributes["ld_signatures.type"], + "RsaSignature2017", + ); + }); + + await t.step( + "malformed (null) signature property records result=rejected, not missing", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const malformed = { ...document, signature: null }; + const verified = await verifyJsonLd(malformed, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }); + assertFalse(verified); + + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + assertEquals( + measurements[0].attributes["activitypub.signature.result"], + "rejected", + ); + assertFalse("ld_signatures.type" in measurements[0].attributes); + }, + ); + + await t.step( + "unknown signature type omits the ld_signatures.type metric attribute", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const exotic = { + ...testVector, + signature: { ...testVector.signature, type: "MadeUpSignature9999" }, + }; + const verified = await verifyJsonLd(exotic, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }); + assertFalse(verified); + + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + assertEquals( + measurements[0].attributes["activitypub.signature.result"], + "rejected", + ); + assertFalse("ld_signatures.type" in measurements[0].attributes); + }, + ); +}); + // cSpell: ignore ostatus diff --git a/packages/fedify/src/sig/ld.ts b/packages/fedify/src/sig/ld.ts index 3234fcb00..3fce6ab0b 100644 --- a/packages/fedify/src/sig/ld.ts +++ b/packages/fedify/src/sig/ld.ts @@ -11,6 +11,11 @@ import { import { decodeBase64, encodeBase64 } from "byte-encodings/base64"; import { encodeHex } from "byte-encodings/hex"; import metadata from "../../deno.json" with { type: "json" }; +import { + getDurationMs, + getFederationMetrics, + type SignatureVerificationResult, +} from "../federation/metrics.ts"; import { fetchKey, type KeyCache, validateCryptoKey } from "./key.ts"; const logger = getLogger(["fedify", "sig", "ld"]); @@ -406,6 +411,37 @@ export async function verifySignature( export interface VerifyJsonLdOptions extends VerifySignatureOptions { } +/** + * Known Linked Data Signature `type` values, used to keep + * `ld_signatures.type` on a bounded set of spec-defined string values. + * Fedify only signs and verifies `RsaSignature2017`; other values come in + * only from external documents and are dropped from the metric attribute to + * avoid attacker-controlled cardinality. + */ +const LD_KNOWN_SIGNATURE_TYPES: ReadonlySet = new Set([ + "RsaSignature2017", +]); + +function hasLdSignatureProperty(jsonLd: unknown): boolean { + return ( + typeof jsonLd === "object" && jsonLd != null && "signature" in jsonLd + ); +} + +function getLdSignatureObject( + jsonLd: unknown, +): Record | undefined { + if ( + typeof jsonLd === "object" && jsonLd != null && "signature" in jsonLd && + typeof (jsonLd as { signature?: unknown }).signature === "object" && + (jsonLd as { signature?: unknown }).signature != null && + !Array.isArray((jsonLd as { signature?: unknown }).signature) + ) { + return (jsonLd as { signature: Record }).signature; + } + return undefined; +} + /** * Verify the authenticity of the given JSON-LD document using Linked Data * Signatures. If the document is signed, this function verifies the signature @@ -424,40 +460,33 @@ export async function verifyJsonLd( return await tracer.startActiveSpan( "ld_signatures.verify", async (span) => { + const start = performance.now(); + let verified = false; + let threw = false; + let signatureType: string | undefined; + // A `signature` property being present at all (even if it is `null` or a + // primitive) counts as "had signature" for classification, so malformed + // signatures are reported as `rejected` rather than as `missing`. + const hadSignature = hasLdSignatureProperty(jsonLd); try { const object = await Object.fromJsonLd(jsonLd, options); if (object.id != null) { span.setAttribute("activitypub.object.id", object.id.href); } span.setAttribute("activitypub.object.type", getTypeId(object).href); - if ( - typeof jsonLd === "object" && jsonLd != null && - "signature" in jsonLd && typeof jsonLd.signature === "object" && - jsonLd.signature != null - ) { - if ( - "creator" in jsonLd.signature && - typeof jsonLd.signature.creator === "string" - ) { - span.setAttribute( - "ld_signatures.key_id", - jsonLd.signature.creator, - ); + const sig = getLdSignatureObject(jsonLd); + if (sig != null) { + if (typeof sig.creator === "string") { + span.setAttribute("ld_signatures.key_id", sig.creator); } - if ( - "signatureValue" in jsonLd.signature && - typeof jsonLd.signature.signatureValue === "string" - ) { - span.setAttribute( - "ld_signatures.signature", - jsonLd.signature.signatureValue, - ); + if (typeof sig.signatureValue === "string") { + span.setAttribute("ld_signatures.signature", sig.signatureValue); } - if ( - "type" in jsonLd.signature && - typeof jsonLd.signature.type === "string" - ) { - span.setAttribute("ld_signatures.type", jsonLd.signature.type); + if (typeof sig.type === "string") { + span.setAttribute("ld_signatures.type", sig.type); + if (LD_KNOWN_SIGNATURE_TYPES.has(sig.type)) { + signatureType = sig.type; + } } } const attributions = new Set( @@ -481,14 +510,30 @@ export async function verifyJsonLd( ); return false; } + verified = true; return true; } catch (error) { + threw = true; span.setStatus({ code: SpanStatusCode.ERROR, message: String(error), }); throw error; } finally { + const classified: SignatureVerificationResult = threw + ? "error" + : verified + ? "verified" + : hadSignature + ? "rejected" + : "missing"; + getFederationMetrics(options.meterProvider) + .recordSignatureVerificationDuration( + getDurationMs(start), + "linked_data", + classified, + { ldType: signatureType }, + ); span.end(); } }, From 3fd1aea8f1d96f8284be30d151aa63ea165fcf72 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 01:29:20 +0900 Subject: [PATCH 04/14] Record Object Integrity Proof verification duration as a metric MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the `activitypub.signature.verification.duration` histogram to cover Object Integrity Proofs. `verifyProof()` now emits exactly one measurement per public call, carrying: - `activitypub.signature.kind` = `object_integrity` - `activitypub.signature.result` = `verified`, `rejected`, or `error`. OIP intentionally has no `missing` value, because `verifyObject()` decides whether to call `verifyProof()` at all. - `object_integrity_proofs.cryptosuite`, gated by a new `OIP_KNOWN_CRYPTOSUITES` whitelist (currently `eddsa-jcs-2022`). Unknown cryptosuite strings keep their span attribute but are dropped from the metric so cardinality stays bounded. Result classification: - thrown error → `error` - `verifyProofInternal()` returns a non-null Multikey → `verified` - `verifyProofInternal()` returns null → `rejected` The two cached-key retry paths inside `verifyProofInternal()` now recurse via `verifyProofInternal()` instead of `verifyProof()`, so the retry stays inside the outer `verifyProof()`'s span and timing context. Each public `verifyProof()` call therefore emits exactly one `object_integrity_proofs.verify` span and one duration measurement. New tests in *packages/fedify/src/sig/proof.test.ts* cover verified, rejected, cached-key retry no-double-count, and `verifyObject()` wrapper no-double-count paths. https://github.com/fedify-dev/fedify/issues/316 https://github.com/fedify-dev/fedify/issues/737 Assisted-by: Claude Code:claude-opus-4-7 Assisted-by: Codex:gpt-5.5 --- packages/fedify/src/sig/proof.test.ts | 159 +++++++++++++++++++++++++- packages/fedify/src/sig/proof.ts | 48 +++++++- 2 files changed, 204 insertions(+), 3 deletions(-) diff --git a/packages/fedify/src/sig/proof.test.ts b/packages/fedify/src/sig/proof.test.ts index f7375d32d..4c5fc08d8 100644 --- a/packages/fedify/src/sig/proof.test.ts +++ b/packages/fedify/src/sig/proof.test.ts @@ -1,4 +1,8 @@ -import { mockDocumentLoader, test } from "@fedify/fixture"; +import { + createTestMeterProvider, + mockDocumentLoader, + test, +} from "@fedify/fixture"; import { normalizeOutgoingActivityJsonLd } from "../compat/outgoing-jsonld.ts"; import { Create, @@ -15,6 +19,7 @@ import { assert, assertEquals, assertFalse, + assertGreaterOrEqual, assertInstanceOf, assertRejects, } from "@std/assert"; @@ -563,6 +568,158 @@ test("verifyProof()", async () => { assertFalse(contextLoaderCalls.includes("https://attacker.example/ctx")); }); +test("verifyProof() records verification duration metric", async (t) => { + const jsonLd = { + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/data-integrity/v1", + ], + id: "https://server.example/activities/1", + type: "Create", + actor: "https://server.example/users/alice", + object: { + id: "https://server.example/objects/1", + type: "Note", + attributedTo: "https://server.example/users/alice", + content: "Hello world", + location: { + type: "Place", + longitude: -71.184902, + latitude: 25.273962, + }, + }, + }; + const proof = new DataIntegrityProof({ + cryptosuite: "eddsa-jcs-2022", + verificationMethod: new URL( + "https://server.example/users/alice#ed25519-key", + ), + proofPurpose: "assertionMethod", + proofValue: decodeMultibase( + // cSpell: disable + "zLaewdp4H9kqtwyrLatK4cjY5oRHwVcw4gibPSUDYDMhi4M49v8pcYk3ZB6D69dNpAPbUmY8ocuJ3m9KhKJEEg7z", + // cSpell: enable + ), + created: Temporal.Instant.from("2023-02-24T23:36:38Z"), + }); + + await t.step( + "verified path records result=verified with bounded cryptosuite", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const key = await verifyProof(jsonLd, proof, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }); + assert(key != null); + + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + const m = measurements[0]; + assertEquals(m.type, "histogram"); + assertGreaterOrEqual(m.value, 0); + assertEquals( + m.attributes["activitypub.signature.kind"], + "object_integrity", + ); + assertEquals(m.attributes["activitypub.signature.result"], "verified"); + assertEquals( + m.attributes["object_integrity_proofs.cryptosuite"], + "eddsa-jcs-2022", + ); + }, + ); + + await t.step("rejected path records result=rejected", async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const tampered = { + ...jsonLd, + object: { ...jsonLd.object, content: "bye" }, + }; + const key = await verifyProof(tampered, proof, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }); + assertEquals(key, null); + + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + assertEquals( + measurements[0].attributes["activitypub.signature.result"], + "rejected", + ); + assertEquals( + measurements[0].attributes["object_integrity_proofs.cryptosuite"], + "eddsa-jcs-2022", + ); + }); + + await t.step("cached-key retry emits one measurement, not two", async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const keyId = "https://server.example/users/alice#ed25519-key"; + // Prime the cache with a wrong-algorithm key (the rsaPublicKey2 is RSA, + // not Ed25519) so that verifyProofInternal falls through to the + // fresh-fetch retry path. + const cache: Record = { + [keyId]: rsaPublicKey2, + }; + const key = await verifyProof(jsonLd, proof, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + keyCache: { + get(id) { + return Promise.resolve(cache[id.href]); + }, + set(id, k) { + cache[id.href] = k; + return Promise.resolve(); + }, + } satisfies KeyCache, + }); + assert(key != null); + assertEquals( + recorder.getMeasurements( + "activitypub.signature.verification.duration", + ).length, + 1, + ); + }); + + await t.step( + "verifyObject() wrapper emits one measurement per inner verifyProof()", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const create = await verifyObject(Create, { + ...jsonLd, + proof: await proof.toJsonLd({ + format: "compact", + contextLoader: mockDocumentLoader, + }), + }, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }); + assert(create != null); + // The fixture has exactly one proof; the wrapper should not + // double-instrument. + assertEquals( + recorder.getMeasurements( + "activitypub.signature.verification.duration", + ).length, + 1, + ); + }, + ); +}); + test("verifyObject()", async () => { const options: VerifyObjectOptions = { documentLoader: mockDocumentLoader, diff --git a/packages/fedify/src/sig/proof.ts b/packages/fedify/src/sig/proof.ts index ebe2febd3..5bcaca79a 100644 --- a/packages/fedify/src/sig/proof.ts +++ b/packages/fedify/src/sig/proof.ts @@ -18,6 +18,11 @@ import serialize from "json-canon"; import metadata from "../../deno.json" with { type: "json" }; import { normalizeOutgoingActivityJsonLd } from "../compat/outgoing-jsonld.ts"; import { preloadedOnlyDocumentLoader } from "../compat/preloaded-context-loader.ts"; +import { + getDurationMs, + getFederationMetrics, + type SignatureVerificationResult, +} from "../federation/metrics.ts"; import { fetchKey, type FetchKeyResult, @@ -25,6 +30,18 @@ import { validateCryptoKey, } from "./key.ts"; +/** + * Known Object Integrity Proof `cryptosuite` values, used to keep + * `object_integrity_proofs.cryptosuite` on a bounded set of spec-defined + * string values. Fedify currently signs and verifies only + * `eddsa-jcs-2022`; other values come in only from external proofs and are + * dropped from the metric attribute to avoid attacker-controlled + * cardinality. + */ +const OIP_KNOWN_CRYPTOSUITES: ReadonlySet = new Set([ + "eddsa-jcs-2022", +]); + const logger = getLogger(["fedify", "sig", "proof"]); /** @@ -310,6 +327,13 @@ export async function verifyProof( return await tracer.startActiveSpan( "object_integrity_proofs.verify", async (span) => { + const start = performance.now(); + let verified = false; + let threw = false; + const cryptosuite = proof.cryptosuite != null && + OIP_KNOWN_CRYPTOSUITES.has(proof.cryptosuite) + ? proof.cryptosuite + : undefined; if (span.isRecording()) { if (proof.cryptosuite != null) { span.setAttribute( @@ -333,14 +357,28 @@ export async function verifyProof( try { const key = await verifyProofInternal(jsonLd, proof, options); if (key == null) span.setStatus({ code: SpanStatusCode.ERROR }); + else verified = true; return key; } catch (error) { + threw = true; span.setStatus({ code: SpanStatusCode.ERROR, message: String(error), }); throw error; } finally { + const classified: SignatureVerificationResult = threw + ? "error" + : verified + ? "verified" + : "rejected"; + getFederationMetrics(options.meterProvider) + .recordSignatureVerificationDuration( + getDurationMs(start), + "object_integrity", + classified, + { cryptosuite }, + ); span.end(); } }, @@ -423,7 +461,10 @@ async function verifyProofInternal( "Ed25519 key:\n{keyId}; retrying with the freshly fetched key...", { proof, keyId: proof.verificationMethodId.href }, ); - return await verifyProof(jsonLd, proof, { + // Recurse into `verifyProofInternal()` (not `verifyProof()`) so the + // retry reuses the outer `object_integrity_proofs.verify` span and + // `activitypub.signature.verification.duration` measurement. + return await verifyProofInternal(jsonLd, proof, { ...options, keyCache: { // Returning `undefined` signals "nothing cached" and forces @@ -479,7 +520,10 @@ async function verifyProofInternal( "with the freshly fetched key...", { keyId: proof.verificationMethodId.href, proof }, ); - return await verifyProof(jsonLd, proof, { + // Recurse into `verifyProofInternal()` (not `verifyProof()`) so the + // retry reuses the outer `object_integrity_proofs.verify` span and + // `activitypub.signature.verification.duration` measurement. + return await verifyProofInternal(jsonLd, proof, { ...options, keyCache: { get: () => Promise.resolve(undefined), From a43272c24c31a073c46e80a4c94d70ddbaac102a Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 01:43:36 +0900 Subject: [PATCH 05/14] Record signature verification key fetch duration as a metric Add the `activitypub.signature.key_fetch.duration` OpenTelemetry histogram (unit `ms`), recorded around every public key lookup that runs inside one of Fedify's three signature verification paths. The new metric lets operators subtract key fetch time from total verification time to isolate crypto-only latency. Each measurement carries: - `activitypub.signature.kind`: `http`, `linked_data`, or `object_integrity` - `activitypub.signature.key_fetch.result`: `hit` (served from the in-process key cache), `fetched` (network roundtrip returned a usable key), or `error` (no usable key, including throws) The metric is recorded one-per-fetch-attempt, deliberately distinct from `activitypub.signature.verification.duration`, which is one-per-public-call. A verification that retries after a cache mismatch therefore emits two key fetch measurements (one `hit` for the stale attempt, one `fetched` for the freshly fetched retry) plus a single verification measurement that covers both. Each verifier wraps its `fetchKey` / `fetchKeyDetailed` calls in a small per-file helper (`measureKeyFetch` in http.ts, `measureLdKeyFetch` in ld.ts, `measureOipKeyFetch` in proof.ts) so: - the timer starts immediately before the fetch and ends as soon as the promise settles, even when concurrent local work (JCS canonicalization, SHA-256 digest, etc.) runs alongside the fetch; - thrown errors are recorded as `error` and rethrown, so verifier behavior is unchanged; - no key IDs, URLs, hostnames, or other unbounded values reach the metric attributes. Tests cover `hit`, `fetched`, and `error` paths for HTTP, plus `fetched` for LD and OIP, plus `hit` for OIP, plus a negative test asserting that an unsigned document under `verifyJsonLd()` emits no key fetch measurement at all. https://github.com/fedify-dev/fedify/issues/316 https://github.com/fedify-dev/fedify/issues/737 Assisted-by: Claude Code:claude-opus-4-7 Assisted-by: Codex:gpt-5.5 --- packages/fedify/src/federation/metrics.ts | 35 +++++++ packages/fedify/src/sig/http.test.ts | 114 ++++++++++++++++++++++ packages/fedify/src/sig/http.ts | 73 ++++++++++---- packages/fedify/src/sig/ld.test.ts | 49 ++++++++++ packages/fedify/src/sig/ld.ts | 59 ++++++++--- packages/fedify/src/sig/proof.test.ts | 72 ++++++++++++++ packages/fedify/src/sig/proof.ts | 44 ++++++++- 7 files changed, 410 insertions(+), 36 deletions(-) diff --git a/packages/fedify/src/federation/metrics.ts b/packages/fedify/src/federation/metrics.ts index ee127bfbe..6e9fcec00 100644 --- a/packages/fedify/src/federation/metrics.ts +++ b/packages/fedify/src/federation/metrics.ts @@ -61,6 +61,20 @@ export type SignatureVerificationResult = | "missing" | "error"; +/** + * The terminal classification of a public key fetch performed as part of + * signature verification, used as the + * `activitypub.signature.key_fetch.result` metric attribute. + * + * - `hit`: the public key was served from the in-process key cache. + * - `fetched`: the public key required a network round trip and returned + * a usable key. + * - `error`: the fetch attempt returned no usable key (HTTP failure, + * invalid response body, cached negative entry, etc.). + * @since 2.3.0 + */ +export type SignatureKeyFetchResult = "hit" | "fetched" | "error"; + /** * Optional attributes recorded alongside an * `activitypub.signature.verification.duration` measurement. Each field is @@ -89,6 +103,7 @@ class FederationMetrics { readonly deliveryPermanentFailure: Counter; readonly signatureVerificationFailure: Counter; readonly signatureVerificationDuration: Histogram; + readonly signatureKeyFetchDuration: Histogram; readonly deliveryDuration: Histogram; readonly inboxProcessingDuration: Histogram; readonly httpServerRequestCount: Counter; @@ -129,6 +144,15 @@ class FederationMetrics { unit: "ms", }, ); + this.signatureKeyFetchDuration = meter.createHistogram( + "activitypub.signature.key_fetch.duration", + { + description: + "Duration of public key lookup performed during ActivityPub " + + "signature verification.", + unit: "ms", + }, + ); this.deliveryDuration = meter.createHistogram( "activitypub.delivery.duration", { @@ -296,6 +320,17 @@ class FederationMetrics { this.signatureVerificationDuration.record(durationMs, attributes); } + recordSignatureKeyFetchDuration( + durationMs: number, + kind: SignatureVerificationKind, + result: SignatureKeyFetchResult, + ): void { + this.signatureKeyFetchDuration.record(durationMs, { + "activitypub.signature.kind": kind, + "activitypub.signature.key_fetch.result": result, + }); + } + recordInboxProcessingDuration( activityType: string, durationMs: number, diff --git a/packages/fedify/src/sig/http.test.ts b/packages/fedify/src/sig/http.test.ts index 153472d2e..b05417e7c 100644 --- a/packages/fedify/src/sig/http.test.ts +++ b/packages/fedify/src/sig/http.test.ts @@ -609,6 +609,120 @@ test("verifyRequestDetailed() records verification duration metric", async (t) = }, ); + await t.step( + "key fetch records result=fetched on a cold cache", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const request = await buildSignedRequest(); + const key = await verifyRequest(request, { + contextLoader: mockDocumentLoader, + documentLoader: mockDocumentLoader, + meterProvider, + }); + assertExists(key); + + const measurements = recorder.getMeasurements( + "activitypub.signature.key_fetch.duration", + ); + assertEquals(measurements.length, 1); + assertEquals(measurements[0].type, "histogram"); + assertGreaterOrEqual(measurements[0].value, 0); + assertEquals( + measurements[0].attributes["activitypub.signature.kind"], + "http", + ); + assertEquals( + measurements[0].attributes[ + "activitypub.signature.key_fetch.result" + ], + "fetched", + ); + }, + ); + + await t.step( + "key fetch records result=hit when served from the key cache", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const cache: Record = { + "https://example.com/key2": rsaPublicKey2, + }; + const request = await buildSignedRequest(); + const key = await verifyRequest(request, { + contextLoader: mockDocumentLoader, + documentLoader: mockDocumentLoader, + meterProvider, + keyCache: { + get(keyId) { + return Promise.resolve(cache[keyId.href]); + }, + set(keyId, k) { + cache[keyId.href] = k; + return Promise.resolve(); + }, + } satisfies KeyCache, + }); + assertExists(key); + + const measurements = recorder.getMeasurements( + "activitypub.signature.key_fetch.duration", + ); + assertEquals(measurements.length, 1); + assertEquals( + measurements[0].attributes[ + "activitypub.signature.key_fetch.result" + ], + "hit", + ); + }, + ); + + await t.step( + "key fetch records result=error when the remote key returns HTTP 410", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + 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: "{}", + }), + rsaPrivateKey2, + keyId, + ); + const result = await verifyRequestDetailed(request, { + contextLoader: mockDocumentLoader, + documentLoader(url) { + if (url === keyId.href) { + throw new FetchError( + keyId, + `HTTP 410: ${keyId.href}`, + new Response(null, { status: 410 }), + ); + } + return mockDocumentLoader(url); + }, + meterProvider, + }); + assertFalse(result.verified); + + const measurements = recorder.getMeasurements( + "activitypub.signature.key_fetch.duration", + ); + assertEquals(measurements.length, 1); + assertEquals( + measurements[0].attributes[ + "activitypub.signature.key_fetch.result" + ], + "error", + ); + }, + ); + await t.step( "draft-cavage with unknown algorithm omits the algorithm metric attribute", async () => { diff --git a/packages/fedify/src/sig/http.ts b/packages/fedify/src/sig/http.ts index 236c976a6..8b138b457 100644 --- a/packages/fedify/src/sig/http.ts +++ b/packages/fedify/src/sig/http.ts @@ -749,6 +749,39 @@ interface HttpSignatureMetricsContext { algorithm?: string; } +/** + * Times an awaited public key fetch and records exactly one + * `activitypub.signature.key_fetch.duration` measurement, classifying the + * outcome as `hit`, `fetched`, or `error` based on the `cached` flag and + * whether the returned key is non-null. Errors thrown by the fetch are + * reported as `error` and rethrown, so verifier behavior is unchanged. + */ +async function measureKeyFetch< + T extends { cached: boolean; key?: unknown }, +>( + kind: "http", + meterProvider: MeterProvider | undefined, + fetch: () => Promise, +): Promise { + const start = performance.now(); + try { + const result = await fetch(); + getFederationMetrics(meterProvider).recordSignatureKeyFetchDuration( + getDurationMs(start), + kind, + result.key != null ? (result.cached ? "hit" : "fetched") : "error", + ); + return result; + } catch (error) { + getFederationMetrics(meterProvider).recordSignatureKeyFetchDuration( + getDurationMs(start), + kind, + "error", + ); + throw error; + } +} + /** * Known draft-cavage `algorithm` parameter values, used to keep the * `http_signatures.algorithm` metric attribute on a bounded set. The header @@ -1133,16 +1166,18 @@ async function verifyRequestDraft( metricsContext.algorithm = normalizedAlgorithm; } } - const { key, cached, fetchError } = await fetchKeyDetailed( - keyIdUrl, - CryptographicKey, - { - documentLoader, - contextLoader, - keyCache, - tracerProvider, - }, + const fetchResult = await measureKeyFetch( + "http", + meterProvider, + () => + fetchKeyDetailed(keyIdUrl, CryptographicKey, { + documentLoader, + contextLoader, + keyCache, + tracerProvider, + }), ); + const { key, cached, fetchError } = fetchResult; if (fetchError != null) { return keyFetchErrorResult(keyIdUrl, fetchError); } @@ -1482,16 +1517,18 @@ async function verifyRequestRfc9421( continue; } - const { key, cached, fetchError } = await fetchKeyDetailed( - keyId, - CryptographicKey, - { - documentLoader, - contextLoader, - keyCache, - tracerProvider, - }, + const rfcFetchResult = await measureKeyFetch( + "http", + meterProvider, + () => + fetchKeyDetailed(keyId, CryptographicKey, { + documentLoader, + contextLoader, + keyCache, + tracerProvider, + }), ); + const { key, cached, fetchError } = rfcFetchResult; if (fetchError != null) { setFailure(keyFetchErrorResult(keyId, fetchError)); diff --git a/packages/fedify/src/sig/ld.test.ts b/packages/fedify/src/sig/ld.test.ts index 93cfe5a3f..4e4613a24 100644 --- a/packages/fedify/src/sig/ld.test.ts +++ b/packages/fedify/src/sig/ld.test.ts @@ -449,6 +449,55 @@ test("verifyJsonLd() records verification duration metric", async (t) => { }, ); + await t.step( + "key fetch records result=fetched on a cold cache", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const verified = await verifyJsonLd(testVector, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }); + assert(verified); + + const measurements = recorder.getMeasurements( + "activitypub.signature.key_fetch.duration", + ); + assertEquals(measurements.length, 1); + assertGreaterOrEqual(measurements[0].value, 0); + assertEquals( + measurements[0].attributes["activitypub.signature.kind"], + "linked_data", + ); + assertEquals( + measurements[0].attributes[ + "activitypub.signature.key_fetch.result" + ], + "fetched", + ); + }, + ); + + await t.step( + "missing signature emits no key_fetch measurement", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const verified = await verifyJsonLd(document, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }); + assertFalse(verified); + + assertEquals( + recorder.getMeasurements( + "activitypub.signature.key_fetch.duration", + ).length, + 0, + ); + }, + ); + await t.step( "unknown signature type omits the ld_signatures.type metric attribute", async () => { diff --git a/packages/fedify/src/sig/ld.ts b/packages/fedify/src/sig/ld.ts index 3fce6ab0b..38723b8aa 100644 --- a/packages/fedify/src/sig/ld.ts +++ b/packages/fedify/src/sig/ld.ts @@ -318,10 +318,9 @@ export async function verifySignature( ); return null; } - const { key, cached } = await fetchKey( - new URL(sig.creator), - CryptographicKey, - options, + const { key, cached } = await measureLdKeyFetch( + options.meterProvider, + () => fetchKey(new URL(sig.creator), CryptographicKey, options), ); if (key == null) return null; const sigOpts: { @@ -375,16 +374,16 @@ export async function verifySignature( "Retrying with the freshly fetched key...", { keyId: sig.creator, ...sig }, ); - const { key } = await fetchKey( - new URL(sig.creator), - CryptographicKey, - { - ...options, - keyCache: { - get: () => Promise.resolve(undefined), - set: async (keyId, key) => await options.keyCache?.set(keyId, key), - }, - }, + const { key } = await measureLdKeyFetch( + options.meterProvider, + () => + fetchKey(new URL(sig.creator), CryptographicKey, { + ...options, + keyCache: { + get: () => Promise.resolve(undefined), + set: async (keyId, key) => await options.keyCache?.set(keyId, key), + }, + }), ); if (key == null) return null; const verified = await crypto.subtle.verify( @@ -411,6 +410,38 @@ export async function verifySignature( export interface VerifyJsonLdOptions extends VerifySignatureOptions { } +/** + * Times an awaited public key fetch and records exactly one + * `activitypub.signature.key_fetch.duration` measurement, classifying the + * outcome as `hit`, `fetched`, or `error` based on the `cached` flag and + * whether the returned key is non-null. Errors thrown by the fetch are + * reported as `error` and rethrown, so verifier behavior is unchanged. + */ +async function measureLdKeyFetch< + T extends { cached: boolean; key: CryptographicKey | null }, +>( + meterProvider: MeterProvider | undefined, + fetch: () => Promise, +): Promise { + const start = performance.now(); + try { + const result = await fetch(); + getFederationMetrics(meterProvider).recordSignatureKeyFetchDuration( + getDurationMs(start), + "linked_data", + result.key != null ? (result.cached ? "hit" : "fetched") : "error", + ); + return result; + } catch (error) { + getFederationMetrics(meterProvider).recordSignatureKeyFetchDuration( + getDurationMs(start), + "linked_data", + "error", + ); + throw error; + } +} + /** * Known Linked Data Signature `type` values, used to keep * `ld_signatures.type` on a bounded set of spec-defined string values. diff --git a/packages/fedify/src/sig/proof.test.ts b/packages/fedify/src/sig/proof.test.ts index 4c5fc08d8..08dcba7e3 100644 --- a/packages/fedify/src/sig/proof.test.ts +++ b/packages/fedify/src/sig/proof.test.ts @@ -692,6 +692,78 @@ test("verifyProof() records verification duration metric", async (t) => { ); }); + await t.step( + "key fetch records result=fetched on a cold cache", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const key = await verifyProof(jsonLd, proof, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }); + assert(key != null); + + const measurements = recorder.getMeasurements( + "activitypub.signature.key_fetch.duration", + ); + assertEquals(measurements.length, 1); + assertGreaterOrEqual(measurements[0].value, 0); + assertEquals( + measurements[0].attributes["activitypub.signature.kind"], + "object_integrity", + ); + assertEquals( + measurements[0].attributes[ + "activitypub.signature.key_fetch.result" + ], + "fetched", + ); + }, + ); + + await t.step( + "key fetch records result=hit when served from the key cache", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + const expectedKey = new Multikey({ + id: new URL("https://server.example/users/alice#ed25519-key"), + controller: new URL("https://server.example/users/alice"), + publicKey: await importMultibaseKey( + "z6MkrJVnaZkeFzdQyMZu1cgjg7k1pZZ6pvBQ7XJPt4swbTQ2", + ), + }); + const cache: Record = { + "https://server.example/users/alice#ed25519-key": expectedKey, + }; + const key = await verifyProof(jsonLd, proof, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + keyCache: { + get(id) { + return Promise.resolve(cache[id.href]); + }, + set(id, k) { + cache[id.href] = k; + return Promise.resolve(); + }, + } satisfies KeyCache, + }); + assert(key != null); + + const measurements = recorder.getMeasurements( + "activitypub.signature.key_fetch.duration", + ); + assertEquals(measurements.length, 1); + assertEquals( + measurements[0].attributes[ + "activitypub.signature.key_fetch.result" + ], + "hit", + ); + }, + ); + await t.step( "verifyObject() wrapper emits one measurement per inner verifyProof()", async () => { diff --git a/packages/fedify/src/sig/proof.ts b/packages/fedify/src/sig/proof.ts index 5bcaca79a..6a08d3d62 100644 --- a/packages/fedify/src/sig/proof.ts +++ b/packages/fedify/src/sig/proof.ts @@ -42,6 +42,43 @@ const OIP_KNOWN_CRYPTOSUITES: ReadonlySet = new Set([ "eddsa-jcs-2022", ]); +/** + * Times an awaited public key fetch and records exactly one + * `activitypub.signature.key_fetch.duration` measurement, classifying the + * outcome as `hit`, `fetched`, or `error` based on the `cached` flag and + * whether the returned key is non-null. Errors thrown by the fetch are + * reported as `error` and rethrown, so verifier behavior is unchanged. + * + * The fetch is wrapped in its own async timer so the recorded duration + * covers only the key fetch itself, not the surrounding JCS canonicalization + * and SHA-256 digest work that runs concurrently in + * {@link verifyProofInternal}. + */ +function measureOipKeyFetch( + meterProvider: MeterProvider | undefined, + fetch: () => Promise>, +): Promise> { + const start = performance.now(); + return fetch().then( + (result) => { + getFederationMetrics(meterProvider).recordSignatureKeyFetchDuration( + getDurationMs(start), + "object_integrity", + result.key != null ? (result.cached ? "hit" : "fetched") : "error", + ); + return result; + }, + (error) => { + getFederationMetrics(meterProvider).recordSignatureKeyFetchDuration( + getDurationMs(start), + "object_integrity", + "error", + ); + throw error; + }, + ); +} + const logger = getLogger(["fedify", "sig", "proof"]); /** @@ -400,10 +437,9 @@ async function verifyProofInternal( proof.proofValue == null || proof.created == null ) return null; - const publicKeyPromise = fetchKey( - proof.verificationMethodId, - Multikey, - options, + const publicKeyPromise = measureOipKeyFetch( + options.meterProvider, + () => fetchKey(proof.verificationMethodId!, Multikey, options), ); const proofConfig = { // deno-lint-ignore no-explicit-any From 963549fe27d8431bccfc5fede24c13182753bf25 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 01:56:28 +0900 Subject: [PATCH 06/14] Document signature verification duration metrics Extend docs/manual/opentelemetry.md to cover the two new histograms: activitypub.signature.verification.duration and activitypub.signature.key_fetch.duration. The metric attribute reference now explains: - that activitypub.signature.kind takes one of http, linked_data, or object_integrity, and activitypub.signature.result takes one of verified, rejected, missing, or error (with a note that object_integrity never produces missing); - that the verification.duration histogram covers the full verification path including local key lookup and remote key fetches, so operators can subtract key_fetch.duration to isolate the non-fetch verification work (canonicalization, hashing, attribution and owner checks, cryptographic verification); - that direct calls to verifyRequest() / verifyRequestDetailed(), verifyJsonLd(), and verifyProof() emit exactly one measurement per public call, while wrappers such as verifyObject() emit one measurement per inner verifyProof() call; - that http_signatures.algorithm, ld_signatures.type, and object_integrity_proofs.cryptosuite are recorded only when the parsed value matches a small spec-bounded set, listing the exact set for each kind; - that http_signatures.failure_reason on rejected rows is one of invalidSignature or keyFetchError, and that missing HTTP signatures use activitypub.signature.result=missing without a failure_reason attribute; - that key IDs, actor IDs, URLs, and object IDs are deliberately excluded from both histograms; - that key_fetch.duration is recorded per fetch attempt, so a cache-mismatch retry emits two key fetch measurements alongside the single verification measurement that covers both; - that hit corresponds to a result served by the configured KeyCache (which may itself be backed by Redis or a database), and fetched corresponds to a key loaded through the document loader, regardless of whether the document loader hits the network or a local store. https://github.com/fedify-dev/fedify/issues/316 https://github.com/fedify-dev/fedify/issues/737 Assisted-by: Claude Code:claude-opus-4-7 Assisted-by: Codex:gpt-5.5 --- docs/manual/opentelemetry.md | 107 ++++++++++++++++++++++++++++++----- 1 file changed, 92 insertions(+), 15 deletions(-) diff --git a/docs/manual/opentelemetry.md b/docs/manual/opentelemetry.md index 3eb5acc64..404293aa1 100644 --- a/docs/manual/opentelemetry.md +++ b/docs/manual/opentelemetry.md @@ -296,21 +296,23 @@ Instrumented metrics Fedify records the following OpenTelemetry metrics: -| Metric name | Instrument | Unit | Description | -| -------------------------------------------- | ------------- | ----------- | --------------------------------------------------------------- | -| `activitypub.delivery.sent` | Counter | `{attempt}` | Counts outgoing ActivityPub delivery attempts. | -| `activitypub.delivery.permanent_failure` | Counter | `{failure}` | Counts outgoing deliveries abandoned as permanent failures. | -| `activitypub.delivery.duration` | Histogram | `ms` | Measures outgoing ActivityPub delivery attempt duration. | -| `activitypub.inbox.processing_duration` | Histogram | `ms` | Measures inbox listener processing duration. | -| `activitypub.signature.verification_failure` | Counter | `{failure}` | Counts failed signature verification for inbox requests. | -| `fedify.http.server.request.count` | Counter | `{request}` | Counts inbound HTTP requests handled by `Federation.fetch()`. | -| `fedify.http.server.request.duration` | Histogram | `ms` | Measures inbound HTTP request duration in `Federation.fetch()`. | -| `fedify.queue.task.enqueued` | Counter | `{task}` | Counts inbox, outbox, and fanout tasks Fedify enqueued. | -| `fedify.queue.task.started` | Counter | `{task}` | Counts queue tasks Fedify began processing as a worker. | -| `fedify.queue.task.completed` | Counter | `{task}` | Counts queue tasks Fedify finished processing without throwing. | -| `fedify.queue.task.failed` | Counter | `{task}` | Counts queue tasks Fedify abandoned because processing threw. | -| `fedify.queue.task.duration` | Histogram | `ms` | Measures queue task processing duration in Fedify workers. | -| `fedify.queue.task.in_flight` | UpDownCounter | `{task}` | Tracks queue tasks currently in flight in this Fedify process. | +| Metric name | Instrument | Unit | Description | +| --------------------------------------------- | ------------- | ----------- | ----------------------------------------------------------------------------------------------- | +| `activitypub.delivery.sent` | Counter | `{attempt}` | Counts outgoing ActivityPub delivery attempts. | +| `activitypub.delivery.permanent_failure` | Counter | `{failure}` | Counts outgoing deliveries abandoned as permanent failures. | +| `activitypub.delivery.duration` | Histogram | `ms` | Measures outgoing ActivityPub delivery attempt duration. | +| `activitypub.inbox.processing_duration` | Histogram | `ms` | Measures inbox listener processing duration. | +| `activitypub.signature.verification_failure` | Counter | `{failure}` | Counts failed signature verification for inbox requests. | +| `activitypub.signature.verification.duration` | Histogram | `ms` | Measures signature verification duration across HTTP, Linked Data, and Object Integrity Proofs. | +| `activitypub.signature.key_fetch.duration` | Histogram | `ms` | Measures public key lookup duration during signature verification. | +| `fedify.http.server.request.count` | Counter | `{request}` | Counts inbound HTTP requests handled by `Federation.fetch()`. | +| `fedify.http.server.request.duration` | Histogram | `ms` | Measures inbound HTTP request duration in `Federation.fetch()`. | +| `fedify.queue.task.enqueued` | Counter | `{task}` | Counts inbox, outbox, and fanout tasks Fedify enqueued. | +| `fedify.queue.task.started` | Counter | `{task}` | Counts queue tasks Fedify began processing as a worker. | +| `fedify.queue.task.completed` | Counter | `{task}` | Counts queue tasks Fedify finished processing without throwing. | +| `fedify.queue.task.failed` | Counter | `{task}` | Counts queue tasks Fedify abandoned because processing threw. | +| `fedify.queue.task.duration` | Histogram | `ms` | Measures queue task processing duration in Fedify workers. | +| `fedify.queue.task.in_flight` | UpDownCounter | `{task}` | Tracks queue tasks currently in flight in this Fedify process. | ### Metric attributes @@ -332,6 +334,81 @@ Fedify records the following OpenTelemetry metrics: : `activitypub.verification.failure_reason`, plus `activitypub.remote.host` when the failed signature includes a key ID. +`activitypub.signature.verification.duration` +: `activitypub.signature.kind` is always present and is one of `http`, + `linked_data`, or `object_integrity`. `activitypub.signature.result` is + always present and is one of: + + - `verified`: the signature was checked and accepted. + - `rejected`: the signature was checked and refused (bad signature, + key fetch failure, owner mismatch, etc.). + - `missing`: no signature was present. Only `http` and `linked_data` + produce this value; `object_integrity` does not, because the caller + decides whether to invoke proof verification at all. + - `error`: verification threw an unexpected error. + + The duration covers the full verification path Fedify performs, + *including* local key lookup and remote key fetches; the separate + `activitypub.signature.key_fetch.duration` histogram lets operators + subtract key lookup latency from the total to isolate the rest of the + verification work (canonicalization, hashing, attribution and owner + checks, cryptographic verification, etc.). Direct calls to + `verifyRequest()` / `verifyRequestDetailed()`, `verifyJsonLd()`, and + `verifyProof()` each emit exactly one measurement, even when the + implementation retries internally after a cache mismatch. Wrappers + such as `verifyObject()` emit one measurement per inner `verifyProof()` + call (and none when the object has no proofs); higher-level inbox + handling can perform several verification attempts in series. + + Kind-specific optional attributes are recorded only when the value + matches a small, spec-bounded set, to keep cardinality safe even when + attacker-supplied JSON-LD or signature headers reach the verifier: + + - `http_signatures.algorithm` (HTTP only) is recorded only when the + parsed algorithm value is one of `rsa-sha1`, `rsa-sha256`, + `rsa-sha512`, `ecdsa-sha256`, `ecdsa-sha384`, `ecdsa-sha512`, + `ed25519`, or `hs2019` (draft-cavage) or one of the keys of the + RFC 9421 algorithm map (`rsa-v1_5-sha256`, `rsa-v1_5-sha512`, + `rsa-pss-sha512`, `ecdsa-p256-sha256`, `ecdsa-p384-sha384`, + `ed25519`). + - `http_signatures.failure_reason` (HTTP only, on `rejected` rows) + is one of `invalidSignature` or `keyFetchError`. HTTP requests + with no signature header are reported as + `activitypub.signature.result=missing` and do not carry a + `http_signatures.failure_reason`. + - `ld_signatures.type` (Linked Data only) is recorded only for the + spec-supported `RsaSignature2017` type. + - `object_integrity_proofs.cryptosuite` (Object Integrity Proofs + only) is recorded only for the spec-supported `eddsa-jcs-2022` + cryptosuite. + + Key IDs, actor IDs, request URLs, and object IDs are deliberately + excluded from this histogram. They remain on the corresponding spans + (`http_signatures.verify`, `ld_signatures.verify`, + `object_integrity_proofs.verify`) for trace-level investigation. + +`activitypub.signature.key_fetch.duration` +: `activitypub.signature.kind` is always present (same values as above). + `activitypub.signature.key_fetch.result` is always present and is one + of: + + - `hit`: the public key was served by the configured `KeyCache` + (which may itself be backed by a remote store such as Redis or a + database; the measurement reflects whatever round trip that + backend incurs). + - `fetched`: the key was not in the cache and was loaded through + the document loader, returning a usable key. This typically + corresponds to a network fetch, but a custom document loader + that serves from a local store will also fall in this bucket. + - `error`: no usable key came back (HTTP failure, invalid response + body, cached negative entry, thrown exception, etc.). + + Unlike `activitypub.signature.verification.duration`, this histogram + is recorded *per fetch attempt*: a verification that retries after a + cache mismatch emits two key fetch measurements (typically one `hit` + for the stale attempt and one `fetched` for the freshly fetched retry) + alongside the single verification measurement that covers both. + `fedify.http.server.request.count` and `fedify.http.server.request.duration` : `http.request.method` and `fedify.endpoint` are always present. `http.request.method` is normalized to one of the standard HTTP methods From 0a86e86e7ca3563c468544cdf8c27557ae18c53c Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 01:57:38 +0900 Subject: [PATCH 07/14] Document signature verification metrics in CHANGES.md Add a 2.3.0 changelog entry for the new activitypub.signature.verification.duration and activitypub.signature.key_fetch.duration histograms, listing the always-present and bounded conditional attributes, and the corresponding issue link reference. https://github.com/fedify-dev/fedify/issues/316 https://github.com/fedify-dev/fedify/issues/737 Assisted-by: Claude Code:claude-opus-4-7 --- CHANGES.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 1e1cbdb97..f622c3aac 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -41,6 +41,21 @@ To be released. attributes, and `TraceActivityRecord.activityJson` is present only when the span event includes full activity JSON. [[#316], [#619], [#755]] + - Added two OpenTelemetry histograms for signature verification: + `activitypub.signature.verification.duration` measures end-to-end + verification time for HTTP Signatures, Linked Data Signatures, and + Object Integrity Proofs (including local key lookup and remote key + fetches), and `activitypub.signature.key_fetch.duration` measures + public key lookup duration separately so operators can isolate + non-fetch verification work. Both instruments carry + `activitypub.signature.kind` (`http`, `linked_data`, or + `object_integrity`) and bounded result attributes; the verification + histogram additionally carries spec-bounded + `http_signatures.algorithm`, `ld_signatures.type`, or + `object_integrity_proofs.cryptosuite` when known, plus + `http_signatures.failure_reason` on rejected HTTP rows. + [[#316], [#737]] + - Added OpenTelemetry HTTP server metrics for inbound requests handled by `Federation.fetch()`: `fedify.http.server.request.count` (Counter) and `fedify.http.server.request.duration` (Histogram). Both instruments carry @@ -80,6 +95,7 @@ To be released. [#619]: https://github.com/fedify-dev/fedify/issues/619 [#735]: https://github.com/fedify-dev/fedify/issues/735 [#736]: https://github.com/fedify-dev/fedify/issues/736 +[#737]: https://github.com/fedify-dev/fedify/issues/737 [#740]: https://github.com/fedify-dev/fedify/issues/740 [#748]: https://github.com/fedify-dev/fedify/pull/748 [#752]: https://github.com/fedify-dev/fedify/issues/752 From 66d174b64573f2d584e01774d00c9a34b7d764b9 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 02:04:28 +0900 Subject: [PATCH 08/14] Align SignatureKeyFetchResult JSDoc with the manual docs The exported type's JSDoc still described `hit` as "served from the in-process key cache" and `fetched` as "required a network round trip," which no longer matches the implementation or docs/manual/opentelemetry.md. `KeyCache` can be backed by a remote store such as Redis or a database, and `fetched` only requires loading through the document loader (typically but not exclusively a network roundtrip). Update the JSDoc to mirror the manual. https://github.com/fedify-dev/fedify/issues/316 https://github.com/fedify-dev/fedify/issues/737 Assisted-by: Codex:gpt-5.5 --- packages/fedify/src/federation/metrics.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/fedify/src/federation/metrics.ts b/packages/fedify/src/federation/metrics.ts index 6e9fcec00..293d8322b 100644 --- a/packages/fedify/src/federation/metrics.ts +++ b/packages/fedify/src/federation/metrics.ts @@ -66,11 +66,17 @@ export type SignatureVerificationResult = * signature verification, used as the * `activitypub.signature.key_fetch.result` metric attribute. * - * - `hit`: the public key was served from the in-process key cache. - * - `fetched`: the public key required a network round trip and returned - * a usable key. + * - `hit`: the public key was served by the configured `KeyCache`. The + * `KeyCache` itself may be backed by a remote store such as Redis or a + * database, in which case the measurement reflects whatever round trip + * that backend incurs. + * - `fetched`: the public key was not in the cache and was loaded + * through the document loader, returning a usable key. This typically + * corresponds to a network fetch, but a custom document loader that + * serves from a local store will also fall in this bucket. * - `error`: the fetch attempt returned no usable key (HTTP failure, - * invalid response body, cached negative entry, etc.). + * invalid response body, cached negative entry, thrown exception, + * etc.). * @since 2.3.0 */ export type SignatureKeyFetchResult = "hit" | "fetched" | "error"; From 99a15a30dcddd1e7e14cdd40f5c32f844ad5f98b Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 04:24:50 +0900 Subject: [PATCH 09/14] Add a PR link to the changelog --- CHANGES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index f622c3aac..664494ca8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -54,7 +54,7 @@ To be released. `http_signatures.algorithm`, `ld_signatures.type`, or `object_integrity_proofs.cryptosuite` when known, plus `http_signatures.failure_reason` on rejected HTTP rows. - [[#316], [#737]] + [[#316], [#737], [#769]] - Added OpenTelemetry HTTP server metrics for inbound requests handled by `Federation.fetch()`: `fedify.http.server.request.count` (Counter) and @@ -103,6 +103,7 @@ To be released. [#755]: https://github.com/fedify-dev/fedify/pull/755 [#757]: https://github.com/fedify-dev/fedify/pull/757 [#759]: https://github.com/fedify-dev/fedify/pull/759 +[#769]: https://github.com/fedify-dev/fedify/pull/769 ### @fedify/fixture From 6246a56ba86524656d3c53bb80be3350eb6715e8 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 04:58:06 +0900 Subject: [PATCH 10/14] Use async/await in measureOipKeyFetch The OIP key-fetch timing helper used `Promise.prototype.then()` with success and error handlers, while the matching helpers in *http.ts* (`measureKeyFetch`) and *ld.ts* (`measureLdKeyFetch`) use async/await with try/catch. Rewriting `measureOipKeyFetch` the same way removes that local inconsistency. The function is still invoked synchronously up to its first `await`, so `verifyProofInternal()`'s concurrent overlap of the key fetch with JCS canonicalization and SHA-256 digest work is preserved. https://github.com/fedify-dev/fedify/pull/769#discussion_r3250554834 Assisted-by: Claude Code:claude-opus-4-7 --- packages/fedify/src/sig/proof.ts | 40 +++++++++++++++++--------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/packages/fedify/src/sig/proof.ts b/packages/fedify/src/sig/proof.ts index 6a08d3d62..3a1ef11c9 100644 --- a/packages/fedify/src/sig/proof.ts +++ b/packages/fedify/src/sig/proof.ts @@ -54,29 +54,31 @@ const OIP_KNOWN_CRYPTOSUITES: ReadonlySet = new Set([ * and SHA-256 digest work that runs concurrently in * {@link verifyProofInternal}. */ -function measureOipKeyFetch( +async function measureOipKeyFetch( meterProvider: MeterProvider | undefined, fetch: () => Promise>, ): Promise> { const start = performance.now(); - return fetch().then( - (result) => { - getFederationMetrics(meterProvider).recordSignatureKeyFetchDuration( - getDurationMs(start), - "object_integrity", - result.key != null ? (result.cached ? "hit" : "fetched") : "error", - ); - return result; - }, - (error) => { - getFederationMetrics(meterProvider).recordSignatureKeyFetchDuration( - getDurationMs(start), - "object_integrity", - "error", - ); - throw error; - }, - ); + // The fetch closure is invoked synchronously here (before the first + // `await`), so the timing window stays tight and the caller still gets a + // Promise it can hold while running other work concurrently. + const pending = fetch(); + try { + const result = await pending; + getFederationMetrics(meterProvider).recordSignatureKeyFetchDuration( + getDurationMs(start), + "object_integrity", + result.key != null ? (result.cached ? "hit" : "fetched") : "error", + ); + return result; + } catch (error) { + getFederationMetrics(meterProvider).recordSignatureKeyFetchDuration( + getDurationMs(start), + "object_integrity", + "error", + ); + throw error; + } } const logger = getLogger(["fedify", "sig", "proof"]); From a9bdffdcbd807c1d131b4339407e1d03c62cd3cd Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 04:58:27 +0900 Subject: [PATCH 11/14] Cover the LD cached-key retry path in tests The LD path (`verifySignature()`) retries with a fresh fetch when the cached key fails to verify, and the surrounding documentation states that each retry emits its own `activitypub.signature.key_fetch.duration` measurement. HTTP and OIP already had regression tests for this; LD did not. Add a step that primes the keyCache with a wrong-but-valid RSA key for the test vector's signer, runs `verifyJsonLd()`, and asserts the two key-fetch measurements appear in order: `result=hit` for the stale cached attempt, then `result=fetched` for the fresh retry that finally verifies. https://github.com/fedify-dev/fedify/pull/769#discussion_r3250570207 Assisted-by: Claude Code:claude-opus-4-7 --- packages/fedify/src/sig/ld.test.ts | 50 ++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/packages/fedify/src/sig/ld.test.ts b/packages/fedify/src/sig/ld.test.ts index 4e4613a24..ed971eb29 100644 --- a/packages/fedify/src/sig/ld.test.ts +++ b/packages/fedify/src/sig/ld.test.ts @@ -498,6 +498,56 @@ test("verifyJsonLd() records verification duration metric", async (t) => { }, ); + await t.step( + "cached-key retry emits two key_fetch measurements: hit then fetched", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + // Prime the cache with a wrong-but-valid RSA key for the signer's + // keyId. The first verification attempt uses the stale cached key, + // fails, and falls through to the fresh-fetch retry path inside + // `verifySignature()`. The retry must record its own key_fetch + // measurement separately from the cached attempt. + const verified = await verifyJsonLd(testVector, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + keyCache: { + async get(keyId: URL) { + return new CryptographicKey({ + id: keyId, + owner: new URL( + "https://activitypub.academy/users/brauca_darradiul", + ), + publicKey: + (await generateCryptoKeyPair("RSASSA-PKCS1-v1_5")).publicKey, + }); + }, + set(_keyId: URL, _key: CryptographicKey) { + return Promise.resolve(); + }, + }, + }); + assert(verified); + + const measurements = recorder.getMeasurements( + "activitypub.signature.key_fetch.duration", + ); + assertEquals(measurements.length, 2); + assertEquals( + measurements[0].attributes[ + "activitypub.signature.key_fetch.result" + ], + "hit", + ); + assertEquals( + measurements[1].attributes[ + "activitypub.signature.key_fetch.result" + ], + "fetched", + ); + }, + ); + await t.step( "unknown signature type omits the ld_signatures.type metric attribute", async () => { From b3d20900353fe098f68a2ed8fcfd5f8537ec1c51 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 04:58:53 +0900 Subject: [PATCH 12/14] Test the OIP cryptosuite cardinality gate Add a regression test that verifies an unrecognized cryptosuite does not flow through to the `object_integrity_proofs.cryptosuite` metric attribute. `DataIntegrityProof`'s constructor and `clone()` reject any cryptosuite other than `eddsa-jcs-2022`, so the exotic proof is built through `DataIntegrityProof.fromJsonLd()`, which is permissive enough to accept arbitrary field values. `verifyProof()` then records `result=rejected` (the internal validator returns null for any cryptosuite other than `eddsa-jcs-2022`), and the test asserts the histogram measurement does not carry an `object_integrity_proofs.cryptosuite` attribute. This locks in the `OIP_KNOWN_CRYPTOSUITES` whitelist as a defense against attacker-controlled cryptosuite strings inflating metric cardinality. https://github.com/fedify-dev/fedify/pull/769#discussion_r3250570215 Assisted-by: Claude Code:claude-opus-4-7 --- packages/fedify/src/sig/proof.test.ts | 47 +++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/packages/fedify/src/sig/proof.test.ts b/packages/fedify/src/sig/proof.test.ts index 08dcba7e3..0fd792e83 100644 --- a/packages/fedify/src/sig/proof.test.ts +++ b/packages/fedify/src/sig/proof.test.ts @@ -790,6 +790,53 @@ test("verifyProof() records verification duration metric", async (t) => { ); }, ); + + await t.step( + "unknown cryptosuite omits the cryptosuite metric attribute", + async () => { + const [meterProvider, recorder] = createTestMeterProvider(); + // `DataIntegrityProof`'s constructor and `clone()` reject any + // cryptosuite other than `eddsa-jcs-2022`, but `fromJsonLd()` does + // not, so build the exotic proof through the JSON-LD path to + // exercise the metric attribute whitelist on an inbound proof that + // the validator would later reject. + const exoticProof = await DataIntegrityProof.fromJsonLd({ + "@context": "https://w3id.org/security/data-integrity/v1", + type: "DataIntegrityProof", + cryptosuite: "made-up-suite-9999", + verificationMethod: "https://server.example/users/alice#ed25519-key", + proofPurpose: "assertionMethod", + proofValue: + // cSpell: disable + "zLaewdp4H9kqtwyrLatK4cjY5oRHwVcw4gibPSUDYDMhi4M49v8pcYk3ZB6D69dNpAPbUmY8ocuJ3m9KhKJEEg7z", + // cSpell: enable + created: "2023-02-24T23:36:38Z", + }, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + }); + assertEquals(exoticProof.cryptosuite, "made-up-suite-9999"); + + const key = await verifyProof(jsonLd, exoticProof, { + documentLoader: mockDocumentLoader, + contextLoader: mockDocumentLoader, + meterProvider, + }); + assertEquals(key, null); + + const measurements = recorder.getMeasurements( + "activitypub.signature.verification.duration", + ); + assertEquals(measurements.length, 1); + assertEquals( + measurements[0].attributes["activitypub.signature.result"], + "rejected", + ); + assertFalse( + "object_integrity_proofs.cryptosuite" in measurements[0].attributes, + ); + }, + ); }); test("verifyObject()", async () => { From 1c7105b6271b883a07cd43fd4b6194e540f06f25 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 08:54:32 +0900 Subject: [PATCH 13/14] Actually exercise the OIP cached-key retry in tests The "cached-key retry emits one measurement, not two" step had two weaknesses. First, it primed the cache with rsaPublicKey2, a CryptographicKey rather than a Multikey, so `fetchKey()` skipped the cache entirely and went straight to a fresh fetch. The retry path inside verifyProofInternal was never triggered; the test passed because verification.duration count was 1 either way. Swap the cached value to ed25519Multikey (a valid Ed25519 Multikey for a different key) so `fetchKey()` returns it as `cached=true`, the Ed25519 algorithm check passes, and the "signature failed with cached key" recursive retry actually runs. Second, the step only checked verification.duration; the per-fetch behavior on the same path was unchecked. Add assertions for the two `activitypub.signature.key_fetch.duration` measurements the retry produces: `result=hit` for the stale cached attempt followed by `result=fetched` for the refetch, mirroring the LD cached-key retry test added earlier in this PR. https://github.com/fedify-dev/fedify/pull/769#discussion_r3250691365 Assisted-by: Claude Code:claude-opus-4-7 --- packages/fedify/src/sig/proof.test.ts | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/packages/fedify/src/sig/proof.test.ts b/packages/fedify/src/sig/proof.test.ts index 0fd792e83..d307c87f4 100644 --- a/packages/fedify/src/sig/proof.test.ts +++ b/packages/fedify/src/sig/proof.test.ts @@ -663,11 +663,13 @@ test("verifyProof() records verification duration metric", async (t) => { await t.step("cached-key retry emits one measurement, not two", async () => { const [meterProvider, recorder] = createTestMeterProvider(); const keyId = "https://server.example/users/alice#ed25519-key"; - // Prime the cache with a wrong-algorithm key (the rsaPublicKey2 is RSA, - // not Ed25519) so that verifyProofInternal falls through to the - // fresh-fetch retry path. + // Prime the cache with a different valid Ed25519 Multikey for the same + // keyId. fetchKey returns it as cached=true, the Ed25519 algorithm + // check passes, and verification fails because the key doesn't match + // the proof, so verifyProofInternal goes through its + // "signature failed with cached key" recursive retry. const cache: Record = { - [keyId]: rsaPublicKey2, + [keyId]: ed25519Multikey, }; const key = await verifyProof(jsonLd, proof, { documentLoader: mockDocumentLoader, @@ -690,6 +692,22 @@ test("verifyProof() records verification duration metric", async (t) => { ).length, 1, ); + // The retry path is observable as a per-fetch sequence on + // `activitypub.signature.key_fetch.duration`: a `hit` for the stale + // cached attempt, then a `fetched` for the fresh refetch. Mirrors the + // LD cached-key retry test. + const keyFetches = recorder.getMeasurements( + "activitypub.signature.key_fetch.duration", + ); + assertEquals(keyFetches.length, 2); + assertEquals( + keyFetches[0].attributes["activitypub.signature.key_fetch.result"], + "hit", + ); + assertEquals( + keyFetches[1].attributes["activitypub.signature.key_fetch.result"], + "fetched", + ); }); await t.step( From 1549d0beb3ad3f495e46714715371e64f131a47a Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sat, 16 May 2026 08:54:57 +0900 Subject: [PATCH 14/14] Reduce cast repetition in getLdSignatureObject Destructure `signature` from `jsonLd` once after the initial type guard, instead of repeating `jsonLd as { signature?: unknown }` for each of the three follow-up checks. Two early returns also replace the original chained conditions, which reads more naturally. Keep the return type as `Record | undefined`. The caller in `verifyJsonLd` reads `sig.creator`, `sig.signatureValue`, and `sig.type` as string properties; switching to `object | undefined` would push the cast burden into the caller at every property access, trading one local cast for three remote ones. https://github.com/fedify-dev/fedify/pull/769#discussion_r3250693884 Assisted-by: Claude Code:claude-opus-4-7 --- packages/fedify/src/sig/ld.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/fedify/src/sig/ld.ts b/packages/fedify/src/sig/ld.ts index 38723b8aa..8b671f2f5 100644 --- a/packages/fedify/src/sig/ld.ts +++ b/packages/fedify/src/sig/ld.ts @@ -463,14 +463,18 @@ function getLdSignatureObject( jsonLd: unknown, ): Record | undefined { if ( - typeof jsonLd === "object" && jsonLd != null && "signature" in jsonLd && - typeof (jsonLd as { signature?: unknown }).signature === "object" && - (jsonLd as { signature?: unknown }).signature != null && - !Array.isArray((jsonLd as { signature?: unknown }).signature) + typeof jsonLd !== "object" || jsonLd == null || !("signature" in jsonLd) ) { - return (jsonLd as { signature: Record }).signature; + return undefined; } - return undefined; + const { signature } = jsonLd as { signature: unknown }; + if ( + typeof signature !== "object" || signature == null || + Array.isArray(signature) + ) { + return undefined; + } + return signature as Record; } /**