eax: Introduce a streaming API

[Xanewok]

Introduces a stream module that implements EaxStream, which is the streaming variant of the regular Eax implemented here.

This can be useful for two reasons, in addition to having the existing one-shot-like API:

1. The associated data can be processed separately
2. The protected data can be processed in a streaming fashion, rather than requiring it to be entirely allocated and contiguous.

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 the EaxStream::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.

[Xanewok]

codecov.io check fails with

Please provide an upload token from codecov.io with valid arguments

[tarcieri]

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"?

[tarcieri]

codecov.io check fails

Will fix, sorry about that.

Edit: #215 should hopefully address it.

[codecov-commenter]

Codecov Report

Merging #214 into master will decrease coverage by 5.01%.
The diff coverage is 0.00%.

@@            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     

Impacted Files	Coverage Δ
eax/src/lib.rs	90.00% <ø> (ø)
eax/src/online.rs	0.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d48b25...3b180b9. Read the comment docs.

[Xanewok]

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 AeadMutInPlace directly. One of the reasons is that we shouldn't need to pass the nonce to the encrypt/decrypt function everytime, once you instantiate the EAX mode. Another one is that the chunk decryption will always perform tag calculation but that may be wasteful and might be hard for the optimizer to recognize. It looks like this interface may not be designed for streaming/online use?

Do you think I should also implement AeadMutInPlace for the newly-introduced EaxOnline?

[tarcieri]

I pushed a commit that prefers using the "online" word rather than stream.

"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").

[Xanewok]

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.

[Xanewok]

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.

[tarcieri]

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)

[newpavlov]

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.

[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)

There are some open design questions regarding buffering, enforcing encryptor/decryptor usage via type system and parallel processing of packets (...)

@newpavlov Do you have any concrete ideas that I could apply already to the API surface introduced here?

[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 ctr crate and CMAC can not be parallelized by design. The implementation could benefit from interleaving MAC update and application of the keystream (one-pass approach is cache-friendlier, than the currently used two-pass one, see #74), but it will not be trivial, since we do not want ctr to touch its inner buffer. Probably we will need deeper changes in our API to make such optimization more ergonomic, e.g. by exposing primitives which would do updates on blocks-only without any inner buffering, on top of which will be built buffering and "all or nothing" counterparts.

[nwalfield]

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.

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.

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.

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.

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)

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.

[tarcieri]

I'd be okay with merging this for now after DropBomb is removed and the unauthenticated decryption method is renamed to note it's hazmat and has an appropriately descriptive comment about how potential misuse could lead to catastrophic cryptographic failures.

[Xanewok]

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 aead test case works in the integration tests format but I would not like to expose the raw set of operations and instead only expose the online mode to be used in either decrypt or encrypt mode. But because these are integration tests, I can't test private API nor I can doc(hidden) because of the trait interface I need to keep around a Key instance solely for tests...

Do you have any idea or preference on how to tackle this for the time being?

Should I adapt the aead::new_test! macro to accept a path to test case blob, rather than defaulting to data/$test_case.blb? This would allow me to define a test using that macro but from within the library unit tests, without exposing the internals.

[tarcieri]

Hmm, it'd really be good if we could figure out some other solution to this.

[tarcieri]

I guess this is my one lingering concern but I don't consider it a showstopper

[tarcieri]

Approved, albeit with at least one notable open question: https://github.com/RustCrypto/AEADs/pull/214/files#r494580906

[tarcieri]

@newpavlov WDYT?

[newpavlov]

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?

[tarcieri]

Released in v0.2.0

[Xanewok]

Thanks!