From 005a3a27211d046a4dc2bd8874c1a3781449eb76 Mon Sep 17 00:00:00 2001
From: Jesse Wierzbinski <contact@cpluspatch.com>
Date: Sun, 24 Nov 2024 23:01:47 +0100
Subject: [PATCH] fix(federation): :ambulance: Don't always try to use instance
 key when an instance is not the request signer

---
 classes/inbox/processor.test.ts |  5 ++++-
 classes/inbox/processor.ts      | 26 ++++++++++++--------------
 worker.ts                       | 13 ++++++++++++-
 3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/classes/inbox/processor.test.ts b/classes/inbox/processor.test.ts
index 7ec2b202..e708ce0d 100644
--- a/classes/inbox/processor.test.ts
+++ b/classes/inbox/processor.test.ts
@@ -123,7 +123,10 @@ describe("InboxProcessor", () => {
         processor = new InboxProcessor(
             mockRequest,
             mockBody,
-            mockSenderInstance,
+            {
+                instance: mockSenderInstance,
+                key: "test-key",
+            },
             mockHeaders,
             undefined,
             {
diff --git a/classes/inbox/processor.ts b/classes/inbox/processor.ts
index 97f1786d..78cb00e3 100644
--- a/classes/inbox/processor.ts
+++ b/classes/inbox/processor.ts
@@ -70,7 +70,7 @@ export class InboxProcessor {
      *
      * @param request Request object.
      * @param body Entity JSON body.
-     * @param senderInstance Sender of the request's instance (from X-Signed-By header). Null if request is from a bridge.
+     * @param sender Sender of the request's instance and key (from X-Signed-By header). Null if request is from a bridge.
      * @param headers Various request headers.
      * @param logger LogTape logger instance.
      * @param requestIp Request IP address. Grabs it from the Hono context if not provided.
@@ -82,7 +82,10 @@ export class InboxProcessor {
             body: string;
         },
         private body: Entity,
-        private senderInstance: Instance | null,
+        private sender: {
+            instance: Instance;
+            key: string;
+        } | null,
         private headers: {
             signature?: string;
             nonce?: string;
@@ -98,20 +101,18 @@ export class InboxProcessor {
      * @returns {Promise<boolean>} - Whether the signature is valid.
      */
     private async isSignatureValid(): Promise<boolean> {
-        if (!this.senderInstance?.data.publicKey?.key) {
-            throw new Error(
-                `Instance ${this.senderInstance?.data.baseUrl} has no public key stored in database`,
-            );
+        if (!this.sender) {
+            throw new Error("Sender is not defined");
         }
 
         if (config.debug.federation) {
             this.logger.debug`Sender public key: ${chalk.gray(
-                this.senderInstance.data.publicKey.key,
+                this.sender.key,
             )}`;
         }
 
         const validator = await SignatureValidator.fromStringKey(
-            this.senderInstance.data.publicKey.key,
+            this.sender.key,
         );
 
         if (!(this.headers.signature && this.headers.nonce)) {
@@ -201,13 +202,10 @@ export class InboxProcessor {
      * @returns {Promise<Response>} - HTTP response to send back.
      */
     public async process(): Promise<Response> {
-        !this.senderInstance &&
+        !this.sender &&
             this.logger.debug`Processing request from potential bridge`;
 
-        if (
-            this.senderInstance &&
-            isDefederated(this.senderInstance.data.baseUrl)
-        ) {
+        if (this.sender && isDefederated(this.sender.instance.data.baseUrl)) {
             // Return 201 to avoid
             // 1. Leaking defederated instance information
             // 2. Preventing the sender from thinking the message was not delivered and retrying
@@ -217,7 +215,7 @@ export class InboxProcessor {
         }
 
         this.logger.debug`Instance ${chalk.gray(
-            this.senderInstance?.data.baseUrl,
+            this.sender?.instance.data.baseUrl,
         )} is not defederated`;
 
         const shouldCheckSignature = this.shouldCheckSignature();
diff --git a/worker.ts b/worker.ts
index fc31d0ec..db4d2ccc 100644
--- a/worker.ts
+++ b/worker.ts
@@ -177,10 +177,21 @@ export const inboxWorker = new Worker<InboxJobData, Response, InboxJobType>(
                     remoteInstance.data.baseUrl,
                 )}`;
 
+                if (!remoteInstance.data.publicKey?.key) {
+                    throw new Error(
+                        `Instance ${remoteInstance.data.baseUrl} has no public key stored in database`,
+                    );
+                }
+
                 const processor = new InboxProcessor(
                     request,
                     data,
-                    remoteInstance,
+                    {
+                        instance: remoteInstance,
+                        key:
+                            sender?.data.publicKey ??
+                            remoteInstance.data.publicKey.key,
+                    },
                     {
                         signature,
                         nonce,