Generating random secret key for secp256k1 is not correct about curv

[omershlo]

You claim that the group order must be explicitly stated?
Btw, this is not a secret key , just a random field element...

[kigawas]

No, actually you can just use SK::new(&mut thread_rng());

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(),
        }
    }
}

[omershlo]

Using SK::new(&mut thread_rng()); is the same as what I am doing.
The secp256k1 library is taking care on how to securely sample the number in the field. When the consumed library provide this service (and in this case it's even constant time) then I prefer to use it than to write my own random sampler. i.e. to avoid stuff like the first vulnerability in the audit:
https://github.com/KZen-networks/curv/blob/master/audit/kzen-curv-audit%20final%2003.01.2019.pdf

[kigawas]

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)
    }
    // ...
}

[kigawas]

Suggest changing it to

    fn new_random() -> Self {
        Self {
            purpose: "random",
            fe: SK::new(&mut thread_rng()),
        }
    }

which looks nicer :)

[omershlo]

this is called rejection sampling and this is the right and secure way to generate this random number.