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

Simplify KEM API #1509

Merged
merged 6 commits into from
Apr 2, 2024
Merged

Simplify KEM API #1509

merged 6 commits into from
Apr 2, 2024

Conversation

rozbb
Copy link
Contributor

@rozbb rozbb commented Feb 16, 2024

This is neat. So per our conversation in #1508, I simplified everything down to two traits which are highly generic. Some benefits:

  1. Way clearer what's going on
  2. No need to make our own SharedSecret type, or any type for that matter
  3. Removed need for std feature
  4. Removed need for generic_array at all (now we don't need to migrate to hybrid-array at this level)
  5. You can still do authenticated encap/decap! All you need is to make you Encapsulate struct have an identity privkey, and your Decapsulate struct have an identity pubkey. The X3DH example in tests/ shows this isn't hard.

One small nit, because I know we've talked about it before: what's the now preferred way to do RNGs? It was between mut rng: impl CryptoRngCore or rng: &mut impl CryptoRngCore + ?Sized.

cc @bifurcation

Copy link

@bifurcation bifurcation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rozbb, this refactor looks good to me modulo a couple of small things.

kem/src/lib.rs Outdated Show resolved Hide resolved
kem/src/lib.rs Outdated Show resolved Hide resolved
kem/src/lib.rs Outdated Show resolved Hide resolved
@rozbb
Copy link
Contributor Author

rozbb commented Mar 28, 2024

Addressed comments

@tarcieri
Copy link
Member

@bifurcation does this look good to you?

@bifurcation
Copy link

LGTM! Thanks for the nudge @tarcieri

@tarcieri tarcieri merged commit 59c34d2 into RustCrypto:master Apr 2, 2024
80 checks passed
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