From: Sammy Kaye Powers <sammyk@sammykmedia.com>
Date: Mon, 19 Nov 2018 23:14:53 +0000 (-0500)
Subject: Improve openssl_random_pseudo_bytes()
X-Git-Tag: php-7.4.0alpha1~1238
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=74c0e580efa8feb282d0da9c830c6bd01b08b45e;p=php

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 df7bc70af7..8385761bb7 100644
--- 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 2da9ad242b..835335bb07 100644
--- 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 7fcab17ed6..e97cd8fda6 100644
--- 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 5727de90a2..6adc78c797 100644
--- 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
index 0000000000..2fb0ca6ed8
--- /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