oxidecomputer / sprockets

Now's the time on sprockets when we dance
Mozilla Public License 2.0
7 stars 1 forks source link

Session: Decouple reading/decrypting ciphertext from copying plaintext #19

Closed andrewjstone closed 2 years ago

andrewjstone commented 2 years ago

This commit is the companion to #17. It attempts to clarify the stages of buffered decryption for easier reading. The buffer manipulation itself is now performed in two typestates in aead_read_buf. Reading from the underlying data source and copying from the buffered reader to the users buffer is performed in decrypting_buf_reader.

While the code is more delineated into its stages, and each individual function is shorter, there is still more cross function control flow dependency than I would like. The code in my opinion should be more linear than it is, and yet I struggled to get it there. I'm not really sure of the reason for this, but I suspect it has to do with manipulating the typestates directly in DecryptingBufReader, rather than encapsulating their ownership and movement in and option containing an enum in an AeadReadBuf struct. While this was not as much of a problem on the write side, the looping flow of the read side seems to make it a bit trickier. This looping also causes ergonomic issues when dealing with ownership transfer of the FnOnce decrypt function. In both the previous iteration of this code, and this version of the code, the decrypt function must be passed back from child functions that don't use it.

In summary, while this code is an improvement IMO, it's not done, and I'd like to continue iterating before merging.