signalapp / libsignal

Home to the Signal Protocol as well as other cryptographic primitives which make Signal possible.
GNU Affero General Public License v3.0
3.08k stars 362 forks source link

document a few constants #473

Closed cosmicexplorer closed 1 year ago

cosmicexplorer commented 2 years ago

Broken out from #287.

  1. Add documentation to several constants.
  2. Put those constants in a limits submodule (this part can be reverted if desired).
cosmicexplorer commented 2 years ago

I'm aware that constants like these are supposed to be implementation details, but I like the idea of bringing these out into the open because some of them have implications on security (e.g. MAX_FORWARD_JUMPS can be mapped directly to a specific line in the public specification for the double ratchet encryption process). A future change might be to make these constants into default values and allow consumers to override those by providing an additional parameter to the methods in {group_cipher,session_cipher,session,sender_keys}.rs that currently consume these constants.

cosmicexplorer commented 1 year ago

So I spent some time to understand what each of these limits actually represents, and I think the more involved explanations I have now are useful, in particular the description of what each limit actually bounds and why that matters (avoiding denial-of-service attacks, for one). I'm going to spend some more time understanding what an archived session state is.

While these descriptions are quite useful for me to understand what the library is doing (as implementation notes for some parts which aren't fully defined in the Double Ratchet spec), they may be too verbose for this file, so one possibility might be to move this more involved documentation to {session,group}_cipher.rs and just leave the pared-down explanation of what each limit actually bounds in these module docs.

If that arrangement seems useful, then we might also consider making limits a pub mod in order to expose these docs? Not sure right now. Going to figure out how session archiving works first before thinking about where to put these docs.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 1 year ago

This issue has been closed due to inactivity.