[lib][web][keyserver] replace Olm with Vodozemac's Olm #325
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| // @flow | ||
|
|
||
| import type { Account as OlmAccount } from '@commapp/vodozemac'; | ||
|
|
||
| import type { | ||
| OlmSessionInitializationInfo, | ||
|
|
@@ -18,20 +18,39 @@ type SessionInitializationKeysSet = { | |
| function retrieveSessionInitializationKeysSet( | ||
| account: OlmAccount, | ||
| ): SessionInitializationKeysSet { | ||
| const identityKeys = JSON.stringify({ | ||
| ed25519: account.ed25519_key, | ||
| curve25519: account.curve25519_key, | ||
| }); | ||
|
Comment on lines
+21
to
+24
Member
There was a problem hiding this comment. 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?
Collaborator
Author
There was a problem hiding this comment. 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. |
||
|
|
||
| const prekey = account.prekey(); | ||
| if (!prekey) { | ||
|
Collaborator
Author
There was a problem hiding this comment. 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 |
||
| throw new ServerError('missing_prekey'); | ||
| } | ||
|
|
||
| // Wrap prekey in old Olm format to match expected structure on all clients | ||
| const prekeyWrapped = JSON.stringify({ | ||
| curve25519: { AAAAAA: prekey }, | ||
| }); | ||
|
|
||
| const prekeySignature = account.prekey_signature(); | ||
| if (!prekeySignature) { | ||
| throw new ServerError('invalid_prekey'); | ||
| } | ||
|
|
||
| account.generate_one_time_keys(1); | ||
| const oneTimeKeysMap = account.one_time_keys(); | ||
| const oneTimeKeysEntries = Array.from(oneTimeKeysMap.entries()); | ||
| const oneTimeKeysObject = Object.fromEntries(oneTimeKeysEntries); | ||
| const oneTimeKey = JSON.stringify({ curve25519: oneTimeKeysObject }); | ||
| account.mark_keys_as_published(); | ||
|
|
||
| return { | ||
| identityKeys, | ||
| oneTimeKey, | ||
| prekey: prekeyWrapped, | ||
| prekeySignature, | ||
| }; | ||
| } | ||
|
|
||
| async function getOlmSessionInitializationDataResponder(): Promise<GetOlmSessionInitializationDataResponse> { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,20 @@ | ||
| // @flow | ||
|
|
||
| import initVodozemac, { | ||
| type Account as OlmAccount, | ||
| Account, | ||
| OlmMessage, | ||
| type Session as OlmSession, | ||
| } from '@commapp/vodozemac'; | ||
| import uuid from 'uuid'; | ||
|
|
||
| import { olmEncryptedMessageTypes } from 'lib/types/crypto-types.js'; | ||
| import { ServerError } from 'lib/utils/errors.js'; | ||
| import { | ||
| getVodozemacPickleKey, | ||
| unpickleVodozemacAccount, | ||
| unpickleVodozemacSession, | ||
| } from 'lib/utils/vodozemac-utils.js'; | ||
|
|
||
| import { getMessageForException } from '../responders/utils.js'; | ||
|
|
||
|
|
@@ -20,16 +27,14 @@ async function unpickleAccountAndUseCallback<T>( | |
| pickledOlmAccount: PickledOlmAccount, | ||
| callback: (account: OlmAccount, picklingKey: string) => Promise<T> | T, | ||
| ): Promise<{ +result: T, +pickledOlmAccount: PickledOlmAccount }> { | ||
| await initVodozemac(); | ||
|
|
||
|
Member
There was a problem hiding this comment. Don't we need an equivalent init call for Vodozemac?
Collaborator
Author
There was a problem hiding this comment. 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.
Member
There was a problem hiding this comment. Let's bring these back so that standalone scripts can still work |
||
| const { picklingKey } = pickledOlmAccount; | ||
| const account = unpickleVodozemacAccount(pickledOlmAccount); | ||
|
|
||
| try { | ||
| const result = await callback(account, picklingKey); | ||
| const updatedAccount = account.pickle(getVodozemacPickleKey(picklingKey)); | ||
| return { | ||
| result, | ||
| pickledOlmAccount: { | ||
|
|
@@ -45,14 +50,12 @@ async function unpickleAccountAndUseCallback<T>( | |
| } | ||
|
|
||
| async function createPickledOlmAccount(): Promise<PickledOlmAccount> { | ||
| await initVodozemac(); | ||
|
|
||
| const account = new Account(); | ||
|
|
||
| const picklingKey = uuid.v4(); | ||
| const pickledAccount = account.pickle(getVodozemacPickleKey(picklingKey)); | ||
| account.free(); | ||
|
|
||
| return { | ||
|
|
@@ -69,16 +72,14 @@ async function unpickleSessionAndUseCallback<T>( | |
| pickledOlmSession: PickledOlmSession, | ||
| callback: (session: OlmSession) => Promise<T> | T, | ||
| ): Promise<{ +result: T, +pickledOlmSession: PickledOlmSession }> { | ||
| await initVodozemac(); | ||
|
|
||
| const { picklingKey } = pickledOlmSession; | ||
| const session = unpickleVodozemacSession(pickledOlmSession); | ||
|
|
||
| try { | ||
| const result = await callback(session); | ||
| const updatedSession = session.pickle(getVodozemacPickleKey(picklingKey)); | ||
| return { | ||
| result, | ||
| pickledOlmSession: { | ||
|
|
@@ -99,19 +100,22 @@ async function createPickledOlmSession( | |
| initialEncryptedMessage: string, | ||
| theirCurve25519Key: string, | ||
| ): Promise<string> { | ||
| await initVodozemac(); | ||
|
|
||
| const olmMessage = new OlmMessage( | ||
| olmEncryptedMessageTypes.PREKEY, | ||
| initialEncryptedMessage, | ||
| ); | ||
| const inboundCreationResult = account.create_inbound_session( | ||
| theirCurve25519Key, | ||
| olmMessage, | ||
| ); | ||
| // into_session() is consuming object. | ||
| // There is no need to call free() on inboundCreationResult | ||
| const session = inboundCreationResult.into_session(); | ||
| const pickledSession = session.pickle( | ||
| getVodozemacPickleKey(accountPicklingKey), | ||
| ); | ||
| session.free(); | ||
|
|
||
| return pickledSession; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only for tests.
@commapp/vodozemacwas 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