ulif / diceware

Passphrases to remember
GNU General Public License v3.0
357 stars 45 forks source link

Switch to cryptographic RNG #70

Closed jtojnar closed 4 years ago

jtojnar commented 4 years ago

The Python docs for random explicitly say:

Warning The pseudo-random generators of this module should not be used for security purposes. For security or cryptographic uses, see the secrets module.

So let’s switch to secrets.

The downside is that the module is only available in Python ≥ 3.6 but that has been released in 2016 so it should not be that much of a problem.

ulif commented 4 years ago

Thank you for your PR, @jtojnar !

I was indeed not aware of secrets substituting random in the standardlib. In the long run, you might be right, but for now I have two problems with the suggested changes.

1) With your PR we drop Python 2 support for no sufficient reason, IMHO. With conditional imports it would be a two-liner to still support legacy stuff, even if Python 2 randomness is partly quirky. I would rather support Python 2 as long as it does not require too much efforts. Efforts here are minimal.

2) I agree, that switching to secrets in the long run might make sense, but for the cryptographic stuff we use in diceware currently, I cannot see any advantages. At least until Python3.8 there seems to be no difference (except naming and wrapping) between the PRNGs in use. Especially secrets does not provide an own CPRNG, but uses (like random) stuff found in the system (usually /dev/urandom). I would consider that strong enough for diceware and the warning in the docs might be a bit misleading in that regard. Also secrets.choice() seems currently only to be a wrapper around random.SystemRandom.choice() (which we use already).

So, in fact we would only get additional wrappers around our security-sensitive stuff.

Please tell, if there is something I missed.

jtojnar commented 4 years ago

You are right, secrets module does in fact re-export the PRNG from random:

https://github.com/python/cpython/blob/6ae2780be0667a8dc52c4fb583171ec86067d700/Lib/secrets.py#L9-L19

I was confused by the docs for random saying:

Warning The pseudo-random generators of this module should not be used for security purposes. For security or cryptographic uses, see the secrets module.

and also

Class that uses the os.urandom() function for generating random numbers from sources provided by the operating system.

and knowing that /dev/urandom is PRNG generated from /dev/random.

The docs for that function make me more at ease:

Return a string of size random bytes suitable for cryptographic use.

plus https://crypto.stackexchange.com/questions/41595/when-to-use-dev-random-over-dev-urandom-in-linux