matrix-org / matrix-rust-sdk

Matrix Client-Server SDK for Rust
Apache License 2.0
1.26k stars 246 forks source link

export_room_keys fails on big database #2914

Closed BillCarsonFr closed 8 months ago

BillCarsonFr commented 11 months ago

see https://github.com/vector-im/element-web/issues/26681

=> Current code tries to extract all inbound session in a single session. In case of big accounts probably room

BillCarsonFr commented 11 months ago

see https://github.com/vector-im/element-web/issues/26488#issuecomment-1843341364

ara4n commented 11 months ago

ftr importing large numbers of keys also fails: https://github.com/vector-im/element-web/issues/26692

andybalaam commented 10 months ago

I can reproduce this in my local dev environment, when I have a lot of keys. I see a failure inside:

.get_all()?.await?

On the IndexedDb object store.

The error message I see is:

11:33:38.615 Error exporting e2e keys: Error: DomException UnknownError (0): The operation failed for reasons unrelated to the database itself and not covered by any other error code.
    __wbindgen_error_new http://127.0.0.1:8080/bundles/_dev_/default-matrix-js-sdk_src_rust-crypto_index_ts.js:13432
rageshake.ts:77:27
    fnName rageshake.ts:77
    <anonymous> logger.ts:97
    startExport ExportE2eKeysDialog.tsx:125
    (Async: promise callback)
    startExport ExportE2eKeysDialog.tsx:124
    ExportE2eKeysDialog ExportE2eKeysDialog.tsx:104
    React 14
    unstable_runWithPriority scheduler.development.js:468
    React 15
    reRender Modal.tsx:409
    run setImmediate.js:40
    runIfPresent setImmediate.js:69
    onGlobalMessage setImmediate.js:109
    (Async: EventListener.handleEvent)
    installPostMessageImplementation setImmediate.js:114
    js setImmediate.js:169
    js setImmediate.js:186
    Webpack 8
andybalaam commented 9 months ago

Here is the stack of calls:

Everything after client.exportRoomKeys, including turning the full list of keys into JSON, then encrypting it as an ArrayBuffer (meaning we have many copies simultaneously) are present in the legacy code as well as the Rust, which works even for heavy users.

So in the Rust case we create a Vec<InboundGroupSession>, copy it into a Vec<ExportedRoomKey>, serialise it with serde into a String, and then parse it as a JS object before we do the above.

andybalaam commented 9 months ago

For future reference, we could base a fully-streaming solution on https://developer.mozilla.org/en-US/docs/Web/API/FileSystemWritableFileStream but it is not currently supported in Safari.

For now, we will focus on creating a Blob of the entire export in a more memory-efficient way.

andybalaam commented 9 months ago

I will consider MegolmExportEncryption outside of the scope of this task because it already exists and works with legacy crypto. But I do think it's worth noting that this could almost certainly be rewritten in streaming form, and doing so would significantly reduce memory usage during export.

andybalaam commented 9 months ago

The most obvious thing to fix first is the fact that the Rust converts the key backup to JSON, and the JS wrapper parses that to return it, only for it to be re-stringified immediately afterwards by the code shared between legacy and Rust crypto.

andybalaam commented 9 months ago

These PRs together avoid the deserialise-reserialise dance on export and import:

andybalaam commented 9 months ago

Now I will focus on generating the String that we return from export_room_keys in the most efficient way. Once that is done we will be at parity with the legacy crypto, and we can evaluate whether we need to go further and provide a streaming API across the Rust-JS boundary.

andybalaam commented 9 months ago

I will consider MegolmExportEncryption outside of the scope of this task because it already exists and works with legacy crypto. But I do think it's worth noting that this could almost certainly be rewritten in streaming form, and doing so would significantly reduce memory usage during export.

Also, and probably a better idea, we implement the same encryption in the Rust code at https://github.com/matrix-org/matrix-rust-sdk/blob/main/crates/matrix-sdk-crypto/src/file_encryption/key_export.rs#L137 so it would be good to re-use this implementation. I note that the current Rust code also makes several copies for encryption, base64 encoding etc. so swapping to use it probably won't fix the problem, but it will mean we re-use more cod.

andybalaam commented 8 months ago

This is potentially fixed by https://github.com/matrix-org/matrix-rust-sdk/pull/3144 and some previous incremental improvements. Waiting for feedback on whether we need a deeper solution like https://github.com/matrix-org/matrix-rust-sdk/pull/3060 + re-using the Rust encryption logic (see previous comment).

andybalaam commented 8 months ago

OK, we have confirmation that an export succeeded, so we consider the fixes we've done so far to be adequate. Ideally, we'd make a fully-scalable solution but we are not going to work more on that now.