Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

General Improvements #268

Merged
merged 25 commits into from
Apr 1, 2022
Merged

General Improvements #268

merged 25 commits into from
Apr 1, 2022

Conversation

daxpedda
Copy link
Contributor

@daxpedda daxpedda commented Feb 25, 2022

  • Require Copy for both public and private key and switch to points instead of using PublicKey-like types.
  • Require Zeroize for both public and private key instead of only using KeGroup::zeroize_sk_on_drop() on the private key.
  • Fixed some missing checks in the ristretto255 implementation for zero scalars and identity elements during deserialization.
  • Base the X25519 implementation on curve25519-dalek instead of x25519-dalek to support Zeroize and drop two dependencies.
  • Use derive_where(ZeroizeOnDrop) instead of manual implementations.
  • Update argon2 to v0.4.
  • Remove unnecessary getrandom and constant_time_eq dependency.
  • Update derive-where to v1.0.0-rc.3
  • Remove unnecessary base64, lazy_static and sha2 dev-dependencies.
  • No multiple versions of the same dependency present anymore 🎉 (clippy::multiple_crate_versions).
  • Rename ristretto255_* and x25519_* crate features to ristretto255-* and x25519-*.
  • Remove unnecessary dependency feature activation of rand/std, rand/std_rng and voprf/std.
  • Fix possibility to produce zero scalars through KeGroup::hash_to_scalar (this is handled by the newest OPAQUE draft and the check added here should be removed).
  • Rename CipherSuite::OprfGroup to CipherSuite::OprfCs.
  • Rename TripleDH to TripleDh.
  • Remove slow-hash crate feature, argon2 should be used instead.
  • Rename SlowHash to Ksf (Key Stretching Function, as it is called in the spec).
  • Remove custom De/Serialize implementation for most types 🎉 (this removes the macro and uses derive instead).

Copy link
Contributor

@kevinlewi kevinlewi left a comment

Choose a reason for hiding this comment

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

Thanks! Is this still meant to be in draft mode?

@daxpedda
Copy link
Contributor Author

daxpedda commented Mar 7, 2022

I definitely plan to do more here, I'm continuing now actually. But I can also split it into multiple PR's or take it out of draft more, whatever you prefer.

@kevinlewi
Copy link
Contributor

It would be nice to be able to sync the implementation with the latest draft and its changes (https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-opaque-08.html). I'd be happy to work on that, but want to avoid any potential merge conflicts, since I will be pulling from the main branch. If you think it would be better to land these changes now, let me know and I can merge before starting my work.

@kevinlewi kevinlewi marked this pull request as ready for review April 1, 2022 23:09
@kevinlewi kevinlewi merged commit 384207a into facebook:main Apr 1, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants