From 3ed61a8be67e69662001ef710cc5e6bed1f3e661 Mon Sep 17 00:00:00 2001
From: lukasIO <mail@lukasseiler.de>
Date: Fri, 20 Oct 2023 20:25:05 +0200
Subject: [PATCH] Guard against overriding newly set key when auto-ratcheting
 (#895)

* Guard against overriding newly set key when auto-ratcheting

* Create curvy-rules-worry.md

* Also guard before resetting to initial material

* Remove unnecessary key reset and check against initialMaterial when available
---
 .changeset/curvy-rules-worry.md |  5 +++++
 src/e2ee/worker/FrameCryptor.ts | 23 +++++++++++------------
 2 files changed, 16 insertions(+), 12 deletions(-)
 create mode 100644 .changeset/curvy-rules-worry.md

diff --git a/.changeset/curvy-rules-worry.md b/.changeset/curvy-rules-worry.md
new file mode 100644
index 0000000000..235e5db553
--- /dev/null
+++ b/.changeset/curvy-rules-worry.md
@@ -0,0 +1,5 @@
+---
+"livekit-client": patch
+---
+
+Guard against overriding newly set key when auto-ratcheting
diff --git a/src/e2ee/worker/FrameCryptor.ts b/src/e2ee/worker/FrameCryptor.ts
index 0c83837c92..78c143236c 100644
--- a/src/e2ee/worker/FrameCryptor.ts
+++ b/src/e2ee/worker/FrameCryptor.ts
@@ -418,7 +418,7 @@ export class FrameCryptor extends BaseFrameCryptor {
           );
 
           let ratchetedKeySet: KeySet | undefined;
-          if (keySet === this.keys.getKeySet(keyIndex)) {
+          if ((initialMaterial ?? keySet) === this.keys.getKeySet(keyIndex)) {
             // only ratchet if the currently set key is still the same as the one used to decrypt this frame
             // if not, it might be that a different frame has already ratcheted and we try with that one first
             const newMaterial = await this.keys.ratchetKey(keyIndex, false);
@@ -431,22 +431,21 @@ export class FrameCryptor extends BaseFrameCryptor {
             encryptionKey: ratchetedKeySet?.encryptionKey,
           });
           if (frame && ratchetedKeySet) {
-            this.keys.setKeySet(ratchetedKeySet, keyIndex, true);
-            // decryption was successful, set the new key index to reflect the ratcheted key set
-            this.keys.setCurrentKeyIndex(keyIndex);
+            // before updating the keys, make sure that the keySet used for this frame is still the same as the currently set key
+            // if it's not, a new key might have been set already, which we don't want to override
+            if ((initialMaterial ?? keySet) === this.keys.getKeySet(keyIndex)) {
+              this.keys.setKeySet(ratchetedKeySet, keyIndex, true);
+              // decryption was successful, set the new key index to reflect the ratcheted key set
+              this.keys.setCurrentKeyIndex(keyIndex);
+            }
           }
           return frame;
         } else {
           /**
-           * Since the key it is first send and only afterwards actually used for encrypting, there were
-           * situations when the decrypting failed due to the fact that the received frame was not encrypted
-           * yet and ratcheting, of course, did not solve the problem. So if we fail RATCHET_WINDOW_SIZE times,
-           * we come back to the initial key.
+           * Because we only set a new key once decryption has been successful,
+           * we can be sure that we don't need to reset the key to the initial material at this point
+           * as the key has not been updated on the keyHandler instance
            */
-          if (initialMaterial) {
-            workerLogger.debug('resetting to initial material');
-            this.keys.setKeyFromMaterial(initialMaterial.material, keyIndex);
-          }
 
           workerLogger.warn('maximum ratchet attempts exceeded');
           throw new CryptorError(