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
Unknown Object (File)
Thu, Nov 7, 3:49 PM
Unknown Object (File)
Thu, Nov 7, 2:34 PM
Unknown Object (File)
Thu, Nov 7, 12:50 PM
Unknown Object (File)
Sun, Nov 3, 8:12 AM
Unknown Object (File)
Oct 12 2024, 10:59 PM
Unknown Object (File)
Oct 12 2024, 10:59 PM
Unknown Object (File)
Oct 12 2024, 10:58 PM
Unknown Object (File)
Oct 12 2024, 10:58 PM
View All 37 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
Branch
publish-session-resetting
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

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

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

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
2 lines
DatabaseManagers/
5 lines
NativeModules/
4 lines
Notifications/
BackgroundDataStorage/
8 lines

Diff 38904

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/DatabaseManagers/SQLiteQueryExecutor.cpp

Loading...
View Options

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp

Loading...
View Options

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

Loading...