Fix undeterminism #2

Activity

1. newpavlov commented on Dec 3, 2018

   

   newpavlov

   on Dec 3, 2018

   Member

   More actions

   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
   
2. newpavlov

   added

   bugSomething isn't workingSomething isn't working

   on Dec 3, 2018
3. dignifiedquire commented on Dec 3, 2018

   

   dignifiedquire

   on Dec 3, 2018

   MemberAuthor

   More actions

   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
4. newpavlov commented on Dec 3, 2018

   

   newpavlov

   on Dec 3, 2018

   Member

   More actions

   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?
5. tarcieri commented on Dec 3, 2018

   

   tarcieri

   on Dec 3, 2018

   Member

   More actions

   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.
6. dhardy commented on Dec 4, 2018

   

   dhardy

   on Dec 4, 2018

   Last edited by dhardy

   More actions

   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.
7. dignifiedquire

   added a commit that references this issue on Dec 4, 2018

   fix(decryption): ensure values are positive

   ...

   This issue will close once commit 645cd68 is merged into the 'master' branch.

   645cd68
8. dignifiedquire commented on Dec 4, 2018

   

   dignifiedquire

   on Dec 4, 2018

   MemberAuthor

   More actions

   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

   👍React with 👍1Reacted by Diggory Hardy
9. dignifiedquire

   closed this as completedin #4on Dec 5, 2018