Page MenuHomePhabricator

Introduce methods in CommCoreModule to initialize olm session for notifications. Refactor method to get one time keys for primary olm account
ClosedPublic

Authored by marcin on Apr 12 2023, 5:06 AM.
Tags
None
Referenced Files
F1657330: D7395.diff
Wed, Apr 24, 3:43 PM
Unknown Object (File)
Sun, Apr 21, 9:43 AM
Unknown Object (File)
Thu, Apr 18, 9:08 PM
Unknown Object (File)
Tue, Apr 16, 3:25 PM
Unknown Object (File)
Tue, Apr 16, 8:41 AM
Unknown Object (File)
Tue, Apr 16, 3:13 AM
Unknown Object (File)
Mon, Apr 15, 8:43 PM
Unknown Object (File)
Mon, Apr 15, 8:43 PM
Subscribers

Details

Summary

This differential introduces methods in CommCoreModule to initialize olm session with keyserver one time keys and prekey for notifications olm account. Additionally method for primary olm account to generate one time keys is
refactored. The refactor gives us control on how many one time keys are generated (previously we were restricted to the default value in CryptoModule.h). Refactored method returns JS
object of type OLMOneTimeKeys. Note that getUserOneTimeKeys` returned raw string previously.
This differential seems to be broad but only changes in CommCoreModule.cpp matter. The rest is boilerplate.

Test Plan

To test refactored getUserOneTimeKeys(<number>) method:
In account-selectors place the following code under commCoreModule.initializeCryptoAccount():

const userOneTimeKeys = await commCoreModule.getUserOneTimeKeys(10);
console.log(userOneTimeKeys);

Build the app, log-in and examine content of keys logged to the console.

To test new methods for notifications identity:
In account-selectors place the following code under commCoreModule.initializeCryptoAccount():

if (!(await commCoreModule.isNotificationsSessionInitialized())) {
    await commCoreModule.initializeNotificationsSession(
         '{"curve25519":"o+tfILWz+bAo5j1q1uRIkJPTgOU499Ib93B8olZW31s","ed25519":"15gDzUxPx5DntkgmAcLrZcnS4kkMQKDtw/wfna4EMPE"}',
         '{"curve25519":{"AAAAAQ":"A/sCRbBRsOwtYBFCJ9H0o/9opVM5wcAHKvpr43aiYnA"}}',
         '{"curve25519":{"AAAAAQ":"MXeHR/2J5zCdF8gsFzaMksNo9hx5QADUOSlYdctyyX8"}}',
    );
 }
const encryptedMessage = await commCoreModule.generateInitialNotificationsEncryptedMessage();
console.log(encryptedMessage);

Build the app, log-in and examine content of message logged to the console.

I created the keys above using olm JS bindings on the keyserver.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

marcin edited the test plan for this revision. (Show Details)
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
965 ↗(On Diff #25030)

Wondering, what is this key value? Is it a number or something else?

975 ↗(On Diff #25030)

I wonder if we find a better term to distinguish notif vs. "main" keys...

"User" seems a bit generic (arguably the notif one-time keys belong to the user as well). Some potential ideas: "App", "Main", "Channel"

1046 ↗(On Diff #25030)

This function's name implies it "get"s the prekey, but in fact it does something much more serious: it generates a new one. If there are already 2 prekeys, this will result in the older prekey being deleted. And if there is at least 1 prekey already, then that prekey will be marked as "old" and will be deleted when forget is called.

We have a 1:1 later today and I can chat about the prekey lifecycle there. I want to make sure we're thinking about this correctly.

Assuming this code is correct (hard to tell without seeing the callsite in JS), we at least should rename this to indicate it's mutating the state in a permanent way, and not just "getting" something.

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
965 ↗(On Diff #25030)

This is the structure of one time keys JSON returned from olm:

{
	"curve25519": 
		{
			"AAAAAQ": "LkYHUy3rFhS4y5xEg6GNyGh0TIFp49aRvXlYUdtp4zg", 
			"AAAAAg": "UpvQ6m8CoG5SQWDCrrgGY4bklE7wYWE4uKzHmablqjU", 
			"AAAAAw": "Mv5T+TgIufUZvlxYEpMBvRuoUyBo5nL6iDpGzRHmjFM", 
			"AAAABA": "qdN1vs3GexiIR3JmXpARWycrm75xAn1LgIKre4wEZXQ", 
			"AAAABQ": "DdJSxatk2maCPpooGn1fJnROZ+dseDTeLQuwmiNH/g0", 
			"AAAABg": "n2JnA1RJoOfKz5omNRWOseVbxbwxIoCMsi2j/G8AZEg", 
			"AAAABw": "4AAT0qaQUBFCakRwxOvhsyWFGvMlMZ6myHHgpFo+pXc", 
			"AAAACA": "0dyiFivVoR5ZfXnln/WY/W+S7F7GvjBHywGn6g3S8kg",
			"AAAACQ": "+KfWVTCJrmLnhck1CYHniPbFYC4sTt61qE94Bqqa1kc", 
			"AAAACg": "UEV3TvTHnNzMlazLFGYLOZ2YLpGtjmQX/Iig7EXX7hQ"
		}
}

I took a look at olm code and this id is a stringified number encoded in base 64.

975 ↗(On Diff #25030)

I guess Primary would be the best. We already use it when uploading signed identity keys to the keyserver.

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
975 ↗(On Diff #25030)

Seems reasonable! We also talk about a "primary device" in the whitepaper, but I think it should be easy to distinguish primary device vs. primary keys. Might get a little confusing if we are ever referring to "primary device's primary keys", but that's probably fine

Add new JS type definitions

Rename getUserOneTimeKeys to getPrimaryOneTimeKeys. Rename getNotificationsPrekey to generateNotificationsPrekey

Adjust to new API of CryptoModule

This revision is now accepted and ready to land.Apr 17 2023, 6:34 AM

As for the parent diff, methods for prekey and one time keys will not be needed for NotificationsCryptoAccount, so I plan to reuse this differential for methods to initialize olm notifications session and checking whether notifications session has already been initialized.

Replace methods to generate prekey and one time keys for notifications with methods to initialize olm notifications session with keyserver prekey and ome time keys.

This revision is now accepted and ready to land.Apr 21 2023, 2:20 AM
marcin retitled this revision from Introduce methods in CommCoreModule to get one time keys and prekey for notifications. Refactor method to get one time keys for primary olm account to Introduce methods in CommCoreModule to initialize olm session for notifications. Refactor method to get one time keys for primary olm account.Apr 21 2023, 2:49 AM
marcin edited the test plan for this revision. (Show Details)

Overwrite by default for outbound session creation.

Add prekey signature to API

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
1008 ↗(On Diff #25996)

I'd define this at the end of the function where it's used

Address comment about jsOneTimeKeys variable definition. Rebase to reorder

Adjust CommCoreModule to NotificationsCryptoModule new API