Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Unable to restore session" error due due to half-initialised legacy indexeddb crypto store #27447

Closed
Tracked by #27490
richvdh opened this issue May 7, 2024 · 1 comment · Fixed by matrix-org/matrix-js-sdk#4218
Assignees
Labels
A-E2EE A-Element-R Issues affecting the port of Element's crypto layer to Rust A-Storage Storage layer of the app, including IndexedDB, local storage, etc. O-Occasional Affects or can be seen by some users regularly or most users rarely S-Critical Prevents work, causes data loss and/or has no workaround T-Defect Z-Element-R-Blocker A blocker for enabling Element R by default

Comments

@richvdh
Copy link
Member

richvdh commented May 7, 2024

We've seen a number of reports from users who are suddenly unable to load their Element-Web (or Desktop) sessions due to an "Unable to restore session" error.

The common theme appears to be that they have a half-initialised matrix-js-sdk:crypto indexeddb store. This makes the Rust crypto code think that it has to do a migration, but there is no actual data to migrate, so it fails.

It's unclear to me what, exactly, is causing the matrix-js-sdk:crypto indexeddb store to be created. I think the most likely cause is something calling matrix.createClient or matrix.createRoomWidgetClient without an explicit cryptoStore; in that case amendClientOpts uses the default cryptoStoreFactory which creates an empty matrix-js-sdk:crypto indexeddb store. [Ironically all this code will go away once we finally rip out the legacy crypto code (#26922), but for now, we're a bit stuck with it.]

What, exactly, is calling matrix.createClient or matrix.createRoomWidgetClient without a crypto store is unknown to me. There are lots of such calls in the react SDK, but they all seem to relate to auth operations, or possibly stuff to do with the ModuleApi, which I don't see much evidence of in the rageshakes. Alternatively, it might be a completely different application on the same domain which happens to use the js-sdk.

I think the best course of action here is to be more conservative in what triggers a migration. In particular, if there is no account.- key in the store, then we should not attempt a migration.

@richvdh richvdh added T-Defect S-Critical Prevents work, causes data loss and/or has no workaround O-Occasional Affects or can be seen by some users regularly or most users rarely A-Element-R Issues affecting the port of Element's crypto layer to Rust Z-Element-R-Blocker A blocker for enabling Element R by default labels May 7, 2024
@richvdh
Copy link
Member Author

richvdh commented May 7, 2024

I think this should be a blocker for further rollout of Element-R

@dosubot dosubot bot added A-E2EE A-Storage Storage layer of the app, including IndexedDB, local storage, etc. labels May 7, 2024
@andybalaam andybalaam self-assigned this May 22, 2024
andybalaam added a commit to matrix-org/matrix-js-sdk that referenced this issue May 24, 2024
andybalaam added a commit to matrix-org/matrix-js-sdk that referenced this issue May 24, 2024
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this issue May 28, 2024
* Don't run migration for Rust crypto if the legacy store is empty

Fixes element-hq/element-web#27447

* Add copyright for the TypeScript files in legacy DB dumps

* Provide a type for the accountPickle we check for before migration

* Remove redundant backup response

This is unused

* Simplify keys response

* Downgrade log message.

---------

