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
livekit · Oct 20, 2023 · 3ed61a8 · 3ed61a8
1 parent bc4b46a
commit 3ed61a8
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 12 deletions.
diff --git a/.changeset/curvy-rules-worry.md 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
@@ -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(