RandomPool encourages insecure code
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Python-Crypto |
Fix Released
|
Undecided
|
Darsey Litzenberger |
Bug Description
RandomPool really really needs to die, at least in its current form. It's not thread-safe, it's not fork-safe, and if it can't get entropy from the OS, it silently produces predictable output. What's worse is that everyone assumes that it's a portable substitute for reading from /dev/urandom on Linux, but it's actually way too fragile to be safely used that way.
When I fixed Paramiko's usage of RandomPool back in January, Robey Pointer broke it again, so I ended up fixing it *twice* in one program.[1] Judging from the rest of the Paramiko code base, I certainly do NOT think Robey is a careless or stupid programmer; Random number generators are just hard to get right and RandomPool encourages people to get it wrong. I have yet to see any code that *explicitly* uses RandomPool correctly. Most of the code I've seen that avoids RandomPool-related security holes seems to do so accidentally---as long as it's not running on Windows and /dev/urandom exists.
I'm not sure that PyCrypto is the right place to implement a good RNG, anyway. Even if we make RandomPool thread-safe and make sure it initializes with enough entropy, it all becomes pointless as soon as somebody calls os.fork(), which duplicates the entire state pool. As far as I know, there's no easy way (and certainly no portable way) to prevent that. Also, absent an OS random number generator, arbitrary programs usually have little to no access to entropy. What makes matters worse is that, unless the RNG gets to run in its own process/thread (RandomPool does not) it relies totally on the user to supply the pool with new entropy at frequent intervals. That almost never happens.
Would there be any objection to replacing RandomPool with a simple wrapper around os.urandom? A quick benchmark shows that reading from /dev/urandom on Linux is about 10-50x faster than using RandomPool.
[1] See http://
Changed in pycrypto: | |
milestone: | none → 2.1.0 |
Dwayne, please go ahead and make any changes you deem necessary for this. Unfortunately, I have no time for PyCrypto maintenance these days, so I won't be able to do anything about this. I certainly agree that RandomPool doesn't make it easy to avoid errors in its usage.