Skip to content
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

change implementation of dusks Fr::random() function to be uniformly distributed #121

Closed
moCello opened this issue Nov 2, 2023 · 2 comments

Comments

@moCello
Copy link
Member

moCello commented Nov 2, 2023

Summary

The current implementation of Fr::random() in the dusk module generates a random array of 64 bytes and wraps them around to fit a Fr. This means that some of the values are hit more often than others. To ensure a uniform distribution, we can randomly generate a bit-pattern of 251 bits until a valid Fr representation is found.
This random scalar generation is not constant time but still as long as the underlying random number generator is functioning as it should, this is no problem.

Relevant Context

bls12_831#124

Possible Solution

    pub fn uni_random<R>(rng: &mut R) -> Self
    where
        R: RngCore + CryptoRng,
    {
        let mut buf = [0; 32];
        let mut scalar: Option<Self> = None;

        // We loop as long as it takes to generate a valid scalar.
        // As long as the random number generator is implemented properly, this
        // loop will terminate.
        while scalar == None {
            rng.fill_bytes(&mut buf);
            // Since modulus has at most 251 bits, we can zero the 5 MSBs and
            // improve our chances of hitting a valid scalar to above 50%
            buf[32 - 1] &= 0b0000_0111;
            scalar = Self::from_bytes(&buf).into();
        }
        scalar.unwrap()
    }
@moCello
Copy link
Member Author

moCello commented Nov 16, 2023

We want to avoid having two different implementations of the random scalar generation, which rules out the above proposal.

Instead we should mimic the bls library as implemented in dusk-network/bls12_381#129

@moCello moCello changed the title change implementation of dusks Fr::random() function to be uniformly distributed (like in BlsScalar::uni_random() change implementation of dusks Fr::random() function to be uniformly distributed Nov 16, 2023
moCello added a commit that referenced this issue Nov 17, 2023
moCello added a commit that referenced this issue Nov 21, 2023
moCello added a commit that referenced this issue Nov 22, 2023
moCello added a commit that referenced this issue Nov 22, 2023
moCello added a commit that referenced this issue Nov 22, 2023
Resolves: #121

Co-authored-by: Victor Lopez <vhrlopes@gmail.com>
moCello added a commit that referenced this issue Nov 22, 2023
Resolves: #121

Co-authored-by: Victor Lopez <vhrlopes@gmail.com>
@moCello
Copy link
Member Author

moCello commented Nov 29, 2023

After extensive discussions we decided to not change the random scalar generation because it was deemed secure enough for our purpose:

We will use this paper to determine whether the current implementation of the random scalar implementation as done in the Field trait implmentation is cryptographically safe.

In the current implementation of random, we sample a random element with 512 bits, and reduce it by the modulo of the field of JubJubScalar to generate a valid scalar.

For JubJubScalar we have:

p := 0x0e7db4ea6533afa906673b0101343b00a6682093ccc81082d0970e5ed6f72cb7
p = 6554484396890773809930967563523245729705921265872317281365359162392183254199

and

s := 2^512
s = 13407807929942597099574024998205846127479365820592393377723561443721764030073546976801874298166903427690031858186486050853753882811946569946433649006084096

To check whether the above approach is secure, we do the following:

For p < s calculate r and m so that

s = rp + m

This leads to:

r := 2045593080716281616348203381729468609728209645786990242449482205581148743408809

and

m := s - p * r
m = 2244478849891746936202736009816130624903096691796347063256129649283183245105

Now we can check whether:

r > 2^64 * root(m/p)
r > 2^64 * root(0.34243408237518300501)
r > 18446744073709551616 * 0.58517867559847308999

This clearly holds true, which means that we fall in the first conversion of the paper above, and can safely sample an element from the field over 2^512 and take it modulo p.

This concludes that we don't need to make any adjustments to the current implementation of random scalar generation.

@moCello moCello closed this as completed Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant