Rip out legacy crypto code #26922
Comments
richvdh
added
the
A-Element-R
|
We'll need to make sure they are all deprecated first, of course: #26919 |
|
Blocked on #27001 |
|
The legacy crypto code treated libolm as an injectable dependency, loading it from the The Rust crypto code doesn't appear to have the same feature and relies on loading WASM. Removing the legacy crypto code would, therefore, effectively cut off the WASM-incapable environments – seemingly without a suitable alternative. Additionally, for environments that can run WASM but don't require encryption, the bundle size is bloated significantly as per matrix-org/matrix-js-sdk#4154. Could we maybe provide the same global injection mechanism for the Rust crypto code? |
Yes, that is regrettable. Unfortunately we're not really in a position to maintain two crypto stacks, so I can't see that we can do much about it. The legacy crypto stack, apart from being a somewhat unmaintanable mess, contains known issues that we really don't have the time or resources to fix, and attempting to maintain it in parallel with the Rust stack would impede our ability to iterate the protocol to fix things in future. I'd be interested to hear which applications require crypto but lack WASM support, but I think the die was cast at the point we decided to rewrite the existing stack in rust: matrix-js-sdk will not be supporting cryptography without WASM. Grand rewrite projects always come at a cost of someone's favourite feature, and in this case crypto-without-wasm is such a feature.
I don't entirely follow what's going on in that issue; certainly the intention is that, if you don't need crypto, you don't need the WASM artifact and you should configure your Webpack/Vite/whatever so that it isn't built in. The global injection mechanism made crypto very hard to use as a developer. Having to inject things into global variables simply isn't a thing a modern Javascript developer expects to do. The only reason we had it was because of some US-export-regulation-hoop-jumping that stopped us shipping cryptography implementations from Github. It made testing hard and was generally a pain in the butt to maintain. I'm very happy to see the back of it :). |
Yes, agreed and I'm not trying to make a case for maintaining both stacks. I'm merely advocating for having the same loading mechanism in the Rust stack that we currently have in the legacy stack.
I don't know if there are other environments but, as I mentioned, React Native lacks WASM support. There is https://github.com/cawfree/react-native-webassembly but it hasn't seen any updates in a year and people seem to be struggling to make it work at all.
The unfortunate thing is that this is not just an arbitrary tiny feature. You're shutting out an entire ecosystem and React Native isn't exactly an exotic technology. Using matrix-js-sdk in React Native has even been semi-officially supported through https://github.com/matrix-org/matrix-rn-sdk in the past. I understand that the |
In theory, if you don't enable crypto in your client, you won't load the WASM artifact. That's why it's dynamically loaded. It should be a fairly simple matter of configuration in your bundler of choice. |
|
My use case is matrix-js-sdk in a React Native app with encryption. I'm currently using the official ASM build of libolm which, so far, works great with the crypto store from matrix-org/matrix-rn-sdk#2. |
|
@Johennes perhaps I'm not understanding what you're asking for, then. It seemed you had two points:
|
|
Sorry, the first point is my actual problem. I clumsily tried to use the second point to underpin it, arguing that having the import injectable would have other benefits. But I think you're right that this particular case is probably solvable via bundler configuration. I'm finding this pretty unfortunate overall, not least because matrix-js-sdk is a matrix-org project. Any chance you'd be open to an external contribution that makes |
Possibly, but how does that help your situation? |
|
So basically with libolm I'm loading the legacy ASM build into the If the |
|
I see. Interesting. We're somewhat off-topic here: could you open a separate issue and we can discuss further? |
|
Hm, looks like even with optimization flags, I suppose I could look into providing it via a React Native "native module" but at that point I could probably just consume the entirety of matrix-rust-sdk as a native module. So I guess I'll retreat Sorry for making a big fuzz about it. I hadn't anticipated the artifact to be this big. It would be nice if we could document that WASM will be mandatory for encryption in the near future to prevent others from sinking their time into trying to use matrix-js-sdk with React Native. |
|
Yeah, I'm not surprised that transpiling the wasm-transpiled Rust into asm.js gives you an abomination :(. Thinking further about this: If anybody wanted to contribute and maintain alternative implementations of To that end: maybe the way to go here is to make the crypto implementation pluggable somehow. I haven't done much thinking about what that might look like in practice (it might be as simple as exposing a But again, work to make it pluggable isn't something Element-the-company is likely to find time for in the near term, so such work would need to be contributed. I'd be happy to review PRs to that end though. @Johennes interested in your thoughts here. |
|
Thanks for getting back on this, Rich. Yes, fully acknowledged that maintaining two crypto stacks in the SDK isn't feasible. Making What I'll need to figure out is how this would compare effort-wise to plugging the Rust SDK as a native module in React Native. I'll have to carve out some time to do this. Generally, if you are open to having |
I did some tests and it turns out using matrix-rust-sdk as a native module in React Native is more or less straightforward because we can reuse the bindings that were written for Element X. So while doing that still requires some efforts, it's probably better longterm than adopting maintenance of the libolm wrappers. That'll also spare us from having to shim Web Crypto and IndexedDb and all the other annoying hacks needed to integrate a node / browser library into React Native's weird environment. |
|
Since I've just discussed this question elsewhere and for the avoidance of doubt: I'm working on creating React Native bindings for the entire Rust SDK, not only for the crypto crate. So I'm not currently planning on contributing a change to make I think it should be possible to use the same approach to write a pluggable replacement for https://github.com/matrix-org/matrix-rust-sdk-crypto-wasm but I've hit other issues with using matrix-js-sdk in React Native (e.g. no |
Relates to: element-hq/element-web#26922 Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
* Fix node.js example Relates to: element-hq/element-web#26922 Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org> * Update examples/node/app.js Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update examples/node/package.json Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Move imports to the top of the file --------- Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
As a result of our work on Element R, lots of the js-sdk code is deprecated or unusable. Once Rust crypto becomes the default (related: #26869), we should begin ripping out the legacy code.
Hopefully, as part of that, we can close #17488.
The text was updated successfully, but these errors were encountered: