Fix undeterminism

[dignifiedquire]

key::tests::key_generation_128 has failed with "unwrap() on None". I've restarted the test and it got "fixed". IIUC you have non-determinism in your tests and implementation has a bug.

Ref: #1 (comment)

[newpavlov]

Backtrace:

thread 'key::tests::key_generation_128' panicked at 'called `Option::unwrap()` on a `None` value', libcore/option.rs:355:21
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at libstd/sys_common/backtrace.rs:59
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:221
   4: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:476
   5: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:390
   6: rust_begin_unwind
             at libstd/panicking.rs:325
   7: core::panicking::panic_fmt
             at libcore/panicking.rs:77
   8: core::panicking::panic
             at libcore/panicking.rs:52
   9: rsa::key::decrypt
  10: rsa::key::tests::test_key_basics
  11: rsa::key::tests::key_generation_128
  12: <F as alloc::boxed::FnBox<A>>::call_box
             at libtest/lib.rs:1471
             at /rustc/f1e2fa8f0469aac1ea69dd5b6164e1d198d57934/src/libcore/ops/function.rs:238
             at /rustc/f1e2fa8f0469aac1ea69dd5b6164e1d198d57934/src/liballoc/boxed.rs:673
  13: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:102

[dignifiedquire]

hmm, so I suspect the issue is that thread_rng doesn’t give us enogh randomness, making the key gen fail during prime generation. I don’t have any good insights into the guarantees that thread_rng gives us, so my best guess would be to do

• move to osrng
• change the test to check for the actual error and if it is actually due to missing ramdomness to try again, as failing in this case is the right thing to do for the implementation

[newpavlov]

For key generation I believe we should use OS RNG directly. So we could use rand_os (once it lands) instead of the whole rand. I am not sure about entropy required for padding, safer (but slower) choice will be to use OS RNG there as well.

@tarcieri any thoughts?

[tarcieri]

As you can probably tell from the rand_os issue thread I'm a fan. That said, some people will probably want deterministic RSA eventually.

[dhardy]

hmm, so I suspect the issue is that thread_rng doesn’t give us enogh randomness, making the key gen fail during prime generation. I don’t have any good insights into the guarantees that thread_rng gives us

It's supposed to be crytographically secure and should always be well initialised. Can you explain what you mean by "not enough randomness"?

It may still be smart to use the OS RNG directly for key generation (if you don't care too much about the performance overhead or no_std compatibility) since a user-space PRNG has less protection than one in kernel memory. But if your tests are failing because of poor randomness that would be very concerning.

[dignifiedquire]

Can you explain what you mean by "not enough randomness"?

Great question, I had the idea that it wasn't able to find enough primes for some reason. But it doesn't make sense, and it turns out the bug had nothing to do with the random number generator. It was simply an edge case where negative numbers, where not moved properly back into the positive range properly. I implemented a fix here #4