Implement xchacha reduced-round variants #355
Conversation
| xchacha20 = ["chacha20/xchacha"] | ||
| xchacha20poly1305 = ["xchacha20"] # alias | ||
| reduced-round = ["chacha20-reduced-round", "xchacha20-reduced-round"] | ||
| chacha20-reduced-round = ["chacha20"] | ||
| xchacha20-reduced-round = ["xchacha20"] |
There was a problem hiding this comment.
I left xchacha20poly1305. You may want to remove this in the future.
|
|
||
| /// Generic ChaCha+Poly1305 Authenticated Encryption with Additional Data (AEAD) construction. | ||
| /// | ||
| /// See the [toplevel documentation](index.html) for a usage example. | ||
| pub struct ChaChaPoly1305<C> | ||
| pub struct ChaChaPoly1305<C, N: ArrayLength<u8> = U12> |
There was a problem hiding this comment.
The default option is used here, so this has no effect on compatibility.
| stream = ["aead/stream"] | ||
| xchacha20poly1305 = ["chacha20/xchacha"] | ||
| xchacha20 = ["chacha20/xchacha"] |
There was a problem hiding this comment.
I feel a bit of explanation is needed as to why there's a chacha20 feature in the first place, as I feel there doesn't need to be an xchacha20 feature.
This feature activates the chacha20 crate.
The reason why it's optional is because the c2-chacha crate provided a faster implementation and also impl'd the RustCrypto traits, so people who really cared about performance could swap it in instead, and disable the chacha20 crate.
With RustCrypto/stream-ciphers#261 the gap has narrowed considerably (ty @str4d).
I think in the next breaking release we can make chacha20 a mandatory dependency again, which would eliminate the need for this feature.
Anyway this is all a long winded way of saying it feels like there are too many features here and I take responsibility for at least part of that. It seems like xchacha20poly1305 and xchacha20-reduced-round could both activate chacha20/xchacha.
Longer term perhaps it would also make sense for simplicity's sake to get rid of chacha20/xchacha and always include the XChaCha family as the code size really isn't that big.
|
Overall this looks great, although it seems the tests are failing due to unused import warnings under I also left one nit about a (cargo) "feature explosion" and am thinking it'd be good to simplify the overall feature set. |
|
I have tried to leave all changes within the PATCH change only. Not breaking change was the goal. It also makes the documentation more beautiful. One of the rust features that we rarely notice, but love. |
|
It might make sense to remove the |
|
Ok, let's start with this, and I can circle back on slimming down features when I can make breaking changes to the |
Codecov Report
@@ Coverage Diff @@
## master #355 +/- ##
==========================================
+ Coverage 89.05% 89.19% +0.14%
==========================================
Files 40 39 -1
Lines 1909 1897 -12
==========================================
- Hits 1700 1692 -8
+ Misses 209 205 -4
Continue to review full report at Codecov.
|
|
Thank you! |
XChaCha8Poly1305 and XChaCha12Poly1305 via ChaChaPoly1305 generalisation.