Co-authored-by: Richard van der Hoff <[email protected]>
thoraj added a commit to verji/matrix-js-sdk that referenced this issue Aug 15, 2024
* Preserve ESM for async imports to work correctly (matrix-org#4187)

* fix: fix lazy rust crypto import

* test: use "commonjs" for tests because of circular deps

* chore: revert commonjs for "module"

* refactor: remove unnecessary example

* refactor: add comments

Signed-off-by: Bayyr Oorjak <[email protected]>

* refactor: improve comment

Signed-off-by: Bayyr Oorjak <[email protected]>

Co-authored-by: Richard van der Hoff <[email protected]>

* Update babel.config.js

---------

Signed-off-by: Bayyr Oorjak <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>

* Update organization (matrix-org#4212)

* v32.4.0

* Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0 (matrix-org#4214)

* Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0

* Disable affected test

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>

* `initRustCrypto`: allow app to pass in the store key directly (matrix-org#4210)

* `initRustCrypto`: allow app to pass in the store key directly

... instead of using the pickleKey. This allows us to avoid a slow PBKDF
operation.

* Fix link in doc-comment

* Bump matrix-sdk-crypto-wasm to 5.0.0 (matrix-org#4216)

Slightly more involved than normal because it requires us to pass a backup version into OlmMachine.importBackedUpRoomKeys.

On the other hand we can now re-enable the test that was disabled in matrix-org#4214 due to matrix-org/matrix-rust-sdk#3447

Fixes: element-hq/element-web#27165

* Remove more deprecated methods, fields, and exports (matrix-org#4217)

* Don't run migration for Rust crypto if the legacy store is empty (matrix-org#4218)

* Don't run migration for Rust crypto if the legacy store is empty

Fixes element-hq/element-web#27447

* Add copyright for the TypeScript files in legacy DB dumps

* Provide a type for the accountPickle we check for before migration

* Remove redundant backup response

This is unused

* Simplify keys response

* Downgrade log message.

---------

Co-authored-by: Richard van der Hoff <[email protected]>

* v33.0.0-rc.0

* v33.0.0

---------

Signed-off-by: Bayyr Oorjak <[email protected]>
Signed-off-by: Michael Telatynski <[email protected]>
Co-authored-by: Bayyr Oorjak <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: R Midhun Suresh <[email protected]>
Co-authored-by: RiotRobot <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>
Co-authored-by: Andy Balaam <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
thoraj added a commit to verji/matrix-js-sdk that referenced this issue Aug 15, 2024
* Preserve ESM for async imports to work correctly (matrix-org#4187)

* fix: fix lazy rust crypto import

* test: use "commonjs" for tests because of circular deps

* chore: revert commonjs for "module"

* refactor: remove unnecessary example

* refactor: add comments

Signed-off-by: Bayyr Oorjak <[email protected]>

* refactor: improve comment

Signed-off-by: Bayyr Oorjak <[email protected]>

Co-authored-by: Richard van der Hoff <[email protected]>

* Update babel.config.js

---------

Signed-off-by: Bayyr Oorjak <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>

* Update organization (matrix-org#4212)

* v32.4.0

* Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0 (matrix-org#4214)

* Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0

* Disable affected test

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>

* `initRustCrypto`: allow app to pass in the store key directly (matrix-org#4210)

* `initRustCrypto`: allow app to pass in the store key directly

... instead of using the pickleKey. This allows us to avoid a slow PBKDF
operation.

* Fix link in doc-comment

* Bump matrix-sdk-crypto-wasm to 5.0.0 (matrix-org#4216)

Slightly more involved than normal because it requires us to pass a backup version into OlmMachine.importBackedUpRoomKeys.

On the other hand we can now re-enable the test that was disabled in matrix-org#4214 due to matrix-org/matrix-rust-sdk#3447

Fixes: element-hq/element-web#27165

* Remove more deprecated methods, fields, and exports (matrix-org#4217)

* Don't run migration for Rust crypto if the legacy store is empty (matrix-org#4218)

* Don't run migration for Rust crypto if the legacy store is empty

Fixes element-hq/element-web#27447

* Add copyright for the TypeScript files in legacy DB dumps

* Provide a type for the accountPickle we check for before migration

* Remove redundant backup response

This is unused

* Simplify keys response

* Downgrade log message.

---------

Co-authored-by: Richard van der Hoff <[email protected]>

* v33.0.0-rc.0

* v33.0.0

---------

Signed-off-by: Bayyr Oorjak <[email protected]>
Signed-off-by: Michael Telatynski <[email protected]>
Co-authored-by: Bayyr Oorjak <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: R Midhun Suresh <[email protected]>
Co-authored-by: RiotRobot <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>
Co-authored-by: Andy Balaam <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
thoraj added a commit to verji/matrix-js-sdk that referenced this issue Aug 15, 2024
* Bringing develop to v 33.0.0 (#5)

* Preserve ESM for async imports to work correctly (matrix-org#4187)

* fix: fix lazy rust crypto import

* test: use "commonjs" for tests because of circular deps

* chore: revert commonjs for "module"

* refactor: remove unnecessary example

* refactor: add comments

Signed-off-by: Bayyr Oorjak <[email protected]>

* refactor: improve comment

Signed-off-by: Bayyr Oorjak <[email protected]>

Co-authored-by: Richard van der Hoff <[email protected]>

* Update babel.config.js

---------

Signed-off-by: Bayyr Oorjak <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>

* Update organization (matrix-org#4212)

* v32.4.0

* Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0 (matrix-org#4214)

* Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0

* Disable affected test

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>

* `initRustCrypto`: allow app to pass in the store key directly (matrix-org#4210)

* `initRustCrypto`: allow app to pass in the store key directly

... instead of using the pickleKey. This allows us to avoid a slow PBKDF
operation.

* Fix link in doc-comment

* Bump matrix-sdk-crypto-wasm to 5.0.0 (matrix-org#4216)

Slightly more involved than normal because it requires us to pass a backup version into OlmMachine.importBackedUpRoomKeys.

On the other hand we can now re-enable the test that was disabled in matrix-org#4214 due to matrix-org/matrix-rust-sdk#3447

Fixes: element-hq/element-web#27165

* Remove more deprecated methods, fields, and exports (matrix-org#4217)

* Don't run migration for Rust crypto if the legacy store is empty (matrix-org#4218)

* Don't run migration for Rust crypto if the legacy store is empty

Fixes element-hq/element-web#27447

* Add copyright for the TypeScript files in legacy DB dumps

* Provide a type for the accountPickle we check for before migration

* Remove redundant backup response

This is unused

* Simplify keys response

* Downgrade log message.

---------

Co-authored-by: Richard van der Hoff <[email protected]>

* v33.0.0-rc.0

* v33.0.0

---------

Signed-off-by: Bayyr Oorjak <[email protected]>
Signed-off-by: Michael Telatynski <[email protected]>
Co-authored-by: Bayyr Oorjak <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: R Midhun Suresh <[email protected]>
Co-authored-by: RiotRobot <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>
Co-authored-by: Andy Balaam <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>

* Bring develop to v33.0.0 (#6)

* Preserve ESM for async imports to work correctly (matrix-org#4187)

* fix: fix lazy rust crypto import

* test: use "commonjs" for tests because of circular deps

* chore: revert commonjs for "module"

* refactor: remove unnecessary example

* refactor: add comments

Signed-off-by: Bayyr Oorjak <[email protected]>

* refactor: improve comment

Signed-off-by: Bayyr Oorjak <[email protected]>

Co-authored-by: Richard van der Hoff <[email protected]>

* Update babel.config.js

---------

Signed-off-by: Bayyr Oorjak <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>

* Update organization (matrix-org#4212)

* v32.4.0

* Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0 (matrix-org#4214)

* Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0

* Disable affected test

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>

* `initRustCrypto`: allow app to pass in the store key directly (matrix-org#4210)

* `initRustCrypto`: allow app to pass in the store key directly

... instead of using the pickleKey. This allows us to avoid a slow PBKDF
operation.

* Fix link in doc-comment

* Bump matrix-sdk-crypto-wasm to 5.0.0 (matrix-org#4216)

Slightly more involved than normal because it requires us to pass a backup version into OlmMachine.importBackedUpRoomKeys.

On the other hand we can now re-enable the test that was disabled in matrix-org#4214 due to matrix-org/matrix-rust-sdk#3447

Fixes: element-hq/element-web#27165

* Remove more deprecated methods, fields, and exports (matrix-org#4217)

* Don't run migration for Rust crypto if the legacy store is empty (matrix-org#4218)

* Don't run migration for Rust crypto if the legacy store is empty

Fixes element-hq/element-web#27447

* Add copyright for the TypeScript files in legacy DB dumps

* Provide a type for the accountPickle we check for before migration

* Remove redundant backup response

This is unused

* Simplify keys response

* Downgrade log message.

---------

Co-authored-by: Richard van der Hoff <[email protected]>

* v33.0.0-rc.0

* v33.0.0

---------

Signed-off-by: Bayyr Oorjak <[email protected]>
Signed-off-by: Michael Telatynski <[email protected]>
Co-authored-by: Bayyr Oorjak <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: R Midhun Suresh <[email protected]>
Co-authored-by: RiotRobot <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>
Co-authored-by: Andy Balaam <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>

---------

Signed-off-by: Bayyr Oorjak <[email protected]>
Signed-off-by: Michael Telatynski <[email protected]>
Co-authored-by: Bayyr Oorjak <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: R Midhun Suresh <[email protected]>
Co-authored-by: RiotRobot <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>
Co-authored-by: Andy Balaam <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-E2EE A-Element-R Issues affecting the port of Element's crypto layer to Rust A-Storage Storage layer of the app, including IndexedDB, local storage, etc. O-Occasional Affects or can be seen by some users regularly or most users rarely S-Critical Prevents work, causes data loss and/or has no workaround T-Defect Z-Element-R-Blocker A blocker for enabling Element R by default
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants