Skip to content

Commit

Permalink
fix(security): random byte generation improved on some systems
Browse files Browse the repository at this point in the history
On some Windows systems, `microtime()` is of insufficient precision and
can yield the same value on multiple calls. In our algorithm this results
in a divide-by-zero error and a `$rounds` count of `0`, so a little extra
entropy is lost. There are numerous other entropy sources tapped, so this
isn't a serious flaw, but here we avoid the error and just fix `$rounds`
to a reasonable value to pick up a little more entropy.

This issue should affect few systems, as it's a fallback mechanism for
several better random sources.

This also adds PHP7's `random_bytes` as the first source.

Fixes #10750
  • Loading branch information
mrclay committed Feb 3, 2017
1 parent 04f84c9 commit 03285ba
Showing 1 changed file with 17 additions and 1 deletion.
18 changes: 17 additions & 1 deletion engine/classes/ElggCrypto.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ public function __construct(SiteSecret $site_secret = null) {
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
public function getRandomBytes($length) {
if (is_callable('random_bytes')) {
try {
return random_bytes($length);
} catch (\Exception $e) {}
}

$SSLstr = '4'; // http://xkcd.com/221/

/**
Expand Down Expand Up @@ -132,6 +138,9 @@ public function getRandomBytes($length) {
if ($handle) {
$entropy .= @fread($handle, $bytes);
} else {
// In the future we should consider outright refusing to generate bytes without an
// official random source, as many consider hacks like this irresponsible.

// Measure the time that the operations will take on average
for ($i = 0; $i < 3; $i++) {
$c1 = microtime(true);
Expand All @@ -145,7 +154,14 @@ public function getRandomBytes($length) {

// Based on the above measurement determine the total rounds
// in order to bound the total running time.
$rounds = (int) ($msec_per_round * 50 / (int) (($c2 - $c1) * 1000000));
if ($c2 - $c1 == 0) {
// This occurs on some Windows systems. On a late 2013 MacBook Pro, 2.6 GHz Intel Core i7
// this averaged 400, so we're just going with that. With all the other entropy gathered
// this should be sufficient.
$rounds = 400;
} else {
$rounds = (int) ($msec_per_round * 50 / (int) (($c2 - $c1) * 1000000));
}

// Take the additional measurements. On average we can expect
// at least $bits_per_round bits of entropy from each measurement.
Expand Down

0 comments on commit 03285ba

Please sign in to comment.