Move docs for sealed sender v1/v2 to the appropriate method docstrings

[cosmicexplorer]

Hello! I am going to finish up my other pending PRs to this repo soon :).

Problem

The sealed sender single-recipient and multi-recipient KEM schemes in Sealed Sender v1/v2 are super neat, and documentation for how they work exists in some (very helpful!) source comments. However, it's not immediately clear e.g. that the Signal server receives the result of sealed_sender_multi_recipient_encrypt(), splits the output into constituent sealed-sender v2 messages, then routes them individually to clients for sealed_sender_multi_recipient_decrypt(). Additionally, the design tradeoffs between v1 and v2 sealed sender mechanisms in general are not made clear in documentation of these public methods.

Solution

1. Move the (very useful!) documentation for the mechanism of v1 and v2 sealed sender into the docstrings for methods sealed_sender_encrypt_from_usmc() and sealed_sender_multi_recipient_encrypt().
2. Clarify the role of sealed_sender_multi_recipient_fan_out() in testing.
   • Clarify how the Signal server will split out sealed sender v2 bulk messages and route them to the appropriate recipients on the backend in sealed_sender_multi_recipient_encrypt().
3. Produce a section "Contrast with Sealed Sender v1" in the docstring for sealed_sender_multi_recipient_encrypt() which describes the pros and cons of the single-recipient vs multi-recipient KEM schemes implemented in the sealed_sender_v1 and sealed_sender_v2 modules.
4. Modify the definition of sealed_sender_v1::{EphemeralKeys,StaticKeys} to avoid repeated Result<Box<...>>ing, and make StaticKeys::calculate() accept &IdentityKeyPair and &IdentityKey instead of structs from curve.rs.
5. Make sealed_sender_v2::{apply_agreement_xor,compute_authentication_tag} accept and return fixed-size arrays instead of unsized slices.
6. Avoid separating the AES-GCM-SIV authentication tag from the rest of the ciphertext in sealed_sender_multi_recipient_encrypt() in order to differentiate it from the asymmetric authentication tags at_i computed later in the function when looping over the recipients.

Result

A rendered version of the docs can be seen here: https://cosmicexplorer.github.io/libsignal-client/target/doc/libsignal_protocol/.

This is a great codebase!!

[jrose-signal]

There's a public/private distinction that's important here: sealed_sender_multi_recipient_encrypt and sealed_sender_decrypt are public, but sealed_sender_v1 and sealed_sender_v2 are implementation details. Whether or not the public entry points mention the specific cryptography involved could go either way, but it can't reference private stuff if we're documenting this like a usual crate.

As an aside, there's nothing wrong with Sealed Sender v1. In fact, it's a smaller payload on the receiving side than Sealed Sender v2. But its encryption key is derived at least partially from the recipient's identity key, so even if the same content is sent to multiple recipients, it has to be re-encrypted each time. SSv2's real change is separating the encryption key from the recipient's identity key, so that only a per-recipient needs to be encrypted differently.

[cosmicexplorer]

SSv2's real change is separating the encryption key from the recipient's identity key

Ok, thanks! That was super helpful! In light of this I have expanded the scope of this PR to also cover documenting the mechanics of v1 sealed sender in sealed_sender_encrypt_to_usmc(), as well as a quick comparison of the design tradeoffs of the two mechanisms that you've just summarized!

I also made a quick change just now in a3e0031 to explicitly append the AES-GCM-SIV authentication tag to the end of the symmetric ciphertext in sealed_sender_multi_recipient_encrypt(), which I thought made it more clear that that authentication tag has nothing to do with the asymmetric authentication tags at_i computed later in the method.

There's a public/private distinction that's important here

Thanks! In light of this, all of the docs I've just added also explicitly avoid referring to "v1" or "v2" sealed sender mechanisms, instead referring to the single- and multi-recipient KEM schemes described in the docstrings for sealed_sender_encrypt_to_usmc() and sealed_sender_multi_recipient_encrypt().

It was a fun exercise writing these docs but if they're more than you'd like to review / aren't appropriate for the repo for some reason feel free to tell me to cut them down!

[cosmicexplorer]

I've pushed a rendered version of the docs from cargo doc to my fork's gh-pages branch, viewable at https://cosmicexplorer.github.io/libsignal-client/target/doc/libsignal_protocol/. The two method docstrings which are the subject of this PR:

• sealed_sender_encrypt_to_usmc()
• sealed_sender_multi_recipient_encrypt()

[cosmicexplorer]

Ok, I've addressed most of your comments and left 9 of them unresolved for further review! As before, the compiled cargo doc output has been pushed to https://cosmicexplorer.github.io/libsignal-client/target/doc/libsignal_protocol/, and the focus of this PR is on these two:

• Sealed Sender v1: sealed_sender_encrypt_from_usmc()
• Sealed Sender v2: sealed_sender_multi_recipient_encrypt()

[cosmicexplorer]

Ok, cf048dd integrates your notes on &...[..] and .expect("...") usage! It does not change any docstrings, so I have not pushed any docs changes since this comment: https://github.com/signalapp/libsignal-client/pull/366#issuecomment-933040985.

Of course, please feel free to make any further comments on code or docs changes, it's very easy for me to oblige.

I'll also probably do a once-over again and see if there's anything else I think could be improved. I think the result is DRASTICALLY better than my initial attempt here and I learned quite a lot from this PR :DDDD. Thanks again for that.

[cosmicexplorer]

After reviewing the docs again, I think I can sign off on this so merge it whenever! Thanks again for your time making this amazing, it helped me a lot and I hope many others too :)))