Page MenuHomePhabricator

[native] Calculate SHA256 of encrypted blob
ClosedPublic

Authored by bartek on Apr 27 2023, 3:53 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 3, 7:57 PM
Unknown Object (File)
Wed, Apr 3, 7:57 PM
Unknown Object (File)
Wed, Apr 3, 7:56 PM
Unknown Object (File)
Wed, Apr 3, 7:56 PM
Unknown Object (File)
Wed, Apr 3, 7:47 PM
Unknown Object (File)
Mar 5 2024, 8:24 AM
Unknown Object (File)
Mar 5 2024, 8:24 AM
Unknown Object (File)
Feb 23 2024, 8:42 PM
Subscribers

Details

Summary

We need the blob hash in order to upload the encrypted blob to Blob service.
According to whitepaper, we use SHA256 to calculate the hash of encrypted blob.

Exposed the calculated SHA256 within encryption result payload.

Test Plan

Added console.log and sent encrypted video message. Verified that both video and thumbnail have blobHash field.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Apr 27 2023, 4:36 AM
bartek added inline comments.
web/media/encryption-utils.js
72 ↗(On Diff #25810)

This will be set in one of the web-part stack diffs

atul added inline comments.
lib/types/media-types.js
270 ↗(On Diff #25810)

We need the blob hash in order to upload the encrypted blob to Blob service.

Should sha256 be string instead of ?string?

EDIT: Looks like we can make this string after web/media/encryption-utils.js is updated?

native/media/encryption-utils.js
110–115 ↗(On Diff #25810)

Would it make sense to have a different reason instead of "encryption_failed" if computing sha256 fails (not sure in what scenario this would happen)?

185–189 ↗(On Diff #25810)

We could destructure these fields from encryptionResult, but whatever you prefer

This revision is now accepted and ready to land.May 3 2023, 10:45 AM
lib/types/media-types.js
270 ↗(On Diff #25810)

Yes, I'll do that in the web diff for this

native/media/encryption-utils.js
110–115 ↗(On Diff #25810)

Sure, will do

native/media/encryption-utils.js
185–189 ↗(On Diff #25810)

In this case, I'd prefer not to because this function is already too long and this wouldn't improve readability IMO