Merge commit from fork

Remove insecure MatrixClient.sendSharedHistoryKeys method

Author: dbkr

spec/unit/models/MSC3089TreeSpace.spec.ts

@@ -107,7 +107,7 @@ describe("MSC3089TreeSpace", () => {
             return Promise.resolve();
         });
         client.invite = fn;
-        await tree.invite(target, false, false);
+        await tree.invite(target, false);
         expect(fn).toHaveBeenCalledTimes(1);
     });
 
@@ -120,7 +120,7 @@ describe("MSC3089TreeSpace", () => {
             return Promise.resolve();
         });
         client.invite = fn;
-        await tree.invite(target, false, false);
+        await tree.invite(target, false);
         expect(fn).toHaveBeenCalledTimes(2);
     });
 
@@ -133,7 +133,7 @@ describe("MSC3089TreeSpace", () => {
         });
         client.invite = fn;
 
-        await expect(tree.invite(target, false, false)).rejects.toThrow("MatrixError: Sample Failure");
+        await expect(tree.invite(target, false)).rejects.toThrow("MatrixError: Sample Failure");
 
         expect(fn).toHaveBeenCalledTimes(1);
     });
@@ -155,61 +155,10 @@ describe("MSC3089TreeSpace", () => {
             { invite: (userId) => fn(tree.roomId, userId) } as MSC3089TreeSpace,
         ];
 
-        await tree.invite(target, true, false);
+        await tree.invite(target, true);
         expect(fn).toHaveBeenCalledTimes(4);
     });
 
-    it("should share keys with invitees", async () => {
-        const target = targetUser;
-        const sendKeysFn = jest.fn().mockImplementation((inviteRoomId: string, userIds: string[]) => {
-            expect(inviteRoomId).toEqual(roomId);
-            expect(userIds).toMatchObject([target]);
-            return Promise.resolve();
-        });
-        client.invite = () => Promise.resolve({}); // we're not testing this here - see other tests
-        client.sendSharedHistoryKeys = sendKeysFn;
-
-        // Mock the history check as best as possible
-        const historyVis = "shared";
-        const historyFn = jest.fn().mockImplementation((eventType: string, stateKey?: string) => {
-            // We're not expecting a super rigid test: the function that calls this internally isn't
-            // really being tested here.
-            expect(eventType).toEqual(EventType.RoomHistoryVisibility);
-            expect(stateKey).toEqual("");
-            return { getContent: () => ({ history_visibility: historyVis }) }; // eslint-disable-line camelcase
-        });
-        room.currentState.getStateEvents = historyFn;
-
-        // Note: inverse test is implicit from other tests, which disable the call stack of this
-        // test in order to pass.
-        await tree.invite(target, false, true);
-        expect(sendKeysFn).toHaveBeenCalledTimes(1);
-        expect(historyFn).toHaveBeenCalledTimes(1);
-    });
-
-    it("should not share keys with invitees if inappropriate history visibility", async () => {
-        const target = targetUser;
-        const sendKeysFn = jest.fn().mockImplementation((inviteRoomId: string, userIds: string[]) => {
-            expect(inviteRoomId).toEqual(roomId);
-            expect(userIds).toMatchObject([target]);
-            return Promise.resolve();
-        });
-        client.invite = () => Promise.resolve({}); // we're not testing this here - see other tests
-        client.sendSharedHistoryKeys = sendKeysFn;
-
-        const historyVis = "joined"; // NOTE: Changed.
-        const historyFn = jest.fn().mockImplementation((eventType: string, stateKey?: string) => {
-            expect(eventType).toEqual(EventType.RoomHistoryVisibility);
-            expect(stateKey).toEqual("");
-            return { getContent: () => ({ history_visibility: historyVis }) }; // eslint-disable-line camelcase
-        });
-        room.currentState.getStateEvents = historyFn;
-
-        await tree.invite(target, false, true);
-        expect(sendKeysFn).toHaveBeenCalledTimes(0);
-        expect(historyFn).toHaveBeenCalledTimes(1);
-    });
-
     async function evaluatePowerLevels(pls: any, role: TreePermissions, expectedPl: number) {
         makePowerLevels(pls);
         const fn = jest

src/client.ts

@@ -4087,43 +4087,6 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         await this.http.authedRequest(Method.Delete, path.path, path.queryData, undefined, { prefix: ClientPrefix.V3 });
     }
 
