Skip to content

Commit 74c0e58

Browse files
SammyKnikic
authored andcommitted
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
1 parent 894e78b commit 74c0e58

File tree

5 files changed

+34
-7
lines changed

5 files changed

+34
-7
lines changed

NEWS

+2
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ PHP NEWS
4242

4343
- OpenSSL:
4444
. Added openssl_x509_verify function. (Ben Scholzen)
45+
. openssl_random_pseudo_bytes() now throws in error conditions.
46+
(Sammy Kaye Powers)
4547

4648
- PDO_OCI:
4749
. Implemented FR #76908 (PDO_OCI getColumnMeta() not implemented).

UPGRADING

+9
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,15 @@ PHP 7.4 UPGRADE NOTES
3737
. The default parameter value of idn_to_ascii() and idn_to_utf8() is now
3838
INTL_IDNA_VARIANT_UTS46 instead of the deprecated INTL_IDNA_VARIANT_2003.
3939

40+
- Openssl:
41+
. The openssl_random_pseudo_bytes() function will now throw an exception in
42+
error situations, similar to random_bytes(). In particular, an Error is
43+
thrown if the number of requested bytes is smaller *or equal* than zero,
44+
and an Exception is thrown is sufficient randomness cannot be gathered.
45+
The $crypto_strong output argument is guaranteed to always be true if the
46+
function does not throw, so explicitly checking it is not necessary.
47+
RFC: http://php.net/manual/de/function.openssl-random-pseudo-bytes.php
48+
4049
- PDO:
4150
. Attempting to serialize a PDO or PDOStatement instance will now generate
4251
an Exception rather than a PDOException, consistent with other internal

ext/openssl/openssl.c

+7-3
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include "php.h"
2929
#include "php_ini.h"
3030
#include "php_openssl.h"
31+
#include "zend_exceptions.h"
3132

3233
/* PHP Includes */
3334
#include "ext/standard/file.h"
@@ -6861,7 +6862,8 @@ PHP_FUNCTION(openssl_random_pseudo_bytes)
68616862
|| ZEND_LONG_INT_OVFL(buffer_length)
68626863
#endif
68636864
) {
6864-
RETURN_FALSE;
6865+
zend_throw_exception(zend_ce_error, "Length must be greater than 0", 0);
6866+
return;
68656867
}
68666868
buffer = zend_string_alloc(buffer_length, 0);
68676869

@@ -6872,7 +6874,8 @@ PHP_FUNCTION(openssl_random_pseudo_bytes)
68726874
if (zstrong_result_returned) {
68736875
ZVAL_FALSE(zstrong_result_returned);
68746876
}
6875-
RETURN_FALSE;
6877+
zend_throw_exception(zend_ce_exception, "Error reading from source device", 0);
6878+
return;
68766879
}
68776880
#else
68786881

@@ -6884,7 +6887,8 @@ PHP_FUNCTION(openssl_random_pseudo_bytes)
68846887
if (zstrong_result_returned) {
68856888
ZVAL_FALSE(zstrong_result_returned);
68866889
}
6887-
RETURN_FALSE;
6890+
zend_throw_exception(zend_ce_exception, "Error reading from source device", 0);
6891+
return;
68886892
} else {
68896893
php_openssl_store_errors();
68906894
}

ext/openssl/tests/openssl_random_pseudo_bytes_basic.phpt

+2-4
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,11 @@ openssl_random_pseudo_bytes() tests
44
<?php if (!extension_loaded("openssl")) print "skip"; ?>
55
--FILE--
66
<?php
7-
for ($i = 0; $i < 10; $i++) {
8-
var_dump(bin2hex(openssl_random_pseudo_bytes($i, $strong)));
7+
for ($i = 1; $i < 10; $i++) {
8+
var_dump(bin2hex(openssl_random_pseudo_bytes($i)));
99
}
10-
1110
?>
1211
--EXPECTF--
13-
string(0) ""
1412
string(2) "%s"
1513
string(4) "%s"
1614
string(6) "%s"
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
--TEST--
2+
Test error operation of openssl_random_pseudo_bytes()
3+
--SKIPIF--
4+
<?php if (!extension_loaded("openssl")) print "skip"; ?>
5+
--FILE--
6+
<?php
7+
try {
8+
openssl_random_pseudo_bytes(0);
9+
} catch (Error $e) {
10+
echo $e->getMessage().PHP_EOL;
11+
}
12+
?>
13+
--EXPECTF--
14+
Length must be greater than 0

0 commit comments

Comments
 (0)