Implemented method to creating olm session.
Depends on D10375
Details
Details
This code is really hard to test as it's impossible to verify partial results.
I generated some keys and values, verified them with olm-utils.test.js, and checked if they succeeded/failed when calling the method introduced here in the same cases as JS tests.
Full functionality tested later in the stack.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp | ||
|---|---|---|
| 796–797 ↗ | (On Diff #34790) | wondering if we should add default 0 type |
| native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp | ||
|---|---|---|
| 796–797 ↗ | (On Diff #34790) | Not sure if this is what you're asking about, but I think all messages we send via Olm should be a structured JSON blob |
| native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp | ||
|---|---|---|
| 796–797 ↗ | (On Diff #34790) | While decrypting, we need to manually add message type like here which I believe it's always 1, only when decrypting for the very first time is 0, and was wondering if we need to do it explicitly because it work the way it is right now |
| native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp | ||
|---|---|---|
| 796–797 ↗ | (On Diff #34790) | Ah, I understand the question now. I'm not sure, but I'd be worried that default initialization of the size_t messageType parameter of struct EncryptedData might not always initialize to 0. My vague memory of C / C++ default initialization is that if you don't explicitly initialize to 0, it might be a "garbage value". Separately, my comment above was actually about the contents of the Olm-encrypted message. I think we should avoid encrypted raw text data, and instead make sure all of the text we're encrypted is "structured". This will help future-proof our system, for when we might have multiple categories of encrypted message. |
| native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp | ||
|---|---|---|
| 796–797 ↗ | (On Diff #34790) | It would be best if messageType field of EncryptedData structure was an enum with two possible values. If you think it is out of the scope of this diff, then please add sth like const prekeyOlmMessageType = 0; similarly as you did with the content of initial encrypted message returned from the counterpart method. Relying on C++ default initialization here is brittle and unreadable |
| native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp | ||
|---|---|---|
| 800 ↗ | (On Diff #34790) | This comment is relevant here as well. |
| native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp | ||
|---|---|---|
| 796 ↗ | (On Diff #35142) | This diff is almost LGTM, but there is one last change we need to introduce. We need to remove one time key from the account when creating inbound session just like we do on the keyserver. However after a quick glance at CryptoModule.cpp I realized that we don't even have an API in this class to remove a one time key after indound session creation! |