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

export_room_keys fails on big database #2914

Closed
BillCarsonFr opened this issue Dec 7, 2023 · 12 comments
Closed

export_room_keys fails on big database #2914

BillCarsonFr opened this issue Dec 7, 2023 · 12 comments
Assignees

Comments

@BillCarsonFr
Copy link
Member

see element-hq/element-web#26681

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

@BillCarsonFr
Copy link
Member Author

@ara4n
Copy link
Member

ara4n commented Dec 7, 2023

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

@andybalaam andybalaam self-assigned this Dec 13, 2023
@andybalaam
Copy link
Member

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
Copy link
Member

andybalaam commented Feb 6, 2024

Here is the stack of calls:

  • ExportE2eKeysDialog.startExport (matrix-react-sdk)
    • client.exportRoomKeys (matrix-js-sdk) -> JS object
      • RustCrypto.exportRoomKeys (matrix-js-sdk) -> Array<IMegolmSessionData>
        • OlmMachine::export_room_keys (matrix-rust-sdk-crypto-wasm) -> String
          • OlmMachine::export_room_keys (matrix-rust-sdk/crypto) -> Vec<ExportedRoomKey>
            • IndexeddbCryptoStore::get_inbound_group_sessions (matrix-rust-sdk/indexeddb) -> Vec<InboundGroupSession>
            • for loop over InboundGroupSession::export -> Vec<ExportedRoomKey>
          • serde_json::to_string -> String
        • JSON.parse -> JS object
    • JSON.stringify -> JS string
    • MegolmExportEncryption.encryptMegolmKeyFile -> JS ArrayBuffer
      • TextEncoder.encode -> JS UInt8Array (utf-8)
      • subtlecrypto.encrypt -> JS ArrayBuffer
      • new UInt8Array -> JS UInt8Array
      • MegolmExportEncryption.packageMegolmKeyFile -> JS ArrayBuffer
    • new Blob -> JS Blob
    • FileSaver.saveAs

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
Copy link
Member

andybalaam commented Feb 6, 2024

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

This is potentially fixed by #3144 and some previous incremental improvements. Waiting for feedback on whether we need a deeper solution like #3060 + re-using the Rust encryption logic (see previous comment).

@andybalaam
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants