fix(e2ee): pin the verified identity key on re-pin, rekey on keyed-peer leave

Multi-agent F3 security review surfaced two voice-E2EE defects:

- Re-pin TOCTOU (voice-E2EE MITM): the identity-mismatch modal showed a
  fingerprint from one membersStore read, but rePinPeerIdentity re-read the
  server-writable store to decide what to pin. A malicious server (F3's threat
  model) could swap in an attacker key via a user_update during the human
  out-of-band verification window and have it pinned, silently defeating the
  mismatch prompt. rePinPeerIdentity now takes the exact verified key as a
  parameter; ChannelSidebar passes the bytes whose fingerprint it displayed.

- Membership forward secrecy: the key holder rotated the room key only when the
  holder ROLE transferred, so a departed non-key-holder kept a valid room key
  until the next periodic (<=5 min) rotation. The holder now also rotates when a
  peer that held the key leaves (reusing rotateKeyPeriodically), gated on the
  leaver having actually held a key.

Client gates green: typecheck, lint (0 errors), prettier, vitest (3361).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Client/tauri-client/src/components/ChannelSidebar.ts

@@ -107,10 +107,17 @@ async function openIdentityMismatchModal(
     fingerprint,
     onAccept: () => {
       closeIdentityModal();
+      // Pin the EXACT key whose fingerprint we displayed and the user verified
+      // out-of-band (captured above), NOT a fresh membersStore re-read — a
+      // malicious server could mutate the store (user_update) during the human
+      // verification window and get its key pinned instead (TOCTOU). If the
+      // server had stripped the key, publishedKey is null and there was nothing
+      // to verify, so there is nothing to re-pin.
+      if (publishedKey === null) return;
       // Surface keyring/IO failures instead of dropping them — this re-pins a
       // trust anchor, so a silent failure would leave the user believing they
       // recovered when they did not.
-      void rePinPeerIdentity(userId).catch((err: unknown) => {
+      void rePinPeerIdentity(userId, publishedKey).catch((err: unknown) => {
         log.error("E2EE: failed to re-pin peer identity", err);
       });
     },

Client/tauri-client/src/lib/livekitSession.ts

@@ -1296,23 +1296,28 @@ export class LiveKitSession {
   }
 
   /**
-   * F3 TOFU re-pin recovery (finding #4). Accept the peer's CURRENT published
-   * identity key, overwriting the stored pin for {host,userId} and clearing the
-   * mismatch block — the identity-key analogue of accepting a changed TLS cert.
-   * A legitimate key rotation (reinstall / new device / wiped keyring) is thus
-   * recoverable instead of a permanent lockout; the next announce re-verifies
-   * against the new pin. Returns false when there is no host or no published key
-   * to pin. The voice-panel mismatch confirm should call this.
+   * F3 TOFU re-pin recovery (finding #4). Pin the EXACT identity key
+   * `verifiedKey` — the bytes whose fingerprint the caller displayed and the
+   * user confirmed out-of-band — overwriting the stored pin for {host,userId}
+   * and clearing the mismatch block (the identity-key analogue of accepting a
+   * changed TLS cert). A legitimate key rotation (reinstall / new device /
+   * wiped keyring) is thus recoverable instead of a permanent lockout; the next
+   * announce re-verifies against the new pin.
+   *
+   * The verified key MUST be passed in, never re-read from membersStore here:
+   * the store is server-writable (a `user_update` mutates it), so re-reading it
+   * would let a malicious server swap in an attacker key during the human
+   * out-of-band verification window and have us pin THAT — a TOCTOU that
+   * silently defeats the mismatch prompt. Returns false when there is no host
+   * or no key to pin.
    */
-  async rePinPeerIdentity(userId: number): Promise<boolean> {
+  async rePinPeerIdentity(userId: number, verifiedKey: string): Promise<boolean> {
     const host = this.serverHost;
-    const publishedIdentity =
-      membersStore.getState().members.get(userId)?.identityPublicKey ?? null;
-    if (!host || !publishedIdentity) {
-      log.warn("E2EE: cannot re-pin peer without a host and published identity key", { userId });
+    if (!host || !verifiedKey) {
+      log.warn("E2EE: cannot re-pin peer without a host and the verified identity key", { userId });
       return false;
     }
-    await storeIdentityPin(host, String(userId), publishedIdentity);
+    await storeIdentityPin(host, String(userId), verifiedKey);
     clearPeerVerification(userId);
     log.info("E2EE: re-pinned peer identity key (TOFU recovery)", { userId });
     return true;
@@ -1454,13 +1459,17 @@ export class LiveKitSession {
 
   /**
    * Handle a participant leaving the voice channel. If we become the new key
-   * holder, rotate the room key and distribute to remaining peers.
+   * holder, rotate the room key and distribute to remaining peers. If we are
+   * ALREADY the key holder and a peer that held the room key left, we also
+   * rotate — so the departed member's copy can no longer decrypt future audio
+   * against the untrusted SFU (membership forward secrecy).
    *
    * Key holder election: the participant with the lowest user ID among remaining
    * participants is elected. This is deterministic and does not depend on Map
    * insertion order (which is not guaranteed to match server join order).
    */
   async handleParticipantLeft(userId: number): Promise<void> {
+    const hadPeerKey = this._peerPublicKeys.has(userId);
     this._peerPublicKeys.delete(userId);
     clearPeerVerification(userId);
 
@@ -1543,6 +1552,14 @@ export class LiveKitSession {
         this._rotatingKey = false;
         this.startKeyRotationTimer();
       }
+    } else if (wasKeyHolder && hadPeerKey) {
+      // Membership forward secrecy: I remain the key holder and a peer that held
+      // the room key left, so rotate + redistribute to the CURRENT peer set
+      // (which already excludes the leaver, deleted above) — otherwise the
+      // departed member keeps a valid room key against the untrusted SFU until
+      // the next periodic rotation. rotateKeyPeriodically self-guards on
+      // _isKeyHolder / _rotatingKey.
+      await this.rotateKeyPeriodically();
     }
   }
 

Client/tauri-client/tests/unit/channel-sidebar.test.ts

@@ -1488,8 +1488,22 @@ describe("ChannelSidebar voice identity badge", () => {
     });
   });
 
-  it("re-pins the peer when the mismatch modal's Trust button is clicked", async () => {
+  it("re-pins the displayed key when the mismatch modal's Trust button is clicked", async () => {
     addVoiceUser(VOICE_CH, 10, "Alice");
+    // The peer must have a published key so its fingerprint is shown and there
+    // is a concrete verified key to re-pin.
+    membersStore.setState((prev) => {
+      const members = new Map(prev.members);
+      members.set(10, {
+        id: 10,
+        username: "Alice",
+        avatar: null,
+        role: "member",
+        status: "online",
+        identityPublicKey: "alice-published-key-b64",
+      });
+      return { ...prev, members };
+    });
     setPeerVerif(10, "mismatch", null);
     sidebar.mount(container);
 
@@ -1501,7 +1515,8 @@ describe("ChannelSidebar voice identity badge", () => {
     });
     trustBtn.click();
 
-    expect(mockRePinPeerIdentity).toHaveBeenCalledWith(10);
+    // Pins the exact key whose fingerprint was displayed, not a bare userId.
+    expect(mockRePinPeerIdentity).toHaveBeenCalledWith(10, "alice-published-key-b64");
     expect(document.body.querySelector(".modal-overlay")).toBeNull();
   });
 

Client/tauri-client/tests/unit/livekit-session.test.ts

@@ -2263,8 +2263,9 @@ describe("LiveKitSession", () => {
       );
 
       // User accepts the new key (analogous to accepting a changed TLS cert):
-      // re-pin overwrites the stored pin and clears the mismatch block.
-      const recovered = await session.rePinPeerIdentity(PEER_ID);
+      // re-pin overwrites the stored pin with the verified key and clears the
+      // mismatch block.
+      const recovered = await session.rePinPeerIdentity(PEER_ID, "new-identity-b64");
       expect(recovered).toBe(true);
       expect(storeIdentityPin).toHaveBeenCalledWith(HOST, String(PEER_ID), "new-identity-b64");
 
@@ -2281,6 +2282,52 @@ describe("LiveKitSession", () => {
       expect(offerSends(ws)).toHaveLength(1);
     });
 
+    it("re-pins the verified key, not a store re-read a malicious server mutated (TOCTOU)", async () => {
+      // The store holds whatever the server most recently pushed. If re-pin
+      // re-read the store it would pin the attacker's swapped-in key; it must
+      // instead pin the exact key it was handed — the one whose fingerprint the
+      // user verified out-of-band.
+      seedPeer("attacker-swapped-key-b64");
+      const ws = { send: vi.fn() };
+      await joinAsKeyHolder(ws);
+
+      const pinned = await session.rePinPeerIdentity(PEER_ID, "verified-key-b64");
+
+      expect(pinned).toBe(true);
+      expect(storeIdentityPin).toHaveBeenCalledWith(HOST, String(PEER_ID), "verified-key-b64");
+      expect(storeIdentityPin).not.toHaveBeenCalledWith(
+        HOST,
+        String(PEER_ID),
+        "attacker-swapped-key-b64",
+      );
+    });
+
+    it("rotates the room key when a keyed peer leaves while I stay key holder (forward secrecy)", async () => {
+      seedPeer("peer-identity-b64");
+      const ws = { send: vi.fn() };
+      await joinAsKeyHolder(ws); // I hold the key for channel 1
+      await session.handleE2EEAnnounce(PEER_ID, "cGVlcg==", "sig"); // peer now holds the room key
+      // A participant remains, so the leave handler proceeds past the empty check.
+      (mockVoiceState as any).voiceUsers = new Map([[1, new Map([[1, {}]])]]);
+      const epochBefore = (session as any)._e2eeEpoch;
+
+      await session.handleParticipantLeft(PEER_ID);
+
+      // Room key rotated (epoch advanced) so the departed peer's copy is dead.
+      expect((session as any)._e2eeEpoch).toBe(epochBefore + 1);
+    });
+
+    it("does not rotate the room key when the leaver never held it", async () => {
+      const ws = { send: vi.fn() };
+      await joinAsKeyHolder(ws);
+      (mockVoiceState as any).voiceUsers = new Map([[1, new Map([[1, {}]])]]);
+      const epochBefore = (session as any)._e2eeEpoch;
+
+      await session.handleParticipantLeft(999); // 999 never announced → held no key
+
+      expect((session as any)._e2eeEpoch).toBe(epochBefore);
+    });
+
     it("verifies a server-substituted key when drained from the pending queue", async () => {
       seedPeer("peer-identity-b64");
       (verifyEphemeralKeySignature as any).mockResolvedValue(false);