From a6dc14c021a1f858a39e63f47ea300fbc25d4d86 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Fri, 19 Apr 2024 16:00:00 -0400 Subject: [PATCH 1/2] use a different error code for UTDs when user was not in the room --- spec/integ/crypto/crypto.spec.ts | 43 +++++++++++++++++++++++++++++++- src/@types/event.ts | 7 ++++++ src/crypto-api.ts | 5 ++++ src/models/event.ts | 18 +++++++++++++ src/rust-crypto/rust-crypto.ts | 12 +++++++++ 5 files changed, 84 insertions(+), 1 deletion(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 1c9ebb12781..844726f0755 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -101,6 +101,7 @@ import { } from "./olm-utils"; import { ToDevicePayload } from "../../../src/models/ToDeviceMessage"; import { AccountDataAccumulator } from "../../test-utils/AccountDataAccumulator"; +import { UNSIGNED_MEMBERSHIP_FIELD } from "../../../src/@types/event"; import { KnownMembership } from "../../../src/@types/membership"; afterEach(() => { @@ -533,7 +534,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, }); describe("Historical events", () => { - async function sendEventAndAwaitDecryption(): Promise { + async function sendEventAndAwaitDecryption(props: Partial = {}): Promise { // A promise which resolves, with the MatrixEvent which wraps the event, once the decryption fails. const awaitDecryption = emitPromise(aliceClient, MatrixEventEvent.Decrypted); @@ -541,6 +542,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, const encryptedEvent = { ...testData.ENCRYPTED_EVENT, origin_server_ts: Date.now() - 24 * 3600 * 1000, + ...props, }; const syncResponse = { @@ -611,6 +613,45 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, const ev = await sendEventAndAwaitDecryption(); expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_WORKING_BACKUP); }); + + newBackendOnly("fails with NOT_JOINED if user is not member of room", async () => { + fetchMock.get("path:/_matrix/client/v3/room_keys/version", { + status: 404, + body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, + }); + expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + await startClientAndAwaitFirstSync(); + + const ev = await sendEventAndAwaitDecryption({ + unsigned: { + [UNSIGNED_MEMBERSHIP_FIELD.name]: "leave", + }, + }); + expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_USER_NOT_JOINED); + }); + + newBackendOnly( + "fails with another error when the server reports user was a member of the room", + async () => { + // This tests that when the server reports the user's + // membership, and reports that the user was joined, then we + // don't get a HISTORICAL_MESSAGE_USER_NOT_JOINED error, and + // instead get some other error. + fetchMock.get("path:/_matrix/client/v3/room_keys/version", { + status: 404, + body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, + }); + expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + await startClientAndAwaitFirstSync(); + + const ev = await sendEventAndAwaitDecryption({ + unsigned: { + [UNSIGNED_MEMBERSHIP_FIELD.name]: "join", + }, + }); + expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP); + }, + ); }); it("Decryption fails with Unable to decrypt for other errors", async () => { diff --git a/src/@types/event.ts b/src/@types/event.ts index 0a144a352df..04f1e969c11 100644 --- a/src/@types/event.ts +++ b/src/@types/event.ts @@ -298,6 +298,13 @@ export const LOCAL_NOTIFICATION_SETTINGS_PREFIX = new UnstableValue( */ export const UNSIGNED_THREAD_ID_FIELD = new UnstableValue("thread_id", "org.matrix.msc4023.thread_id"); +/** + * https://github.com/matrix-org/matrix-spec-proposals/pull/4115 + * + * @experimental + */ +export const UNSIGNED_MEMBERSHIP_FIELD = new UnstableValue("membership", "io.element.msc4115.membership"); + /** * @deprecated in favour of {@link EncryptedFile} */ diff --git a/src/crypto-api.ts b/src/crypto-api.ts index 1c3a3a6356c..7dfdfbfcf08 100644 --- a/src/crypto-api.ts +++ b/src/crypto-api.ts @@ -560,6 +560,11 @@ export enum DecryptionFailureCode { */ HISTORICAL_MESSAGE_WORKING_BACKUP = "HISTORICAL_MESSAGE_WORKING_BACKUP", + /** + * Message was sent when the user was not a member of the room. + */ + HISTORICAL_MESSAGE_USER_NOT_JOINED = "HISTORICAL_MESSAGE_USER_NOT_JOINED", + /** Unknown or unclassified error. */ UNKNOWN_ERROR = "UNKNOWN_ERROR", diff --git a/src/models/event.ts b/src/models/event.ts index 7c2725cda02..3e5df8fbf7c 100644 --- a/src/models/event.ts +++ b/src/models/event.ts @@ -31,6 +31,7 @@ import { RelationType, ToDeviceMessageId, UNSIGNED_THREAD_ID_FIELD, + UNSIGNED_MEMBERSHIP_FIELD, } from "../@types/event"; import { Crypto } from "../crypto"; import { deepSortedObjectEntries, internaliseString } from "../utils"; @@ -76,6 +77,7 @@ export interface IUnsigned { "invite_room_state"?: StrippedState[]; "m.relations"?: Record; // No common pattern for aggregated relations [UNSIGNED_THREAD_ID_FIELD.name]?: string; + [UNSIGNED_MEMBERSHIP_FIELD.name]?: Membership | string; } export interface IThreadBundledRelationship { @@ -721,6 +723,22 @@ export class MatrixEvent extends TypedEventEmitter Date: Thu, 25 Apr 2024 15:02:19 -0400 Subject: [PATCH 2/2] if user is invited, treat it as unexpected UTD --- spec/integ/crypto/crypto.spec.ts | 24 ++++++++++++++++++++++++ src/rust-crypto/rust-crypto.ts | 2 +- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/spec/integ/crypto/crypto.spec.ts b/spec/integ/crypto/crypto.spec.ts index 844726f0755..50b5f4dea05 100644 --- a/spec/integ/crypto/crypto.spec.ts +++ b/spec/integ/crypto/crypto.spec.ts @@ -630,6 +630,30 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_USER_NOT_JOINED); }); + newBackendOnly( + "fails with another error when the server reports user was a member of the room", + async () => { + // This tests that when the server reports that the user + // was invited at the time the event was sent, then we + // don't get a HISTORICAL_MESSAGE_USER_NOT_JOINED error, + // and instead get some other error, since the user should + // have gotten the key for the event. + fetchMock.get("path:/_matrix/client/v3/room_keys/version", { + status: 404, + body: { errcode: "M_NOT_FOUND", error: "No current backup version." }, + }); + expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} }); + await startClientAndAwaitFirstSync(); + + const ev = await sendEventAndAwaitDecryption({ + unsigned: { + [UNSIGNED_MEMBERSHIP_FIELD.name]: "invite", + }, + }); + expect(ev.decryptionFailureReason).toEqual(DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP); + }, + ); + newBackendOnly( "fails with another error when the server reports user was a member of the room", async () => { diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index f428ab80d36..29373fe3b48 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -1745,7 +1745,7 @@ class EventDecryptor { // If the server is telling us our membership at the time the event // was sent, and it isn't "join", we use a different error code. const membership = event.getMembershipAtEvent(); - if (membership && membership !== KnownMembership.Join) { + if (membership && membership !== KnownMembership.Join && membership !== KnownMembership.Invite) { throw new DecryptionError( DecryptionFailureCode.HISTORICAL_MESSAGE_USER_NOT_JOINED, "This message was sent when we were not a member of the room.",