Revert "Write random_mod in terms of new random_bits"

[tarcieri]

This reverts commit ba4f0e0 (#1026)

For whatever reason this breaks the dsa test suite.

I'm in the middle of a major refactoring in crypto-bigint and this took quite a bit of bisecting to figure out on top of all of that, so I don't have time to investigate why.

cc @mrdomino

[codecov]

Codecov Report

❌ Patch coverage is 88.88889% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.81%. Comparing base (34a24b7) to head (34e59bd).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/uint/rand.rs	90.19%	5 Missing ⚠️
src/uint/boxed/rand.rs	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1060      +/-   ##
==========================================
+ Coverage   78.75%   78.81%   +0.06%     
==========================================
  Files         173      173              
  Lines       17671    17656      -15     
==========================================
- Hits        13917    13916       -1     
+ Misses       3754     3740      -14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mrdomino]

Ach, annoying. Sorry to cost you debugging time.

This is dsa?

I can try to take a look.

[tarcieri]

Yes, more specifically: https://github.com/RustCrypto/signatures/tree/master/dsa

Both sign and verify tests seemed to fail, though I'm not sure if the test hardcodes a particular keygen.

If you're interested in taking a look, let me know if you need help reproducing it.

[mrdomino]

I have a local repro working. Indeed, the tests generate a deterministic keypair in a way that ultimately winds up depending on random_mod_vartime, so the hardcoded expected signatures do not match up, because the different random numbers yield a different deterministic keypair.

With any downstream dependency of random_mod_vartime, for #1026, we’d have to update anything that makes expectations based on its output. (I gather dsa is not currently tested on 32-bit platforms, so the workflow does not notice #1009.)

How would we like to proceed? Options I see:

1. Change the dsa tests and expect to fix these differences in crates that depend on random_mod.
2. Provide two different functions, one with the old platform-dependent behavior and one with the new behavior; e.g. random_mod_vartime_old vs random_mod_vartime.
3. Go forward with a different fix for random_mod is not platform-independent #1009 that encodes the current 64-bit behavior on 32-bit platforms.

My preference would be 1 or 2 but I’m not opposed to any of them.

[tarcieri]

@mrdomino I'm fine with #1. It's annoying the test hardcodes a specific keygen. Sorry I didn't have time to examine it before reverting (though it's nice to be unblocked on upgrading crypto-bigint).

I can probably just revert the revert, but it would be good to get the dsa tests updated before then.

They can probably just be changed to verify a known good key/signature/message, and to generate a random key, sign a message, and then verify the signature, without hardcoding a specific signature produced.

[mrdomino]

Yeah, that makes sense to me. Sounds good - let me know if I can be of further assistance.

[mrdomino]

Went ahead and made an attempt at fixing dsa, but there are likely to be other crates that will need to change.

[tarcieri]

Hopefully we don't hardcode the RNG output anywhere else!