disable the osrandom engine on 1.1.1d+ (#5317)

* disable the osrandom engine on 1.1.1d+

* skip (and run) some tests on 1.1.1d+

* simplify our conditionals

* Update src/_cffi_src/openssl/src/osrandom_engine.c

Co-authored-by: Alex Gaynor <[email protected]>

* words

* more words

* language

* get coverage more cleverly

* a word

* Update .github/workflows/ci.yml

Co-authored-by: Alex Gaynor <[email protected]>

Co-authored-by: Alex Gaynor <[email protected]>

.github/workflows/ci.yml

@@ -14,11 +14,11 @@ jobs:
     strategy:
       matrix:
         PYTHON:
-          - {VERSION: "2.7", TOXENV: "py27"}
-          - {VERSION: "3.5", TOXENV: "py35"}
-          - {VERSION: "3.6", TOXENV: "py36"}
-          - {VERSION: "3.7", TOXENV: "py37"}
-          - {VERSION: "3.8", TOXENV: "py38"}
+          - {VERSION: "2.7", TOXENV: "py27", EXTRA_CFLAGS: ""}
+          - {VERSION: "3.5", TOXENV: "py35", EXTRA_CFLAGS: ""}
+          - {VERSION: "3.6", TOXENV: "py36", EXTRA_CFLAGS: ""}
+          - {VERSION: "3.7", TOXENV: "py37", EXTRA_CFLAGS: ""}
+          - {VERSION: "3.8", TOXENV: "py38", EXTRA_CFLAGS: "-DUSE_OSRANDOM_RNG_FOR_TESTING"}
     name: "Python ${{ matrix.PYTHON.VERSION }} on macOS"
     steps:
       - uses: actions/checkout@master
@@ -40,10 +40,11 @@ jobs:
         run: |
           CRYPTOGRAPHY_SUPPRESS_LINK_FLAGS=1 \
             LDFLAGS="${HOME}/openssl-macos/lib/libcrypto.a ${HOME}/openssl-macos/lib/libssl.a" \
-            CFLAGS="-I${HOME}/openssl-macos/include -Werror -Wno-error=deprecated-declarations -Wno-error=incompatible-pointer-types-discards-qualifiers -Wno-error=unused-function -Wno-error=unused-command-line-argument -mmacosx-version-min=10.10 -march=core2" \
+            CFLAGS="-I${HOME}/openssl-macos/include -Werror -Wno-error=deprecated-declarations -Wno-error=incompatible-pointer-types-discards-qualifiers -Wno-error=unused-function -Wno-error=unused-command-line-argument -mmacosx-version-min=10.10 -march=core2 $EXTRA_CFLAGS" \
             tox -r --  --color=yes --wycheproof-root=wycheproof
         env:
           TOXENV: ${{ matrix.PYTHON.TOXENV }}
+          EXTRA_CFLAGS: ${{ matrix.PYTHON.EXTRA_CFLAGS }}
 
       - name: Upload coverage
         run: |
@@ -58,11 +59,11 @@ jobs:
           - {ARCH: 'x86', WINDOWS: 'win32'}
           - {ARCH: 'x64', WINDOWS: 'win64'}
         PYTHON:
-          - {VERSION: "2.7", TOXENV: "py27", MSVC_VERSION: "2010"}
-          - {VERSION: "3.5", TOXENV: "py35", MSVC_VERSION: "2019"}
-          - {VERSION: "3.6", TOXENV: "py36", MSVC_VERSION: "2019"}
-          - {VERSION: "3.7", TOXENV: "py37", MSVC_VERSION: "2019"}
-          - {VERSION: "3.8", TOXENV: "py38", MSVC_VERSION: "2019"}
+          - {VERSION: "2.7", TOXENV: "py27", MSVC_VERSION: "2010", CL_FLAGS: ""}
+          - {VERSION: "3.5", TOXENV: "py35", MSVC_VERSION: "2019", CL_FLAGS: ""}
+          - {VERSION: "3.6", TOXENV: "py36", MSVC_VERSION: "2019", CL_FLAGS: ""}
+          - {VERSION: "3.7", TOXENV: "py37", MSVC_VERSION: "2019", CL_FLAGS: ""}
+          - {VERSION: "3.8", TOXENV: "py38", MSVC_VERSION: "2019", CL_FLAGS: "/D USE_OSRANDOM_RNG_FOR_TESTING"}
     name: "Python ${{ matrix.PYTHON.VERSION }} on ${{ matrix.WINDOWS.WINDOWS }}"
     steps:
       - uses: actions/checkout@master
@@ -85,6 +86,7 @@ jobs:
             python .github/workflows/download_openssl.py windows openssl-${{ matrix.WINDOWS.WINDOWS }}-${{ matrix.PYTHON.MSVC_VERSION }}
             echo "::set-env name=INCLUDE::C:/openssl-${{ matrix.WINDOWS.WINDOWS }}-${{ matrix.PYTHON.MSVC_VERSION }}/include;%INCLUDE%"
             echo "::set-env name=LIB::C:/openssl-${{ matrix.WINDOWS.WINDOWS }}-${{ matrix.PYTHON.MSVC_VERSION }}/lib;%LIB%"
+            echo "::set-env name=CL::${{ matrix.PYTHON.CL_FLAGS }}"
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
       - run: git clone https://github.com/google/wycheproof

CHANGELOG.rst

@@ -46,6 +46,9 @@ Changelog
   :meth:`~cryptography.x509.CertificateSigningRequest.get_attribute_for_oid`.
 * Added support for encoding attributes in certificate signing requests via
   :meth:`~cryptography.x509.CertificateSigningRequestBuilder.add_attribute`.
+* On OpenSSL 1.1.1d and higher ``cryptography`` now uses OpenSSL's
+  built-in CSPRNG instead of its own OS random engine because these versions of
+  OpenSSL properly reseed on fork.
 
 .. _v2-9-2:
 

docs/hazmat/backends/openssl.rst

@@ -68,6 +68,12 @@ greater.
 OS random engine
 ----------------
 
+.. note::
+
+    As of OpenSSL 1.1.1d its CSPRNG is fork-safe by default.
+    ``cryptography`` does not compile or load the custom engine on
+    these versions.
+
 By default OpenSSL uses a user-space CSPRNG that is seeded from system random (
 ``/dev/urandom`` or ``CryptGenRandom``). This CSPRNG is not reseeded
 automatically when a process calls ``fork()``. This can result in situations

src/_cffi_src/openssl/cryptography.py

@@ -50,6 +50,14 @@
     (OPENSSL_VERSION_NUMBER < 0x10101000 || CRYPTOGRAPHY_IS_LIBRESSL)
 #define CRYPTOGRAPHY_OPENSSL_LESS_THAN_111B \
     (OPENSSL_VERSION_NUMBER < 0x10101020 || CRYPTOGRAPHY_IS_LIBRESSL)
+#define CRYPTOGRAPHY_OPENSSL_LESS_THAN_111D \
+    (OPENSSL_VERSION_NUMBER < 0x10101040 || CRYPTOGRAPHY_IS_LIBRESSL)
+#if (CRYPTOGRAPHY_OPENSSL_LESS_THAN_111D && !defined(OPENSSL_NO_ENGINE)) || \
+    defined(USE_OSRANDOM_RNG_FOR_TESTING)
+#define CRYPTOGRAPHY_NEEDS_OSRANDOM_ENGINE 1
+#else
+#define CRYPTOGRAPHY_NEEDS_OSRANDOM_ENGINE 0
+#endif
 """
 
 TYPES = """
@@ -60,6 +68,7 @@
 static const int CRYPTOGRAPHY_OPENSSL_LESS_THAN_102I;
 static const int CRYPTOGRAPHY_OPENSSL_LESS_THAN_111;
 static const int CRYPTOGRAPHY_OPENSSL_LESS_THAN_111B;
+static const int CRYPTOGRAPHY_NEEDS_OSRANDOM_ENGINE;
 
 static const int CRYPTOGRAPHY_IS_LIBRESSL;
 """

src/_cffi_src/openssl/src/osrandom_engine.c

@@ -17,8 +17,9 @@
 #include <poll.h>
 #endif
 
-#ifndef OPENSSL_NO_ENGINE
-/* OpenSSL has ENGINE support so build the engine. */
+#if CRYPTOGRAPHY_NEEDS_OSRANDOM_ENGINE
+/* OpenSSL has ENGINE support and is older than 1.1.1d (the first version that
+ * properly implements fork safety in its RNG) so build the engine. */
 static const char *Cryptography_osrandom_engine_id = "osrandom";
 
 /****************************************************************************
@@ -650,7 +651,7 @@ int Cryptography_add_osrandom_engine(void) {
  * to compile the osrandom engine, but we do need some
  * placeholders */
 static const char *Cryptography_osrandom_engine_id = "no-engine-support";
-static const char *Cryptography_osrandom_engine_name = "osrandom_engine disabled due to no engine support";
+static const char *Cryptography_osrandom_engine_name = "osrandom_engine disabled";
 
 int Cryptography_add_osrandom_engine(void) {
     return 0;

src/cryptography/hazmat/backends/openssl/backend.py

@@ -135,7 +135,7 @@ def openssl_assert(self, ok):
         return binding._openssl_assert(self._lib, ok)
 
     def activate_builtin_random(self):
-        if self._lib.Cryptography_HAS_ENGINE:
+        if self._lib.CRYPTOGRAPHY_NEEDS_OSRANDOM_ENGINE:
             # Obtain a new structural reference.
             e = self._lib.ENGINE_get_default_RAND()
             if e != self._ffi.NULL:
@@ -168,7 +168,7 @@ def _get_osurandom_engine(self):
             self.openssl_assert(res == 1)
 
     def activate_osrandom_engine(self):
-        if self._lib.Cryptography_HAS_ENGINE:
+        if self._lib.CRYPTOGRAPHY_NEEDS_OSRANDOM_ENGINE:
             # Unregister and free the current engine.
             self.activate_builtin_random()
             with self._get_osurandom_engine() as e:

src/cryptography/hazmat/bindings/openssl/binding.py

@@ -114,7 +114,7 @@ def _register_osrandom_engine(cls):
         # reliably clear the error queue. Once we clear it here we will
         # error on any subsequent unexpected item in the stack.
         cls.lib.ERR_clear_error()
-        if cls.lib.Cryptography_HAS_ENGINE:
+        if cls.lib.CRYPTOGRAPHY_NEEDS_OSRANDOM_ENGINE:
             result = cls.lib.Cryptography_add_osrandom_engine()
             _openssl_assert(cls.lib, result in (1, 2))
 

tests/hazmat/backends/test_openssl.py

@@ -169,8 +169,8 @@ def test_bn_to_int(self):
 
 
 @pytest.mark.skipif(
-    backend._lib.Cryptography_HAS_ENGINE == 0,
-    reason="Requires OpenSSL with ENGINE support")
+    not backend._lib.CRYPTOGRAPHY_NEEDS_OSRANDOM_ENGINE,
+    reason="Requires OpenSSL with ENGINE support and OpenSSL < 1.1.1d")
 class TestOpenSSLRandomEngine(object):
     def setup(self):
         # The default RAND engine is global and shared between
@@ -292,16 +292,16 @@ def test_activate_osrandom_already_default(self):
 
 
 @pytest.mark.skipif(
-    backend._lib.Cryptography_HAS_ENGINE == 1,
-    reason="Requires OpenSSL without ENGINE support")
+    backend._lib.CRYPTOGRAPHY_NEEDS_OSRANDOM_ENGINE,
+    reason="Requires OpenSSL without ENGINE support or OpenSSL >=1.1.1d")
 class TestOpenSSLNoEngine(object):
     def test_no_engine_support(self):
         assert backend._ffi.string(
             backend._lib.Cryptography_osrandom_engine_id
         ) == b"no-engine-support"
         assert backend._ffi.string(
             backend._lib.Cryptography_osrandom_engine_name
-        ) == b"osrandom_engine disabled due to no engine support"
+        ) == b"osrandom_engine disabled"
 
     def test_activate_builtin_random_does_nothing(self):
         backend.activate_builtin_random()