-
Notifications
You must be signed in to change notification settings - Fork 59
Group revamp #261
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
Group revamp #261
Conversation
df4bb8d to
7310dac
Compare
c95668e to
55fad1b
Compare
|
This is currently broken for a couple of reasons:
Will figure out how to fix/report this upstream. |
0428627 to
dcd4d93
Compare
c4a7f92 to
1d5ba34
Compare
1d5ba34 to
fc91a6a
Compare
|
@kevinlewi this should be ready to review. I will keep in draft because I'm going to push some more general code improvements, if that's alright. |
kevinlewi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, this was a big one, thanks so much!!
Should I wait to merge for the general improvements you wanted to add, or go ahead and merge now?
|
I can make the general improvements as a follow-up, so go ahead! |
|
Pushed the improvements I had here for now, feel free to merge (or tell me to stop 😄) anytime. |
TheZeroizeimplementation and testing got out of hand here. I will improve on that later. See RustCrypto/utils#652, RustCrypto/utils#699 and RustCrypto/utils#700 on progress how we can improve on that.KeGroupimplementation(X25519 still outstanding).Hashassociated type inCipherSuiteand now the one fromOprfGroupis used.ZeroizeOnDroptrait where appropriate.Keytype and instead use type provided byKeGroup.644(probably messed up by me because I was using a MacBook).SlowHashand move it to the method instead.crypto-bigint.DeriveKeyPairimplementation using the wrong group forHashToField.Issues introduced 😅:
ZeroizeOnDrophad to be written by hand because the derive proc-macro doesn't supportGenericArray.curve25519-dalekv4.0.0-pre.2 because it constrainedzeroizeto<=1.4. See Update curve25519-dalek requirement from =4.0.0-pre.1 to =4.0.0-pre.2 voprf#64.curve25519-dalekv3 optionally forx25519-dalekbecause the newest version doesn't use v4. See Update dependencies dalek-cryptography/x25519-dalek#87.Fixes #254.
Fixes #256.
Fixes #258.
Fixes #267.