-
Notifications
You must be signed in to change notification settings - Fork 161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
eax: Introduce a streaming API #214
Conversation
codecov.io check fails with
|
Some previous discussion on #132. One tricky naming thing here is we intend to implement STREAM, a provably secure segmented AEAD (a.k.a. OAE) construction designed by Phil Rogaway. This can be implemented generically for any AEAD mode (a.k.a. nonce-based OAE). As a contrast to that, perhaps we could find another word for the concept in this PR (i.e. #132) within the RustCrypto/AEAD implementations. Perhaps "incremental encryption"? |
Will fix, sorry about that. Edit: #215 should hopefully address it. |
Not to conflict with the generic STREAM scheme introduced in https://web.cs.ucdavis.edu/~rogaway/papers/oae.pdf.
Codecov Report
@@ Coverage Diff @@
## master #214 +/- ##
==========================================
- Coverage 88.56% 83.55% -5.02%
==========================================
Files 32 33 +1
Lines 1067 1131 +64
==========================================
Hits 945 945
- Misses 122 186 +64
Continue to review full report at Codecov.
|
Thanks for taking a look! I pushed a commit that prefers using the "online" word rather than stream. I'm kind of torn about not implementing Do you think I should also implement |
"Online" is also problematic, as STREAM (as in the Rogaway version) is a mode of Online Authenticated Encryption (specifically a nonce-based OAE or "nOAE"). |
As much as I understand reserving the "STREAM" name because it's introduced as a name/term for a useful generic framework capable of "online-ifying" a given nAE scheme, I don't think we should also reserve the "online" noun. After all, it's a well-known name of a property of an algorithm, just like EAX is online by design in general. Hypothetically, if I were to find anything related to the OAE/CHAIN/STREAM framework, I'd expect to find it under "OAE{,1,2}" as a, for better or worse, yet another introduced term in the nomenclature of authenticated encryption. |
Maybe "chunk(ed)" name would be better, then? Hopefully that would not imply the data to be of equal-length chunks, since "block" is a more commonly used name for this concept here. |
Regardless of what name we choose, I think it might be worth designing a trait for this use case and adding it to https://github.com/RustCrypto/traits/tree/master/aead /cc @newpavlov (Note: that's not necessarily a blocker for this PR, but might be worth investigating before another release) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am also not sure about a good name for such mode of operation. Maybe we could employ the IUF terminology here?
I think ideally we should design a trait for "online mode" first and update the crates later. There are some open design questions regarding buffering, enforcing encryptor/decryptor usage via type system and parallel processing of packets, but overall changes should be quite straightforward.
@Xanewok
Would you be willing to wait until such changes and use your fork in meantime? If you plan to use it in Sequoia in the near future, I guess we could merge this variant for now and push a minor release and migrate to the new traits in the next major release.
We currently use our own implementation but we decided that upstreaming the implementation would be a better idea and less frowned upon by the users AIUI. I believe that it'd be great for us to merge the approved version of this PR and use a newly-published minor release, as we'd like to ship our own 1.0 soon (cc @nwalfield @teythoon for release details)
@newpavlov Do you have any concrete ideas that I could apply already to the API surface introduced here? |
You already get it for free by using the |
Using a git fork is difficult, because as long as we are using a git fork, we can't make a release to crates.io, which, AIUI, only allows crates that rely on other crates published on crates.io.
There are a couple of things going on here. First, we don't want to use our own crypto, even if it is only a few dozen lines of code. Second, if we rely on something else, it would be good that it not pull in too many dependencies. That is, if we use the eax crate to avoid including 50 lines of crypto code and eax pulls in x000 LOC, it's unclear that we've made the right tradeoff.
We're getting very close to a 1.0 release. But, if we don't make it in the next few weeks, we'll almost certainly do another point release. So, we can't merge the eax code until the eax crate is released. |
I'd be okay with merging this for now after |
Releases versions that are upgraded to use `block-cipher` v0.8 and `stream-cipher` v0.7. - `aes-gcm` v0.7.0 - `aes-gcm-siv` v0.8.0 - `aes-siv` v0.4.0 - `ccm` v0.2.0 - `chacha20poly1305` v0.6.0 - `crypto_box` v0.4.0 - `eax` v0.2.0-pre (final release pending resolution of #214) - `xsalsa20poly1305` v0.5.0
Releases versions that are upgraded to use `block-cipher` v0.8 and `stream-cipher` v0.7. - `aes-gcm` v0.7.0 - `aes-gcm-siv` v0.8.0 - `aes-siv` v0.4.0 - `ccm` v0.2.0 - `chacha20poly1305` v0.6.0 - `crypto_box` v0.4.0 - `eax` v0.2.0-pre (final release pending resolution of #214) - `xsalsa20poly1305` v0.5.0
Releases versions that are upgraded to use `block-cipher` v0.8 and `stream-cipher` v0.7. - `aes-gcm` v0.7.0 - `aes-gcm-siv` v0.8.0 - `aes-siv` v0.4.0 - `ccm` v0.2.0 - `chacha20poly1305` v0.6.0 - `crypto_box` v0.4.0 - `eax` v0.2.0-pre (final release pending resolution of #214) - `xsalsa20poly1305` v0.5.0
Releases versions that are upgraded to use `block-cipher` v0.8 and `stream-cipher` v0.7. - `aes-gcm` v0.7.0 - `aes-gcm-siv` v0.8.0 - `aes-siv` v0.4.0 - `ccm` v0.2.0 - `chacha20poly1305` v0.6.0 - `crypto_box` v0.4.0 - `eax` v0.2.0-pre (final release pending resolution of #214) - `xsalsa20poly1305` v0.5.0
Brings back the old behavior - it's harder to misuse but separates the implementations of online and offline variants.
Dropped the drop bomb code. Before we merge this, I wanted to bring back the old behavior of first checking the tag and only then decrypting on success, in the offline variant. This would, however, separate the offline/online implementations but I don't feel comfortable not testing the online variant at all. Unfortunately, the Do you have any idea or preference on how to tackle this for the time being? Should I adapt the |
// HACK: Needed for the test harness due to AEAD trait online/offline interface mismatch | ||
#[cfg(test)] | ||
key: Key<Cipher>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it'd really be good if we could figure out some other solution to this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is my one lingering concern but I don't consider it a showstopper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, albeit with at least one notable open question: https://github.com/RustCrypto/AEADs/pull/214/files#r494580906
@newpavlov WDYT? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good as a temporary solution, I think we can finalize the naming after adding streaming API to the aead
crate.
@tarcieri
Can you do a release after merge?
Released in v0.2.0 |
Thanks! |
Introduces a
stream
module that implementsEaxStream
, which is the streaming variant of the regularEax
implemented here.This can be useful for two reasons, in addition to having the existing one-shot-like API:
I'm trying to reuse the EAX implementation for use in Sequoia (Rust OpenPGP implementation) but unfortunately I need the streaming API variant for that.
Because the newly-introduced in-place API can be somewhat easily misused, I tried to mimic a linear type, forcing the user to explicitly consume and handle the resulting value after decryption. With
std
feature enabled, it aborts the process using a drop bomb pattern, unless it's forgotten by theEaxStream::finish
associated function.Note that this changes the existing
Eax
implementation by first performing the decryption in-place and then checking against the expected tag, rather than scanning the ciphertext first to calculate tag and then decrypting if the tags match. I have to admit that I did that to share more of the implementation and leverage the testing framework test both implementations but I can revert that change if needed.Let me know what you think about this and if this is something you'd consider merging in one form or another.