From bae18c037f3103a4163e8283556255605daf5a3a Mon Sep 17 00:00:00 2001 From: Bruno Windels <274386+bwindels@users.noreply.github.com> Date: Tue, 7 Mar 2023 10:53:32 +0100 Subject: [PATCH] return enum explaining user trust level rather than boolean --- src/matrix/e2ee/DeviceTracker.ts | 4 +- src/matrix/e2ee/common.ts | 14 +++- src/matrix/e2ee/olm/Encryption.ts | 4 +- src/matrix/verification/CrossSigning.ts | 107 ++++++++++++++++++------ 4 files changed, 97 insertions(+), 32 deletions(-) diff --git a/src/matrix/e2ee/DeviceTracker.ts b/src/matrix/e2ee/DeviceTracker.ts index 98221523..ae00b1e0 100644 --- a/src/matrix/e2ee/DeviceTracker.ts +++ b/src/matrix/e2ee/DeviceTracker.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import {verifyEd25519Signature, getEd25519Signature, SIGNATURE_ALGORITHM} from "./common"; +import {verifyEd25519Signature, getEd25519Signature, SIGNATURE_ALGORITHM, SignatureVerification} from "./common"; import {HistoryVisibility, shouldShareKey, DeviceKey, getDeviceEd25519Key, getDeviceCurve25519Key} from "./common"; import {RoomMember} from "../room/members/RoomMember.js"; import {getKeyUsage, getKeyEd25519Key, getKeyUserId, KeyUsage} from "../verification/CrossSigning"; @@ -462,7 +462,7 @@ export class DeviceTracker { log.log("ed25519 and/or curve25519 key invalid").set({deviceKey}); return false; } - const isValid = verifyEd25519Signature(this._olmUtil, userId, deviceId, ed25519Key, deviceKey, log); + const isValid = verifyEd25519Signature(this._olmUtil, userId, deviceId, ed25519Key, deviceKey, log) === SignatureVerification.Valid; if (!isValid) { log.log({ l: "ignore device with invalid signature", diff --git a/src/matrix/e2ee/common.ts b/src/matrix/e2ee/common.ts index 27078135..c8a5ec0f 100644 --- a/src/matrix/e2ee/common.ts +++ b/src/matrix/e2ee/common.ts @@ -68,11 +68,17 @@ export function getEd25519Signature(signedValue: SignedValue, userId: string, de return signedValue?.signatures?.[userId]?.[`${SIGNATURE_ALGORITHM}:${deviceOrKeyId}`]; } -export function verifyEd25519Signature(olmUtil: Olm.Utility, userId: string, deviceOrKeyId: string, ed25519Key: string, value: SignedValue, log?: ILogItem) { +export enum SignatureVerification { + Valid, + Invalid, + NotSigned, +} + +export function verifyEd25519Signature(olmUtil: Olm.Utility, userId: string, deviceOrKeyId: string, ed25519Key: string, value: SignedValue, log?: ILogItem): SignatureVerification { const signature = getEd25519Signature(value, userId, deviceOrKeyId); if (!signature) { log?.set("no_signature", true); - return false; + return SignatureVerification.NotSigned; } const clone = Object.assign({}, value) as object; delete clone["unsigned"]; @@ -81,14 +87,14 @@ export function verifyEd25519Signature(olmUtil: Olm.Utility, userId: string, dev try { // throws when signature is invalid olmUtil.ed25519_verify(ed25519Key, canonicalJson, signature); - return true; + return SignatureVerification.Valid; } catch (err) { if (log) { const logItem = log.log({l: "Invalid signature, ignoring.", ed25519Key, canonicalJson, signature}); logItem.error = err; logItem.logLevel = log.level.Warn; } - return false; + return SignatureVerification.Invalid; } } diff --git a/src/matrix/e2ee/olm/Encryption.ts b/src/matrix/e2ee/olm/Encryption.ts index c4fee911..ef16ba45 100644 --- a/src/matrix/e2ee/olm/Encryption.ts +++ b/src/matrix/e2ee/olm/Encryption.ts @@ -15,7 +15,7 @@ limitations under the License. */ import {groupByWithCreator} from "../../../utils/groupBy"; -import {verifyEd25519Signature, OLM_ALGORITHM, getDeviceCurve25519Key, getDeviceEd25519Key} from "../common"; +import {verifyEd25519Signature, OLM_ALGORITHM, getDeviceCurve25519Key, getDeviceEd25519Key, SignatureVerification} from "../common"; import {createSessionEntry} from "./Session"; import type {OlmMessage, OlmPayload, OlmEncryptedMessageContent} from "./types"; @@ -260,7 +260,7 @@ export class Encryption { const device = devicesByUser.get(userId)?.get(deviceId); if (device) { const isValidSignature = verifyEd25519Signature( - this.olmUtil, userId, deviceId, getDeviceEd25519Key(device), keySection, log); + this.olmUtil, userId, deviceId, getDeviceEd25519Key(device), keySection, log) === SignatureVerification.Valid; if (isValidSignature) { const target = EncryptionTarget.fromOTK(device, keySection.key); verifiedEncryptionTargets.push(target); diff --git a/src/matrix/verification/CrossSigning.ts b/src/matrix/verification/CrossSigning.ts index 975f0d43..c35914bb 100644 --- a/src/matrix/verification/CrossSigning.ts +++ b/src/matrix/verification/CrossSigning.ts @@ -16,7 +16,7 @@ limitations under the License. import { ILogItem } from "../../lib"; import {pkSign} from "./common"; -import {verifyEd25519Signature} from "../e2ee/common"; +import {verifyEd25519Signature, SignatureVerification} from "../e2ee/common"; import type {SecretStorage} from "../ssss/SecretStorage"; import type {Storage} from "../storage/idb/Storage"; @@ -43,6 +43,27 @@ export enum KeyUsage { UserSigning = "user_signing" }; +export enum UserTrust { + /** We trust the user, the whole signature chain checks out from our MSK to all of their device keys. */ + Trusted = 1, + /** We haven't signed this user's identity yet. Verify this user first to sign it. */ + UserNotSigned, + /** We have signed the user already, but the signature isn't valid. + One possible cause could be that an attacker is uploading signatures in our name. */ + UserSignatureMismatch, + /** We trust the user, but they don't trust one of their devices. */ + UserDeviceNotSigned, + /** We trust the user, but the signatures of one of their devices is invalid. + * One possible cause could be that an attacker is uploading signatures in their name. */ + UserDeviceSignatureMismatch, + /** The user doesn't have a valid signature for the SSK with their MSK, or the SSK is missing. + * This likely means bootstrapping cross-signing on their end didn't finish correctly. */ + UserSetupError, + /** We don't have a valid signature for our SSK with our MSK, the SSK is missing, or we don't trust our own MSK. + * This likely means bootstrapping cross-signing on our end didn't finish correctly. */ + OwnSetupError +} + export class CrossSigning { private readonly storage: Storage; private readonly secretStorage: SecretStorage; @@ -161,31 +182,69 @@ export class CrossSigning { }); } - async isUserTrusted(userId: string, log: ILogItem): Promise { - return log.wrap("isUserTrusted", async log => { + async getUserTrust(userId: string, log: ILogItem): Promise { + return log.wrap("getUserTrust", async log => { log.set("id", userId); if (!this.isMasterKeyTrusted) { - return false; - } - const theirDeviceKeys = await log.wrap("get their devices", log => this.deviceTracker.devicesForUsers([userId], this.hsApi, log)); - const theirSSK = await log.wrap("get their ssk", log => this.deviceTracker.getCrossSigningKeyForUser(userId, KeyUsage.SelfSigning, this.hsApi, log)); - if (!theirSSK) { - return false; - } - const hasUnsignedDevice = theirDeviceKeys.some(dk => log.wrap({l: "verify device", id: dk.device_id}, log => !this.hasValidSignatureFrom(dk, theirSSK, log))); - if (hasUnsignedDevice) { - return false; - } - const theirMSK = await log.wrap("get their msk", log => this.deviceTracker.getCrossSigningKeyForUser(userId, KeyUsage.Master, this.hsApi, log)); - if (!theirMSK || !log.wrap("verify their ssk", log => this.hasValidSignatureFrom(theirSSK, theirMSK, log))) { - return false; - } - const ourUSK = await log.wrap("get our usk", log => this.deviceTracker.getCrossSigningKeyForUser(this.ownUserId, KeyUsage.UserSigning, this.hsApi, log)); - if (!ourUSK || !log.wrap("verify their msk", log => this.hasValidSignatureFrom(theirMSK, ourUSK, log))) { - return false; + return UserTrust.OwnSetupError; } const ourMSK = await log.wrap("get our msk", log => this.deviceTracker.getCrossSigningKeyForUser(this.ownUserId, KeyUsage.Master, this.hsApi, log)); - return !!ourMSK && log.wrap("verify our usk", log => this.hasValidSignatureFrom(ourUSK, ourMSK, log)); + if (!ourMSK) { + return UserTrust.OwnSetupError; + } + const ourUSK = await log.wrap("get our usk", log => this.deviceTracker.getCrossSigningKeyForUser(this.ownUserId, KeyUsage.UserSigning, this.hsApi, log)); + if (!ourUSK) { + return UserTrust.OwnSetupError; + } + const ourUSKVerification = log.wrap("verify our usk", log => this.hasValidSignatureFrom(ourUSK, ourMSK, log)); + if (ourUSKVerification !== SignatureVerification.Valid) { + return UserTrust.OwnSetupError; + } + const theirMSK = await log.wrap("get their msk", log => this.deviceTracker.getCrossSigningKeyForUser(userId, KeyUsage.Master, this.hsApi, log)); + if (!theirMSK) { + /* assume that when they don't have an MSK, they've never enabled cross-signing on their client + (or it's not supported) rather than assuming a setup error on their side. + Later on, for their SSK, we _do_ assume it's a setup error as it doesn't make sense to have an MSK without a SSK */ + return UserTrust.UserNotSigned; + } + const theirMSKVerification = log.wrap("verify their msk", log => this.hasValidSignatureFrom(theirMSK, ourUSK, log)); + if (theirMSKVerification !== SignatureVerification.Valid) { + if (theirMSKVerification === SignatureVerification.NotSigned) { + return UserTrust.UserNotSigned; + } else { /* SignatureVerification.Invalid */ + return UserTrust.UserSignatureMismatch; + } + } + const theirSSK = await log.wrap("get their ssk", log => this.deviceTracker.getCrossSigningKeyForUser(userId, KeyUsage.SelfSigning, this.hsApi, log)); + if (!theirSSK) { + return UserTrust.UserSetupError; + } + const theirSSKVerification = log.wrap("verify their ssk", log => this.hasValidSignatureFrom(theirSSK, theirMSK, log)); + if (theirSSKVerification !== SignatureVerification.Valid) { + return UserTrust.UserSetupError; + } + const theirDeviceKeys = await log.wrap("get their devices", log => this.deviceTracker.devicesForUsers([userId], this.hsApi, log)); + const lowestDeviceVerification = theirDeviceKeys.reduce((lowest, dk) => log.wrap({l: "verify device", id: dk.device_id}, log => { + const verification = this.hasValidSignatureFrom(dk, theirSSK, log); + // first Invalid, then NotSigned, then Valid + if (lowest === SignatureVerification.Invalid || verification === SignatureVerification.Invalid) { + return SignatureVerification.Invalid; + } else if (lowest === SignatureVerification.NotSigned || verification === SignatureVerification.NotSigned) { + return SignatureVerification.NotSigned; + } else if (lowest === SignatureVerification.Valid || verification === SignatureVerification.Valid) { + return SignatureVerification.Valid; + } + // should never happen as we went over all the enum options + return SignatureVerification.Invalid; + }), SignatureVerification.Valid); + if (lowestDeviceVerification !== SignatureVerification.Valid) { + if (lowestDeviceVerification === SignatureVerification.NotSigned) { + return UserTrust.UserDeviceNotSigned; + } else { /* SignatureVerification.Invalid */ + return UserTrust.UserDeviceSignatureMismatch; + } + } + return UserTrust.Trusted; }); } @@ -217,10 +276,10 @@ export class CrossSigning { pkSign(this.olm, keyToSign, signingKey, this.ownUserId, ""); } - private hasValidSignatureFrom(key: DeviceKey | CrossSigningKey, signingKey: CrossSigningKey, log: ILogItem): boolean { + private hasValidSignatureFrom(key: DeviceKey | CrossSigningKey, signingKey: CrossSigningKey, log: ILogItem): SignatureVerification { const pubKey = getKeyEd25519Key(signingKey); if (!pubKey) { - return false; + return SignatureVerification.NotSigned; } return verifyEd25519Signature(this.olmUtil, signingKey.user_id, pubKey, pubKey, key, log); }