Security Implications of #675 (Delete rand.py)

[tbaxx]

In #675, the rand module was removed from the codebase, and users of the module were instructed to use os.urandom instead. However, using os.urandom is not an alternative for the whole rand module, just for some functions like rand.bytes.

There are other functions in the module, such as rand.add, that are changing the PRNG state of OpenSSL. Using such functions is of particular importance for fork safety, as described here: https://wiki.openssl.org/index.php/Random_fork-safety

and here:
https://docs.python.org/2/library/ssl.html#multi-processing

Please consider reverting #675, or at least bringing back the minimum subset of the rand module that will allow pyopenssl users to deal with fork safety.

[alex]

Hmm, we could restore just the Add function, or we could have pyOpenSSL install cryptograpy's alternate RNG which is fork-safe.

Opinions?

[tiran]

I'm for option 2, that is "install osrandom engine". It provides security w/o forcing the user to mess with OpenSSL internals.

[reaperhulk]

I think restoring the status quo with Add is the least invasive fix here. Since pyOpenSSL gets run in some very odd embedded contexts I'd prefer not installing the cryptography engine by default (although it will continue to be installed if you use the methods that convert to cryptography objects since that happens as a side effect of the lazy backend import).

[hynek]

Maybe we should add some blurb to it explaining why it’s important while doing so.

[tbaxx]

I came across this page, which describes a new approach for fork-safety in OpenSSL 1.1.1:
https://github.com/openssl/openssl/blob/master/doc/man3/OPENSSL_fork_prepare.pod

I guess it will take some time until 1.1.1 becomes the minimum supported version, but perhaps this is worth considering while you decide how to best tackle this issue.

[reaperhulk]

For posterity: we're aware of the upcoming improvements in OpenSSL 1.1.1 re: the userland CSPRNG. cryptography itself actually ships a default random engine that sources from urandom, getentropy, getrandom, or CryptGenRandom (depending on platform and OS version) to avoid this issue as well. However, to minimize any risk of incompatibility we've chosen to not import that engine by default in pyOpenSSL. You can, however, get it by importing cryptography's backend (from cryptography.hazmat.backends import default_backend) since it is an import time side effect.

#708 restores the rand subset to allow manual entropy mixing for conscientious forkers who can't utilize the cryptography random engine. In the future when these functions are called against 1.1.1 they will be a no-op.

[tbaxx]

@reaperhulk many thanks for the pull request #708 to restore part of the rand module.

Note that the security implications I am raising here have to do with OpenSSL's internal use of its own RNG. I can't think how the import of cryptography's random engine will change that. Please clarify.

[reaperhulk]

cryptography contains a C implementation of rand for the ENGINE interface within OpenSSL. During startup we register it and set it to the default random. This entirely replaces OpenSSL's internal RNG and makes it fork safe 😄 You can see the implementation here: https://github.com/pyca/cryptography/blob/master/src/_cffi_src/openssl/src/osrandom_engine.c

[tbaxx]

Thanks for clarifying!