-    /**
-     * Share shared-history decryption keys with the given users.
-     *
-     * @param roomId - the room for which keys should be shared.
-     * @param userIds - a list of users to share with.  The keys will be sent to
-     *     all of the user's current devices.
-     *
-     * @deprecated Do not use this method. It does not work with the Rust crypto stack, and even with the legacy
-     *     stack it introduces a security vulnerability.
-     */
-    public async sendSharedHistoryKeys(roomId: string, userIds: string[]): Promise<void> {
-        if (!this.crypto) {
-            throw new Error("End-to-end encryption disabled");
-        }
-
-        const roomEncryption = this.crypto?.getRoomEncryption(roomId);
-        if (!roomEncryption) {
-            // unknown room, or unencrypted room
-            this.logger.error("Unknown room.  Not sharing decryption keys");
-            return;
-        }
-
-        const deviceInfos = await this.crypto.downloadKeys(userIds);
-        const devicesByUser: Map<string, DeviceInfo[]> = new Map();
-        for (const [userId, devices] of deviceInfos) {
-            devicesByUser.set(userId, Array.from(devices.values()));
-        }
-
-        // XXX: Private member access
-        const alg = this.crypto.getRoomDecryptor(roomId, roomEncryption.algorithm);
-        if (alg.sendSharedHistoryInboundSessions) {
-            await alg.sendSharedHistoryInboundSessions(devicesByUser);
-        } else {
-            this.logger.warn("Algorithm does not support sharing previous keys", roomEncryption.algorithm);
-        }
-    }
-
     /**
      * Get the config for the media repository.
      * @returns Promise which resolves with an object containing the config.

src/models/MSC3089TreeSpace.ts

@@ -30,7 +30,6 @@ import {
     simpleRetryOperation,
 } from "../utils.ts";
 import { MSC3089Branch } from "./MSC3089Branch.ts";
-import { isRoomSharedHistory } from "../crypto/algorithms/megolm.ts";
 import { ISendEventResponse } from "../@types/requests.ts";
 import { FileType } from "../http-api/index.ts";
 import { KnownMembership } from "../@types/membership.ts";
@@ -136,28 +135,14 @@ export class MSC3089TreeSpace {
      * @param userId - The user ID to invite.
      * @param andSubspaces - True (default) to invite the user to all
      * directories/subspaces too, recursively.
-     * @param shareHistoryKeys - True (default) to share encryption keys
-     * with the invited user. This will allow them to decrypt the events (files)
-     * in the tree. Keys will not be shared if the room is lacking appropriate
-     * history visibility (by default, history visibility is "shared" in trees,
-     * which is an appropriate visibility for these purposes).
      * @returns Promise which resolves when complete.
      */
-    public async invite(userId: string, andSubspaces = true, shareHistoryKeys = true): Promise<void> {
+    public async invite(userId: string, andSubspaces = true): Promise<void> {
         const promises: Promise<void>[] = [this.retryInvite(userId)];
         if (andSubspaces) {
-            promises.push(...this.getDirectories().map((d) => d.invite(userId, andSubspaces, shareHistoryKeys)));
+            promises.push(...this.getDirectories().map((d) => d.invite(userId, andSubspaces)));
         }
-        return Promise.all(promises).then(() => {
-            // Note: key sharing is default on because for file trees it is relatively important that the invite
-            // target can actually decrypt the files. The implied use case is that by inviting a user to the tree
-            // it means the sender would like the receiver to view/download the files contained within, much like
-            // sharing a folder in other circles.
-            if (shareHistoryKeys && isRoomSharedHistory(this.room)) {
-                // noinspection JSIgnoredPromiseFromCall - we aren't concerned as much if this fails.
-                this.client.sendSharedHistoryKeys(this.roomId, [userId]);
-            }
-        });
+        await Promise.all(promises);
     }
 
     private retryInvite(userId: string): Promise<void> {