From ff05041a5a240410460ce3176fd07351679dccb6 Mon Sep 17 00:00:00 2001 From: Zoe Date: Wed, 15 Jan 2020 13:57:29 +0000 Subject: [PATCH 1/6] Update room shield icons to represent encrypted but unverified chats in less alarmed tones --- src/components/structures/RoomView.js | 58 ++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index a717f485f0..3adcd22d87 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -172,6 +172,7 @@ module.exports = createReactClass({ MatrixClientPeg.get().on("accountData", this.onAccountData); MatrixClientPeg.get().on("crypto.keyBackupStatus", this.onKeyBackupStatus); MatrixClientPeg.get().on("deviceVerificationChanged", this.onDeviceVerificationChanged); + MatrixClientPeg.get().on("userTrustStatusChanged", this.onUserVerificationChanged); // Start listening for RoomViewStore updates this._roomStoreToken = RoomViewStore.addListener(this._onRoomViewStoreUpdate); this._onRoomViewStoreUpdate(true); @@ -491,6 +492,7 @@ module.exports = createReactClass({ MatrixClientPeg.get().removeListener("accountData", this.onAccountData); MatrixClientPeg.get().removeListener("crypto.keyBackupStatus", this.onKeyBackupStatus); MatrixClientPeg.get().removeListener("deviceVerificationChanged", this.onDeviceVerificationChanged); + MatrixClientPeg.get().removeListener("userTrustStatusChanged", this.onUserVerificationChanged); } window.removeEventListener('beforeunload', this.onPageUnload); @@ -761,6 +763,14 @@ module.exports = createReactClass({ this._updateE2EStatus(room); }, + onUserVerificationChanged: function(userId, _trustStatus) { + const room = this.state.room; + if (!room.currentState.getMember(userId)) { + return; + } + this._updateE2EStatus(room); + }, + _updateE2EStatus: async function(room) { const cli = MatrixClientPeg.get(); if (!cli.isRoomEncrypted(room.roomId)) { @@ -784,29 +794,57 @@ module.exports = createReactClass({ return; } const e2eMembers = await room.getEncryptionTargetMembers(); + + /* + Ensure we trust our own signing key, ie, nobody's used our credentials to + replace it and sign all our devices + */ + if (!cli.checkUserTrust(cli.getUserId())) { + this.setState({ + e2eStatus: "warning", + }); + debuglog("e2e status set to warning due to not trusting our own signing key"); + return; + } + + /* + Gather verification state of every user in the room. + If _any_ user is verified then _every_ user must be verified, or we'll bail. + Note we don't count our own user so that the all/any check behaves properly. + */ + const verificationState = e2eMembers.map(({userId}) => userId) + .filter((userId) => userId !== cli.getUserId()) + .map((userId) => cli.checkUserTrust(userId).isCrossSigningVerified()); + if (verificationState.includes(true) && verificationState.includes(false)) { + this.setState({ + e2eStatus: "warning", + }); + debuglog("e2e status set to warning as some, but not all, users are verified"); + return; + } + + /* + Whether we verify or not, a user having an untrusted device requires warnings. + Check every user's devices, including ourselves. + */ for (const member of e2eMembers) { const { userId } = member; - const userVerified = cli.checkUserTrust(userId).isCrossSigningVerified(); - if (!userVerified) { - this.setState({ - e2eStatus: "warning", - }); - return; - } const devices = await cli.getStoredDevicesForUser(userId); - const allDevicesVerified = devices.every(device => { - const { deviceId } = device; + const allDevicesVerified = devices.every(({deviceId}) => { return cli.checkDeviceTrust(userId, deviceId).isCrossSigningVerified(); }); if (!allDevicesVerified) { this.setState({ e2eStatus: "warning", }); + debuglog("e2e status set to warning as not all users trust all of their devices." + + " Aborted on user", userId); return; } } + this.setState({ - e2eStatus: "verified", + e2eStatus: verificationState.includes(true) ? "verified" : "normal", }); }, From 82c5349c4ed1cf516985f5d5de693d9caf4a7fa6 Mon Sep 17 00:00:00 2001 From: Zoe Date: Thu, 16 Jan 2020 16:31:50 +0000 Subject: [PATCH 2/6] Updated to properly handle logic --- src/components/structures/RoomView.js | 47 +++++++++++---------------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 1843a7b64a..3cdc308758 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -792,31 +792,25 @@ export default createReactClass({ e2eStatus: hasUnverifiedDevices ? "warning" : "verified", }); }); + debuglog("e2e check is warning/verified only as cross-signing is off"); return; } + + /* At this point, the user has encryption on and cross-signing on */ const e2eMembers = await room.getEncryptionTargetMembers(); - - /* - Ensure we trust our own signing key, ie, nobody's used our credentials to - replace it and sign all our devices - */ - if (!cli.checkUserTrust(cli.getUserId())) { - this.setState({ - e2eStatus: "warning", - }); - debuglog("e2e status set to warning due to not trusting our own signing key"); - return; - } - - /* - Gather verification state of every user in the room. - If _any_ user is verified then _every_ user must be verified, or we'll bail. - Note we don't count our own user so that the all/any check behaves properly. - */ - const verificationState = e2eMembers.map(({userId}) => userId) + const verified = []; + const unverified = []; + e2eMembers.map(({userId}) => userId) .filter((userId) => userId !== cli.getUserId()) - .map((userId) => cli.checkUserTrust(userId).isCrossSigningVerified()); - if (verificationState.includes(true) && verificationState.includes(false)) { + .forEach((userId) => { + (cli.checkUserTrust(userId).isCrossSigningVerified() ? + verified : unverified).push(userId) + }); + + debuglog("e2e verified", verified, "unverified", unverified); + + /* If we verify any users in this room, expect to verify every user in the room */ + if (verified.length > 0 && unverified.length > 0) { this.setState({ e2eStatus: "warning", }); @@ -824,12 +818,9 @@ export default createReactClass({ return; } - /* - Whether we verify or not, a user having an untrusted device requires warnings. - Check every user's devices, including ourselves. - */ - for (const member of e2eMembers) { - const { userId } = member; + /* At this point, either `verified` or `unverified` is empty, or both */ + /* Check all verified user devices. We don't care if everyone's unverified anyway. */ + for (const userId of [...verified, cli.getUserId()]) { const devices = await cli.getStoredDevicesForUser(userId); const allDevicesVerified = devices.every(({deviceId}) => { return cli.checkDeviceTrust(userId, deviceId).isCrossSigningVerified(); @@ -845,7 +836,7 @@ export default createReactClass({ } this.setState({ - e2eStatus: verificationState.includes(true) ? "verified" : "normal", + e2eStatus: unverified.length === 0 ? "verified" : "normal", }); }, From 8efc45b31a504cfe65593077a9c6e8fc01f3c857 Mon Sep 17 00:00:00 2001 From: Zoe Date: Fri, 17 Jan 2020 10:04:34 +0000 Subject: [PATCH 3/6] no need to verify our own devices for every room --- src/components/structures/RoomView.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 3cdc308758..e0997f87da 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -820,7 +820,7 @@ export default createReactClass({ /* At this point, either `verified` or `unverified` is empty, or both */ /* Check all verified user devices. We don't care if everyone's unverified anyway. */ - for (const userId of [...verified, cli.getUserId()]) { + for (const userId of verified) { const devices = await cli.getStoredDevicesForUser(userId); const allDevicesVerified = devices.every(({deviceId}) => { return cli.checkDeviceTrust(userId, deviceId).isCrossSigningVerified(); From 510b08c88bd6f0dfeac1f56c39a12855f67990c1 Mon Sep 17 00:00:00 2001 From: Zoe Date: Fri, 17 Jan 2020 10:18:50 +0000 Subject: [PATCH 4/6] changed logic to reflect the task --- src/components/structures/RoomView.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index e0997f87da..aa3e86fa60 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -809,17 +809,16 @@ export default createReactClass({ debuglog("e2e verified", verified, "unverified", unverified); - /* If we verify any users in this room, expect to verify every user in the room */ - if (verified.length > 0 && unverified.length > 0) { + /* If we've not verified anyone, set state to "normal" */ + if (verified.length == 0) { this.setState({ - e2eStatus: "warning", + e2eStatus: "normal", }); - debuglog("e2e status set to warning as some, but not all, users are verified"); + debuglog("e2e state set to normal as we have no verified users to worry about"); return; } - /* At this point, either `verified` or `unverified` is empty, or both */ - /* Check all verified user devices. We don't care if everyone's unverified anyway. */ + /* Check all verified user devices. */ for (const userId of verified) { const devices = await cli.getStoredDevicesForUser(userId); const allDevicesVerified = devices.every(({deviceId}) => { @@ -836,7 +835,7 @@ export default createReactClass({ } this.setState({ - e2eStatus: unverified.length === 0 ? "verified" : "normal", + e2eStatus: "verified", }); }, From d02185e4af661542e39a568d43aa58f19670ec0c Mon Sep 17 00:00:00 2001 From: Zoe Date: Fri, 17 Jan 2020 10:22:53 +0000 Subject: [PATCH 5/6] whoops, the number of unverified users matters to the logic --- src/components/structures/RoomView.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index aa3e86fa60..8ecb6a6a02 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -809,15 +809,6 @@ export default createReactClass({ debuglog("e2e verified", verified, "unverified", unverified); - /* If we've not verified anyone, set state to "normal" */ - if (verified.length == 0) { - this.setState({ - e2eStatus: "normal", - }); - debuglog("e2e state set to normal as we have no verified users to worry about"); - return; - } - /* Check all verified user devices. */ for (const userId of verified) { const devices = await cli.getStoredDevicesForUser(userId); @@ -835,7 +826,7 @@ export default createReactClass({ } this.setState({ - e2eStatus: "verified", + e2eStatus: unverified.length === 0 ? "verified" : "normal", }); }, From 908630c0d942de6ec9115c7b197b0d2b47f87488 Mon Sep 17 00:00:00 2001 From: Zoe Date: Fri, 17 Jan 2020 11:30:45 +0000 Subject: [PATCH 6/6] *rude grumbling noises about @dbkr* --- src/components/structures/RoomView.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 8ecb6a6a02..9b02f6d503 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -813,7 +813,7 @@ export default createReactClass({ for (const userId of verified) { const devices = await cli.getStoredDevicesForUser(userId); const allDevicesVerified = devices.every(({deviceId}) => { - return cli.checkDeviceTrust(userId, deviceId).isCrossSigningVerified(); + return cli.checkDeviceTrust(userId, deviceId).isVerified(); }); if (!allDevicesVerified) { this.setState({