-
Notifications
You must be signed in to change notification settings - Fork 47
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
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. |
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. |
TheZeroize
implementation 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.KeGroup
implementation(X25519 still outstanding).Hash
associated type inCipherSuite
and now the one fromOprfGroup
is used.ZeroizeOnDrop
trait where appropriate.Key
type and instead use type provided byKeGroup
.644
(probably messed up by me because I was using a MacBook).SlowHash
and move it to the method instead.crypto-bigint
.DeriveKeyPair
implementation using the wrong group forHashToField
.Issues introduced 😅:
ZeroizeOnDrop
had to be written by hand because the derive proc-macro doesn't supportGenericArray
.curve25519-dalek
v4.0.0-pre.2 because it constrainedzeroize
to<=1.4
. See Update curve25519-dalek requirement from =4.0.0-pre.1 to =4.0.0-pre.2 voprf#64.curve25519-dalek
v3 optionally forx25519-dalek
because the newest version doesn't use v4. See Update dependencies dalek-cryptography/x25519-dalek#87.Fixes #254.
Fixes #256.
Fixes #258.
Fixes #267.