From e09038232e367e001ba70d099cfa7c87a2ed024f Mon Sep 17 00:00:00 2001 From: David Baker Date: Sat, 25 Jan 2020 19:42:02 +0000 Subject: [PATCH 1/2] Fix bootstrap cleanup As hopefully explained in the comment. The symptom of this was that bootstrapping would work just fine the first time you called it in any run of the app, but then if called a second time (eg. if you cancelled by dismissing the password prompt) it would create keys and upload the public parts but not store the private parts in SSSS, leaving you with cross signing keys you don't have the private parts of. Also use object.assign in the save keys callback just in case we ever reset a subset of the keys (and also because it makes it a bit simpler to reason about what objects are where). --- src/crypto/index.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/crypto/index.js b/src/crypto/index.js index a81243f75..c9df7d10a 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -375,7 +375,7 @@ Crypto.prototype.bootstrapSecretStorage = async function({ "creating new keys", ); this._baseApis._cryptoCallbacks.saveCrossSigningKeys = - keys => crossSigningPrivateKeys = keys; + keys => Object.assign(crossSigningPrivateKeys, keys); this._baseApis._cryptoCallbacks.getCrossSigningKey = name => crossSigningPrivateKeys[name]; await this.resetCrossSigningKeys( @@ -468,7 +468,15 @@ Crypto.prototype.bootstrapSecretStorage = async function({ } } } finally { - this._baseApis._cryptoCallbacks = appCallbacks; + // Restore the original callbacks. NB. we must do this by manipulating + // the same object since the CrossSigning class has a reference to the + // object, so if we assign the object here then our callbacks will change + // but the instances of the CrossSigning class will be left with our + // random, otherwise dead closures. + for (const cb of Object.keys(this._baseApis._cryptoCallbacks)) { + delete this._baseApis._cryptoCallbacks[cb]; + } + Object.assign(this._baseApis._cryptoCallbacks, appCallbacks); } logger.log("Secure Secret Storage ready"); From 49fd9e90a0d004f57f5a94a66e8d7f03fd8e2d5d Mon Sep 17 00:00:00 2001 From: David Baker Date: Sat, 25 Jan 2020 19:48:36 +0000 Subject: [PATCH 2/2] this can be const now --- src/crypto/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crypto/index.js b/src/crypto/index.js index c9df7d10a..6f610e7b9 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -351,7 +351,7 @@ Crypto.prototype.bootstrapSecretStorage = async function({ // key with the cross-signing master key. The cross-signing master key is also used // to verify the signature on the SSSS default key when adding secrets, so we // effectively need it for both reading and writing secrets. - let crossSigningPrivateKeys = {}; + const crossSigningPrivateKeys = {}; // If we happen to reset cross-signing keys here, then we want access to the // cross-signing private keys, but only for the scope of this method, so we