fix(e2ee): coalesce concurrent room-key rotations; refuse blind re-pin

Follow-up to the F3 re-pin/forward-secrecy fix, closing two residuals found by
adversarial re-review of the first fix:

- Concurrent-leave rotation drop (medium): rotateKeyPeriodically's _rotatingKey
  guard silently skipped a rotation already in flight, so a keyed peer that left
  mid-rotation kept a live room key until the next periodic (<=5 min) rotation.
  A coincident keyed-peer leave now DEFERS its rekey (_rotationPending) instead
  of dropping it; the completing rotation drains it via a shared
  drainPendingRotationOrArmTimer, excluding the departed member. Applied to both
  the become-holder and periodic rotation paths; reset in clearE2EEState.

- Blind re-pin (info, defense-in-depth): the mismatch modal's Trust action pinned
  publishedKey even when its fingerprint could not be computed (nothing shown to
  verify). onAccept now refuses to pin when fingerprint is null.

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

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
J3vb
2026-07-24 09:11:08 +02:00
co-authored by Claude Opus 4.8
parent ef1aca8a67
commit 8b8632774e
4 changed files with 106 additions and 9 deletions
@@ -110,10 +110,13 @@ async function openIdentityMismatchModal(
// 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;
// verification window and get its key pinned instead (TOCTOU).
//
// Only pin a key whose fingerprint was actually SHOWN: publishedKey null
// means the server stripped the key, and fingerprint null means it could
// not be computed (malformed key). In both cases the user saw nothing to
// verify, so pinning would be a blind accept — refuse it.
if (publishedKey === null || fingerprint === 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.
+33 -5
View File
@@ -169,6 +169,10 @@ export class LiveKitSession {
private _roomKeyRejector: ((err: Error) => void) | null = null;
/** Guard: true while a key rotation is in progress (prevents concurrent rotations). */
private _rotatingKey = false;
/** Set when a keyed-peer leave coincides with an in-flight rotation: the rekey
* is deferred (not dropped) and re-run when the current rotation finishes, so
* a member that left mid-rotation is excluded from the fresh room key. */
private _rotationPending = false;
/** Monotonic counter incremented on every key rotation. handleE2EEOffer captures the
* epoch before async work and discards the result if epoch changed (stale offer). */
private _e2eeEpoch = 0;
@@ -1550,16 +1554,26 @@ export class LiveKitSession {
log.error("E2EE: failed to rotate room key", err);
} finally {
this._rotatingKey = false;
this.startKeyRotationTimer();
}
// If a keyed peer left while this become-holder rotation was in flight, its
// rekey was deferred (not dropped) — run it now so the departed member is
// excluded from the fresh key; otherwise re-arm the periodic timer.
await this.drainPendingRotationOrArmTimer();
} 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();
// the next periodic rotation.
if (this._rotatingKey) {
// A rotation is already in flight and may already have sent the current
// key to this leaver before they left. Don't DROP the rekey (that would
// leave the departed member holding a live key) — defer it so it re-runs
// when the in-flight rotation completes, excluding them.
this._rotationPending = true;
} else {
await this.rotateKeyPeriodically();
}
}
}
@@ -1621,7 +1635,20 @@ export class LiveKitSession {
this._rotatingKey = false;
}
// Re-arm the timer for the next rotation.
// Re-arm the periodic timer, or run a rotation deferred by a keyed-peer leave
// that coincided with this one.
await this.drainPendingRotationOrArmTimer();
}
/** After a rotation completes: if a keyed-peer leave coincided with it (its
* rekey was deferred, not dropped), run one more rotation to exclude the
* departed member; otherwise re-arm the periodic rotation timer. */
private async drainPendingRotationOrArmTimer(): Promise<void> {
if (this._rotationPending) {
this._rotationPending = false;
await this.rotateKeyPeriodically();
return;
}
this.startKeyRotationTimer();
}
@@ -1635,6 +1662,7 @@ export class LiveKitSession {
clearPeerVerifications();
this._isKeyHolder = false;
this._rotatingKey = false;
this._rotationPending = false;
this._e2eeEpoch = 0;
this._pendingAnnounces.length = 0;
this.clearKeyRotationTimer();
@@ -39,6 +39,7 @@ import { voiceStore, updateVoiceState } from "../../src/stores/voice.store";
import type { PeerVerification } from "../../src/stores/voice.store";
import { membersStore } from "../../src/stores/members.store";
import type { ReadyChannel } from "../../src/lib/types";
import { computeKeyFingerprint } from "@lib/e2eeCrypto";
function resetStores(): void {
channelsStore.setState(() => ({
@@ -1520,6 +1521,38 @@ describe("ChannelSidebar voice identity badge", () => {
expect(document.body.querySelector(".modal-overlay")).toBeNull();
});
it("does not re-pin when the fingerprint could not be computed (no blind accept)", async () => {
addVoiceUser(VOICE_CH, 10, "Alice");
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);
// The changed key's fingerprint cannot be computed → the modal shows no
// fingerprint, so Trust must not pin a key the user never got to verify.
(computeKeyFingerprint as any).mockRejectedValueOnce(new Error("bad key"));
sidebar.mount(container);
(badgeFor(10) as HTMLElement).click();
const trustBtn = await vi.waitFor(() => {
const btn = document.body.querySelector(".modal-overlay .btn-danger") as HTMLButtonElement;
expect(btn).not.toBeNull();
return btn;
});
trustBtn.click();
expect(mockRePinPeerIdentity).not.toHaveBeenCalled();
expect(document.body.querySelector(".modal-overlay")).toBeNull();
});
it("closes an open mismatch modal on sidebar destroy", async () => {
addVoiceUser(VOICE_CH, 10, "Alice");
setPeerVerif(10, "mismatch", null);
@@ -2328,6 +2328,39 @@ describe("LiveKitSession", () => {
expect((session as any)._e2eeEpoch).toBe(epochBefore);
});
it("defers a keyed-peer leave rotation instead of dropping it when one is in flight", async () => {
seedPeer("peer-identity-b64");
const ws = { send: vi.fn() };
await joinAsKeyHolder(ws);
await session.handleE2EEAnnounce(PEER_ID, "cGVlcg==", "sig"); // peer holds the room key
(mockVoiceState as any).voiceUsers = new Map([[1, new Map([[1, {}]])]]);
// A rotation is already in flight (e.g. an earlier peer's leave).
(session as any)._rotatingKey = true;
const epochBefore = (session as any)._e2eeEpoch;
await session.handleParticipantLeft(PEER_ID);
// Not dropped by the _rotatingKey guard: the rekey is queued and no second
// rotation ran underneath the in-flight one.
expect((session as any)._rotationPending).toBe(true);
expect((session as any)._e2eeEpoch).toBe(epochBefore);
});
it("runs the deferred rotation once the in-flight one completes", async () => {
const ws = { send: vi.fn() };
await joinAsKeyHolder(ws);
(mockVoiceState as any).voiceUsers = new Map([[1, new Map([[1, {}]])]]);
// A keyed-peer leave was deferred while a rotation was in flight.
(session as any)._rotationPending = true;
const epochBefore = (session as any)._e2eeEpoch;
// The completing rotation must drain the pending one (excludes the leaver).
await (session as any).rotateKeyPeriodically();
expect((session as any)._e2eeEpoch).toBe(epochBefore + 2);
expect((session as any)._rotationPending).toBe(false);
});
it("verifies a server-substituted key when drained from the pending queue", async () => {
seedPeer("peer-identity-b64");
(verifyEphemeralKeySignature as any).mockResolvedValue(false);