Skip to content
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

Committing AEAD marker traits and generic constructions #1365

Closed
wants to merge 12 commits into from

Conversation

rlee287
Copy link
Contributor

@rlee287 rlee287 commented Oct 24, 2023

See the documentation comments for an explanation of committing AE and the generic constructions included.

TODO: bikeshedding of names and constructions that need changes to the underlying AEAD traits
While this exact construction hasn't appeared in the literature, it is
similar to CTX, with the original tag being exposed to allow it to be
implemented with the current AEAD traits, and a follow-up substitution of
HMAC instead of a raw hash function to ensure that exposing the original tag
does not open the committment up to length-extension type vulnerabilities.
In-place en/decryption doesn't quite make sense when the padding adds a ciphertext overhead
Turns out Aead and AeadMut do not clash with each other
@rlee287
Copy link
Contributor Author

rlee287 commented Oct 24, 2023

Seems like the test failures are caused by the cipher crate emitting a warning in the minimal version build, as opposed to being related to any of the changes I made.

@newpavlov
Copy link
Member

I think it will be better to move the wrapper into a different crate in the AEADs repository and link it in the new wrappers trait docs.

@tarcieri
Copy link
Member

Yeah, I had a similar first thought: this might make sense in a separate crate.

If it were located in the https://github.com/RustCrypto/AEADs/ repo it would be much easier to test the generic implementation against a concrete AEAD algorithm implementation located there.

@rlee287
Copy link
Contributor Author

rlee287 commented Oct 26, 2023

I'll move the wrappers into a separate PR to the AEADs repo, but I'll also take the time to add some tests using concrete AEADs. However, I'd like to confirm whether you think it'd be better to keep the marker traits in the aead crate of this repo or whether I should also move the traits into the separate crate as well.

@rlee287
Copy link
Contributor Author

rlee287 commented Oct 31, 2023

I opened a PR against the AEADs repo at RustCrypto/AEADs#564, so I'll close this one. I will send in a PR to cross-reference the new crate in the aead crate documentation after that one is merged.

@rlee287 rlee287 closed this Oct 31, 2023
@tarcieri
Copy link
Member

Sounds good. Since committing AEADs can be completely implemented in that crate, it seems like there isn't much value in having upstream marker traits?

We're going to start making breaking changes to aead soon, so if there's anything you do need changed, get a PR in ASAP

@rlee287
Copy link
Contributor Author

rlee287 commented Oct 31, 2023

Forgot to mention this, but I decided to move the marker traits into the new crate as well. Also, thanks for the heads up about upcoming breaking changes to the aead crate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants