Conversation
Hopefully us of the future appreciate it.
zmre
left a comment
There was a problem hiding this comment.
Went over this pretty carefully and I think it's in good shape.
giarc3
left a comment
There was a problem hiding this comment.
Looks good, just a few suggestions
coltfred
left a comment
There was a problem hiding this comment.
I didn't do too in depth of a review because of current time constraints, but if it were me I'd add some proptests around the update and accumulator.
For example a test that feeds the data all in at once vs feeding it in piece by piece in small chunks to ensure you keep the data held aside correctly. You can get a random bytes vec to encrypt and random places in that array to break it up.
I would also add a proptest for our streaming encrypt and the normal decrypt and normal encrypt with streaming decrypt just to better cover that base.
I did look at the presented API as well and it looks good. I'm going to assume you pulled it through to swig to verify that it presents well in the other langs.
|
@coltfred I had actually thought of adding proptests, but seeing as we didn't have any in the repo, didn't know if this rose to the level to bring them in. I'll add them, and probably add some to cover other things as well.
I didn't, I saw no reason it wouldn't. I will before release. (edit: it does come through fine) |
Fixes #358
Adds file encrypt and decrypt with constant memory use via internal streaming. This should be a swig-friendly API (unlike actual iterators/streams or an update/finalize into ?? setup).
See
src/crypto/streaming.rsfor the critical core bits and tests, along with a nice bit of ASCII art trying to explain what's going on.encrypt_streamanddecrypt_streamin there are the next layer up and handle chunking. The next layer up is insrc/internal/document_api/file_ops.rs, which deals with the ironcore header-y bits around the ciphertext, thensrc/document/file.rswhich presents the final API for users (both managed and unmanaged).