Clean up Hacl-rs integration and add Poly1305 and ChaCha20Poly1305 #703
Conversation
- drop portable_hacl feature - don't export hacl modules when hacl feature is set - (on some) add an expose-hacl feature to export hacl module
This commit adds inlines: - plain inline inside hacl-rs code (sometimes) - inline(always) for functions that call hacl-rs code This way our wrapper functions get inlined away and the caller just calls the hacl-rs function, and inside the hacl-rs function there are few calls to internal functions (where it makes sense).
Sounds good to me. |
There was a problem hiding this comment.
Generally lgtm with a couple nits.
The main question I have is around when hacl internals are exposed. The basic design should be that only a libcrux API is exposed (backed by some implementation), and we only expose something backend (hacl) specific if that's required by another backend (hacl) implementation, guarded by some fatures. You started doing that. But there still seem to be a bunch of exports of hacl specific modules. Or maybe it's just the naming that needs to change.
|
Regarding the features: I removed all the hacl features and only have the |
github-actions
bot
dismissed
franziskuskiefer’s stale review
Review re-requested
|
I hope this was it :) |
This PR addresses the following cleanup issues:
It also adds Poly1305 (MAC) and ChaChaPoly1305 (AEAD), which means we also make progress on this issue:
I am currently stuck on implementing the existing AEAD API with the new code, because that uses a single
ctxt_ptxtfunction argument to encrypt and does in-place encryption. The API exposed by hacl-rs uses separate arguments, and we can't put the argument in both because that would result in a shared mutable reference. I suggest we leave the old API be for now and address this once we have a better idea of what the final API should be.Opinions @franziskuskiefer @jschneider-bensch?
TODOs left:
Fixes #696
Fixes #673
Fixes #675