From a95aab3ff0247a621833b90cb36f19a0c7415e4f Mon Sep 17 00:00:00 2001
From: Hubert Chathi <hubertc@matrix.org>
Date: Thu, 6 Jul 2023 09:09:07 -0400
Subject: [PATCH 1/2] don't allow Olm unwedging rate-limiting to race

---
 src/crypto/index.ts | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/crypto/index.ts b/src/crypto/index.ts
index ff3410dea07..ae7400f1406 100644
--- a/src/crypto/index.ts
+++ b/src/crypto/index.ts
@@ -130,7 +130,12 @@ export function isCryptoAvailable(): boolean {
     return Boolean(global.Olm);
 }
 
+// minimum time between attempting to unwedge an Olm session, if we succeeded
+// in creating a new session
 const MIN_FORCE_SESSION_INTERVAL_MS = 60 * 60 * 1000;
+// minimum time between attempting to unwedge an Olm session, if we failed
+// to create a new session
+const FORCE_SESSION_RETRY_INTERVAL_MS = 5 * 60 * 1000;
 
 interface IInitOpts {
     exportedOlmDevice?: IExportedDevice;
@@ -390,7 +395,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
     // to avoid loading room members as long as possible.
     private roomDeviceTrackingState: { [roomId: string]: Promise<void> } = {};
 
-    // The timestamp of the last time we forced establishment
+    // The timestamp of the minimum time at which we will retry forcing establishment
     // of a new session for each device, in milliseconds.
     // {
     //     userId: {
@@ -398,7 +403,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
     //     },
     // }
     // Map: user Id → device Id → timestamp
-    private lastNewSessionForced: MapWithDefault<string, MapWithDefault<string, number>> = new MapWithDefault(
+    private forceNewSessionRetryTime: MapWithDefault<string, MapWithDefault<string, number>> = new MapWithDefault(
         () => new MapWithDefault(() => 0),
     );
 
@@ -3591,25 +3596,27 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
             return;
         }
 
-        // check when we last forced a new session with this device: if we've already done so
+        // check when we can force a new session with this device: if we've already done so
         // recently, don't do it again.
-        const lastNewSessionDevices = this.lastNewSessionForced.getOrCreate(sender);
-        const lastNewSessionForced = lastNewSessionDevices.getOrCreate(deviceKey);
-        if (lastNewSessionForced + MIN_FORCE_SESSION_INTERVAL_MS > Date.now()) {
+        const forceNewSessionRetryTimeDevices = this.forceNewSessionRetryTime.getOrCreate(sender);
+        const forceNewSessionRetryTime = forceNewSessionRetryTimeDevices.getOrCreate(deviceKey);
+        if (forceNewSessionRetryTime > Date.now()) {
             logger.debug(
                 "New session already forced with device " +
                     sender +
                     ":" +
                     deviceKey +
-                    " at " +
-                    lastNewSessionForced +
-                    ": not forcing another",
+                    ": not forcing another until at least ",
+                    forceNewSessionRetryTime
             );
             await this.olmDevice.recordSessionProblem(deviceKey, "wedged", true);
             retryDecryption();
             return;
         }
 
+        // make sure we don't retry to unwedge too soon even if we fail to create a new session
+        forceNewSessionRetryTimeDevices.set(deviceKey, Date.now() + FORCE_SESSION_RETRY_INTERVAL_MS);
+
         // establish a new olm session with this device since we're failing to decrypt messages
         // on a current session.
         // Note that an undecryptable message from another device could easily be spoofed -
@@ -3630,7 +3637,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
         const devicesByUser = new Map([[sender, [device]]]);
         await olmlib.ensureOlmSessionsForDevices(this.olmDevice, this.baseApis, devicesByUser, true);
 
-        lastNewSessionDevices.set(deviceKey, Date.now());
+        forceNewSessionRetryTimeDevices.set(deviceKey, Date.now() + MIN_FORCE_SESSION_INTERVAL_MS);
 
         // Now send a blank message on that session so the other side knows about it.
         // (The keyshare request is sent in the clear so that won't do)

From f55dcb20c89928fdd0c229cce2b439fafd0f9b22 Mon Sep 17 00:00:00 2001
From: Hubert Chathi <hubertc@matrix.org>
Date: Thu, 6 Jul 2023 13:25:33 -0400
Subject: [PATCH 2/2] apply changes from code review

---
 src/crypto/index.ts | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/crypto/index.ts b/src/crypto/index.ts
index ae7400f1406..44ea32ed983 100644
--- a/src/crypto/index.ts
+++ b/src/crypto/index.ts
@@ -132,10 +132,10 @@ export function isCryptoAvailable(): boolean {
 
 // minimum time between attempting to unwedge an Olm session, if we succeeded
 // in creating a new session
-const MIN_FORCE_SESSION_INTERVAL_MS = 60 * 60 * 1000;
+const MIN_FORCE_SESSION_INTERVAL_MS = 60 * 60 * 1000; // 1 hour
 // minimum time between attempting to unwedge an Olm session, if we failed
 // to create a new session
-const FORCE_SESSION_RETRY_INTERVAL_MS = 5 * 60 * 1000;
+const FORCE_SESSION_RETRY_INTERVAL_MS = 5 * 60 * 1000; // 5 minutes
 
 interface IInitOpts {
     exportedOlmDevice?: IExportedDevice;
@@ -3602,12 +3602,8 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
         const forceNewSessionRetryTime = forceNewSessionRetryTimeDevices.getOrCreate(deviceKey);
         if (forceNewSessionRetryTime > Date.now()) {
             logger.debug(
-                "New session already forced with device " +
-                    sender +
-                    ":" +
-                    deviceKey +
-                    ": not forcing another until at least ",
-                    forceNewSessionRetryTime
+                `New session already forced with device ${sender}:${deviceKey}: ` +
+                    `not forcing another until at least ${new Date(forceNewSessionRetryTime).toUTCString()}`,
             );
             await this.olmDevice.recordSessionProblem(deviceKey, "wedged", true);
             retryDecryption();