Reject standalone keys with mismatched IDs

CHANGES.md

@@ -8,6 +8,14 @@ Version 2.0.25
 
 To be released.
 
+### @fedify/fedify
+
+ -  Standalone key documents whose `id` differs from the requested key URL are
+    now rejected instead of being cached under the wrong URL. [[#963]] by
+    ojspp41
+
+[#963]: https://github.com/fedify-dev/fedify/issues/963
+
 
 Version 2.0.24
 --------------

changes.d/fedify/mismatched-key-id.md

@@ -0,0 +1,5 @@
+ -  Standalone key documents whose `id` differs from the requested key URL are
+    now rejected instead of being cached under the wrong URL. [[#963]] by
+    ojspp41
+
+[#963]: https://github.com/fedify-dev/fedify/issues/963

packages/fedify/src/sig/key.test.ts

@@ -352,6 +352,65 @@ test("fetchKey()", async () => {
   );
 });
 
+test("fetchKey() rejects standalone keys with a mismatched id", async () => {
+  for (
+    const { keyId, standaloneKey, fetch } of [
+      {
+        keyId: "https://example.com/key",
+        standaloneKey: rsaPublicKey1,
+        fetch: (keyId: string, options: FetchKeyOptions) =>
+          fetchKey(keyId, CryptographicKey, options),
+      },
+      {
+        keyId: "https://example.com/multikey",
+        standaloneKey: ed25519Multikey,
+        fetch: (keyId: string, options: FetchKeyOptions) =>
+          fetchKey(keyId, Multikey, options),
+      },
+    ]
+  ) {
+    const cache: Record<string, CryptographicKey | Multikey | null> = {};
+    const options: FetchKeyOptions = {
+      async documentLoader(resource) {
+        if (resource === keyId) {
+          const document = await standaloneKey.toJsonLd({
+            contextLoader: mockDocumentLoader,
+          });
+          return {
+            contextUrl: null,
+            documentUrl: resource,
+            document: {
+              ...document as Record<string, unknown>,
+              id: "https://example.com/different-key",
+            },
+          };
+        }
+        return await mockDocumentLoader(resource);
+      },
+      contextLoader: mockDocumentLoader,
+      keyCache: {
+        get(keyId) {
+          return Promise.resolve(cache[keyId.href]);
+        },
+        set(keyId, key) {
+          cache[keyId.href] = key;
+          return Promise.resolve();
+        },
+      } satisfies KeyCache,
+    };
+
+    assertEquals(await fetch(keyId, options), {
+      key: null,
+      cached: false,
+    });
+    assertEquals(cache, { [keyId]: null });
+    assertEquals(await fetch(keyId, options), {
+      key: null,
+      cached: true,
+    });
+  }
+});
+
 test("fetchKey() returns null for a malformed actor publicKey", async () => {
   const actorId = "https://example.com/malformed-public-key";
   const keyId = "https://example.com/malformed-public-key#main-key";

packages/fedify/src/sig/key.ts

@@ -323,7 +323,10 @@ async function fetchKeyInternal<T extends CryptographicKey | Multikey>(
     }
   }
   let key: T | null = null;
-  if (object instanceof cls) key = object;
+  if (
+    object instanceof cls &&
+    (object.id == null || object.id.href === keyId)
+  ) key = object;
   else if (isActor(object)) {
     // Treat malformed remote actor keys as missing keys.
     // @ts-ignore: cls is either CryptographicKey or Multikey