Rust-crypto: fix bootstrapCrossSigning on second call
#3912
Conversation
| @@ -66,18 +66,25 @@ export class CrossSigningIdentity { | |||
| }); | |||
|
|
|||
| if (olmDeviceHasKeys) { | |||
| if (!privateKeysInSecretStorage) { | |||
| if (!(await this.secretStorage.hasKey())) { | |||
There was a problem hiding this comment.
this is the real fix in this PR: we need to check that secretStorage is set up before we try to export to it.
| this.secretStorage.store("m.cross_signing.master", exported.masterKey); | ||
| await this.secretStorage.store("m.cross_signing.master", exported.masterKey); | ||
| } else { | ||
| logger.error(`Cannot export MSK to secret storage, private key unknown`); | ||
| } | ||
| if (exported?.self_signing_key) { | ||
| this.secretStorage.store("m.cross_signing.self_signing", exported.self_signing_key); | ||
| await this.secretStorage.store("m.cross_signing.self_signing", exported.self_signing_key); | ||
| } else { | ||
| logger.error(`Cannot export SSK to secret storage, private key unknown`); | ||
| } | ||
| if (exported?.userSigningKey) { | ||
| this.secretStorage.store("m.cross_signing.user_signing", exported.userSigningKey); | ||
| await this.secretStorage.store("m.cross_signing.user_signing", exported.userSigningKey); | ||
| } else { | ||
| logger.error(`Cannot export USK to secret storage, private key unknown`); | ||
| } |
There was a problem hiding this comment.
these things were missing their await keywords, which meant that when they failed they were shouting into the void.
Currently, `bootstrapCrossSigning` raises an exception if it is called a second time before secret storage is set up. It is easily fixed by checking that 4S is set up before trying to export to 4S. Also a few logging fixes while we're in the area.
richvdh
force-pushed
the
rav/element-r/fix_bootstrap_cross_signing
branch
from
0ef8201
to
4e5915e
Compare
|
will add some more tests |
| const authDict = { type: "test" }; | ||
| await bootstrapCrossSigning(authDict); | ||
|
|
||
| // a second call should do nothing except GET requests |
There was a problem hiding this comment.
It does GET /user/:user/account_data/:type requests, because this test is running before the initialsync completes (so it has not cached any account data in memory).
Would you find this useful to include in the comment?
There was a problem hiding this comment.
No maybe not. Was trying to remember why it would do requests.
Looks like it's checking if it's in 4S or not to eventually update 4S or import. There are some comments about it in bootstrapCrossSigning. I guess the naming is not perfect, maybe it's ensureCrossSigning but also fix it :/
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
Test flake filed as element-hq/element-web#26679 |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Currently,
bootstrapCrossSigningraises an exception if it is called a second time before secret storage is set up. It is easily fixed by checking that 4S is set up before trying to export to 4S.Also a few logging fixes while we're in the area.
Fixes element-hq/element-web#26393
Here's what your changelog entry will look like:
🐛 Bug Fixes
bootstrapCrossSigningon second call (#3912). Fixes Element-R: Exceptions when setting up backup/cross-signing/etc on existing account with no 4S element-hq/element-web#26393.