Merge pull request #2073 from glyph/no-c-random

Replace C implementation of OS Random engine with Python one that just calls os.urandom

10 files changed, 96 insertions, 218 deletions

diff --git a/src/_cffi_src/build_openssl.py b/src/_cffi_src/build_openssl.py
index 1ebadccb..dac3e4d8 100644
--- a/src/_cffi_src/build_openssl.py
+++ b/src/_cffi_src/build_openssl.py
@@ -78,7 +78,6 @@ ffi = build_ffi_for_binding(
         "nid",
         "objects",
         "opensslv",
-        "osrandom_engine",
         "pem",
         "pkcs7",
         "pkcs12",
diff --git a/src/_cffi_src/openssl/engine.py b/src/_cffi_src/openssl/engine.py
index 3ebfa6c1..5079fd69 100644
--- a/src/_cffi_src/openssl/engine.py
+++ b/src/_cffi_src/openssl/engine.py
@@ -17,9 +17,16 @@ typedef ... DSA_METHOD;
 typedef ... ECDH_METHOD;
 typedef ... ECDSA_METHOD;
 typedef ... DH_METHOD;
-typedef ... RAND_METHOD;
+typedef struct {
+    void (*seed)(const void *, int);
+    int (*bytes)(unsigned char *, int);
+    void (*cleanup)();
+    void (*add)(const void *, int, double);
+    int (*pseudorand)(unsigned char *, int);
+    int (*status)();
+} RAND_METHOD;
 typedef ... STORE_METHOD;
-typedef ... *ENGINE_GEN_INT_FUNC_PTR;
+typedef int (*ENGINE_GEN_INT_FUNC_PTR)(ENGINE *);
 typedef ... *ENGINE_CTRL_FUNC_PTR;
 typedef ... *ENGINE_LOAD_KEY_PTR;
 typedef ... *ENGINE_CIPHERS_PTR;
diff --git a/src/_cffi_src/openssl/err.py b/src/_cffi_src/openssl/err.py
index 0ee19c9e..eebf19ba 100644
--- a/src/_cffi_src/openssl/err.py
+++ b/src/_cffi_src/openssl/err.py
@@ -251,6 +251,7 @@ unsigned long ERR_peek_error_line(const char **, int *);
 unsigned long ERR_peek_last_error_line(const char **, int *);
 unsigned long ERR_get_error_line_data(const char **, int *,
                                       const char **, int *);
+void ERR_clear_error(void);
 unsigned long ERR_peek_error_line_data(const char **,
                                        int *, const char **, int *);
 unsigned long ERR_peek_last_error_line_data(const char **,
diff --git a/src/_cffi_src/openssl/osrandom_engine.py b/src/_cffi_src/openssl/osrandom_engine.py
deleted file mode 100644
index a8479b07..00000000
--- a/src/_cffi_src/openssl/osrandom_engine.py
+++ /dev/null
@@ -1,31 +0,0 @@
-# This file is dual licensed under the terms of the Apache License, Version
-# 2.0, and the BSD License. See the LICENSE file in the root of this repository
-# for complete details.
-
-from __future__ import absolute_import, division, print_function
-
-import os
-
-with open(os.path.join(
-    os.path.dirname(__file__), "src/osrandom_engine.h"
-)) as f:
-    INCLUDES = f.read()
-
-TYPES = """
-static const char *const Cryptography_osrandom_engine_name;
-static const char *const Cryptography_osrandom_engine_id;
-"""
-
-FUNCTIONS = """
-int Cryptography_add_osrandom_engine(void);
-"""
-
-MACROS = """
-"""
-
-with open(os.path.join(
-    os.path.dirname(__file__), "src/osrandom_engine.c"
-)) as f:
-    CUSTOMIZATIONS = f.read()
-
-CONDITIONAL_NAMES = {}
diff --git a/src/_cffi_src/openssl/src/osrandom_engine.c b/src/_cffi_src/openssl/src/osrandom_engine.c
deleted file mode 100644
index 27894712..00000000
--- a/src/_cffi_src/openssl/src/osrandom_engine.c
+++ /dev/null
@@ -1,167 +0,0 @@
-static const char *Cryptography_osrandom_engine_id = "osrandom";
-static const char *Cryptography_osrandom_engine_name = "osrandom_engine";
-
-#if defined(_WIN32)
-static HCRYPTPROV hCryptProv = 0;
-
-static int osrandom_init(ENGINE *e) {
-    if (hCryptProv > 0) {
-        return 1;
-    }
-    if (CryptAcquireContext(&hCryptProv, NULL, NULL,
-                            PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) {
-        return 1;
-    } else {
-        return 0;
-    }
-}
-
-static int osrandom_rand_bytes(unsigned char *buffer, int size) {
-    if (hCryptProv == 0) {
-        return 0;
-    }
-
-    if (!CryptGenRandom(hCryptProv, (DWORD)size, buffer)) {
-        ERR_put_error(
-            ERR_LIB_RAND, 0, ERR_R_RAND_LIB, "osrandom_engine.py", 0
-        );
-        return 0;
-    }
-    return 1;
-}
-
-static int osrandom_finish(ENGINE *e) {
-    if (CryptReleaseContext(hCryptProv, 0)) {
-        hCryptProv = 0;
-        return 1;
-    } else {
-        return 0;
-    }
-}
-
-static int osrandom_rand_status(void) {
-    if (hCryptProv == 0) {
-        return 0;
-    } else {
-        return 1;
-    }
-}
-#else
-static int urandom_fd = -1;
-
-static int osrandom_finish(ENGINE *e);
-
-static int osrandom_init(ENGINE *e) {
-    if (urandom_fd > -1) {
-        return 1;
-    }
-    urandom_fd = open("/dev/urandom", O_RDONLY);
-    if (urandom_fd > -1) {
-        int flags = fcntl(urandom_fd, F_GETFD);
-        if (flags == -1) {
-            osrandom_finish(e);
-            return 0;
-        } else if (fcntl(urandom_fd, F_SETFD, flags | FD_CLOEXEC) == -1) {
-            osrandom_finish(e);
-            return 0;
-        }
-        return 1;
-    } else {
-        return 0;
-    }
-}
-
-static int osrandom_rand_bytes(unsigned char *buffer, int size) {
-    ssize_t n;
-    while (size > 0) {
-        do {
-            n = read(urandom_fd, buffer, (size_t)size);
-        } while (n < 0 && errno == EINTR);
-        if (n <= 0) {
-            ERR_put_error(
-                ERR_LIB_RAND, 0, ERR_R_RAND_LIB, "osrandom_engine.py", 0
-            );
-            return 0;
-        }
-        buffer += n;
-        size -= n;
-    }
-    return 1;
-}
-
-static int osrandom_finish(ENGINE *e) {
-    int n;
-    do {
-        n = close(urandom_fd);
-    } while (n < 0 && errno == EINTR);
-    urandom_fd = -1;
-    if (n < 0) {
-        return 0;
-    } else {
-        return 1;
-    }
-}
-
-static int osrandom_rand_status(void) {
-    if (urandom_fd == -1) {
-        return 0;
-    } else {
-        return 1;
-    }
-}
-#endif
-
-/* This replicates the behavior of the OpenSSL FIPS RNG, which returns a
-   -1 in the event that there is an error when calling RAND_pseudo_bytes. */
-static int osrandom_pseudo_rand_bytes(unsigned char *buffer, int size) {
-    int res = osrandom_rand_bytes(buffer, size);
-    if (res == 0) {
-        return -1;
-    } else {
-        return res;
-    }
-}
-
-static RAND_METHOD osrandom_rand = {
-    NULL,
-    osrandom_rand_bytes,
-    NULL,
-    NULL,
-    osrandom_pseudo_rand_bytes,
-    osrandom_rand_status,
-};
-
-/* Returns 1 if successfully added, 2 if engine has previously been added,
-   and 0 for error. */
-int Cryptography_add_osrandom_engine(void) {
-    ENGINE *e;
-    e = ENGINE_by_id(Cryptography_osrandom_engine_id);
-    if (e != NULL) {
-        ENGINE_free(e);
-        return 2;
-    } else {
-        ERR_clear_error();
-    }
-
-    e = ENGINE_new();
-    if (e == NULL) {
-        return 0;
-    }
-    if(!ENGINE_set_id(e, Cryptography_osrandom_engine_id) ||
-            !ENGINE_set_name(e, Cryptography_osrandom_engine_name) ||
-            !ENGINE_set_RAND(e, &osrandom_rand) ||
-            !ENGINE_set_init_function(e, osrandom_init) ||
-            !ENGINE_set_finish_function(e, osrandom_finish)) {
-        ENGINE_free(e);
-        return 0;
-    }
-    if (!ENGINE_add(e)) {
-        ENGINE_free(e);
-        return 0;
-    }
-    if (!ENGINE_free(e)) {
-        return 0;
-    }
-
-    return 1;
-}
diff --git a/src/_cffi_src/openssl/src/osrandom_engine.h b/src/_cffi_src/openssl/src/osrandom_engine.h
deleted file mode 100644
index 11a3159e..00000000
--- a/src/_cffi_src/openssl/src/osrandom_engine.h
+++ /dev/null
@@ -1,6 +0,0 @@
-#ifdef _WIN32
-#include <Wincrypt.h>
-#else
-#include <fcntl.h>
-#include <unistd.h>
-#endif
diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py
index 88a17de0..91bc304f 100644
--- a/src/cryptography/hazmat/backends/openssl/backend.py
+++ b/src/cryptography/hazmat/backends/openssl/backend.py
@@ -200,7 +200,7 @@ class Backend(object):
         self.activate_builtin_random()
         # Fetches an engine by id and returns it. This creates a structural
         # reference.
-        e = self._lib.ENGINE_by_id(self._lib.Cryptography_osrandom_engine_id)
+        e = self._lib.ENGINE_by_id(self._binding._osrandom_engine_id)
         assert e != self._ffi.NULL
         # Initialize the engine for use. This adds a functional reference.
         res = self._lib.ENGINE_init(e)
diff --git a/src/cryptography/hazmat/bindings/openssl/binding.py b/src/cryptography/hazmat/bindings/openssl/binding.py
index e0a83972..b7178bb2 100644
--- a/src/cryptography/hazmat/bindings/openssl/binding.py
+++ b/src/cryptography/hazmat/bindings/openssl/binding.py
@@ -4,11 +4,25 @@
 
 from __future__ import absolute_import, division, print_function
 
+import os
 import threading
 
 from cryptography.hazmat.bindings._openssl import ffi, lib
 
 
+@ffi.callback("int (*)(unsigned char *, int)", error=-1)
+def _osrandom_rand_bytes(buf, size):
+    signed = ffi.cast("char *", buf)
+    result = os.urandom(size)
+    signed[0:size] = result
+    return 1
+
+
+@ffi.callback("int (*)(void)")
+def _osrandom_rand_status():
+    return 1
+
+
 class Binding(object):
     """
     OpenSSL API wrapper.
@@ -21,10 +35,42 @@ class Binding(object):
     _init_lock = threading.Lock()
     _lock_init_lock = threading.Lock()
 
+    _osrandom_engine_id = ffi.new("const char[]", b"osrandom")
+    _osrandom_engine_name = ffi.new("const char[]", b"osrandom_engine")
+    _osrandom_method = ffi.new(
+        "RAND_METHOD *",
+        dict(bytes=_osrandom_rand_bytes, pseudorand=_osrandom_rand_bytes,
+             status=_osrandom_rand_status)
+    )
+
     def __init__(self):
         self._ensure_ffi_initialized()
 
     @classmethod
+    def _register_osrandom_engine(cls):
+        assert cls.lib.ERR_peek_error() == 0
+        looked_up_engine = cls.lib.ENGINE_by_id(cls._osrandom_engine_id)
+        if looked_up_engine != ffi.NULL:
+            raise RuntimeError("osrandom engine already registered")
+
+        cls.lib.ERR_clear_error()
+
+        engine = cls.lib.ENGINE_new()
+        assert engine != cls.ffi.NULL
+        try:
+            result = cls.lib.ENGINE_set_id(engine, cls._osrandom_engine_id)
+            assert result == 1
+            result = cls.lib.ENGINE_set_name(engine, cls._osrandom_engine_name)
+            assert result == 1
+            result = cls.lib.ENGINE_set_RAND(engine, cls._osrandom_method)
+            assert result == 1
+            result = cls.lib.ENGINE_add(engine)
+            assert result == 1
+        finally:
+            result = cls.lib.ENGINE_free(engine)
+            assert result == 1
+
+    @classmethod
     def _ensure_ffi_initialized(cls):
         if cls._lib_loaded:
             return
@@ -32,8 +78,7 @@ class Binding(object):
         with cls._init_lock:
             if not cls._lib_loaded:
                 cls._lib_loaded = True
-                res = cls.lib.Cryptography_add_osrandom_engine()
-                assert res != 0
+                cls._register_osrandom_engine()
 
     @classmethod
     def init_static_locks(cls):
diff --git a/tests/hazmat/backends/test_openssl.py b/tests/hazmat/backends/test_openssl.py
index 34fff277..6a2e8a77 100644
--- a/tests/hazmat/backends/test_openssl.py
+++ b/tests/hazmat/backends/test_openssl.py
@@ -224,7 +224,7 @@ class TestOpenSSLRandomEngine(object):
         backend.activate_osrandom_engine()
         current_default = backend._lib.ENGINE_get_default_RAND()
         name = backend._lib.ENGINE_get_name(current_default)
-        assert name == backend._lib.Cryptography_osrandom_engine_name
+        assert name == backend._binding._osrandom_engine_name
 
     def test_osrandom_engine_is_default(self, tmpdir):
         engine_printer = textwrap.dedent(
@@ -258,7 +258,7 @@ class TestOpenSSLRandomEngine(object):
             )
 
         osrandom_engine_name = backend._ffi.string(
-            backend._lib.Cryptography_osrandom_engine_name
+            backend._binding._osrandom_engine_name
         )
 
         assert engine_name.read().encode('ascii') == osrandom_engine_name
@@ -277,7 +277,7 @@ class TestOpenSSLRandomEngine(object):
         backend.activate_osrandom_engine()
         e = backend._lib.ENGINE_get_default_RAND()
         name = backend._lib.ENGINE_get_name(e)
-        assert name == backend._lib.Cryptography_osrandom_engine_name
+        assert name == backend._binding._osrandom_engine_name
         res = backend._lib.ENGINE_free(e)
         assert res == 1
 
@@ -285,7 +285,7 @@ class TestOpenSSLRandomEngine(object):
         e = backend._lib.ENGINE_get_default_RAND()
         assert e != backend._ffi.NULL
         name = backend._lib.ENGINE_get_name(e)
-        assert name == backend._lib.Cryptography_osrandom_engine_name
+        assert name == backend._binding._osrandom_engine_name
         res = backend._lib.ENGINE_free(e)
         assert res == 1
         backend.activate_builtin_random()
@@ -303,13 +303,13 @@ class TestOpenSSLRandomEngine(object):
     def test_activate_osrandom_already_default(self):
         e = backend._lib.ENGINE_get_default_RAND()
         name = backend._lib.ENGINE_get_name(e)
-        assert name == backend._lib.Cryptography_osrandom_engine_name
+        assert name == backend._binding._osrandom_engine_name
         res = backend._lib.ENGINE_free(e)
         assert res == 1
         backend.activate_osrandom_engine()
         e = backend._lib.ENGINE_get_default_RAND()
         name = backend._lib.ENGINE_get_name(e)
-        assert name == backend._lib.Cryptography_osrandom_engine_name
+        assert name == backend._binding._osrandom_engine_name
         res = backend._lib.ENGINE_free(e)
         assert res == 1
 
diff --git a/tests/hazmat/bindings/test_openssl.py b/tests/hazmat/bindings/test_openssl.py
index e6d6fc45..f3f2eaf4 100644
--- a/tests/hazmat/bindings/test_openssl.py
+++ b/tests/hazmat/bindings/test_openssl.py
@@ -4,11 +4,27 @@
 
 from __future__ import absolute_import, division, print_function
 
+import os
+
 import pytest
 
 from cryptography.hazmat.bindings.openssl.binding import Binding
 
 
+def skip_if_libre_ssl(openssl_version):
+    if b'LibreSSL' in openssl_version:
+        pytest.skip("LibreSSL hard-codes RAND_bytes to use arc4random.")
+
+
+class TestLibreSkip(object):
+    def test_skip_no(self):
+        assert skip_if_libre_ssl(b"OpenSSL 0.9.8zf 19 Mar 2015") is None
+
+    def test_skip_yes(self):
+        with pytest.raises(pytest.skip.Exception):
+            skip_if_libre_ssl(b"LibreSSL 2.1.6")
+
+
 class TestOpenSSL(object):
     def test_binding_loads(self):
         binding = Binding()
@@ -89,8 +105,22 @@ class TestOpenSSL(object):
 
     def test_add_engine_more_than_once(self):
         b = Binding()
-        res = b.lib.Cryptography_add_osrandom_engine()
-        assert res == 2
+        with pytest.raises(RuntimeError):
+            b._register_osrandom_engine()
+
+    def test_actual_osrandom_bytes(self, monkeypatch):
+        b = Binding()
+        skip_if_libre_ssl(b.ffi.string(b.lib.OPENSSL_VERSION_TEXT))
+        sample_data = (b"\x01\x02\x03\x04" * 4)
+        length = len(sample_data)
+
+        def notrandom(size):
+            assert size == length
+            return sample_data
+        monkeypatch.setattr(os, "urandom", notrandom)
+        buf = b.ffi.new("char[]", length)
+        b.lib.RAND_bytes(buf, length)
+        assert b.ffi.buffer(buf)[0:length] == sample_data
 
     def test_ssl_ctx_options(self):
         # Test that we're properly handling 32-bit unsigned on all platforms.