Skip to content

Commit

Permalink
Improve openssl_random_pseudo_bytes()
Browse files Browse the repository at this point in the history
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
  • Loading branch information
SammyK authored and nikic committed Jan 11, 2019
1 parent 894e78b commit 74c0e58
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 7 deletions.
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
9 changes: 9 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 7 additions & 3 deletions ext/openssl/openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);

Expand All @@ -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

Expand All @@ -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();
}
Expand Down
6 changes: 2 additions & 4 deletions ext/openssl/tests/openssl_random_pseudo_bytes_basic.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
14 changes: 14 additions & 0 deletions ext/openssl/tests/openssl_random_pseudo_bytes_error.phpt
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 74c0e58

Please sign in to comment.