[native] Expose Olm sha256 via JSI
ClosedPublic
Actions

Authored by bartek on Apr 25 2023, 4:48 AM.

Details

Reviewers

marcin
atul

Commits

rCOMMce5f64f378ba: [native] Expose Olm sha256 via JSI

Summary

I'm going to need this for calculating the sha256 blob hash for Blob service.
As suggested in one of "Cryptography discussion" threads, I exposed the Olm sha256 via JSI.

I was basing on the official Olm unit test example.

Test Plan

Called commUtilsModule.sha256() and compared the result with the unit test above:

const helloWorldArray = new Uint8Array([0x48, 0x65, 0x6c, 0x6c, 0x6f, 0x2c, 0x20, 0x57, 0x6f, 0x72, 0x6c, 0x64]);
const helloWorldHash = commUtilsModule.sha256(helloWorldArray.buffer);
console.log('Actual:\t', helloWorldHash);
console.log('Expected:\t', 'A2daxT/5zRU1zMffzfosRYxSGDcfQY3BNvLRmsH76KU');

Results can be validated via: https://emn178.github.io/online-tools/sha256.html - first use SHA256, then Base64 encode

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Apr 25 2023, 4:48 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptApr 25 2023, 4:48 AM

Harbormaster completed remote builds in B18733: Diff 25655.Apr 25 2023, 5:05 AM

bartek requested review of this revision.Apr 25 2023, 5:05 AM

marcin accepted this revision.Apr 26 2023, 5:53 AM

marcin added inline comments.

native/cpp/CommonCpp/NativeModules/CommUtilsModule.cpp
122 ↗	(On Diff #25655)	Lets stick to the convention: OlmBuffer sha256Bytes{sha256Size};
native/cpp/CommonCpp/NativeModules/CommUtilsModule.h
21 ↗	(On Diff #25655)	This variable appears to be used just to initialize `olmUtility` memory. Perhaps we can create it `lazily` in the constructor and not keep it as a class member?

This revision is now accepted and ready to land.Apr 26 2023, 5:53 AM

bartek added inline comments.Apr 26 2023, 6:06 AM

native/cpp/CommonCpp/NativeModules/CommUtilsModule.h
21 ↗	(On Diff #25655)	As I understand Olm internals, this is used to store the `OlmUtility` data, that the `olmUtility` variable points to. Holding it as a class member leverages RAII to manage its memory and keep it for the module's lifetime. If it was created in constructor only, it would be deallocated right after constructor finishes its execution, leaving the `olmUtility` pointer dangling. We're using the same approach in `CryptoTools.h` to store `OlmAccount`. Correct me if I'm wrong. Personally I don't like this solution either (why is Olm designed this way? maybe C compatibility?)*. I'd prefer to initialize it directly (no ptr) or use `unique_ptr`, but not sure if utility functions like `olm_utility_last_error` or `olm_sha256` would work with `OlmUtility` allocated this way.

bartek added inline comments.Apr 26 2023, 7:00 AM

native/cpp/CommonCpp/NativeModules/CommUtilsModule.cpp
122 ↗	(On Diff #25655)	This won't work here - braces syntax will lead to the wrong constructor overload ((10) instead of (4) here) so we'd get a single-element vector with 1st elem equal to `sha256Size` instead of `sha256Size`-sized vector.