Improve openssl_random_pseudo_bytes()

CSPRNG implementations should always fail closed. Now
openssl_random_pseudo_bytes() will fail closed by throwing an
`\Exception` in fail conditions.

RFC: https://wiki.php.net/rfc/improve-openssl-random-pseudo-bytes

diff --git a/NEWS b/NEWS
index df7bc70af7995417e31add06022c342574a77a52..8385761bb7045da1021cda25a314fbf71033a1e5 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -42,6 +42,8 @@ PHP                                                                        NEWS
 
 - OpenSSL:
   . Added openssl_x509_verify function. (Ben Scholzen)
+  . openssl_random_pseudo_bytes() now throws in error conditions.
+    (Sammy Kaye Powers)
 
 - PDO_OCI:
   . Implemented FR #76908 (PDO_OCI getColumnMeta() not implemented).

diff --git a/UPGRADING b/UPGRADING
index 2da9ad242b9bd22d72308d94d668268b173732f2..835335bb07ec42252ea59f16ca8e00587dc58bb4 100644 (file)
--- a/UPGRADING
+++ b/UPGRADING
@@ -37,6 +37,15 @@ PHP 7.4 UPGRADE NOTES
   . The default parameter value of idn_to_ascii() and idn_to_utf8() is now
     INTL_IDNA_VARIANT_UTS46 instead of the deprecated INTL_IDNA_VARIANT_2003.
 
+- Openssl:
+  . The openssl_random_pseudo_bytes() function will now throw an exception in
+    error situations, similar to random_bytes(). In particular, an Error is
+    thrown if the number of requested bytes is smaller *or equal* than zero,
+    and an Exception is thrown is sufficient randomness cannot be gathered.
+    The $crypto_strong output argument is guaranteed to always be true if the
+    function does not throw, so explicitly checking it is not necessary.
+    RFC: http://php.net/manual/de/function.openssl-random-pseudo-bytes.php
+
 - PDO:
   . Attempting to serialize a PDO or PDOStatement instance will now generate
     an Exception rather than a PDOException, consistent with other internal

diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c
index 7fcab17ed66b53dc1876cc413aa6d151b92bc93c..e97cd8fda60d80dda4b14c4f682cf5c582065eda 100644 (file)
--- a/ext/openssl/openssl.c
+++ b/ext/openssl/openssl.c
@@ -28,6 +28,7 @@
 #include "php.h"
 #include "php_ini.h"
 #include "php_openssl.h"
+#include "zend_exceptions.h"
 
 /* PHP Includes */
 #include "ext/standard/file.h"
@@ -6861,7 +6862,8 @@ PHP_FUNCTION(openssl_random_pseudo_bytes)
                || ZEND_LONG_INT_OVFL(buffer_length)
 #endif
                        ) {
-               RETURN_FALSE;
+               zend_throw_exception(zend_ce_error, "Length must be greater than 0", 0);
+               return;
        }
        buffer = zend_string_alloc(buffer_length, 0);
 
@@ -6872,7 +6874,8 @@ PHP_FUNCTION(openssl_random_pseudo_bytes)
                if (zstrong_result_returned) {
                        ZVAL_FALSE(zstrong_result_returned);
                }
-               RETURN_FALSE;
+               zend_throw_exception(zend_ce_exception, "Error reading from source device", 0);
+               return;
        }
 #else
 
@@ -6884,7 +6887,8 @@ PHP_FUNCTION(openssl_random_pseudo_bytes)
                if (zstrong_result_returned) {
                        ZVAL_FALSE(zstrong_result_returned);
                }
-               RETURN_FALSE;
+               zend_throw_exception(zend_ce_exception, "Error reading from source device", 0);
+               return;
        } else {
                php_openssl_store_errors();
        }

diff --git a/ext/openssl/tests/openssl_random_pseudo_bytes_basic.phpt b/ext/openssl/tests/openssl_random_pseudo_bytes_basic.phpt
index 5727de90a20ee635c9fc5d7c998f03ecb86a8e91..6adc78c797461e4544825c5bc2e3c965e7651191 100644 (file)
--- a/ext/openssl/tests/openssl_random_pseudo_bytes_basic.phpt
+++ b/ext/openssl/tests/openssl_random_pseudo_bytes_basic.phpt
@@ -4,13 +4,11 @@ openssl_random_pseudo_bytes() tests
 <?php if (!extension_loaded("openssl")) print "skip"; ?>
 --FILE--
 <?php
-for ($i = 0; $i < 10; $i++) {
-    var_dump(bin2hex(openssl_random_pseudo_bytes($i, $strong)));
+for ($i = 1; $i < 10; $i++) {
+    var_dump(bin2hex(openssl_random_pseudo_bytes($i)));
 }
-
 ?>
 --EXPECTF--
-string(0) ""
 string(2) "%s"
 string(4) "%s"
 string(6) "%s"

diff --git a/ext/openssl/tests/openssl_random_pseudo_bytes_error.phpt b/ext/openssl/tests/openssl_random_pseudo_bytes_error.phpt
new file mode 100644 (file)
index 0000000..2fb0ca6
--- /dev/null
+++ b/ext/openssl/tests/openssl_random_pseudo_bytes_error.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Test error operation of openssl_random_pseudo_bytes()
+--SKIPIF--
+<?php if (!extension_loaded("openssl")) print "skip"; ?>
+--FILE--
+<?php
+try {
+    openssl_random_pseudo_bytes(0);
+} catch (Error $e) {
+    echo $e->getMessage().PHP_EOL;
+}
+?>
+--EXPECTF--
+Length must be greater than 0