various cross-signing fixes and improvements

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

@@ -50,11 +50,16 @@ describe("Cross Signing", function() {
         const alice = await makeTestClient(
             {userId: "@alice:example.com", deviceId: "Osborne2"},
         );
+        alice.uploadDeviceSigningKeys = async function(e) {return;};
+        alice.uploadKeySignatures = async function(e) {return;};
         // set Alice's cross-signing key
         let privateKeys;
         alice.on("cross-signing:savePrivateKeys", function(e) {
             privateKeys = e;
         });
+        alice.on("cross-signing:getKey", function(e) {
+            e.done(privateKeys[e.type]);
+        });
         await alice.resetCrossSigningKeys();
         // Alice downloads Bob's device key
         alice._crypto._deviceList.storeCrossSigningForUser("@bob:example.com", {
@@ -69,10 +74,6 @@ describe("Cross Signing", function() {
             },
         });
         // Alice verifies Bob's key
-        alice.on("cross-signing:getKey", function(e) {
-            expect(e.type).toBe("user_signing");
-            e.done(privateKeys.user_signing);
-        });
         const promise = new Promise((resolve, reject) => {
             alice.uploadKeySignatures = (...args) => {
                 resolve(...args);
@@ -110,6 +111,10 @@ describe("Cross Signing", function() {
         alice.once("cross-signing:newKey", (e) => {
             e.done(masterKey);
         });
+        alice.on("cross-signing:getKey", (e) => {
+            // will be called to sign our own device
+            e.done(selfSigningKey);
+        });
 
         const deviceInfo = alice._crypto._deviceList._devices["@alice:example.com"]
             .Osborne2;
@@ -198,8 +203,13 @@ describe("Cross Signing", function() {
                     },
                 },
             },
+            {
+                method: "POST",
+                path: "/keys/signatures/upload",
+                data: {},
+            },
         ];
-        setHttpResponses(alice, responses);
+        setHttpResponses(alice, responses, true, true);
 
         await alice.startClient();
 
@@ -213,11 +223,16 @@ describe("Cross Signing", function() {
         const alice = await makeTestClient(
             {userId: "@alice:example.com", deviceId: "Osborne2"},
         );
+        alice.uploadDeviceSigningKeys = async function(e) {return;};
+        alice.uploadKeySignatures = async function(e) {return;};
         // set Alice's cross-signing key
         let privateKeys;
         alice.on("cross-signing:savePrivateKeys", function(e) {
             privateKeys = e;
         });
+        alice.on("cross-signing:getKey", function(e) {
+            e.done(privateKeys[e.type]);
+        });
         await alice.resetCrossSigningKeys();
         // Alice downloads Bob's ssk and device key
         const bobMasterSigning = new global.Olm.PkSigning();
@@ -275,10 +290,6 @@ describe("Cross Signing", function() {
         expect(alice.checkUserTrust("@bob:example.com")).toBe(2);
         expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(2);
         // Alice verifies Bob's SSK
-        alice.on("cross-signing:getKey", function(e) {
-            expect(e.type).toBe("user_signing");
-            e.done(privateKeys.user_signing);
-        });
         alice.uploadKeySignatures = () => {};
         await alice.setDeviceVerified("@bob:example.com", bobMasterPubkey, true);
         // Bob's device key should be trusted
@@ -292,17 +303,18 @@ describe("Cross Signing", function() {
         );
         alice._crypto._deviceList.startTrackingDeviceList("@bob:example.com");
         alice._crypto._deviceList.stopTrackingAllDeviceLists = () => {};
+        alice.uploadDeviceSigningKeys = async function(e) {return;};
+        alice.uploadKeySignatures = async function(e) {return;};
 
         // set Alice's cross-signing key
         let privateKeys;
         alice.on("cross-signing:savePrivateKeys", function(e) {
             privateKeys = e;
         });
-        await alice.resetCrossSigningKeys();
+        alice.on("cross-signing:getKey", (e) => {
-
-        alice.once("cross-signing:getKey", (e) => {
             e.done(privateKeys[e.type]);
         });
+        await alice.resetCrossSigningKeys();
 
         const selfSigningKey = new Uint8Array([
             0x1e, 0xf4, 0x01, 0x6d, 0x4f, 0xa1, 0x73, 0x66,
@@ -450,11 +462,16 @@ describe("Cross Signing", function() {
         const alice = await makeTestClient(
             {userId: "@alice:example.com", deviceId: "Osborne2"},
         );
+        alice.uploadDeviceSigningKeys = async function(e) {return;};
+        alice.uploadKeySignatures = async function(e) {return;};
         // set Alice's cross-signing key
         let privateKeys;
         alice.on("cross-signing:savePrivateKeys", function(e) {
             privateKeys = e;
         });
+        alice.on("cross-signing:getKey", (e) => {
+            e.done(privateKeys[e.type]);
+        });
         await alice.resetCrossSigningKeys();
         // Alice downloads Bob's ssk and device key
         // (NOTE: device key is not signed by ssk)
@@ -506,12 +523,9 @@ describe("Cross Signing", function() {
         // Bob's device key should be untrusted
         expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(0);
         // Alice verifies Bob's SSK
-        alice.on("cross-signing:getKey", function(e) {
+        console.log("verifying bob's device");
-            expect(e.type).toBe("user_signing");
-            e.done(privateKeys.user_signing);
-        });
-        alice.uploadKeySignatures = () => {};
         await alice.setDeviceVerified("@bob:example.com", bobMasterPubkey, true);
+        console.log("done");
         // Bob's device key should be untrusted
         expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(0);
     });
@@ -520,10 +534,15 @@ describe("Cross Signing", function() {
         const alice = await makeTestClient(
             {userId: "@alice:example.com", deviceId: "Osborne2"},
         );
+        alice.uploadDeviceSigningKeys = async function(e) {return;};
+        alice.uploadKeySignatures = async function(e) {return;};
         let privateKeys;
         alice.on("cross-signing:savePrivateKeys", function(e) {
             privateKeys = e;
         });
+        alice.on("cross-signing:getKey", function(e) {
+            e.done(privateKeys[e.type]);
+        });
         await alice.resetCrossSigningKeys();
         // Alice downloads Bob's keys
         const bobMasterSigning = new global.Olm.PkSigning();
@@ -577,10 +596,6 @@ describe("Cross Signing", function() {
             Dynabook: bobDevice,
         });
         // Alice verifies Bob's SSK
-        alice.on("cross-signing:getKey", function(e) {
-            expect(e.type).toBe("user_signing");
-            e.done(privateKeys.user_signing);
-        });
         alice.uploadKeySignatures = () => {};
         await alice.setDeviceVerified("@bob:example.com", bobMasterPubkey, true);
         // Bob's device key should be trusted
@@ -624,8 +639,7 @@ describe("Cross Signing", function() {
         expect(alice.checkDeviceTrust("@bob:example.com", "Dynabook")).toBe(0);
         // Alice verifies Bob's SSK
         alice.on("cross-signing:getKey", function(e) {
-            expect(e.type).toBe("user_signing");
+            e.done(privateKeys[e.type]);
-            e.done(privateKeys.user_signing);
         });
         alice.uploadKeySignatures = () => {};
         await alice.setDeviceVerified("@bob:example.com", bobMasterPubkey2, true);

src/base-apis.js

@@ -1676,9 +1676,10 @@ MatrixBaseApis.prototype.getKeyChanges = function(oldToken, newToken) {
     );
 };
 
-MatrixBaseApis.prototype.uploadDeviceSigningKeys = function(keys) {
+MatrixBaseApis.prototype.uploadDeviceSigningKeys = function(auth, keys) {
+    const data = Object.assign({}, keys, {auth});
     return this._http.authedRequestWithPrefix(
-        undefined, "POST", "/keys/device_signing/upload", undefined, keys,
+        undefined, "POST", "/keys/device_signing/upload", undefined, data,
         httpApi.PREFIX_UNSTABLE,
     );
 };

src/client.js

@@ -559,6 +559,9 @@ MatrixClient.prototype.initCrypto = async function() {
         "crypto.roomKeyRequest",
         "crypto.roomKeyRequestCancellation",
         "crypto.warning",
+        "crypto.devicesUpdated",
+        "cross-signing:savePrivateKeys",
+        "cross-signing:getKey",
     ]);
 
     logger.log("Crypto: initialising crypto object...");
@@ -795,27 +798,34 @@ MatrixClient.prototype.getGlobalBlacklistUnverifiedDevices = function() {
 };
 
 /**
- * returns a function that just calls the corresponding function from this._crypto.
+ * add methods that call the corresponding method in this._crypto
  *
- * @param {string} name the function to call
+ * @param {class} MatrixClient the class to add the method to
- *
+ * @param {string} names the names of the methods to call
- * @return {Function} a wrapper function
  */
-function wrapCryptoFunc(name) {
+function wrapCryptoFuncs(MatrixClient, names) {
-    return function(...args) {
+    for (const name of names) {
-        if (!this._crypto) { // eslint-disable-line no-invalid-this
+        MatrixClient.prototype[name] = function(...args) {
-            throw new Error("End-to-end encryption disabled");
+            if (!this._crypto) { // eslint-disable-line no-invalid-this
-        }
+                throw new Error("End-to-end encryption disabled");
+            }
 
-        return this._crypto[name](...args); // eslint-disable-line no-invalid-this
+            return this._crypto[name](...args); // eslint-disable-line no-invalid-this
-    };
+        };
+    }
 }
 
-MatrixClient.prototype.checkUserTrust
+wrapCryptoFuncs(MatrixClient, [
-    = wrapCryptoFunc("checkUserTrust");
+    "checkUserTrust",
+    "checkDeviceTrust",
+]);
 
-MatrixClient.prototype.checkDeviceTrust
+wrapCryptoFuncs(MatrixClient, [
-    = wrapCryptoFunc("checkDeviceTrust");
+    "storeSecret",
+    "getSecret",
+    "isSecretStored",
+    "requestSecret",
+]);
 
 /**
  * Get e2e information on the device that sent an event
@@ -848,14 +858,11 @@ MatrixClient.prototype.isEventSenderVerified = async function(event) {
     return device.isVerified();
 };
 
-MatrixClient.prototype.resetCrossSigningKeys
+wrapCryptoFuncs(MatrixClient, [
-    = wrapCryptoFunc("resetCrossSigningKeys");
+    "resetCrossSigningKeys",
-
+    "getCrossSigningId",
-MatrixClient.prototype.setCrossSigningKeys
+    "getStoredCrossSigningForUser",
-    = wrapCryptoFunc("setCrossSigningKeys");
+]);
-
-MatrixClient.prototype.getCrossSigningId
-    = wrapCryptoFunc("getCrossSigningId");
 
 /**
  * Cancel a room key request for this event if one is ongoing and resend the

src/crypto/CrossSigning.js

@@ -195,8 +195,13 @@ export class CrossSigningInfo extends EventEmitter {
                 logger.error(error);
                 throw new Error(error);
             }
-            // First-Use is true if and only if we had no previous key for the user
+            if (!this.keys.master) {
-            this.fu = !(this.keys.self_signing);
+                // this is the first key we've seen, so first-use is true
+                this.fu = true;
+            } else if (getPublicKey(keys.master)[1] !== this.getId()) {
+                // this is a different key, so first-use is false
+                this.fu = false;
+            } // otherwise, same key, so no change
             signingKeys.master = keys.master;
         } else if (this.keys.master) {
             signingKeys.master = this.keys.master;
@@ -254,7 +259,11 @@ export class CrossSigningInfo extends EventEmitter {
     }
 
     async signUser(key) {
+        if (!this.keys.user_signing) {
+            return;
+        }
         const [pubkey, usk] = await getPrivateKey(this, "user_signing", (key) => {
+            // FIXME:
             return;
         });
         try {
@@ -268,9 +277,15 @@ export class CrossSigningInfo extends EventEmitter {
 
     async signDevice(userId, device) {
         if (userId !== this.userId) {
-            throw new Error("Urgh!");
+            throw new Error(
+                `Trying to sign ${userId}'s device; can only sign our own device`,
+            );
+        }
+        if (!this.keys.self_signing) {
+            return;
         }
         const [pubkey, ssk] = await getPrivateKey(this, "self_signing", (key) => {
+            // FIXME:
             return;
         });
         try {
@@ -288,6 +303,8 @@ export class CrossSigningInfo extends EventEmitter {
     }
 
     checkUserTrust(userCrossSigning) {
+        // if we're checking our own key, then it's trusted if the master key
+        // and self-signing key match
         if (this.userId === userCrossSigning.userId
             && this.getId() && this.getId() === userCrossSigning.getId()
             && this.getId("self_signing")
@@ -298,7 +315,8 @@ export class CrossSigningInfo extends EventEmitter {
         }
 
         if (!this.keys.user_signing) {
-            return 0;
+            return (userCrossSigning.fu ? CrossSigningVerification.TOFU
+                    : CrossSigningVerification.UNVERIFIED);
         }
 
         let userTrusted;

src/crypto/DeviceList.js

@@ -655,6 +655,7 @@ export default class DeviceList extends EventEmitter {
                 }
             });
             this.saveIfDirty();
+            this.emit("crypto.devicesUpdated", users);
         };
 
         return prom;

src/crypto/Secrets.js

@@ -17,6 +17,9 @@ limitations under the License.
 import {EventEmitter} from 'events';
 import logger from '../logger';
 import olmlib from './olmlib';
+import { randomString } from '../randomstring';
+import { keyForNewBackup } from './backup_password';
+import { encodeRecoveryKey, decodeRecoveryKey } from './recoverykey';
 
 /** Implements MSC-1946
  */
@@ -28,6 +31,56 @@ export default class SecretStorage extends EventEmitter {
         this._incomingRequests = {};
     }
 
+    async addKey(type, opts) {
+        const keyData = {
+            algorithm: opts.algorithm,
+        };
+
+        switch (opts.algorithm) {
+        case "m.secret_storage.v1.curve25519-aes-sha2":
+        {
+            const decryption = new global.Olm.PkDecryption();
+            try {
+                if (opts.passphrase) {
+                    const key = await keyForNewBackup(opts.passphrase);
+                    keyData.passphrase = {
+                        algorithm: "m.pbkdf2",
+                        iterations: key.iterations,
+                        salt: key.salt,
+                    };
+                    opts.encodedkey = encodeRecoveryKey(key.key);
+                    keyData.pubkey = decryption.init_with_private_key(key.key);
+                } else if (opts.privkey) {
+                    keyData.pubkey = decryption.init_with_private_key(opts.privkey);
+                    opts.encodedkey = encodeRecoveryKey(opts.privkey);
+                } else {
+                    keyData.pubkey = decryption.generate_key();
+                    opts.encodedkey = encodeRecoveryKey(decryption.get_private_key());
+                }
+            } finally {
+                decryption.free();
+            }
+            break;
+        }
+        default:
+            throw new Error(`Unknown key algorithm ${opts.algorithm}`);
+        }
+
+        let keyName;
+
+        do {
+            keyName = randomString(32);
+        } while (!this._baseApis.getAccountData(`m.secret_storage.key.${keyName}`));
+
+        // FIXME: sign keyData?
+
+        await this._baseApis.setAccountData(
+            `m.secret_storage.key.${keyName}`, keyData,
+        );
+
+        return keyName;
+    }
+
     /** store an encrypted secret on the server
      *
      * @param {string} name The name of the secret
@@ -285,7 +338,11 @@ export default class SecretStorage extends EventEmitter {
                 });
             }
         } else if (content.action === "request") {
-            // if from us and device is trusted (or else check trust)
+            if (deviceId === this._baseApis.deviceId) {
+                // no point in trying to send ourself the secret
+                return;
+            }
+
             // check if we have the secret
             logger.info("received request for secret (" + sender
                         + ", " + deviceId + ", " + content.request_id + ")");
@@ -308,6 +365,15 @@ export default class SecretStorage extends EventEmitter {
                         sender_key: this._baseApis._crypto._olmDevice.deviceCurve25519Key,
                         ciphertext: {},
                     };
+                    await olmlib.ensureOlmSessionsForDevices(
+                        this._baseApis._crypto._olmDevice,
+                        this._baseApis,
+                        {
+                            [sender]: [
+                                await this._baseApis.getStoredDevice(sender, deviceId),
+                            ],
+                        },
+                    );
                     await olmlib.encryptMessageForDevice(
                         encryptedContent.ciphertext,
                         this._baseApis.getUserId(),

src/crypto/index.js

@@ -25,6 +25,7 @@ limitations under the License.
 const anotherjson = require('another-json');
 import Promise from 'bluebird';
 import {EventEmitter} from 'events';
+import ReEmitter from '../ReEmitter';
 
 import logger from '../logger';
 const utils = require("../utils");
@@ -107,6 +108,7 @@ export default function Crypto(baseApis, sessionStore, userId, deviceId,
     clientStore, cryptoStore, roomList, verificationMethods) {
     this._onDeviceListUserCrossSigningUpdated = this._onDeviceListUserCrossSigningUpdated.bind(this);
 
+    this._reEmitter = new ReEmitter(this);
     this._baseApis = baseApis;
     this._sessionStore = sessionStore;
     this._userId = userId;
@@ -148,6 +150,7 @@ export default function Crypto(baseApis, sessionStore, userId, deviceId,
     // XXX: This isn't removed at any point, but then none of the event listeners
     // this class sets seem to be removed at any point... :/
     this._deviceList.on('userCrossSigningUpdated', this._onDeviceListUserCrossSigningUpdated);
+    this._reEmitter.reEmit(this._deviceList, ["crypto.devicesUpdated"]);
 
     // the last time we did a check for the number of one-time-keys on the
     // server.
@@ -200,18 +203,35 @@ export default function Crypto(baseApis, sessionStore, userId, deviceId,
     this._verificationTransactions = new Map();
 
     this._crossSigningInfo = new CrossSigningInfo(userId);
-    this._crossSigningInfo.on("cross-signing:savePrivateKeys", (...args) => {
+    this._reEmitter.reEmit(this._crossSigningInfo, [
-        this._baseApis.emit("cross-signing:savePrivateKeys", ...args);
+        "cross-signing:savePrivateKeys",
-    });
+        "cross-signing:getKey",
-    this._crossSigningInfo.on("cross-signing:getKey", (...args) => {
+    ]);
-        this._baseApis.emit("cross-signing:getKey", ...args);
-    });
 
     this._secretStorage = new SecretStorage(baseApis);
     // TODO: expose SecretStorage methods
 }
 utils.inherits(Crypto, EventEmitter);
 
+Crypto.prototype.storeSecret = function(name, secret, keys) {
+    return this._secretStorage.store(name, secret, keys);
+};
+
+Crypto.prototype.getSecret = function(name) {
+    return this._secretStorage.get(name);
+};
+
+Crypto.prototype.isSecretStored = function(name, checkKey) {
+    return this._secretStorage.isStored(name, checkKey);
+};
+
+Crypto.prototype.requestSecret = function(name, devices) {
+    if (!devices) {
+        devices = Object.keys(this._deviceList.getRawStoredDevicesForUser(this._userId));
+    }
+    return this._secretStorage.request(name, devices);
+};
+
 /**
  * Initialise the crypto module so that it is ready for use
  *
@@ -271,19 +291,22 @@ Crypto.prototype.init = async function() {
  * keys will be created for the given level and below.  Defaults to
  * regenerating all keys.
  */
-Crypto.prototype.resetCrossSigningKeys = async function(level) {
+Crypto.prototype.resetCrossSigningKeys = async function(authDict, level) {
     await this._crossSigningInfo.resetKeys(level);
+    const keys = {};
+    for (const [name, key] of Object.entries(this._crossSigningInfo.keys)) {
+        keys[name + "_key"] = key;
+    }
+    await this._baseApis.uploadDeviceSigningKeys(authDict || {}, keys);
     this._baseApis.emit("cross-signing:keysChanged", {});
-};
 
-/**
+    const device = this._deviceList.getStoredDevice(this._userId, this._deviceId);
- * Set the user's cross-signing keys to use.
+    const signedDevice = await this._crossSigningInfo.signDevice(this._userId, device);
- *
+    await this._baseApis.uploadKeySignatures({
- * @param {object} keys A mapping of key type to key data.
+        [this._userId]: {
- */
+            [this._deviceId]: signedDevice,
-Crypto.prototype.setCrossSigningKeys = function(keys) {
+        },
-    this._crossSigningInfo.setKeys(keys);
+    });
-    this._baseApis.emit("cross-signing:keysChanged", {});
 };
 
 /**
@@ -298,6 +321,10 @@ Crypto.prototype.getCrossSigningId = function(type) {
     return this._crossSigningInfo.getId(type);
 };
 
+Crypto.prototype.getStoredCrossSigningForUser = function(userId) {
+    return this._deviceList.getStoredCrossSigningForUser(userId);
+};
+
 /**
  * Check whether a given user is trusted.
  *
@@ -419,19 +446,29 @@ Crypto.prototype.checkOwnCrossSigningTrust = async function() {
         logger.info("Got private key");
     }
 
-    // FIXME: fetch the private key?
+    const oldSelfSigningId = this._crossSigningInfo.getId("self_signing");
-    if (this._crossSigningInfo.getId("self_signing")
+    const oldUserSigningId = this._crossSigningInfo.getId("user_signing")
-        !== newCrossSigning.getId("self_signing")) {
-        logger.info("Got new self-signing key", newCrossSigning.getId("self_signing"));
-    }
-    if (this._crossSigningInfo.getId("user_signing")
-        !== newCrossSigning.getId("user_signing")) {
-        logger.info("Got new user-signing key", newCrossSigning.getId("user_signing"));
-    }
 
     this._crossSigningInfo.setKeys(newCrossSigning.keys);
     // FIXME: save it ... somewhere?
 
+    if (oldSelfSigningId !== newCrossSigning.getId("self_signing")) {
+        logger.info("Got new self-signing key", newCrossSigning.getId("self_signing"));
+
+        const device = this._deviceList.getStoredDevice(this._userId, this._deviceId);
+        const signedDevice = await this._crossSigningInfo.signDevice(
+            this._userId, device,
+        );
+        await this._baseApis.uploadKeySignatures({
+            [this._userId]: {
+                [this._deviceId]: signedDevice,
+            },
+        });
+    }
+    if (oldUserSigningId !== newCrossSigning.getId("user_signing")) {
+        logger.info("Got new user-signing key", newCrossSigning.getId("user_signing"));
+    }
+
     if (changed) {
         this._baseApis.emit("cross-signing:keysChanged", {});
     }
@@ -1062,12 +1099,13 @@ Crypto.prototype.setDeviceVerification = async function(
     if (xsk && xsk.getId() === deviceId) {
         if (verified) {
             const device = await this._crossSigningInfo.signUser(xsk);
-            // FIXME: mark xsk as dirty in device list
+            if (device) {
-            this._baseApis.uploadKeySignatures({
+                this._baseApis.uploadKeySignatures({
-                [userId]: {
+                    [userId]: {
-                    [deviceId]: device,
+                        [deviceId]: device,
-                },
+                    },
-            });
+                });
+            }
             return device;
         } else {
             // FIXME: ???
@@ -1109,13 +1147,16 @@ Crypto.prototype.setDeviceVerification = async function(
 
     // do cross-signing
     if (verified && userId === this._userId) {
-        const device = await this._crossSigningInfo.signDevice(userId, dev);
+        const device = await this._crossSigningInfo.signDevice(
-        // FIXME: mark device as dirty in device list
+            userId, DeviceInfo.fromStorage(dev, deviceId),
-        this._baseApis.uploadKeySignatures({
+        );
-            [userId]: {
+        if (device) {
-                [deviceId]: device,
+            this._baseApis.uploadKeySignatures({
-            },
+                [userId]: {
-        });
+                    [deviceId]: device,
+                },
+            });
+        }
     }
 
     return DeviceInfo.fromStorage(dev, deviceId);