-
Notifications
You must be signed in to change notification settings - Fork 111
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
Generating random secret key for secp256k1 is not correct #62
Comments
You claim that the group order must be explicitly stated? |
No, actually you can just use I mean for this code: impl Secp256k1Point {
pub fn random_point() -> Self {
let random_scalar: Secp256k1Scalar = Secp256k1Scalar::new_random(); // <- if it's not a valid secret key, the whole thing crashes
let base_point = Self::generator();
let pk = base_point.scalar_mul(&random_scalar.get_element());
Self {
purpose: "random_point",
ge: pk.get_element(),
}
}
} |
Using |
Hmm...It's little different since it's possibly continuously generating valid random bytes in the library: impl SecretKey {
/// Creates a new random secret key. Requires compilation with the "rand" feature.
#[inline]
#[cfg(any(test, feature = "rand"))]
pub fn new<R: Rng + ?Sized>(rng: &mut R) -> SecretKey {
let mut data = random_32_bytes(rng);
unsafe {
while ffi::secp256k1_ec_seckey_verify(
ffi::secp256k1_context_no_precomp,
data.as_ptr(),
) == 0
{
data = random_32_bytes(rng);
}
}
SecretKey(data)
}
// ...
} |
Suggest changing it to fn new_random() -> Self {
Self {
purpose: "random",
fe: SK::new(&mut thread_rng()),
}
} which looks nicer :) |
this is called rejection sampling and this is the right and secure way to generate this random number. |
This is not enough, a valid secret key should be [1, Group order), and group order is
fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141
The text was updated successfully, but these errors were encountered: