[native] replace Olm with Vodozemac's Olm #327

xsanm · 2025-12-02T16:03:30Z

Summary:
ENG-11531.

Making Vodozemac available on native.

Depends on D15549

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

Reviewers: ashoat

Subscribers: tomek

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

CLAassistant · 2025-12-02T16:03:38Z

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

Only got through CryptoModule

Ashoat · 2025-12-03T16:42:22Z

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

-  } else {
+CryptoModule::CryptoModule(std::string secretKey, Persist persist)
+    : vodozemacAccount(::account_new()) {
+  this->secretKey = secretKey;


Why do we need this? I can't find where it's used

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

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

+  for (const auto &key : oneTimeKeysVec) {
+    this->keys.oneTimeKeys.push_back(std::string(key));
  }
-  return ::olm_account_mark_keys_as_published(this->getOlmAccount());


There is a slight difference here it seems... previously we returned the result of olm_account_mark_keys_as_published, but now we return oneTimeKeysVec.size. Are these always the same?

This is because mark_keys_as_published in vodozemac is returning void, but the values should be the same

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

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

+    folly::dynamic identityKeysObj = folly::parseJson(this->getIdentityKeys());
+    std::string signingPublicKey = identityKeysObj["ed25519"].asString();


Why'd we get rid of getSigningPublicKey? We seem to be doing the same thing here, but I guess with less robust error handling. I guess we didn't need the error handling here?

That was strange. I am reintroducing getSigningPublicKey

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

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

+    std::string signingPublicKey = identityKeysObj["ed25519"].asString();
+
+    std::string prekey = this->getPrekey();
+    std::string prekeySignature = this->getPrekeySignature();


Why'd you remove the const keyword from these declarations?

Ashoat · 2025-12-03T16:47:33Z

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

    std::string rawMessage{e.what()};
-    if (rawMessage.find("BAD_MESSAGE_MAC") != std::string::npos) {
+    if (rawMessage.find("BAD_MESSAGE_MAC") != std::string::npos ||
+        rawMessage.find("Signature") != std::string::npos) {


The same issue with checking Signature as in a prior PR

Ashoat · 2025-12-03T16:51:55Z

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

-          random.data(),
-          random.size())) {
-    throw std::runtime_error{
-        "error generateOneTimeKeys => " +


Are we no longer handling this error generateOneTimeKeys?

Ashoat · 2025-12-03T16:52:42Z

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

+  try {
+    std::string unpublished =
+        std::string(this->vodozemacAccount->unpublished_prekey());
+    if (unpublished.empty()) {


Add a comment explaining why we use the empty string

Ashoat · 2025-12-03T16:54:21Z

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

-      idKeys);
+
+  auto [newSession, plaintext] = Session::createSessionAsResponder(
+      this->vodozemacAccount, encryptedData, idKeys);


~~Why don't we need to pass this->keys.identityKeys.data() anymore?~~

EDIT – looking at Session.cpp, it apparently wasn't even used before. Feel free to ignore this comment

Ashoat · 2025-12-03T16:54:29Z

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

+      this->vodozemacAccount, encryptedData, idKeys);
  newSession->setVersion(sessionVersion);
  this->sessions.insert(make_pair(targetDeviceId, std::move(newSession)));
+  return plaintext;


Why do we return plaintext now?

Previously (C++ Olm), create_inbound_session was creating the session only, and we had to decrypt the initial message separately; it was done in CommCoreModule. In Vodoezmac, decryption is part of create_inbound_session, so we have to return it

Ashoat · 2025-12-03T16:59:17Z

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

  for (it = this->sessions.begin(); it != this->sessions.end(); ++it) {
-    OlmBuffer buffer = it->second->storeAsB64(secretKey);
+    std::string sessionPickle = it->second->storeAsB64(secretKey);
+    OlmBuffer buffer(sessionPickle.begin(), sessionPickle.end());


Why do we still need to use OlmBuffer? Is this type from inside of the old Olm library? Will it prevent us from removing that library?

OlmBuffer is just an alias for std::vector<std::uint8_t>:

comm/native/cpp/CommonCpp/CryptoTools/Tools.h

Line 18 in 3b4513a

typedef std::vector<std::uint8_t> OlmBuffer;

Ashoat · 2025-12-03T20:01:39Z

native/cpp/CommonCpp/CryptoTools/Session.cpp

+        account->create_outbound_session(
+            curve25519Key,
+            ed25519Key,
+            oneTimeKey.value_or(""),


Would be good to add a comment here explaining why we use the empty string

Ashoat · 2025-12-03T20:05:43Z

native/cpp/CommonCpp/CryptoTools/Session.cpp

+        static_cast<uint32_t>(encryptedData.messageType)));
+  } catch (const std::exception &e) {
+    throw std::runtime_error(
+        "error decrypt => " + olmErrorFlag + " " + std::string(e.what()));


We used to print the message hash here in this error, but we don't anymore. Did we add the message hash previously because it helped with debugging? Is it difficult to reintroduce?

Did we add the message hash previously because it helped with debugging?

I don't know, but I think so, perhaps when having some issues with notifications in the past.

Reintroduced

Ashoat · 2025-12-03T20:06:17Z

native/cpp/CommonCpp/CryptoTools/Session.h

  int version;

 public:
+  ::rust::Box<::VodozemacSession> vodozemacSession;


Why does this need to be public?

It was because we used it in:

comm/native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp

Lines 677 to 680 in 3b4513a

isSenderChainEmpty = maybeSessionWithPicklingKey.value()

.first->vodozemacSession->is_sender_chain_empty();

hasReceivedMessage = maybeSessionWithPicklingKey.value()

.first->vodozemacSession->has_received_message();

I implemented methods for isSenderChainEmpty and hasReceivedMessage and updated vodozemacSession to be private

Ashoat · 2025-12-04T03:53:51Z

native/crypto/olm-api.js

@@ -1,6 +1,5 @@
 // @flow

-import { getOneTimeKeyValues } from 'lib/shared/crypto-utils.js';


Is getOneTimeKeyValues used anywhere anymore?

It is used as part of getOneTimeKeyValuesFromBlob, which is used to parse the result from getOlmSessionInitializationDataResponder endpoint (this endpoint has to still support old format for older clients, as we discussed yesterday)

Ashoat · 2025-12-04T03:55:16Z

native/schema/CommCoreModuleSchema.js

 import type { ClientDBReportStoreOperation } from 'lib/ops/report-store-ops.js';
 import type { ClientDBThreadStoreOperation } from 'lib/ops/thread-store-ops.js';
 import type {
-  OneTimeKeysResult,


Is the OneTimeKeysResult type used anywhere anymore? Can it be removed?

Ashoat · 2025-12-04T03:56:05Z

I reviewed only the final commit, as this branch needs to be rebased on #323's branch

Ashoat · 2025-12-04T11:13:51Z

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

-  const OlmBuffer prekeyBytes = Base64::decode(prekey);
  try {
-    this->verifySignature(signingPublicKey, prekeyBytes, preKeySignature);
+    std::string signingPublicKey = getSigningPublicKey(this->getIdentityKeys());


const was still removed here

Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). Making vodozemac available on native. Depends on D15549 Test Plan: Test encrypt/decrypt and creating session with Olm and Vodozemac. Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15550

xsanm · 2025-12-04T11:26:01Z

I triggered CI in ac50bc4 to make sure it passes before merging to master

Summary: [ENG-11531](https://linear.app/comm/issue/ENG-11531/migrate-the-full-crypto-api-to-vodozemac). Making vodozemac available on native. Depends on D15549 Test Plan: Test encrypt/decrypt and creating session with Olm and Vodozemac. Reviewers: ashoat Subscribers: tomek Differential Revision: https://phab.comm.dev/D15550

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

xsanm force-pushed the vodozemac-work-part-4 branch 2 times, most recently from 6018853 to 865c668 Compare December 3, 2025 16:15

xsanm force-pushed the vodozemac-work-part-5 branch from a811562 to 58cb6e6 Compare December 3, 2025 16:17

xsanm force-pushed the vodozemac-work-part-4 branch 2 times, most recently from de66670 to de276f7 Compare December 3, 2025 16:58

Ashoat reviewed Dec 3, 2025

View reviewed changes

Ashoat approved these changes Dec 4, 2025

View reviewed changes

xsanm changed the base branch from vodozemac-work-part-4 to vodozemac-work-part-1 December 4, 2025 10:03

xsanm force-pushed the vodozemac-work-part-5 branch 2 times, most recently from 3b4513a to e4858cf Compare December 4, 2025 11:08

Ashoat approved these changes Dec 4, 2025

View reviewed changes

xsanm force-pushed the vodozemac-work-part-5 branch from e4858cf to b943500 Compare December 4, 2025 11:24

Ashoat approved these changes Dec 4, 2025

View reviewed changes

Ashoat merged commit c234a98 into vodozemac-work-part-1 Dec 4, 2025

Ashoat deleted the vodozemac-work-part-5 branch December 4, 2025 11:43

		folly::dynamic identityKeysObj = folly::parseJson(this->getIdentityKeys());
		std::string signingPublicKey = identityKeysObj["ed25519"].asString();

	isSenderChainEmpty = maybeSessionWithPicklingKey.value()
	.first->vodozemacSession->is_sender_chain_empty();
	hasReceivedMessage = maybeSessionWithPicklingKey.value()
	.first->vodozemacSession->has_received_message();

		@@ -1,6 +1,5 @@
		// @flow

		import { getOneTimeKeyValues } from 'lib/shared/crypto-utils.js';

[native] replace Olm with Vodozemac's Olm #327

[native] replace Olm with Vodozemac's Olm #327

Uh oh!

Conversation

xsanm commented Dec 2, 2025

Uh oh!

CLAassistant commented Dec 2, 2025

Uh oh!

Ashoat left a comment

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!

Ashoat 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!

Ashoat commented Dec 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xsanm commented Dec 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Ashoat Dec 3, 2025 •

edited

Loading