diff --git a/CHANGES.md b/CHANGES.md index 6eae6a6a6..8945574fb 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,11 @@ To be released. ### @fedify/fedify + - Changed compound portable-object verification to check each map's proof + policy before resolving its key or verifying its signature, avoiding work + for maps that will be rejected. Policy-rejected maps now appear in + `onRequestFinished` verification reports with their policy failure and no + signature checks. [[#1177], [#1264]] - Fixed log entries emitted during an individual outbound activity delivery or inbound activity processing carrying the enclosing HTTP request's or queue worker's `traceId`/`spanId` instead of the delivery or processing @@ -21,7 +26,9 @@ To be released. [[#1030], [#1205] by Yujin Jung\] [#1030]: https://github.com/fedify-dev/fedify/issues/1030 +[#1177]: https://github.com/fedify-dev/fedify/issues/1177 [#1205]: https://github.com/fedify-dev/fedify/pull/1205 +[#1264]: https://github.com/fedify-dev/fedify/pull/1264 ### @fedify/cli diff --git a/changes.d/fedify/portable-proof-policy-order.md b/changes.d/fedify/portable-proof-policy-order.md new file mode 100644 index 000000000..f052fa733 --- /dev/null +++ b/changes.d/fedify/portable-proof-policy-order.md @@ -0,0 +1,10 @@ +--- +links: + '#1177': https://github.com/fedify-dev/fedify/issues/1177 + '#1264': https://github.com/fedify-dev/fedify/pull/1264 +--- + - Changed compound portable-object verification to check each map's proof + policy before resolving its key or verifying its signature, avoiding work + for maps that will be rejected. Policy-rejected maps now appear in + `onRequestFinished` verification reports with their policy failure and no + signature checks. [[#1177], [#1264]] diff --git a/packages/fedify/src/sig/compound-proof-verification.test.ts b/packages/fedify/src/sig/compound-proof-verification.test.ts index eebd65d28..a5aa27cd1 100644 --- a/packages/fedify/src/sig/compound-proof-verification.test.ts +++ b/packages/fedify/src/sig/compound-proof-verification.test.ts @@ -1179,3 +1179,208 @@ test("compound proof observation does not throw for malformed subject IDs", asyn assertEquals(evidence.attempts[0].subject.id, null); assertEquals(evidence.attempts[0].subject.pointer, ""); }); + +test("compound portable policy rejects actor gateways before key work", async () => { + const { verificationObservation } = await import("./verification.ts"); + for ( + const gateways of [undefined, [], ["https://gw.example/path"], [ + "https://gw.example", + ]] + ) { + const inner = await secureRawDocument( + { + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/data-integrity/v1", + ], + id: vector.documents.innerUnsecuredDocument.id, + type: "Person", + inbox: "https://inner.example/inbox", + outbox: "https://inner.example/outbox", + ...(gateways === undefined ? {} : { gateways }), + }, + vector.keys.inner.testPrivateKeyJwk, + vector.keys.inner.verificationMethod, + ); + const outer = await secureRawDocument( + { + ...vector.documents.outerUnsecuredDocument, + object: inner, + }, + vector.keys.outer.testPrivateKeyJwk, + vector.keys.outer.verificationMethod, + ); + const reads: string[] = []; + const evidence = { + attempts: [] as import("./verification.ts").InboxVerificationAttempt[], + }; + const result = await verifyCompoundPortableObjectProofs(outer, limits, { + ...options, + keyCache: { + get(id) { + reads.push(id.href); + return Promise.resolve(undefined); + }, + async set() {}, + }, + [verificationObservation]: evidence, + }); + assert(result.status === "ok"); + const valid = gateways?.[0] === "https://gw.example"; + assertEquals(result.verified, valid); + assertEquals( + reads.sort(), + (valid + ? [ + vector.keys.inner.verificationMethod, + vector.keys.outer.verificationMethod, + ] + : [vector.keys.outer.verificationMethod]).sort(), + ); + assertEquals(result.proofs.map((p) => [p.path, p.verified]), [[ + "/object", + valid, + ], ["", true]]); + assertEquals(result.snapshot, outer); + assert(Object.isFrozen(result.snapshot)); + assert(Object.isFrozen(result.proofs)); + assert(Object.isFrozen(result.portableObjects)); + const child = result.portableObjects[0]; + if (!valid) { + assert(!child.verified); + assertEquals(child.reason.type, "invalidGateways"); + assertEquals(result.proofs[0], { + path: "/object", + id: vector.documents.innerUnsecuredDocument.id, + depth: 1, + verified: false, + reason: { type: "invalidProof" }, + }); + const attempt = evidence.attempts.find((a) => + a.subject.pointer === "/object" + ); + assert(attempt?.status === "rejected"); + assertEquals(attempt.reason, { + type: "proofPolicy", + reason: { type: "invalidGateways" }, + }); + assertEquals(attempt.checks, []); + } + } +}); + +test("compound portable policy skips doomed proofs without pruning parents", async () => { + const context = createInlineProofContext(); + const scoped = createInlineProofContext(); + scoped.proof = { + "@id": "https://w3id.org/security#proof", + "@context": { assertionMethod: "https://w3id.org/security#authentication" }, + }; + const cases = [ + { + id: vector.documents.outerUnsecuredDocument.id, + type: "Note", + method: vector.keys.inner.verificationMethod, + context, + reason: "verificationMethodMismatch", + }, + { + id: vector.documents.innerUnsecuredDocument.id, + type: "Note", + method: "https://attacker.example/key", + context, + reason: "unsupportedVerificationMethod", + }, + { + id: "ap://did%ZZkey/objects/1", + type: "Note", + method: vector.keys.inner.verificationMethod, + context, + reason: "invalidPortableObject", + }, + { + id: vector.documents.innerUnsecuredDocument.id, + type: "Link", + method: vector.keys.inner.verificationMethod, + context: { + ...context, + Link: "https://www.w3.org/ns/activitystreams#Link", + href: { + "@id": "https://www.w3.org/ns/activitystreams#href", + "@type": "@id", + }, + }, + extra: { href: "https://example.com/resource" }, + reason: "unsupportedObjectType", + }, + { + id: vector.documents.innerUnsecuredDocument.id, + type: "Note", + method: vector.keys.inner.verificationMethod, + context: scoped, + reason: "invalidProof", + }, + { + id: vector.documents.innerUnsecuredDocument.id, + type: "Note", + method: vector.keys.inner.verificationMethod, + context, + extra: { "@id": "https://example.com/ordinary" }, + reason: "notPortableObject", + }, + ]; + for (const candidate of cases) { + const child = await secureRawDocument( + { + "@context": candidate.context, + id: candidate.id, + type: candidate.type, + ...candidate.extra, + content: "Rejected attachment", + }, + vector.keys.inner.testPrivateKeyJwk, + candidate.method, + ); + if (candidate.context === scoped) delete asRecord(child.proof)["@context"]; + // This ordinary HTTPS parent still has to verify its complete raw input. + const parent: Record = { + ...vector.documents.outerUnsecuredDocument, + id: "https://social.example/activities/1", + attachment: [child], + }; + delete parent.object; + const outer = await secureRawDocument( + parent, + vector.keys.outer.testPrivateKeyJwk, + vector.keys.outer.verificationMethod, + ); + const reads: string[] = []; + let loads = 0; + const result = await verifyCompoundPortableObjectProofs(outer, limits, { + ...options, + documentLoader() { + loads++; + throw new TypeError("unexpected key fetch"); + }, + keyCache: { + get(id) { + reads.push(id.href); + return Promise.resolve(undefined); + }, + async set() {}, + }, + }); + assert(result.status === "ok"); + assertEquals(result.verified, false); + assertEquals(reads, [vector.keys.outer.verificationMethod]); + assertEquals(loads, 0); + assertEquals(result.proofs.map((p) => [p.path, p.verified]), [[ + "/attachment/0", + false, + ], ["", true]]); + const [rejected] = result.portableObjects; + assert(!rejected.verified); + assertEquals(rejected.reason.type, candidate.reason); + assertEquals(rejected.path, "/attachment/0"); + } +}); diff --git a/packages/fedify/src/sig/compound-proof.ts b/packages/fedify/src/sig/compound-proof.ts index 8ea9f1f70..cadf16eba 100644 --- a/packages/fedify/src/sig/compound-proof.ts +++ b/packages/fedify/src/sig/compound-proof.ts @@ -11,10 +11,13 @@ import { } from "./portable-key-id.ts"; import { classifyFep2277CoreType, + type PreparedPortableObjectProofPolicy, + preparePortableObjectProofPolicy, verifyMapLocalProof, type VerifyPortableObjectProofFailureReason, type VerifyPortableObjectProofOptions, verifyPortableObjectProofPolicy, + verifyPreparedPortableObjectProofPolicy, type VerifyProofOptions, } from "./proof.ts"; @@ -99,7 +102,10 @@ export type CompoundProofVerificationFailureReason = readonly type: "missingContext"; } | { - /** The direct proof is malformed, unsupported, or invalid. */ + /** + * The direct proof is malformed, unsupported, invalid, or left unverified + * because the portable-object policy rejects its map. + */ readonly type: "invalidProof"; }; @@ -926,6 +932,8 @@ export async function verifyCompoundProofDocuments( async function verifyDiscoveredProofDocuments( discovered: Extract, options: VerifyProofOptions, + rejectedPolicies: ReadonlyMap = + new Map(), ): Promise { const documents = await Promise.all( discovered.documents.map(async (document) => { @@ -964,8 +972,15 @@ async function verifyDiscoveredProofDocuments( }, }, "objectIntegrity", - (observation) => - verifyMapLocalProof( + async (observation) => { + const reason = rejectedPolicies.get(document.path); + if (reason != null) { + if (observation.attempt != null) { + observation.attempt.reason = { type: "proofPolicy", reason }; + } + return null; + } + return await verifyMapLocalProof( document.securedDocument, { ...options, @@ -973,7 +988,8 @@ async function verifyDiscoveredProofDocuments( ? undefined : observation, }, - ), + ); + }, (key) => key != null, ); return key == null @@ -1002,33 +1018,71 @@ export async function verifyCompoundPortableObjectProofs( const discovered = discoverCompoundProofDocuments(json, limits); if (discovered.status !== "ok") return discovered; - const proofs = await verifyDiscoveredProofDocuments(discovered, options); - const proofByPath = new Map(proofs.map((proof) => [proof.path, proof])); - const verifyPortableObject = async ( - object: CompoundPortableObject, - ): Promise => { - const metadata = { - path: object.path, - id: object.id, - depth: object.depth, + const collected = collectPortableObjects(discovered.snapshot); + const objectByPath = new Map( + collected.map((object) => [object.path, object]), + ); + // Unsigned maps under the publicKey or assertionMethod of a portable map + // may be the keys of a portable actor, e.g., its gateway keys, whose IDs + // are compatible identifiers, or the keys of an FEP-ae97 client, whose IDs + // are ap: URIs. They are decided after their parents: + const isKeyCandidate = (object: CompoundPortableObject) => + object.keyOf != null && !Object.hasOwn(object.document, "proof"); + type PreparedPolicy = + | PreparedPortableObjectProofPolicy + | { + readonly prepared: false; + readonly reason: CompoundPortableObjectFailureReason; }; - if ( - object.depth > 0 && !Object.hasOwn(object.document, "@context") - ) { - return Object.freeze({ - ...metadata, - verified: false as const, - reason: { type: "missingContext" as const }, - }); + const prepareObject = async ( + object: CompoundPortableObject, + ): Promise => { + if (object.depth > 0 && !Object.hasOwn(object.document, "@context")) { + return { prepared: false, reason: { type: "missingContext" } }; } - const proof = proofByPath.get(object.path); - const key = proof?.verified === true ? proof.key : null; try { - const policy = await verifyPortableObjectProofPolicy( + const policy = await preparePortableObjectProofPolicy( object.document, - key, options, ); + return policy.prepared + ? policy + : { prepared: false, reason: policy.result.reason }; + } catch { + return { prepared: false, reason: { type: "invalidPortableObject" } }; + } + }; + // Prepare policy once per ordinary portable map before any key work. + const preparedByPath = new Map( + await Promise.all( + collected.filter((object) => !isKeyCandidate(object)).map( + async (object) => [object.path, await prepareObject(object)] as const, + ), + ), + ); + const rejectedPolicies = new Map( + [...preparedByPath].flatMap(([path, prepared]) => + prepared.prepared ? [] : [[path, prepared.reason] as const] + ), + ); + const proofs = await verifyDiscoveredProofDocuments( + discovered, + options, + rejectedPolicies, + ); + const proofByPath = new Map(proofs.map((proof) => [proof.path, proof])); + const verifyPortableObject = async ( + object: CompoundPortableObject, + ): Promise => { + const metadata = { path: object.path, id: object.id, depth: object.depth }; + try { + const prepared = preparedByPath.get(object.path) ?? + await prepareObject(object); + const proof = proofByPath.get(object.path); + const key = proof?.verified === true ? proof.key : null; + const policy = prepared.prepared + ? verifyPreparedPortableObjectProofPolicy(prepared, key) + : { verified: false as const, reason: prepared.reason }; return policy.verified ? Object.freeze({ ...metadata, @@ -1049,16 +1103,6 @@ export async function verifyCompoundPortableObjectProofs( }); } }; - const collected = collectPortableObjects(discovered.snapshot); - const objectByPath = new Map( - collected.map((object) => [object.path, object]), - ); - // Unsigned maps under the publicKey or assertionMethod of a portable map - // may be the keys of a portable actor, e.g., its gateway keys, whose IDs - // are compatible identifiers, or the keys of an FEP-ae97 client, whose IDs - // are ap: URIs. They are decided after their parents: - const isKeyCandidate = (object: CompoundPortableObject) => - object.keyOf != null && !Object.hasOwn(object.document, "proof"); const results = new Map(); for ( const result of await Promise.all( diff --git a/packages/fedify/src/sig/proof.ts b/packages/fedify/src/sig/proof.ts index 53d6466dc..4b22caeb2 100644 --- a/packages/fedify/src/sig/proof.ts +++ b/packages/fedify/src/sig/proof.ts @@ -2181,21 +2181,41 @@ async function preparePortableObjectProof( }; } +type PortableObjectProofPolicyResult = + | Extract + | { + readonly verified: true; + readonly keys: readonly Multikey[]; + readonly objectId: string; + }; + /** - * Applies portable-object proof policy to one map-local cryptographic result. + * Policy data for a map whose proof still needs cryptographic verification. * * @internal */ -export async function verifyPortableObjectProofPolicy( +export interface PreparedPortableObjectProofPolicy { + readonly prepared: true; + readonly verificationMethod: URL; + readonly objectId: string; +} + +/** + * Checks portable-object policy without resolving keys or verifying signatures. + * + * @internal + */ +export async function preparePortableObjectProofPolicy( jsonLd: unknown, - key: Multikey | null, options: VerifyPortableObjectProofOptions = {}, ): Promise< - | Extract + | PreparedPortableObjectProofPolicy | { - readonly verified: true; - readonly keys: readonly Multikey[]; - readonly objectId: string; + readonly prepared: false; + readonly result: Extract< + VerifyPortableObjectProofResult, + { verified: false } + >; } > { const policyOptions = { @@ -2203,7 +2223,7 @@ export async function verifyPortableObjectProofPolicy( contextLoader: preloadedOnlyDocumentLoader, }; const prepared = await preparePortableObjectProof(jsonLd, policyOptions); - if (!prepared.prepared) return prepared.result; + if (!prepared.prepared) return { prepared: false, result: prepared.result }; const policyProof = prepared.proofs[0]; const literalProof = isJsonLdNode(jsonLd) && isJsonLdNode(jsonLd.proof) ? (await parseRawProofCandidates( @@ -2215,26 +2235,62 @@ export async function verifyPortableObjectProofPolicy( : null; const verificationMethod = policyProof?.verificationMethodId; if ( - prepared.proofs.length !== 1 || key == null || + prepared.proofs.length !== 1 || policyProof == null || literalProof == null || - !sameProof(policyProof, literalProof) || verificationMethod == null || - key.id?.href !== verificationMethod.href + !sameProof(policyProof, literalProof) || verificationMethod == null ) { return { - verified: false, - reason: { - type: "invalidProof", - proofIndex: prepared.proofs.length > 1 ? 1 : 0, + prepared: false, + result: { + verified: false, + reason: { + type: "invalidProof", + proofIndex: prepared.proofs.length > 1 ? 1 : 0, + }, }, }; } return { - verified: true, - keys: [key], + prepared: true, + verificationMethod, objectId: formatIri(prepared.objectId), }; } +/** + * Binds prepared portable-object policy to the map-local verified key. + * + * @internal + */ +export function verifyPreparedPortableObjectProofPolicy( + prepared: PreparedPortableObjectProofPolicy, + key: Multikey | null, +): PortableObjectProofPolicyResult { + if (key == null || key.id?.href !== prepared.verificationMethod.href) { + return { + verified: false, + reason: { type: "invalidProof", proofIndex: 0 }, + }; + } + return { verified: true, keys: [key], objectId: prepared.objectId }; +} + +/** + * Applies portable-object proof policy to one map-local cryptographic result. + * + * @internal + */ +export async function verifyPortableObjectProofPolicy( + jsonLd: unknown, + key: Multikey | null, + options: VerifyPortableObjectProofOptions = {}, +): Promise { + const prepared = await preparePortableObjectProofPolicy(jsonLd, options); + return prepared.prepared + ? verifyPreparedPortableObjectProofPolicy(prepared, key) + : prepared.result; +} + /** * Verifies the FEP-ef61 Object Integrity Proof policy for a portable object. *