On Fri, Oct 16, 2015 at 9:04 PM, Steven D'Aprano <steve@pearwood.info> wrote:
On Fri, Oct 16, 2015 at 08:57:24AM +0200, Victor Stinner wrote:
Hi,
I like the PEP. IMHO it's a better solution than using a CPRNG for random by default.
I suggest to raise an error if token_bytes(n) if calls with n < 16 bytes (128 bits). Well, I'm not sure that 16 is the good compromise between performance and security, but we must enforce users to use a minimum number of bits of entropy. token_bytes(1) looks valid, even token_bytes(0), according to the Python code in the PEP.
Youtube URLs have what looks like about 8 or 9 bytes of randomness:
https://www.youtube.com/watch?v=B3KBuQHHKx0
(assuming it is base64 encoded, 11 chars is about 8 or 9 bytes).
I don't think that's randomness - it's some kind of unique ID, and I believe they lengthen with the log of the number of videos on Youtube.
Unless anyone has a good argument for why we should do this, I would be inclined to allow any value for nbytes. At most, I would be prepared to raise a warning if the nbytes was less than 16, but I don't think an error is appropriate.
Thoughts?
Not even a warning, IMO. If you ask for a certain number of bytes, you should get that many bytes. Raise ValueError on token_bytes(-1), and maybe token_bytes(0), but otherwise, do what you were told. For people who want "good enough security", token_bytes() is the correct choice; if you explicitly choose, you get what you ask for. And secrets.DEFAULT_ENTROPY is intentionally public, right? I see no __all__, and it doesn't start with an underscore. So if someone wants "twice as long as the default", s/he can spell that secrets.token_bytes(secrets.DEFAULT_ENTROPY*2). There should be no reason to hard-code a number if you don't specifically want that number.
You may also be a little bit more explicit on the CPRNG: it *looks* like secrets will always use a CRPNG implemented in the kernel. Is it a property of the secrets module, or can it be ssl.RAND_bytes() for example? IMHO we must always use a CRPNG implemented in the kernel, there is still an issue with ssl.RAND_bytes() and fork() (two child process can produce exactly the same random numbers after a lot of fork()...). I understood that OpenSSL developers doesn't want to fix it.
You may even be very explicit, list CPRNG that will be used on Python 3.6:
* Linux: getrandom() syscall if available (Linux 3.17 or newer), or /dev/urandom * Solaris: getrandom() function if available (Solaris 11.3 or newer), or /dev/urandom * OpenBSD: getentropy() function (OpenBSD 5.6 or newer), or /dev/urandom * Windows: CryptAcquireContext(PROV_RSA_FULL, CRYPT_VERIFYCONTEXT) and CryptGenRandom() * Other UNIXes: /dev/urandom
Does anyone else think that the secrets module should promise specific implementations?
I think it should NOT. The point of this is to provide whatever is believed to be secure, and that should never be promised. If RHEL is still shipping CPython 3.6 ten years from now, the best source of entropy might have changed, and the module should be able to be patched. It may be worth having the implementation introspectable, though - have some attribute that identifies the source and any CSPRNG algorithms used. Might not be much use in practice, given that you can just look at the source code. ChrisA