[lib][web][keyserver] replace Olm with Vodozemac's Olm #325

xsanm · 2025-12-02T16:00:19Z

There is one thing with naming which is not obvious, this is using Olm name - Olm is still part of Vodozemac crate, so I think this is still correct, but I am okay with updating this too.

I tried to make the API as similar as it is possible, so there are files where changing the import is everything we have to do.

I redefined EncryptResult in lib/. Importing it from Olm was a bad idea because it was used on native too, which causes Olm/Vodozemac wasm to be bundled into native, which worked with Olm but not with Vodozemac.

Depends on D15547

Test Plan: Test encrypt/decrypt and create a session with Olm and Vodozemac.

Reviewers: ashoat

Subscribers: tomek

Differential Revision: https://phab.comm.dev/D15548

CLAassistant · 2025-12-02T16:00:31Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Ashoat · 2025-12-02T23:43:09Z

keyserver/package.json

    },
    "transformIgnorePatterns": [
-      "/node_modules/(?!@babel/runtime)"
+      "/node_modules/(?!(@babel/runtime|@commapp/vodozemac))"


Why do we need this?

This is only for tests. @commapp/vodozemac was configured with "type": "module" (using ES6), while Jest runs in Node's CommonJS environment by default, so without transformation, it can't understand these ES6 export statements

Ashoat · 2025-12-02T23:44:03Z

keyserver/src/responders/keys-responders.js


  const prekey = account.prekey();
-  const prekeySignature = account.prekey_signature();
+  if (!prekey) {


Why are we adding this check?

In Olm, prekey was typed as defined because it was always returned. Olm just read bytes of memory allocated for the prekey struct, even though it wasn't generated, so there was a chance to get some trash bytes as prekey. We even had an issue with it (link).

In Rust, it is not easy to just read a chunk of memory where the prekey should be, so to make it work, I've made it optional, and we need to check if this exists here.

Prekey is set during account creation, so this couldn't have happened. Taking a step back, probably throwing an error was a better fit than making it optional, but fixing this now is probably too much work

Ashoat · 2025-12-02T23:45:16Z

keyserver/src/responders/keys-responders.js

+  const identityKeys = JSON.stringify({
+    ed25519: account.ed25519_key,
+    curve25519: account.curve25519_key,
+  });


Will we need these conversions on the other platforms? Should we have functions to implent this conversion, as wella s the prekey and one-time key conversions below?

Prekey and one-time key conversions are deprecated, the only exception is this particular endpoint.

For identity keys, this is something that can be improved even further, created ENG-11553 because here it gets too messy.

Ashoat · 2025-12-02T23:46:31Z

keyserver/src/utils/olm-objects.js

 ): Promise<{ +result: T, +pickledOlmAccount: PickledOlmAccount }> {
-  const { picklingKey, pickledAccount } = pickledOlmAccount;
-
-  await olm.init();


Don't we need an equivalent init call for Vodozemac?

I am not really sure why this call was added here. Both Olm before, and Vodozemac now are initialized at keyserver start here, I think this is unnecessary no-op, but I can add it if you think it is better to keep it as it is.

Let's bring these back so that standalone scripts can still work

Ashoat · 2025-12-02T23:51:06Z

lib/utils/olm-utils.js

 } {
-  const prekey = getPrekeyValueFromBlob(account.prekey());
+  const prekey = account.prekey();
+  if (!prekey) {


Why are we adding this check?

#325 (comment)

Ashoat · 2025-12-03T00:00:15Z

web/shared-worker/worker/worker-crypto.js

      throw new Error(`error decrypt => ${OLM_ERROR_FLAG} ` + e.message);
    }

+    olmMessage.free();


We could do this in a finally block

Ashoat · 2025-12-03T00:00:46Z

web/push-notif/notif-crypto-utils.js

+    // into_session() is consuming object.
+    // There is no need to call free() on inboundCreationResult
+    session = inboundCreationResult.into_session();
+    olmMessage.free();


Will this not be freed if an exception is thrown above?

Ashoat · 2025-12-03T00:00:59Z

web/push-notif/notif-crypto-utils.js

+
+  const olmMessage = new OlmMessage(type, encryptedPayload);
+  const decryptedString = session.decrypt(olmMessage);
+  olmMessage.free();


Will this not be freed if an exception is thrown above?

Ashoat · 2025-12-03T00:01:31Z

web/shared-worker/worker/worker-crypto.js

      throw new Error(`error decrypt => ${OLM_ERROR_FLAG} ` + e.message);
    }

+    olmMessage.free();


This doesn't seem to have been addressed

Ashoat · 2025-12-03T00:01:40Z

web/shared-worker/worker/worker-crypto.js

+    // There is no need to call free() on inboundCreationResult
+    const initialEncryptedMessage = inboundCreationResult.plaintext;
+    const session = inboundCreationResult.into_session();
+    olmMessage.free();


Ashoat · 2025-12-03T15:55:22Z

web/shared-worker/worker/worker-crypto.js

-      );
+      result = olmSession.session.decrypt(olmMessage);
    } catch (e) {
+      olmMessage.free();


You don't need this one

Ashoat · 2025-12-03T15:55:37Z

web/shared-worker/worker/worker-crypto.js

      throw new Error(`error decrypt => ${OLM_ERROR_FLAG} ` + e.message);
    }

