Sign portable compound activities with one proof

CHANGES.md

@@ -10,6 +10,24 @@ To be released.
 
 ### @fedify/fedify
 
+ -  Changed `Context.sendActivity()` so that an activity containing
+    [FEP-ef61] portable objects gets at most one Object Integrity Proof.
+    Previously Fedify signed every outgoing activity once for each Ed25519
+    key, which produced a proof set that Fedify's own inbox rejects in a
+    compound portable document.  [[#288], [#1041], [#1045], [#1073]]
+
+     -  An activity that already carries a proof is sent as is.
+     -  With several Ed25519 keys, a portable activity is signed only by the
+        key whose ID is a DID URL for the activity's DID.
+     -  If no single key qualifies, or a non-portable activity that embeds
+        portable objects has several Ed25519 keys, `sendActivity()` rejects
+        with a `TypeError` before anything is delivered or queued.  Pass
+        explicit sender keys with exactly one Ed25519 key, or sign the
+        activity with `signObject()` beforehand.
+     -  An activity with portable objects in which any map carries a proof
+        set is rejected the same way.
+     -  Activities without portable objects are signed as before.
+
  -  Changed cached actor public keys and remembered per-origin HTTP Message
     Signatures specs to expire, so a `KvStore` that never sees an explicit
     clear no longer accumulates entries for actors and origins that have
@@ -158,8 +176,8 @@ To be released.
     traversal limits are rejected as unsupported.
     [[#288], [#938], [#1041]]
 
-[key–value store guide]: https://fedify.dev/manual/kv
 [FEP-ef61]: https://w3id.org/fep/ef61
+[key–value store guide]: https://fedify.dev/manual/kv
 [FEP-8b32]: https://w3id.org/fep/8b32
 [FEP-fe34]: https://w3id.org/fep/fe34
 [ActivityPub Media Upload extension]: https://www.w3.org/wiki/SocialCG/ActivityPub/MediaUpload
@@ -190,7 +208,9 @@ To be released.
 [#1027]: https://github.com/fedify-dev/fedify/pull/1027
 [#1041]: https://github.com/fedify-dev/fedify/pull/1041
 [#1044]: https://github.com/fedify-dev/fedify/issues/1044
+[#1045]: https://github.com/fedify-dev/fedify/issues/1045
 [#1051]: https://github.com/fedify-dev/fedify/pull/1051
+[#1073]: https://github.com/fedify-dev/fedify/pull/1073
 
 ### @fedify/adonisjs
 

changes.d/fedify/compound-proof-key-selection.md

@@ -0,0 +1,26 @@
+---
+links:
+  '#1041': https://github.com/fedify-dev/fedify/pull/1041
+  '#1045': https://github.com/fedify-dev/fedify/issues/1045
+  '#1073': https://github.com/fedify-dev/fedify/pull/1073
+  '#288': https://github.com/fedify-dev/fedify/issues/288
+---
+ -  Changed `Context.sendActivity()` so that an activity containing
+    [FEP-ef61] portable objects gets at most one Object Integrity Proof.
+    Previously Fedify signed every outgoing activity once for each Ed25519
+    key, which produced a proof set that Fedify's own inbox rejects in a
+    compound portable document.  [[#288], [#1041], [#1045], [#1073]]
+
+     -  An activity that already carries a proof is sent as is.
+     -  With several Ed25519 keys, a portable activity is signed only by the
+        key whose ID is a DID URL for the activity's DID.
+     -  If no single key qualifies, or a non-portable activity that embeds
+        portable objects has several Ed25519 keys, `sendActivity()` rejects
+        with a `TypeError` before anything is delivered or queued.  Pass
+        explicit sender keys with exactly one Ed25519 key, or sign the
+        activity with `signObject()` beforehand.
+     -  An activity with portable objects in which any map carries a proof
+        set is rejected the same way.
+     -  Activities without portable objects are signed as before.
+
+[FEP-ef61]: https://w3id.org/fep/ef61

docs/manual/send.md

@@ -1082,7 +1082,9 @@ Fedify automatically includes the integrity proof of activities by signing
 them with the sender's private key if the [actor keys dispatcher is
 set](./actor.md#public-keys-of-an-actor) and the actor has any Ed25519 key pair.
 If there are multiple key pairs, Fedify creates the number of integrity proofs
-equal to the number of Ed25519 key pairs.
+equal to the number of Ed25519 key pairs.  An activity containing [FEP-ef61]
+portable objects is the exception: it gets at most one proof, as described in
+[*Choosing the proof key*](#choosing-the-proof-key).
 
 When verifying incoming Object Integrity Proofs, Fedify can resolve Ed25519
 `did:key` verification methods locally.  A proof whose `verificationMethod`
@@ -1228,6 +1230,69 @@ the returned object afterwards, and `clone()` never carries it, because a
 clone may differ from the document the proof covers.  Sign the clone again
 when it has to be embedded as a secured child.
 
+#### Choosing the proof key
+
+Outside the compound profile, `sendActivity()` signs an activity once for each
+Ed25519 key it is given, which yields a proof set when there are several keys.
+Fedify inboxes reject a proof set in a document that contains a portable
+object, so an activity whose JSON contains a map identified by an `ap:` or
+`ap+ef61:` URI, whether the activity itself or anything embedded in it, gets at
+most one proof:
+
+ -  An activity that already carries a proof is sent as is.  Fedify does not
+    add another proof to it, not even with the keys from the [actor key pairs
+    dispatcher](./actor.md#public-keys-of-an-actor).
+ -  With a single Ed25519 key, that key signs the activity.
+ -  With several Ed25519 keys, a portable activity is signed only by the key
+    whose ID is a DID URL for the activity's own DID, such as
+    `did:key:z6Mk…#z6Mk…` for `ap://did:key:z6Mk…/activities/1`.  This is the
+    only proof [FEP-ef61] accepts for it.
+
+When no key or more than one key qualifies, or when a non-portable activity
+embeds portable objects and several Ed25519 keys are available,
+`sendActivity()` rejects with a `TypeError` instead of guessing.  Nothing is
+delivered or queued in that case.  To choose the key yourself, pass explicit
+sender keys that contain exactly one Ed25519 key, or sign the activity with
+`signObject()` before sending it.  RSA keys in the same list keep signing the
+HTTP request and the Linked Data Signature as usual:
+
+~~~~ typescript twoslash
+import type { Context } from "@fedify/fedify";
+import type { Create, Recipient } from "@fedify/vocab";
+const ctx = null as unknown as Context<void>;
+const rsaPrivateKey = null as unknown as CryptoKey;
+const ed25519PrivateKey = null as unknown as CryptoKey;
+const recipient = null as unknown as Recipient;
+const activity = null as unknown as Create;
+// ---cut-before---
+await ctx.sendActivity(
+  [
+    {
+      keyId: new URL("https://example.com/users/alice#main-key"),
+      privateKey: rsaPrivateKey,
+    },
+    {
+      keyId: new URL("did:key:z6Mkabc#z6Mkabc"),
+      privateKey: ed25519PrivateKey,
+    },
+  ],
+  recipient,
+  activity,
+);
+~~~~
+
+The Multikey IDs Fedify derives for the actor key pairs dispatcher are
+fragments of the actor URI, such as `…/actor#multikey-1`, not DID URLs.  With
+several dispatched Ed25519 keys, an unsigned portable activity sent by actor
+identifier is therefore always rejected; send it with explicit sender keys or
+pre-sign it instead.
+
+Fedify also refuses to send an activity with portable objects if any map in it
+already carries a proof set, which happens, for example, when `signObject()`
+is called twice on the same object.  The error names the JSON Pointer of the
+offending `proof`.  These checks only prevent unsupported proof shapes.  They
+do not otherwise validate a proof created with a single key.
+
 > [!WARNING]
 > Several things take a signed child outside this supported path, and each
 > one falls back to ordinary serialization, which rebuilds the child under the
@@ -1239,9 +1304,9 @@ when it has to be embedded as a secured child.
 >     [`forwardActivity()`](./outbox.md#federating-posted-activities) to avoid
 >     a vocabulary-object round trip.
 >  -  An object that already carried a proof.  The profile accepts exactly one
->     direct proof per map, while Fedify's ordinary activity signer creates one
->     proof for each Ed25519 key, so a sender producing a portable compound
->     document must arrange for exactly one direct proof on each map.
+>     direct proof per map, so sign each object with exactly one key.  Fedify
+>     refuses to send an activity whose portable content carries a proof set;
+>     see [*Choosing the proof key*](#choosing-the-proof-key).
 >  -  A `toJsonLd()` call whose `context` option could hide the marker Fedify
 >     uses to place the captured document, for example a context that aliases
 >     `@id` under a term other than `id`, declares `@nest`, uses an `@id` or

packages/fedify/src/federation/middleware.ts

@@ -78,7 +78,7 @@ import {
   wrapContextLoaderForJsonLd,
 } from "../sig/ld.ts";
 import { getKeyOwner, type GetKeyOwnerOptions } from "../sig/owner.ts";
-import { hasProofLike, signObject, verifyObject } from "../sig/proof.ts";
+import { hasProofLike, verifyObject } from "../sig/proof.ts";
 import { getAuthenticatedDocumentLoader } from "../utils/docloader.ts";
 import { kvCache } from "../utils/kv-cache.ts";
 import {
@@ -153,6 +153,10 @@ import {
 } from "./metrics.ts";
 import type { MessageQueue } from "./mq.ts";
 import { acceptsJsonLd } from "./negotiation.ts";
+import {
+  assertSupportedCompoundProofShape,
+  signOutgoingActivity,
+} from "./outgoing-proof.ts";
 import type {
   FanoutMessage,
   InboxMessage,
@@ -2339,8 +2343,6 @@ export class FederationImpl<TContextData>
       this.#getLoaderOptions(ctx.origin),
     );
     const activityId = activity.id.href;
-    let hasProof = false;
-    let proofCreated = false;
     let rsaKey: { keyId: URL; privateKey: CryptoKey } | null = null;
     for (const { keyId, privateKey } of keys) {
       validateCryptoKey(privateKey, "private");
@@ -2350,22 +2352,20 @@ export class FederationImpl<TContextData>
     }
     // If Object Integrity Proofs were already created before fanout (e.g., in
     // sendActivityInternal()), skip signing to avoid duplicates.
-    for await (const _ of activity.getProofs({ contextLoader })) {
-      hasProof = true;
-      break;
-    }
-    if (!hasProof) {
-      for (const { keyId, privateKey } of keys) {
-        if (privateKey.algorithm.name === "Ed25519") {
-          activity = await signObject(activity, privateKey, keyId, {
-            contextLoader,
-            tracerProvider: this.tracerProvider,
-          });
-          hasProof = true;
-          proofCreated = true;
-        }
-      }
-    }
+    const signed = await signOutgoingActivity(
+      activity,
+      keys.map(({ keyId, privateKey }) => ({
+        verificationMethod: keyId,
+        privateKey,
+      })),
+      {
+        contextLoader,
+        tracerProvider: this.tracerProvider,
+        appendToExistingProofs: false,
+      },
+    );
+    activity = signed.activity;
+    const { hasProof, proofCreated } = signed;
     let jsonLd = !proofCreated && options.activityJsonLd != null
       ? options.activityJsonLd
       : await activity.toJsonLd({
@@ -2381,6 +2381,7 @@ export class FederationImpl<TContextData>
         preserveNestedSecuredDocuments: true,
       });
     }
+    assertSupportedCompoundProofShape(jsonLd, activityId);
     if (rsaKey == null) {
       logger.warn(
         "No supported key found to create a Linked Data signature for " +
@@ -3919,48 +3920,41 @@ export class ContextImpl<TContextData> implements Context<TContextData> {
     // Pre-sign with Object Integrity Proofs before fanout so that all
     // recipients receive the same signed activity.  Uses Multikey IDs so that
     // verifiers can look up the correct key type in the actor document.
-    let proofCreated = false;
-    if (actorKeyPairs != null) {
-      const contextLoader = this.contextLoader;
-      for (const kp of actorKeyPairs) {
-        if (
-          kp.privateKey.algorithm.name !== "Ed25519" ||
-          kp.multikey.id == null
-        ) continue;
-        activity = await signObject(activity, kp.privateKey, kp.multikey.id, {
-          contextLoader,
+    //
+    // Explicit sender keys carry no Multikey, so they sign with the key ID
+    // the caller supplied, which is exactly what the delivery worker would do
+    // after reparsing the activity.  Signing here instead keeps a signed
+    // child's retained representation intact: the reparsed activity no
+    // longer carries one, so a worker-side proof would cover a rebuilt child
+    // whose own proof no longer verifies.
+    //
+    // An activity the caller already signed keeps its own proof when sent
+    // with explicit keys: appending another would turn a single-proof
+    // document into a proof set, which the map-local compound-proof profile
+    // does not accept.  This mirrors the guard `FederationImpl.sendActivity()`
+    // applies before signing.  Actor key pairs keep appending outside that
+    // profile, as they always have.
+    const { activity: signedActivity, proofCreated } =
+      await signOutgoingActivity(
+        activity,
+        actorKeyPairs == null
+          ? keys.map(({ keyId, privateKey }) => ({
+            verificationMethod: keyId,
+            privateKey,
+          }))
+          : actorKeyPairs.flatMap((kp) =>
+            kp.multikey.id == null ? [] : [{
+              verificationMethod: kp.multikey.id,
+              privateKey: kp.privateKey,
+            }]
+          ),
+        {
+          contextLoader: this.contextLoader,
           tracerProvider: this.tracerProvider,
-        });
-        proofCreated = true;
-      }
-    } else {
-      // Explicit sender keys carry no Multikey, so sign with the key ID the
-      // caller supplied, which is exactly what the delivery worker would do
-      // after reparsing the activity.  Signing here instead keeps a signed
-      // child's retained representation intact: the reparsed activity no
-      // longer carries one, so a worker-side proof would cover a rebuilt
-      // child whose own proof no longer verifies.
-      const contextLoader = this.contextLoader;
-      // An activity the caller already signed keeps its own proof: appending
-      // another would turn a single-proof document into a proof set, which
-      // the map-local compound-proof profile does not accept.  This mirrors
-      // the guard `FederationImpl.sendActivity()` applies before signing.
-      let hasProof = false;
-      for await (const _ of activity.getProofs({ contextLoader })) {
-        hasProof = true;
-        break;
-      }
-      if (!hasProof) {
-        for (const { keyId, privateKey } of keys) {
-          if (privateKey.algorithm.name !== "Ed25519") continue;
-          activity = await signObject(activity, privateKey, keyId, {
-            contextLoader,
-            tracerProvider: this.tracerProvider,
-          });
-          proofCreated = true;
-        }
-      }
-    }
+          appendToExistingProofs: actorKeyPairs != null,
+        },
+      );
+    activity = signedActivity;
     const inboxes = extractInboxes({
       recipients: expandedRecipients,
       preferSharedInbox: options.preferSharedInbox,
@@ -3990,6 +3984,13 @@ export class ContextImpl<TContextData> implements Context<TContextData> {
       });
       return true;
     }
+    const activityJsonLd = await activity.toJsonLd({
+      format: "compact",
+      contextLoader: this.contextLoader,
+    });
+    // Reject before anything is enqueued, so the caller learns about it
+    // instead of the fanout worker.
+    assertSupportedCompoundProofShape(activityJsonLd, activity.id?.href);
     const keyJwkPairs = await Promise.all(
       keys.map(async ({ keyId, privateKey }) => ({
         keyId: keyId.href,
@@ -4008,10 +4009,7 @@ export class ContextImpl<TContextData> implements Context<TContextData> {
           [k, { actorIds, sharedInbox }],
         ) => [k, { actorIds: [...actorIds], sharedInbox }]),
       ),
-      activity: await activity.toJsonLd({
-        format: "compact",
-        contextLoader: this.contextLoader,
-      }),
+      activity: activityJsonLd,
       activityId: activity.id?.href,
       activityType: getTypeId(activity).href,
       collectionSync: opts.collectionSync,