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

When using the in-memory crypto store, keys restored from backup are re-uploaded to backup #3447

Closed
richvdh opened this issue May 23, 2024 · 0 comments · Fixed by #3448 or matrix-org/matrix-rust-sdk-crypto-wasm#121
Assignees
Labels
bug Something isn't working

Comments

@richvdh
Copy link
Member

richvdh commented May 23, 2024

BackupMachine::backup() returns a list of message keys which need backing up to the server.

When we restore a key from backup, it is imported via BackupMachine::import_backed_up_room_keys(). The intention is that such keys are not returned by a later call to BackupMachine::backup() (since, obviously, they are already backed up).

However, this has broken, at least for MemoryStore; the regression happened in #3320. The problem is that, as of that PR, MemoryStore keeps a separate record of "which backup version was this session backed up to", which it uses to populate the results for BackupMachine::backup(); and BackupMachine::import_backed_up_room_keys() does not populate that field.

@richvdh richvdh added the bug Something isn't working label May 23, 2024
@richvdh richvdh self-assigned this May 23, 2024
richvdh added a commit to matrix-org/matrix-rust-sdk-crypto-wasm that referenced this issue May 24, 2024
Bump to a version of the rust SDK that includes matrix-org/matrix-rust-sdk#3448 and matrix-org/matrix-rust-sdk#3456, to fix matrix-org/matrix-rust-sdk#3447.

Also, pass the backup version into import_backed_up_room_keys to avoid using the deprecated method on BackupMachine.
richvdh added a commit to matrix-org/matrix-js-sdk that referenced this issue May 24, 2024
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 #4214 due to matrix-org/matrix-rust-sdk#3447

Fixes: element-hq/element-web#27165
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
bug Something isn't working
Projects
None yet
1 participant