+    olmMessage.free();


This doesn't seem to have been addressed

Ashoat · 2025-12-03T15:55:46Z

web/shared-worker/worker/worker-crypto.js

      );
    } catch (e) {
-      session.free();
+      olmMessage.free();


You don't need this one

Ashoat · 2025-12-03T16:03:46Z

keyserver/src/utils/olm-objects.js

 ): Promise<{ +result: T, +pickledOlmAccount: PickledOlmAccount }> {
-  const { picklingKey, pickledAccount } = pickledOlmAccount;
-
-  await olm.init();


Let's bring these back so that standalone scripts can still work

Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). There is one thing with naming which is not obvious, this is using Olm name - Olm is still part of Vodozemac crate, so I think this is still correct, but I am okay with updating this too. I tried to make API as similar as it is possible, so there are files when changing import is everything we have to do. I redefined `EncryptResult` in `lib/`. Importing it from Olm was bad idea because it was used on native too, which causes Olm/Vodozemac wasm to be bundled into native, which worked with Olm but not with Vodozemac. Depends on D15547 Test Plan: Test encrypt/decrypt and creating session with Olm and Vodozemac. Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15548

Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). Making vodozemac available on native. Branch needs to be updated after PRs are merged Test Plan: Tested in next diff Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15549

…#324) * [lib][web][keyserver] add Vodozemac library and `.wasm` file handling Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). Using the same pattern as have for existing `.wasm` files (Olm, SQLite and Backup Client). Currently linking my local library, so CI will fail. This should be updated to NPM package after CommE2E/vodozemac#3 is merged and published. This diff shouldn't be landed without following one with API migration, because it replaces initializing Olm with Vodozemac, so Olm is not working anymore. Depends on D15546 Test Plan: Run web and keyserver, vodozemac should be available, usage tested in next diff Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15547 * [lib][web][keyserver] replace Olm with Vodozemac's Olm (#325) * [lib][web][keyserver] replace Olm with Vodozemac's Olm Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). There is one thing with naming which is not obvious, this is using Olm name - Olm is still part of Vodozemac crate, so I think this is still correct, but I am okay with updating this too. I tried to make API as similar as it is possible, so there are files when changing import is everything we have to do. I redefined `EncryptResult` in `lib/`. Importing it from Olm was bad idea because it was used on native too, which causes Olm/Vodozemac wasm to be bundled into native, which worked with Olm but not with Vodozemac. Depends on D15547 Test Plan: Test encrypt/decrypt and creating session with Olm and Vodozemac. Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15548 * [native] implement CXX bindings to Vodozemac (#326) Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). Making vodozemac available on native. Branch needs to be updated after PRs are merged Test Plan: Tested in next diff Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15549

xsanm requested a review from Ashoat December 2, 2025 16:00

Ashoat reviewed Dec 3, 2025

View reviewed changes

xsanm force-pushed the vodozemac-work-part-2 branch from f3e1025 to 080ab37 Compare December 3, 2025 12:09

xsanm force-pushed the vodozemac-work-part-3 branch 2 times, most recently from 17ec456 to 2c625b5 Compare December 3, 2025 15:47

Ashoat approved these changes Dec 3, 2025

View reviewed changes

xsanm force-pushed the vodozemac-work-part-3 branch from 2c625b5 to d87e0e8 Compare December 3, 2025 16:11

Ashoat approved these changes Dec 3, 2025

View reviewed changes

Ashoat merged commit df4e62a into vodozemac-work-part-2 Dec 3, 2025

Ashoat deleted the vodozemac-work-part-3 branch December 3, 2025 17:09

[lib][web][keyserver] replace Olm with Vodozemac's Olm #325

[lib][web][keyserver] replace Olm with Vodozemac's Olm #325

Uh oh!

Conversation

xsanm commented Dec 2, 2025

Uh oh!

CLAassistant commented Dec 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xsanm Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xsanm Dec 3, 2025 •

edited

Loading