address review comments on osrandom engine, reorganize some code

2 files changed, 79 insertions, 60 deletions

diff --git a/cryptography/hazmat/bindings/openssl/osrand_engine.py b/cryptography/hazmat/bindings/openssl/osrand_engine.py
index bca2d79d..a04daed1 100644
--- a/cryptography/hazmat/bindings/openssl/osrand_engine.py
+++ b/cryptography/hazmat/bindings/openssl/osrand_engine.py
@@ -32,35 +32,60 @@ int Cryptography_add_osrandom_engine(void);
 MACROS = """
 """
 
-CUSTOMIZATIONS = """
-static const char *Cryptography_osrandom_engine_id= "osrandom";
-static const char *Cryptography_osrandom_engine_name = "osrandom_engine";
+WIN32_CUSTOMIZATIONS = """
+static HCRYPTPROV hCryptProv = 0;
 
-#ifndef _WIN32
-static int urandom_fd = -1;
+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) {
-    ssize_t n;
-    while (0 < size) {
-        do {
-            n = read(urandom_fd, buffer, (size_t)size);
-        } while (n < 0 && errno == EINTR);
-        if (n <= 0) {
+    size_t chunk;
+
+    if (hCryptProv == 0) {
+        return 0;
+    }
+
+    while (size > 0) {
+        chunk = size;
+        if (!CryptGenRandom(hCryptProv, (DWORD)chunk, buffer)) {
+            ERR_put_error(ERR_LIB_RAND, 0, ERR_R_RAND_LIB, "osrandom.py", 0);
             return 0;
         }
-        buffer += n;
-        size -= n;
+        buffer += chunk;
+        size -= chunk;
     }
     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 (urandom_fd == -1) {
+    if (hCryptProv == 0) {
         return 0;
     } else {
         return 1;
     }
 }
+"""
+
+POSIX_CUSTOMIZATIONS = """
+static int urandom_fd = -1;
 
 static int osrandom_init(ENGINE *e) {
     if (urandom_fd > -1) {
@@ -77,78 +102,71 @@ static int osrandom_init(ENGINE *e) {
     }
 }
 
+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.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) {
-        urandom_fd = -1;
         return 0;
     } else {
-        urandom_fd = -1;
         return 1;
     }
 }
-#endif
 
-#ifdef _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 {
+static int osrandom_rand_status(void) {
+    if (urandom_fd == -1) {
         return 0;
+    } else {
+        return 1;
     }
 }
+"""
 
-static int osrandom_rand_bytes(unsigned char *buffer, int size) {
-    size_t chunk;
-
-    if (hCryptProv == 0) {
-        return 0;
-    }
-
-    while (size > 0) {
-        chunk = size;
-        if (!CryptGenRandom(hCryptProv, (DWORD)chunk, buffer)) {
-            return 0;
-        }
-        buffer += chunk;
-        size -= chunk;
-    }
-    return 1;
-}
+CUSTOMIZATIONS = """
+static const char *Cryptography_osrandom_engine_id = "osrandom";
+static const char *Cryptography_osrandom_engine_name = "osrandom_engine";
 
-static int osrandom_finish(ENGINE *e) {
-    if (CryptReleaseContext(hCryptProv, 0)) {
-        hCryptProv = 0;
-        return 1;
-    } else {
-        return 0;
-    }
-}
+#if defined(_WIN32)
+""" + WIN32_CUSTOMIZATIONS + """
+#else
+""" + POSIX_CUSTOMIZATIONS + """
+#endif
 
-static int osrandom_rand_status(void) {
-    if (hCryptProv == 0) {
-        return 0;
+/* 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 1;
+        return res;
     }
 }
-#endif /* MS_WINDOWS */
 
 static RAND_METHOD osrandom_rand = {
     NULL,
     osrandom_rand_bytes,
     NULL,
     NULL,
-    osrandom_rand_bytes,
+    osrandom_pseudo_rand_bytes,
     osrandom_rand_status,
 };
 
@@ -162,6 +180,7 @@ int Cryptography_add_osrandom_engine(void) {
             !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)) {
diff --git a/docs/hazmat/backends/openssl.rst b/docs/hazmat/backends/openssl.rst
index 8eb02ea4..79c58857 100644
--- a/docs/hazmat/backends/openssl.rst
+++ b/docs/hazmat/backends/openssl.rst
@@ -55,7 +55,7 @@ utilizes the `Yarrow`_ algorithm.
 On Windows ``CryptGenRandom`` is backed by `Fortuna`_.
 
 Linux uses its own PRNG design. ``/dev/urandom`` is a non-blocking source seeded
-from the ``/dev/random`` pool.
+from the same pool as ``/dev/random``.
 
 
 .. _`OpenSSL`: https://www.openssl.org/