Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't allow Olm unwedging rate-limiting to race #3549

Merged
merged 2 commits into from
Jul 7, 2023
Merged

Don't allow Olm unwedging rate-limiting to race #3549

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a95aab3
don't allow Olm unwedging rate-limiting to race
uhoreg Jul 6, 2023
f55dcb2
apply changes from code review
uhoreg Jul 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions src/crypto/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,12 @@ export function isCryptoAvailable(): boolean {
return Boolean(global.Olm);
}

const MIN_FORCE_SESSION_INTERVAL_MS = 60 * 60 * 1000;
// 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; // 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; // 5 minutes

interface IInitOpts {
exportedOlmDevice?: IExportedDevice;
Expand Down Expand Up @@ -390,15 +395,15 @@ 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: {
// deviceId: 1234567890000,
// },
// }
// 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),
);

Expand Down Expand Up @@ -3591,25 +3596,23 @@ 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",
`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();
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 -
Expand All @@ -3630,7 +3633,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)
Expand Down