From 328de4e9c867ca3f41f04a942d7c8f8aa36ab7ed Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 1 Dec 2023 10:59:18 +0000 Subject: [PATCH] `shieldStatusForRoom`: guard against client shutdown (#11957) If `shieldStatusForRoom` races against a logout operation, then some of the (many) crypto operations that it does can fail. Catch the error and handle it gracefully. --- src/utils/ShieldUtils.ts | 93 ++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 41 deletions(-) diff --git a/src/utils/ShieldUtils.ts b/src/utils/ShieldUtils.ts index fa01778b67..5003083eeb 100644 --- a/src/utils/ShieldUtils.ts +++ b/src/utils/ShieldUtils.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { MatrixClient, Room } from "matrix-js-sdk/src/matrix"; +import { ClientStoppedError, MatrixClient, Room } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; import DMRoomMap from "./DMRoomMap"; @@ -32,48 +32,59 @@ export async function shieldStatusForRoom(client: MatrixClient, room: Room): Pro return E2EStatus.Warning; } - const members = (await room.getEncryptionTargetMembers()).map(({ userId }) => userId); - const inDMMap = !!DMRoomMap.shared().getUserIdForRoomId(room.roomId); + try { + const members = (await room.getEncryptionTargetMembers()).map(({ userId }) => userId); + const inDMMap = !!DMRoomMap.shared().getUserIdForRoomId(room.roomId); - const verified: string[] = []; - const unverified: string[] = []; - for (const userId of members) { - if (userId === client.getUserId()) continue; - const userTrust = await crypto.getUserVerificationStatus(userId); + const verified: string[] = []; + const unverified: string[] = []; + for (const userId of members) { + if (userId === client.getUserId()) continue; + const userTrust = await crypto.getUserVerificationStatus(userId); - /* Alarm if any unverified users were verified before. */ - if (userTrust.wasCrossSigningVerified() && !userTrust.isCrossSigningVerified()) { - return E2EStatus.Warning; + /* Alarm if any unverified users were verified before. */ + if (userTrust.wasCrossSigningVerified() && !userTrust.isCrossSigningVerified()) { + return E2EStatus.Warning; + } + (userTrust.isCrossSigningVerified() ? verified : unverified).push(userId); } - (userTrust.isCrossSigningVerified() ? verified : unverified).push(userId); + + /* Check all verified user devices. */ + /* Don't alarm if no other users are verified */ + const includeUser = + (verified.length > 0 && // Don't alarm for self in rooms where nobody else is verified + !inDMMap && // Don't alarm for self in DMs with other users + members.length !== 2) || // Don't alarm for self in 1:1 chats with other users + members.length === 1; // Do alarm for self if we're alone in a room + const targets = includeUser ? [...verified, client.getUserId()!] : verified; + const devicesByUser = await crypto.getUserDeviceInfo(targets); + for (const userId of targets) { + const devices = devicesByUser.get(userId); + if (!devices) { + // getUserDeviceInfo returned nothing about this user, which means we know nothing about their device list. + // That seems odd, so treat it as a warning. + logger.warn(`No device info for user ${userId}`); + return E2EStatus.Warning; + } + + const anyDeviceNotVerified = await asyncSome(devices.keys(), async (deviceId) => { + const verificationStatus = await crypto.getDeviceVerificationStatus(userId, deviceId); + return !verificationStatus?.isVerified(); + }); + if (anyDeviceNotVerified) { + return E2EStatus.Warning; + } + } + + return unverified.length === 0 ? E2EStatus.Verified : E2EStatus.Normal; + } catch (e) { + if (!(e instanceof ClientStoppedError)) { + throw e; + } + + // The client has been stopped while we were figuring out what to do. Catch the exception to stop it being + // logged. It probably doesn't really matter what we return. + logger.warn("shieldStatusForRoom: client stopped"); + return E2EStatus.Normal; } - - /* Check all verified user devices. */ - /* Don't alarm if no other users are verified */ - const includeUser = - (verified.length > 0 && // Don't alarm for self in rooms where nobody else is verified - !inDMMap && // Don't alarm for self in DMs with other users - members.length !== 2) || // Don't alarm for self in 1:1 chats with other users - members.length === 1; // Do alarm for self if we're alone in a room - const targets = includeUser ? [...verified, client.getUserId()!] : verified; - const devicesByUser = await crypto.getUserDeviceInfo(targets); - for (const userId of targets) { - const devices = devicesByUser.get(userId); - if (!devices) { - // getUserDeviceInfo returned nothing about this user, which means we know nothing about their device list. - // That seems odd, so treat it as a warning. - logger.warn(`No device info for user ${userId}`); - return E2EStatus.Warning; - } - - const anyDeviceNotVerified = await asyncSome(devices.keys(), async (deviceId) => { - const verificationStatus = await crypto.getDeviceVerificationStatus(userId, deviceId); - return !verificationStatus?.isVerified(); - }); - if (anyDeviceNotVerified) { - return E2EStatus.Warning; - } - } - - return unverified.length === 0 ? E2EStatus.Verified : E2EStatus.Normal; }