Reject relay re-handshake key changes (#1037)

Closes #366
Closes #368

packages/relay/src/encrypted-channel.test.ts

@@ -35,6 +35,10 @@ function createMockTransportPair(): [Transport, Transport] {
   return [transportA, transportB];
 }
 
+async function waitForAsyncDelivery(): Promise<void> {
+  await new Promise((resolve) => setTimeout(resolve, 50));
+}
+
 describe("EncryptedChannel", () => {
   it("establishes encrypted channel between daemon and client", async () => {
     const [daemonTransport, clientTransport] = createMockTransportPair();
@@ -96,7 +100,7 @@ describe("EncryptedChannel", () => {
     await clientChannel.send("Second message from client");
 
     // Wait for async delivery
-    await new Promise((r) => setTimeout(r, 50));
+    await waitForAsyncDelivery();
 
     expect(daemonMessages).toEqual(["Hello from client", "Second message from client"]);
     expect(clientMessages).toEqual(["Hello from daemon"]);
@@ -194,4 +198,75 @@ describe("EncryptedChannel", () => {
 
     await expect(daemonChannelPromise).rejects.toThrow("Invalid hello message");
   });
+
+  it("accepts duplicate hello from the same client without re-keying", async () => {
+    const [daemonTransport, clientTransport] = createMockTransportPair();
+
+    const daemonKeyPair = generateKeyPair();
+    const daemonPubKeyB64 = exportPublicKey(daemonKeyPair.publicKey);
+    const daemonMessages: (string | ArrayBuffer)[] = [];
+
+    let clientOpenedResolve: (() => void) | null = null;
+    const clientOpened = new Promise<void>((resolve) => {
+      clientOpenedResolve = resolve;
+    });
+
+    const daemonChannelPromise = createDaemonChannel(daemonTransport, daemonKeyPair, {
+      onmessage: (data) => daemonMessages.push(data),
+    });
+
+    const clientChannel = await createClientChannel(clientTransport, daemonPubKeyB64, {
+      onopen: () => clientOpenedResolve?.(),
+    });
+
+    await daemonChannelPromise;
+    await clientOpened;
+
+    const firstHello = (clientTransport.send as ReturnType<typeof vi.fn>).mock.calls.find(
+      ([data]) => typeof data === "string" && data.includes('"type":"e2ee_hello"'),
+    )?.[0];
+    expect(typeof firstHello).toBe("string");
+
+    daemonTransport.onmessage?.(firstHello as string);
+    await waitForAsyncDelivery();
+
+    expect(daemonTransport.close).not.toHaveBeenCalled();
+
+    await clientChannel.send("still encrypted with original key");
+    await waitForAsyncDelivery();
+
+    expect(daemonMessages).toEqual(["still encrypted with original key"]);
+  });
+
+  it("closes an open daemon channel when a different client key sends hello", async () => {
+    const [daemonTransport, clientTransport] = createMockTransportPair();
+
+    const daemonKeyPair = generateKeyPair();
+    const daemonPubKeyB64 = exportPublicKey(daemonKeyPair.publicKey);
+
+    let clientOpenedResolve: (() => void) | null = null;
+    const clientOpened = new Promise<void>((resolve) => {
+      clientOpenedResolve = resolve;
+    });
+
+    const daemonChannelPromise = createDaemonChannel(daemonTransport, daemonKeyPair);
+
+    await createClientChannel(clientTransport, daemonPubKeyB64, {
+      onopen: () => clientOpenedResolve?.(),
+    });
+
+    await daemonChannelPromise;
+    await clientOpened;
+
+    const attackerKeyPair = generateKeyPair();
+    const attackerHello = JSON.stringify({
+      type: "e2ee_hello",
+      key: exportPublicKey(attackerKeyPair.publicKey),
+    });
+
+    daemonTransport.onmessage?.(attackerHello);
+    await waitForAsyncDelivery();
+
+    expect(daemonTransport.close).toHaveBeenCalledWith(1008, "E2EE re-handshake key mismatch");
+  });
 });

packages/relay/src/encrypted-channel.ts

@@ -92,6 +92,8 @@ function buildInvalidHelloError(rawText: string, parsed?: unknown): Error {
 
 const HANDSHAKE_RETRY_MS = 1000;
 const MAX_PENDING_SENDS = 200;
+const REHANDSHAKE_KEY_MISMATCH_CLOSE_CODE = 1008;
+const REHANDSHAKE_KEY_MISMATCH_CLOSE_REASON = "E2EE re-handshake key mismatch";
 
 interface TimeoutWithUnref {
   unref(): void;
@@ -420,16 +422,14 @@ export class EncryptedChannel {
       return;
     }
 
-    // Different key implies a new client connection (common with relays
-    // where the daemon's socket stays open while the client reconnects).
-    // Re-key and re-send "ready". Drop any queued sends to avoid leaking
-    // messages between logical client sessions.
-    this.state = "handshaking";
-    this.sharedKey = nextSharedKey;
-    this.pendingSends = [];
-    this.transport.send(JSON.stringify({ type: "e2ee_ready" } satisfies E2EEReadyMessage));
-    this.state = "open";
-    await this.flushPendingSends();
+    // A different key on an already-open encrypted channel is not an
+    // authenticated reconnect. Close and require a fresh transport instead of
+    // allowing the relay to switch this channel to an attacker-chosen key.
+    this.state = "closed";
+    this.transport.close(
+      REHANDSHAKE_KEY_MISMATCH_CLOSE_CODE,
+      REHANDSHAKE_KEY_MISMATCH_CLOSE_REASON,
+    );
   }
 
   close(code = 1000, reason = "Normal closure"): void {
@@ -452,8 +452,9 @@ export class EncryptedChannel {
 
 function keysEqual(a: Uint8Array, b: Uint8Array): boolean {
   if (a.byteLength !== b.byteLength) return false;
+  let difference = 0;
   for (let i = 0; i < a.byteLength; i += 1) {
-    if (a[i] !== b[i]) return false;
+    difference |= a[i] ^ b[i];
   }
-  return true;
+  return difference === 0;
 }