Page MenuHomePhabricator
Differential D11582

[native] update C++ crypto module to handle session version
ClosedPublic
Actions

Authored by kamil on Apr 8 2024, 6:38 AM.
Tags
None
Referenced Files
F1776748: D11582.diff
Thu, May 16, 6:10 PM
Unknown Object (File)
Mon, Apr 22, 11:58 AM
Unknown Object (File)
Apr 17 2024, 6:18 AM
Unknown Object (File)
Apr 16 2024, 9:24 AM
Unknown Object (File)
Apr 14 2024, 1:03 PM
Unknown Object (File)
Apr 13 2024, 2:22 PM
Unknown Object (File)
Apr 13 2024, 6:12 AM
Unknown Object (File)
Apr 11 2024, 2:41 PM
View All 9 Files
Subscribers
ashoat

Details

Reviewers
bartek
marcin
tomek
Commits
rCOMMf380159b7402: [native] update C++ crypto module to handle session version
Summary

Updates in-memory CryptoModule to work with session versions.

Depends on D11580

Test Plan

Tested in TBD

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil created this revision.Apr 8 2024, 6:38 AM
kamil held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptApr 8 2024, 6:38 AM
kamil added inline comments.Apr 8 2024, 6:44 AM
native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp
79 ↗(On Diff #38904)

for notifs it's hardcoded now - we should update this in the future while working at [ENG-7658](https://linear.app/comm/issue/ENG-7658/add-olm-session-versioning-for-notifs

kamil added a child revision: D11583: [web] update `WorkerCryptoStore` to handle session version.Apr 8 2024, 6:47 AM
Harbormaster failed remote builds in B28054: Diff 38904!Apr 8 2024, 7:16 AM
kamil updated this revision to Diff 38911.Apr 8 2024, 7:20 AM

build wasm

Harbormaster completed remote builds in B28061: Diff 38911.Apr 8 2024, 7:49 AM
kamil published this revision for review.Apr 9 2024, 6:01 AM
marcin added inline comments.Apr 10 2024, 1:34 AM
native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp
79 ↗(On Diff #38904)

This code is only used to migrate old way of persisting session. Currently we serialize and persist the following JSON to MMKV:

{
    "session":<session bytes>
    "picklingKey": <picklingKey>
}

That said we need to update this JSON to have version field as well.

kamil added inline comments.Apr 10 2024, 6:07 AM
native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp
79 ↗(On Diff #38904)

since this value will be hardcoded for notifs - can we just ignore updating JSON and add it when it is needed? Not sure what could we gain from persisting mock value in MMKV

marcin accepted this revision.Apr 10 2024, 6:28 AM
marcin added inline comments.
native/cpp/CommonCpp/CryptoTools/Session.h
17 ↗(On Diff #38911)

Nit: Could we make it private and introduce getter?

This revision is now accepted and ready to land.Apr 10 2024, 6:28 AM
kamil updated this revision to Diff 39034.Apr 11 2024, 3:55 AM

introduce setter & getter

Harbormaster completed remote builds in B28160: Diff 39034.Apr 11 2024, 4:11 AM
Closed by commit rCOMMf380159b7402: [native] update C++ crypto module to handle session version (authored by kamil). · Explain WhyApr 11 2024, 4:47 AM
This revision was automatically updated to reflect the committed changes.
kamil added a commit: rCOMMf380159b7402: [native] update C++ crypto module to handle session version.

Revision Contents
Changeset List

PathSize
native/
cpp/
CommonCpp/
CryptoTools/
8 lines
7 lines
3 lines
8 lines
DatabaseManagers/
5 lines
NativeModules/
4 lines
Notifications/
BackgroundDataStorage/
8 lines
web/
shared-worker/
_generated/

Diff 39034

View Options

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

Loading...
View Options

native/cpp/CommonCpp/CryptoTools/Persist.h

Loading...
View Options

native/cpp/CommonCpp/CryptoTools/Session.h

Loading...
View Options

native/cpp/CommonCpp/CryptoTools/Session.cpp

Loading...
View Options

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp

Loading...
View Options

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp

Loading...
View Options

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

Loading...
View Options

web/shared-worker/_generated/comm_query_executor.wasm

Loading...