We have completed the security review of the new Pseudorandom Number Generator (PRNG) for OpenSSL1.1.1.
This security review was sponsored by Private Internet Access, ExpressVPN, DuckDuckGo, OpenVPN, and the privacy community.
Random number generation is a crucial component in all cryptography, because the “randomness” of numbers is the mechanism that makes secret numbers hard to guess. Problems with number generation can lead to serious consequences. When we learned that OpenSSL is getting a shiny new PRNG, we decided that it was crucial that we focus on making sure that the new code is as safe and robust as possible.
We had a team specifically review the new PRNG, which is also covered with a 2nd review by the cryptography review from QuarksLab. JP Aumasson and Antony Vennard carried out this component of our comprehensive review.
A total of ten issues were raised to improve the performance and/or randomness of the data created by the PRNG.
Because of these issues being found and corrected, OpenSSL’s new PRNG is safer, performs better, and will provide safer crypto to the world!
Issues Identified in OpenSSLs New PRNG:
- Issue 1: Insufficient Privileges Check
The OpenSSL PRNG checks privileges before allowing random bytes to be called. This check did not account for any future changes to the structure of privileges in Linux, specifically, POSIX privileges in Fedora and its downstream neighbors.
Status: Issue reported and patch issued here: https://github.com/openssl/openssl/pull/6993
- Issue 2: Entropy Bytes Discarded
The OpenSSL PRNG was structured in such a way that it frequently threw away bytes if the function called too many or too few bytes of random data, leading to sub-optimal performance.
Status: Issue reported and patches issued here: https://github.com/openssl/openssl/issues/6978 and here https://github.com/openssl/openssl/pull/6990
- Issue 3: Insufficient Size Comparison
The OpenSSL PRNG function ctr_128 checked if exactly 128 bits of keylength were in use, instead of greater than 128 bits of keylength.
The OpenSSL security team responded that these checks are sufficient in the context that they are used, because if the keylength doesn’t match the behavior of the software changes in a safe way, regardless of the keylength being larger or smaller than intended.
- Issue 4: Missing Null Pointer Checks in API Functions
The API for some PRNG functions fails to check for null pointers.
The OpenSSL security team responded that these checks are omitted by design. There is significant debate within the OpenSSL community on whether OpenSSL should employ null pointer checks or not. The full response by Matt Caswell of OpenSSL is below:
Passing NULL through in certain parameters is disallowed by the API. If you do it then you are incorrectly using the API – and the behavior is undefined.
Similarly if you pass NULL through to certain C library functions then you might see a crash.
It’s an ongoing debate within the OpenSSL community as to whether we should add NULL pointer checks or not. The big advantage to not having these checks (as per the argument of those in favour of not having them) is that you get immediate feedback that you’ve got a bug in your code, rather than potentially attempting to carry on even through an earlier operation failed. There is of course an overhead involved continually checking for NULL pointers through the whole library. Others take the opposite view that we should never allow a crash in the library if we can avoid it.
The Debate continues with no clear consensus at the current time.
In any case not having NULL checks in the specific locations identified was a deliberate choice. No changes were made a result of this issue.
- Issue 5: Ordering of Seed Sources
The Linux getrandom() function is prioritized over all others, even if other sources of entropy are specified. It is recommended that OpenSSL mixes entropy sources if others are specified through XOR or some other mixing function.
The OpenSSL team has requested that their exact response be shown here for clarity:
For performance reasons, and also in order not to hog a scarce system resource (see e.g. issue #5849), we should not collect more than the requested amount of entropy and return as soon as we succeeded. Also, XORing the random data of different sources would not be necessary, because we use a derivation function: Concatenating the data is sufficient, it will be mixed by the derivation function in the end. Note that if getrandom() fails for some reason then the other entropy sources will be used as fallbacks.
- Issue 6: Support for NONE Entropy Source
OpenSSL can be compiled with NONE as an entropy option. It is suggested that a warning be issued when a user attempts to do this as it is far less secure.
Status: Issue reported and patch issued here: https://github.com/openssl/openssl/issues/6980and https://github.com/openssl/openssl/pull/6981
The OpenSSL team has requested that their exact response be shown here for clarity:
This option -with-rand-seed=none is only meant as last resort in cases where OpenSSL does not know which entropy source to use. Note, however, that it does *not* relax the requirement for seeding the DRBG properly, it only disables *automatic* (re-)seeding. The DRBG will remain in an uninitialized state unless the application seeds it manually using RAND_add(). Provided that the application seeds (and reseeds) the DRBG manually with sufficient entropy from a reliable entropy source, this should be just as good as automatic reseeding.
The -with-rand-seed option and the manual reseeding process is documented in detail in the RAND_DRBG(7) manual page.
- Issue 7: Role of Additional Data
OpenSSL allows optional personalization data to be added to the PRNG. This data is low-entropy and will not help security in any scenario.
The OpenSSL team disagreed with our analysis. The full response by Matt Caswell of OpenSSL is below:
This is a requirement of SP800-90A. The idea is to differentiate the output from different instantiations of the DRBG. See section 8.7.1 of that document for more details.
- Issue 8: UEFI Randomness Generation
UEFI and VXWorks only support the “none” option for generating random data in OpenSSL. This is far less secure than other options. It is suggested that the OpenSSL team look into options like EFI_RNG_PROTOCOL or just rdrand. Both options are superior to using “none” as an entropy source.
The OpenSSL team wanted to contact the UEFI team to expand on this issue. As of this writing no changes have been made to our knowledge. We are waiting for additional comment from the OpenSSL Security Team on this issue.
- Issue 9: Modern Windows Randomness Generation
Older versions of Windows used an old entropy source called CryptGenRandom(). This was replaced in Windows XP and above by RtlGenRandom() which is a more modern and better source of entropy.
The team agreed that this may be a good change. As of this writing no changes have been made.
- Issue 10: Confusing Terminology in Documentation
OpenSSL comments and documentation use the words “entropy” and “entropy length” in non-standard ways, which can lead to confusion among developers trying to review and/or implement OpenSSL-based code.
The OpenSSL team said that the use of the words are in compliance with NISTs definition. The concern is that the NIST definition may be confusing to developers and lead to implementation errors for devs using OpenSSL as a library for their software.
The full report is below, please do not hot link to this report. Linking to this page instead allows us to bring more awareness to our cause! Thank you!
https://ostif.org/wp-content/uploads/2018/09/opensslrng-audit-report.pdf