Bug #12801: User password hashes pseudo-random number generator may return insecure salt value - pfSense - pfSense bugtracker

Actions

Copy link

Bug #12801

closed

User password hashes pseudo-random number generator may return insecure salt value

Added by Viktor Gurov about 2 years ago. Updated over 1 year ago.

Status:

Resolved

Priority:

Normal

Assignee:

Viktor Gurov

Category:

Authentication

Target version:

2.7.0

Start date:

Due date:

% Done:

100%

Estimated time:

Plus Target Version:

22.05

Release Notes:

Default

Affected Version:

2.6.0

Affected Architecture:

Description

https://github.com/pfsense/pfsense/blob/master/src/etc/inc/auth.inc#L819:

$salt = substr(bin2hex(openssl_random_pseudo_bytes(16)),0,16);

openssl_random_pseudo_bytes(16) omits the strong_result parameter (null)

from https://www.php.net/manual/en/function.openssl-random-pseudo-bytes.php:

Parameters 

length
The length of the desired string of bytes. Must be a positive integer. 
PHP will try to cast this parameter to a non-null integer to use it.

strong_result
If passed into the function, this will hold a bool value that determines 
if the algorithm used was "cryptographically strong", e.g., safe for usage with GPG,
passwords, etc. true if it did, otherwise false

must be:
openssl_random_pseudo_bytes(16, true)

Actions

Copy link

Updated by Jim Pingle about 2 years ago

That second parameter needs to be a variable -- it's not a flag telling it to use a secure method, it's a variable which returns true/false based on whether the underlying result came from a secure source. This may already be true which makes this a bit of a non-issue, but we should check and test it. If it contains false after the call we should use an alternate method of obtaining randomness like random_bytes().

Actions

Copy link

Updated by Viktor Gurov about 2 years ago

Subject changed from User password hashes use an insecure pseudo-random number generator to User password hashes pseudo-random number generator may return insecure salt value

Jim Pingle wrote in #note-1:

That second command needs to be a variable -- it's not a flag telling it to use a secure method, it's a variable which returns true/false based on whether the underlying result came from a secure source. This may already be true which makes this a bit of a non-issue, but we should check and test it. If it contains false after the call we should use an alternate method of obtaining randomness like random_bytes().

It's better to use random_bytes() than to try N rounds of openssl_random_pseudo_bytes() until strong_result == true.

Actions

Copy link