From 93982716951ce2583904bfc26b27d6a86ba17a87 Mon Sep 17 00:00:00 2001 From: Timo <16718859+toger5@users.noreply.github.com> Date: Thu, 22 May 2025 15:43:47 +0200 Subject: [PATCH] Check for `unknown variant` on to-device sending and fall back to room event encryption. (#4847) * Check for `unknown variant` on to-device sending and fallback to room event encryption. * fix tests * fix error js-sdk api type * Change logger from debug to warn for unsupported to-device transport and improve error message comments * also add case for not supported This will be send by the driver in case we sent an encrypted to-device but do not have support of that. --------- Co-authored-by: Robin <robin@robin.town> --- .../RoomAndToDeviceTransport.spec.ts | 33 +++++++++++++++++++ .../matrixrtc/ToDeviceKeyTransport.spec.ts | 2 +- src/matrixrtc/IKeyTransport.ts | 2 ++ src/matrixrtc/RoomAndToDeviceKeyTransport.ts | 16 +++++++-- src/matrixrtc/ToDeviceKeyTransport.ts | 26 ++++++++++++++- 5 files changed, 75 insertions(+), 4 deletions(-) diff --git a/spec/unit/matrixrtc/RoomAndToDeviceTransport.spec.ts b/spec/unit/matrixrtc/RoomAndToDeviceTransport.spec.ts index a4ce40aa6..1a5e4e22f 100644 --- a/spec/unit/matrixrtc/RoomAndToDeviceTransport.spec.ts +++ b/spec/unit/matrixrtc/RoomAndToDeviceTransport.spec.ts @@ -124,6 +124,39 @@ describe("RoomAndToDeviceTransport", () => { expect(toDeviceSendKeySpy).toHaveBeenCalledTimes(0); expect(onTransportEnabled).toHaveBeenCalledWith({ toDevice: false, room: true }); }); + + it("enables room transport and disables to device transport on widget driver error", async () => { + mockClient.encryptAndSendToDevice.mockRejectedValue({ + message: + "unknown variant `send_to_device`, expected one of `supported_api_versions`, `content_loaded`, `get_openid`, `org.matrix.msc2876.read_events`, `send_event`, `org.matrix.msc4157.update_delayed_event` at line 1 column 22", + }); + + transport.start(); + const membership = mockCallMembership(membershipTemplate, roomId, "@alice:example.org"); + const onTransportEnabled = jest.fn(); + transport.on(RoomAndToDeviceEvents.EnabledTransportsChanged, onTransportEnabled); + + // We start with toDevice transport enabled + expect(transport.enabled.room).toBeFalsy(); + expect(transport.enabled.toDevice).toBeTruthy(); + + await transport.sendKey("1235", 0, [membership]); + + // We switched transport, now room transport is enabled + expect(onTransportEnabled).toHaveBeenCalledWith({ toDevice: false, room: true }); + expect(transport.enabled.room).toBeTruthy(); + expect(transport.enabled.toDevice).toBeFalsy(); + + // sanity check that we called the failang to device send key. + expect(toDeviceKeyTransport.sendKey).toHaveBeenCalledWith("1235", 0, [membership]); + expect(toDeviceKeyTransport.sendKey).toHaveBeenCalledTimes(1); + // We re-sent the key via the room transport + expect(roomKeyTransport.sendKey).toHaveBeenCalledWith("1235", 0, [membership]); + expect(roomKeyTransport.sendKey).toHaveBeenCalledTimes(1); + + mockClient.encryptAndSendToDevice.mockRestore(); + }); + it("does log that it did nothing when disabled", () => { transport.start(); const onNewKeyFromTransport = jest.fn(); diff --git a/spec/unit/matrixrtc/ToDeviceKeyTransport.spec.ts b/spec/unit/matrixrtc/ToDeviceKeyTransport.spec.ts index 80ff7fc4e..6764c6675 100644 --- a/spec/unit/matrixrtc/ToDeviceKeyTransport.spec.ts +++ b/spec/unit/matrixrtc/ToDeviceKeyTransport.spec.ts @@ -34,7 +34,7 @@ describe("ToDeviceKeyTransport", () => { beforeEach(() => { mockClient = getMockClientWithEventEmitter({ - encryptAndSendToDevice: jest.fn(), + encryptAndSendToDevice: jest.fn().mockImplementation(() => Promise.resolve()), }); mockLogger = { debug: jest.fn(), diff --git a/src/matrixrtc/IKeyTransport.ts b/src/matrixrtc/IKeyTransport.ts index b9776b90f..0bed388a3 100644 --- a/src/matrixrtc/IKeyTransport.ts +++ b/src/matrixrtc/IKeyTransport.ts @@ -18,10 +18,12 @@ import { type CallMembership } from "./CallMembership.ts"; export enum KeyTransportEvents { ReceivedKeys = "received_keys", + NotSupportedError = "not_supported_error", } export type KeyTransportEventsHandlerMap = { [KeyTransportEvents.ReceivedKeys]: KeyTransportEventListener; + [KeyTransportEvents.NotSupportedError]: () => void; }; export type KeyTransportEventListener = ( diff --git a/src/matrixrtc/RoomAndToDeviceKeyTransport.ts b/src/matrixrtc/RoomAndToDeviceKeyTransport.ts index a3d656eeb..659c094b2 100644 --- a/src/matrixrtc/RoomAndToDeviceKeyTransport.ts +++ b/src/matrixrtc/RoomAndToDeviceKeyTransport.ts @@ -18,7 +18,7 @@ import { logger as rootLogger, type Logger } from "../logger.ts"; import { KeyTransportEvents, type KeyTransportEventsHandlerMap, type IKeyTransport } from "./IKeyTransport.ts"; import { type CallMembership } from "./CallMembership.ts"; import type { RoomKeyTransport } from "./RoomKeyTransport.ts"; -import type { ToDeviceKeyTransport } from "./ToDeviceKeyTransport.ts"; +import { NotSupportedError, type ToDeviceKeyTransport } from "./ToDeviceKeyTransport.ts"; import { TypedEventEmitter } from "../models/typed-event-emitter.ts"; // Deprecate RoomAndToDeviceTransport: This whole class is only a stop gap until we remove RoomKeyTransport. @@ -114,6 +114,18 @@ export class RoomAndToDeviceTransport (this._enabled.toDevice ? "to device transport" : ""), ); if (this._enabled.room) await this.roomKeyTransport.sendKey(keyBase64Encoded, index, members); - if (this._enabled.toDevice) await this.toDeviceTransport.sendKey(keyBase64Encoded, index, members); + if (this._enabled.toDevice) { + try { + await this.toDeviceTransport.sendKey(keyBase64Encoded, index, members); + } catch (error) { + if (error instanceof NotSupportedError && !this._enabled.room) { + this.logger.warn( + "To device is not supported enabling room key transport, disabling toDevice transport", + ); + this.setEnabled({ toDevice: false, room: true }); + await this.sendKey(keyBase64Encoded, index, members); + } + } + } } } diff --git a/src/matrixrtc/ToDeviceKeyTransport.ts b/src/matrixrtc/ToDeviceKeyTransport.ts index 8486b4205..f562cbc54 100644 --- a/src/matrixrtc/ToDeviceKeyTransport.ts +++ b/src/matrixrtc/ToDeviceKeyTransport.ts @@ -14,6 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { type WidgetApiResponseError } from "matrix-widget-api"; + import { TypedEventEmitter } from "../models/typed-event-emitter.ts"; import { type IKeyTransport, KeyTransportEvents, type KeyTransportEventsHandlerMap } from "./IKeyTransport.ts"; import { type Logger, logger as rootLogger } from "../logger.ts"; @@ -23,6 +25,14 @@ import { ClientEvent, type MatrixClient } from "../client.ts"; import type { MatrixEvent } from "../models/event.ts"; import { EventType } from "../@types/event.ts"; +export class NotSupportedError extends Error { + public constructor(message?: string) { + super(message); + } + public get name(): string { + return "NotSupportedError"; + } +} /** * ToDeviceKeyTransport is used to send MatrixRTC keys to other devices using the * to-device CS-API. @@ -91,7 +101,21 @@ export class ToDeviceKeyTransport }); if (targets.length > 0) { - await this.client.encryptAndSendToDevice(EventType.CallEncryptionKeysPrefix, targets, content); + await this.client + .encryptAndSendToDevice(EventType.CallEncryptionKeysPrefix, targets, content) + .catch((error: WidgetApiResponseError) => { + const msg: string = error.message; + // This is not ideal. We would want to have a custom error type for unsupported actions. + // This is not part of the widget API spec. Since as of now there are only two implementations: + // Rust SDK + JS-SDK, and the JS-SDK does support to-device sending, we can assume that + // this is a widget driver issue error message. + if ( + (msg.includes("unknown variant") && msg.includes("send_to_device")) || + msg.includes("not supported") + ) { + throw new NotSupportedError("The widget driver does not support to-device encryption"); + } + }); this.statistics.counters.roomEventEncryptionKeysSent += 1; } else { this.logger.warn("No targets found for sending key"); -- GitLab