Change check{User|Device}Trust interfaces

...to return objects with functions rather than a bitmask

spec/unit/crypto/cross-signing.spec.js

@@ -259,8 +259,17 @@ describe("Cross Signing", function() {
         // once ssk is confirmed, device key should be trusted
         await keyChangePromise;
         await uploadSigsPromise;
-        expect(alice.checkUserTrust("@alice:example.com")).toBe(6);
-        expect(alice.checkDeviceTrust("@alice:example.com", "Osborne2")).toBe(7);
+
+        const aliceTrust = alice.checkUserTrust("@alice:example.com");
+        expect(aliceTrust.isCrossSigningVerified()).toBeTruthy();
+        expect(aliceTrust.isTofu()).toBeTruthy();
+        expect(aliceTrust.isVerified()).toBeTruthy();
+
+        const aliceDeviceTrust = alice.checkDeviceTrust("@alice:example.com", "Osborne2");
+        expect(aliceDeviceTrust.isCrossSigningVerified()).toBeTruthy();
+        expect(aliceDeviceTrust.isLocallyVerified()).toBeTruthy();
+        expect(aliceDeviceTrust.isTofu()).toBeTruthy();
+        expect(aliceDeviceTrust.isVerified()).toBeTruthy();
     });
 
     it("should use trust chain to determine device verification", async function() {
@@ -324,14 +333,27 @@ describe("Cross Signing", function() {
             Dynabook: bobDevice,
         });
         // Bob's device key should be TOFU
-        expect(alice.checkUserTrust("@bob:example.com")).toBe(2);
-        expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(2);
+        const bobTrust = alice.checkUserTrust("@bob:example.com");
+        expect(bobTrust.isVerified()).toBeFalsy();
+        expect(bobTrust.isTofu()).toBeTruthy();
+
+        const bobDeviceTrust = alice.checkDeviceTrust("@bob:example.com", "Dynabook");
+        expect(bobDeviceTrust.isVerified()).toBeFalsy();
+        expect(bobDeviceTrust.isTofu()).toBeTruthy();
+
         // Alice verifies Bob's SSK
         alice.uploadKeySignatures = () => {};
         await alice.setDeviceVerified("@bob:example.com", bobMasterPubkey, true);
+
         // Bob's device key should be trusted
-        expect(alice.checkUserTrust("@bob:example.com")).toBe(6);
-        expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(6);
+        const bobTrust2 = alice.checkUserTrust("@bob:example.com");
+        expect(bobTrust2.isCrossSigningVerified()).toBeTruthy();
+        expect(bobTrust2.isTofu()).toBeTruthy();
+
+        const bobDeviceTrust2 = alice.checkDeviceTrust("@bob:example.com", "Dynabook");
+        expect(bobDeviceTrust2.isCrossSigningVerified()).toBeTruthy();
+        expect(bobDeviceTrust2.isLocallyVerified()).toBeFalsy();
+        expect(bobDeviceTrust2.isTofu()).toBeTruthy();
     });
 
     it("should trust signatures received from other devices", async function() {
@@ -487,8 +509,14 @@ describe("Cross Signing", function() {
         await keyChangePromise;
 
         // Bob's device key should be trusted
-        expect(alice.checkUserTrust("@bob:example.com")).toBe(6);
-        expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(6);
+        const bobTrust = alice.checkUserTrust("@bob:example.com");
+        expect(bobTrust.isCrossSigningVerified()).toBeTruthy();
+        expect(bobTrust.isTofu()).toBeTruthy();
+
+        const bobDeviceTrust = alice.checkDeviceTrust("@bob:example.com", "Dynabook");
+        expect(bobDeviceTrust.isCrossSigningVerified()).toBeTruthy();
+        expect(bobDeviceTrust.isLocallyVerified()).toBeFalsy();
+        expect(bobDeviceTrust.isTofu()).toBeTruthy();
     });
 
     it("should dis-trust an unsigned device", async function() {
@@ -547,11 +575,17 @@ describe("Cross Signing", function() {
             Dynabook: bobDevice,
         });
         // Bob's device key should be untrusted
-        expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(0);
+        const bobDeviceTrust = alice.checkDeviceTrust("@bob:example.com", "Dynabook");
+        expect(bobDeviceTrust.isVerified()).toBeFalsy();
+        expect(bobDeviceTrust.isTofu()).toBeFalsy();
+
         // Alice verifies Bob's SSK
         await alice.setDeviceVerified("@bob:example.com", bobMasterPubkey, true);
+
         // Bob's device key should be untrusted
-        expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(0);
+        const bobDeviceTrust2 = alice.checkDeviceTrust("@bob:example.com", "Dynabook");
+        expect(bobDeviceTrust2.isVerified()).toBeFalsy();
+        expect(bobDeviceTrust2.isTofu()).toBeFalsy();
     });
 
     it("should dis-trust a user when their ssk changes", async function() {
@@ -615,8 +649,12 @@ describe("Cross Signing", function() {
         // Alice verifies Bob's SSK
         alice.uploadKeySignatures = () => {};
         await alice.setDeviceVerified("@bob:example.com", bobMasterPubkey, true);
+
         // Bob's device key should be trusted
-        expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(6);
+        const bobDeviceTrust = alice.checkDeviceTrust("@bob:example.com", "Dynabook");
+        expect(bobDeviceTrust.isVerified()).toBeTruthy();
+        expect(bobDeviceTrust.isTofu()).toBeTruthy();
+
         // Alice downloads new SSK for Bob
         const bobMasterSigning2 = new global.Olm.PkSigning();
         const bobMasterPrivkey2 = bobMasterSigning2.generate_seed();
@@ -652,23 +690,38 @@ describe("Cross Signing", function() {
             unsigned: {},
         });
         // Bob's and his device should be untrusted
-        expect(alice.checkUserTrust("@bob:example.com")).toBe(0);
-        expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(0);
+        const bobTrust = alice.checkUserTrust("@bob:example.com");
+        expect(bobTrust.isVerified()).toBeFalsy();
+        expect(bobTrust.isTofu()).toBeFalsy();
+
+        const bobDeviceTrust2 = alice.checkDeviceTrust("@bob:example.com", "Dynabook");
+        expect(bobDeviceTrust2.isVerified()).toBeFalsy();
+        expect(bobDeviceTrust2.isTofu()).toBeFalsy();
+
         // Alice verifies Bob's SSK
         alice.uploadKeySignatures = () => {};
         await alice.setDeviceVerified("@bob:example.com", bobMasterPubkey2, true);
+
         // Bob should be trusted but not his device
-        expect(alice.checkUserTrust("@bob:example.com")).toBe(4);
-        expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(0);
+        const bobTrust2 = alice.checkUserTrust("@bob:example.com");
+        expect(bobTrust2.isVerified()).toBeTruthy();
+
+        const bobDeviceTrust3 = alice.checkDeviceTrust("@bob:example.com", "Dynabook");
+        expect(bobDeviceTrust3.isVerified()).toBeFalsy();
+
         // Alice gets new signature for device
         const sig2 = bobSigning2.sign(bobDeviceString);
         bobDevice.signatures["@bob:example.com"]["ed25519:" + bobPubkey2] = sig2;
         alice._crypto._deviceList.storeDevicesForUser("@bob:example.com", {
             Dynabook: bobDevice,
         });
+
         // Bob's device should be trusted again (but not TOFU)
-        expect(alice.checkUserTrust("@bob:example.com")).toBe(4);
-        expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(4);
+        const bobTrust3 = alice.checkUserTrust("@bob:example.com");
+        expect(bobTrust3.isVerified()).toBeTruthy();
+
+        const bobDeviceTrust4 = alice.checkDeviceTrust("@bob:example.com", "Dynabook");
+        expect(bobDeviceTrust4.isCrossSigningVerified()).toBeTruthy();
     });
 
     it("should offer to upgrade device verifications to cross-signing", async function() {
@@ -722,13 +775,17 @@ describe("Cross Signing", function() {
         await alice.resetCrossSigningKeys();
         await upgradePromise;
 
-        expect(alice.checkUserTrust("@bob:example.com")).toBe(6);
+        const bobTrust = alice.checkUserTrust("@bob:example.com");
+        expect(bobTrust.isCrossSigningVerified()).toBeTruthy();
+        expect(bobTrust.isTofu()).toBeTruthy();
 
         // "forget" that Bob is trusted
         delete alice._crypto._deviceList._crossSigningInfo["@bob:example.com"]
             .keys.master.signatures["@alice:example.com"];
 
-        expect(alice.checkUserTrust("@bob:example.com")).toBe(2);
+        const bobTrust2 = alice.checkUserTrust("@bob:example.com");
+        expect(bobTrust2.isCrossSigningVerified()).toBeFalsy();
+        expect(bobTrust2.isTofu()).toBeTruthy();
 
         upgradePromise = new Promise((resolve) => {
             upgradeResolveFunc = resolve;
@@ -736,6 +793,8 @@ describe("Cross Signing", function() {
         alice._crypto._deviceList.emit("userCrossSigningUpdated", "@bob:example.com");
         await upgradePromise;
 
-        expect(alice.checkUserTrust("@bob:example.com")).toBe(6);
+        const bobTrust3 = alice.checkUserTrust("@bob:example.com");
+        expect(bobTrust.isCrossSigningVerified()).toBeTruthy();
+        expect(bobTrust.isTofu()).toBeTruthy();
     });
 });

spec/unit/crypto/verification/sas.spec.js

@@ -318,9 +318,17 @@ describe("SAS verification", function() {
 
             await verifyProm;
 
-            expect(alice.client.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(1);
-            expect(bob.client.checkUserTrust("@alice:example.com")).toBe(6);
-            expect(bob.client.checkDeviceTrust("@alice:example.com", "Osborne2")).toBe(1);
+            const bobDeviceTrust = alice.client.checkDeviceTrust("@bob:example.com", "Dynabook");
+            expect(bobDeviceTrust.isLocallyVerified()).toBeTruthy();
+            expect(bobDeviceTrust.isCrossSigningVerified()).toBeFalsy();
+
+            const aliceTrust = bob.client.checkUserTrust("@alice:example.com");
+            expect(aliceTrust.isCrossSigningVerified()).toBeTruthy();
+            expect(aliceTrust.isTofu()).toBeTruthy();
+
+            const aliceDeviceTrust = bob.client.checkDeviceTrust("@alice:example.com", "Osborne2");
+            expect(aliceDeviceTrust.isLocallyVerified()).toBeTruthy();
+            expect(aliceDeviceTrust.isCrossSigningVerified()).toBeFalsy();
         });
     });
 

src/client.js

@@ -235,9 +235,8 @@ function keyFromRecoverySession(session, decryptionKey) {
  *   {string} request_id The ID of the request. Used to match a
  *     corresponding `crypto.secrets.request_cancelled`. The request ID will be
  *     unique per sender, device pair.
- *   {int} device_trust: The trust status of the device requesting
- *     the secret.  Will be a bit mask in the same form as returned by {@link
- *     module:client~MatrixClient#checkDeviceTrust}.
+ *   {DeviceTrustLevel} device_trust: The trust status of the device requesting
+ *     the secret as returned by {@link module:client~MatrixClient#checkDeviceTrust}.
  */
 function MatrixClient(opts) {
     opts.baseUrl = utils.ensureNoTrailingSlash(opts.baseUrl);
@@ -1004,16 +1003,7 @@ function wrapCryptoFuncs(MatrixClient, names) {
  * @function module:client~MatrixClient#checkUserTrust
  * @param {string} userId The ID of the user to check.
  *
- * @returns {integer} a bit mask indicating how the user is trusted (if at all)
- *  - returnValue & 1: unused
- *  - returnValue & 2: trust-on-first-use cross-signing key
- *  - returnValue & 4: user's cross-signing key is verified
- *
- * TODO: is this a good way of representing it?  Or we could return an object
- * with different keys, or a set?  The advantage of doing it this way is that
- * you can define which methods you want to use, "&" with the appopriate mask,
- * then test for truthiness.  Or if you want to just trust everything, then use
- * the value alone.  However, I wonder if bit masks are too obscure...
+ * @returns {UserTrustLevel}
  */
 
 /**
@@ -1024,12 +1014,7 @@ function wrapCryptoFuncs(MatrixClient, names) {
  * @param {string} userId The ID of the user whose devices is to be checked.
  * @param {string} deviceId The ID of the device to check
  *
- * @returns {integer} a bit mask indicating how the user is trusted (if at all)
- *  - returnValue & 1: device marked as verified
- *  - returnValue & 2: trust-on-first-use cross-signing key
- *  - returnValue & 4: user's cross-signing key is verified and device is signed
- *
- * TODO: see checkUserTrust
+ * @returns {DeviuceTrustLevel}
  */
 
 wrapCryptoFuncs(MatrixClient, [

src/crypto/CrossSigning.js

@@ -313,15 +313,15 @@ export class CrossSigningInfo extends EventEmitter {
         if (this.userId === userCrossSigning.userId
             && this.getId() && this.getId() === userCrossSigning.getId()
             && this.getId("self_signing")
-            && this.getId("self_signing") === userCrossSigning.getId("self_signing")) {
-            return CrossSigningVerification.VERIFIED
-                | (this.firstUse ? CrossSigningVerification.TOFU
-                   : CrossSigningVerification.UNVERIFIED);
+            && this.getId("self_signing") === userCrossSigning.getId("self_signing")
+        ) {
+            return new UserTrustLevel(true, this.firstUse);
         }
 
         if (!this.keys.user_signing) {
-            return (userCrossSigning.firstUse ? CrossSigningVerification.TOFU
-                    : CrossSigningVerification.UNVERIFIED);
+            // If there's no user signing key, they can't possibly be verified.
+            // They may be TOFU trusted though.
+            return new UserTrustLevel(false, userCrossSigning.firstUse);
         }
 
         let userTrusted;
@@ -333,28 +333,31 @@ export class CrossSigningInfo extends EventEmitter {
         } catch (e) {
             userTrusted = false;
         }
-        return (userTrusted ? CrossSigningVerification.VERIFIED
-                : CrossSigningVerification.UNVERIFIED)
-             | (userCrossSigning.firstUse ? CrossSigningVerification.TOFU
-                : CrossSigningVerification.UNVERIFIED);
+        return new UserTrustLevel(userTrusted, userCrossSigning.firstUse);
     }
 
-    checkDeviceTrust(userCrossSigning, device) {
+    checkDeviceTrust(userCrossSigning, device, localTrust) {
         const userTrust = this.checkUserTrust(userCrossSigning);
 
         const userSSK = userCrossSigning.keys.self_signing;
         if (!userSSK) {
-            return 0;
+            // if the user has no self-signing key then we cannot make any
+            // trust assertions about this device from cross-signing
+            return new DeviceTrustLevel(false, false, localTrust);
         }
+
         const deviceObj = deviceToObject(device, userCrossSigning.userId);
         try {
+            // if we can verify the user's SSK from their master key...
             pkVerify(userSSK, userCrossSigning.getId(), userCrossSigning.userId);
+            // ...and this device's key from their SSK...
             pkVerify(
                 deviceObj, publicKeyFromKeyInfo(userSSK)[1], userCrossSigning.userId,
             );
-            return userTrust;
+            // ...then we trust this device as much as far as we trust the user
+            return DeviceTrustLevel.fromUserTrustLevel(userTrust, localTrust);
         } catch (e) {
-            return 0;
+            return new DeviceTrustLevel(false, false, localTrust);
         }
     }
 }
@@ -377,8 +380,80 @@ export const CrossSigningLevel = {
     USER_SIGNING: 2,
 };
 
-export const CrossSigningVerification = {
-    UNVERIFIED: 0,
-    TOFU: 1,
-    VERIFIED: 2,
+/**
+ * Represents the ways in which we trust a user
+ */
+export class UserTrustLevel {
+    constructor(crossSigningVerified, tofu) {
+        this._crossSigningVerified = crossSigningVerified;
+        this._tofu = tofu;
+    }
+
+    /**
+     * @returns {bool} true if this user is verified via any means
+     */
+    isVerified() {
+        return this.isCrossSigningVerified();
+    }
+
+    /**
+     * @returns {bool} true if this user is verified via cross signing
+     */
+    isCrossSigningVerified() {
+        return this._crossSigningVerified;
+    }
+
+    /**
+     * @returns {bool} true if this user's key is trusted on first use
+     */
+    isTofu() {
+        return this._tofu;
+    }
+};
+
+/**
+ * Represents the ways in which we trust a device
+ */
+export class DeviceTrustLevel {
+    constructor(crossSigningVerified, tofu, localVerified) {
+        this._crossSigningVerified = crossSigningVerified;
+        this._tofu = tofu;
+        this._localVerified = localVerified;
+    }
+
+    static fromUserTrustLevel(userTrustLevel, localVerified) {
+        return new DeviceTrustLevel(
+            userTrustLevel._crossSigningVerified,
+            userTrustLevel._tofu,
+            localVerified,
+        );
+    }
+
+    /**
+     * @returns {bool} true if this user is verified via any means
+     */
+    isVerified() {
+        return this.isCrossSigningVerified() || this.isLocallyVerified();
+    }
+
+    /**
+     * @returns {bool} true if this user is verified via cross signing
+     */
+    isCrossSigningVerified() {
+        return this._crossSigningVerified;
+    }
+
+    /**
+     * @returns {bool} true if this user is verified via cross signing
+     */
+    isLocallyVerified() {
+        return this._localVerified;
+    }
+
+    /**
+     * @returns {bool} true if this user's key is trusted on first use
+     */
+    isTofu() {
+        return this._tofu;
+    }
 };

src/crypto/index.js

@@ -36,7 +36,7 @@ const DeviceInfo = require("./deviceinfo");
 const DeviceVerification = DeviceInfo.DeviceVerification;
 const DeviceList = require('./DeviceList').default;
 import { randomString } from '../randomstring';
-import { CrossSigningInfo } from './CrossSigning';
+import { CrossSigningInfo, UserTrustLevel, DeviceTrustLevel } from './CrossSigning';
 import SecretStorage from './Secrets';
 
 import OutgoingRoomKeyRequestManager from './OutgoingRoomKeyRequestManager';
@@ -411,8 +411,8 @@ Crypto.prototype._checkForDeviceVerificationUpgrade = async function(
 ) {
     // only upgrade if this is the first cross-signing key that we've seen for
     // them, and if their cross-signing key isn't already verified
-    if (crossSigningInfo.firstUse
-        && !(this._crossSigningInfo.checkUserTrust(crossSigningInfo) & 2)) {
+    const trustLevel = this._crossSigningInfo.checkUserTrust(crossSigningInfo);
+    if (crossSigningInfo.firstUse && !trustLevel.verified) {
         const devices = this._deviceList.getRawStoredDevicesForUser(userId);
         const deviceIds = await this._checkForValidDeviceSignature(
             userId, crossSigningInfo.keys.master, devices,
@@ -487,25 +487,14 @@ Crypto.prototype.getStoredCrossSigningForUser = function(userId) {
  *
  * @param {string} userId The ID of the user to check.
  *
- * @returns {integer} a bit mask indicating how the user is trusted (if at all)
- *  - returnValue & 1: unused
- *  - returnValue & 2: trust-on-first-use cross-signing key
- *  - returnValue & 4: user's cross-signing key is verified
- *
- * TODO: is this a good way of representing it?  Or we could return an object
- * with different keys, or a set?  The advantage of doing it this way is that
- * you can define which methods you want to use, "&" with the appopriate mask,
- * then test for truthiness.  Or if you want to just trust everything, then use
- * the value alone.  However, I wonder if bit masks are too obscure...
+ * @returns {UserTrustLevel}
  */
 Crypto.prototype.checkUserTrust = function(userId) {
     const userCrossSigning = this._deviceList.getStoredCrossSigningForUser(userId);
     if (!userCrossSigning) {
-        return 0;
+        return new UserTrustLevel(false, false);
     }
-    // We shift the result from CrossSigningInfo.checkUserTrust so this
-    // function's return is consistent with checkDeviceTrust
-    return this._crossSigningInfo.checkUserTrust(userCrossSigning) << 1;
+    return this._crossSigningInfo.checkUserTrust(userCrossSigning);
 };
 
 /**
@@ -514,25 +503,20 @@ Crypto.prototype.checkUserTrust = function(userId) {
  * @param {string} userId The ID of the user whose devices is to be checked.
  * @param {string} deviceId The ID of the device to check
  *
- * @returns {integer} a bit mask indicating how the user is trusted (if at all)
- *  - returnValue & 1: device marked as verified
- *  - returnValue & 2: trust-on-first-use cross-signing key
- *  - returnValue & 4: user's cross-signing key is verified and device is signed
- *
- * TODO: see checkUserTrust
+ * @returns {DeviceTrustLevel}
  */
 Crypto.prototype.checkDeviceTrust = function(userId, deviceId) {
-    let rv = 0;
-
     const device = this._deviceList.getStoredDevice(userId, deviceId);
-    if (device && device.isVerified()) {
-        rv |= 1;
-    }
+    const trustedLocally = device && device.isVerified();
+
     const userCrossSigning = this._deviceList.getStoredCrossSigningForUser(userId);
     if (device && userCrossSigning) {
-        rv |= this._crossSigningInfo.checkDeviceTrust(userCrossSigning, device) << 1;
+        return this._crossSigningInfo.checkDeviceTrust(
+            userCrossSigning, device, trustedLocally,
+        );
+    } else {
+        return new DeviceTrustLevel(false, false, trustedLocally);
     }
-    return rv;
 };
 
 /*