Page MenuHomePhabricator

Implement methods in NotificationsCryptoModule to decrypt data
ClosedPublic

Authored by marcin on May 11 2023, 7:17 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 20, 6:58 AM
Unknown Object (File)
Wed, Apr 17, 6:46 PM
Unknown Object (File)
Tue, Apr 16, 3:54 PM
Unknown Object (File)
Tue, Apr 16, 3:11 AM
Unknown Object (File)
Mon, Apr 15, 8:40 PM
Unknown Object (File)
Mon, Apr 15, 3:47 PM
Unknown Object (File)
Mon, Apr 15, 3:39 PM
Unknown Object (File)
Mon, Apr 15, 3:38 PM
Subscribers

Details

Summary

This differential implements a method to decrypt data in NotificationsCryptoModule. Additionally some CryptoModule code is
simplified. I don't see the reason the ckeck for matching inbound sessions every time we receive a message. Additionally we would
have to store keyserver identity keys which would make the code more complicated.

Test Plan

After succesfull log-in execute in the keyserver code to encrypt some mock data. Then hardcode this data in
NotificationsService and call NotificationsCryptoModule::decrypt() method on it. Build the app again and send a notif to trigger
decryption code execution. Examine logs from the NSE.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ashoat added a subscriber: anunay.

I don't see the reason the ckeck for matching inbound sessions every time we receive a message.

This makes sense to me, but it'd be good to get @anunay's perspective on this as well. Is it safe for us to skip calling ::olm_matches_inbound_session_from when decrypting an inbound session?

I think it makes sense to remove the check... it doesn't make sense that we'd be checking the identity keys of the caller on each message, partly because we have no "proof" that the message is directly associated with the identity keys (outside of the standard chain-of-trust stuff to the chain key, which is already checked when we call ::olm_decrypt), and partly because the chain-of-trust should be enough "proof" for us.

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp
354 ↗(On Diff #26404)

Should we delete matchesInboundSession from the codebase? It appears we have no other uses

This revision is now accepted and ready to land.May 12 2023, 9:21 AM
native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp
354 ↗(On Diff #26404)

Since we recently we agreed on not to treat CryptoModule as a library, we can delete this methods that it is not used anywhere.

I confirmed with @anunay that we don't need this check and can remove it

Remove unecessary 'matchInboundSession' method