Add support for receiving sender keys #171
Conversation
… groups with sender keys.
gferon
changed the title
libsignal-service/src/cipher.rs
Outdated
| } else if envelope.content.is_some() { | ||
| let plaintext = self.decrypt(&envelope).await?; | ||
| let message = | ||
| crate::proto::Content::decode(plaintext.data.as_slice())?; | ||
| Ok(Content::from_proto(message, plaintext.metadata)) | ||
| if let Some(bytes) = message.sender_key_distribution_message { | ||
| dbg!("PROCESSING SENDER KEYSSSSSSSS -------------------------"); |
There was a problem hiding this comment.
Clippy says it too, but this should probably go away. If you want, use log::debug!() etc. :-)
There was a problem hiding this comment.
the joys of printf debugging
libsignal-service/src/cipher.rs
Outdated
| sender_device: envelope.source_device(), | ||
| sender_device: envelope.source_device().into(), |
There was a problem hiding this comment.
Indeed, I started to change it to DeviceId everywhere and reverted because it's somewhat unrelated. This is just a remnant, I just filed #173.
libsignal-service/src/cipher.rs
Outdated
| sender_device: envelope.source_device(), | ||
| sender_device: envelope.source_device().into(), |
| /// is then validated against the `trust_root` baked into the client to ensure that the sender's | ||
| /// identity was not forged. | ||
| #[allow(clippy::too_many_arguments)] | ||
| async fn sealed_sender_decrypt( |
There was a problem hiding this comment.
I don't immediately see how this works. Should the application call this method when the ContentBody matches some specific variant? This function is, as far as I can see, never used (but clippy doesn't call you out on it, so I'm potentially blind).
Since you tested in Presage, I assume this works, for both receiving and sending.
There was a problem hiding this comment.
This is (unfortunately) a copy-pasta of the current impl in libsignal and is used here
libsignal, which somehow doesn't have a branch for CiphertextMessageType::SenderKey https://github.com/signalapp/libsignal/blob/main/rust/protocol/src/sealed_sender.rs#L1647
There was a problem hiding this comment.
I don't quite understand why this is another function. I checked the sources of Signal Desktop. And there this code is part of open_envelope.
There was a problem hiding this comment.
Basically, what they do there:
- Unseal the envelope with
sealedSenderDecryptToUsmc - Check the
msgtype(here is should be part of case for unidentified sender) - Depending on the type of
msgtypecalldecryptorgroupDecrypt
There was a problem hiding this comment.
Ah, now I got it. Basically, you are doing it one level deeper compared to Signal Desktop, which actually makes sense.
|
this PR is missing using sender keys to send group messages, I'll implement this here as well so that we can really test it. |
That would be great. I've just started to work towards unidentified sending and sender keys in Whisperfish: https://gitlab.com/whisperfish/whisperfish/-/merge_requests/306 It already compiles against this branch, and I've set the unidentified keys and flag :-) |
Actually, it makes sense to merge support for receiving as fast as possible, sending will take a bit more time as usual. I feel like we need to have clients in the wild that can receive for a few weeks before we switch to sending using sender keys. Wdyt? |
gferon
changed the title
Agreed. Then again, I don't have sealed sending yet in WF. I'd like to get that in ASAP too, and then I can push receiving via Sender Keys at the same time, and start working on sending via Sender Keys. What scares me most is the registration and upgrade procedure... but I'll figure it out. Let me know when you want another review. |
Exactly, it's actually fine to have sealed sender enabled, receive sender keys and groups encrypted with sender keys, without being able to send with them (at least for now, it works). I'm up for doing the sending part as well after that! |
|
If/when signalapp/libsignal#494 is merged, we can remove most of the copy-pasted code here with a bump of |
There's two parts to supporting sender keys:
SenderKeyDistributionMessagewhen it arrives andI believe one oversight (that prevented us from diagnosing this earlier) was to swallow errors when converting the Protobuf to
ContentBody, I changed the signature of the function to actually report a failure when we get some unsupported (from our side) data type.Note #1: while I think this is it, I'm currently testing it with
presageNote #2: not supported is generating and sending sender keys