Niklas Keller wrote on 23.08.2015 16:30:
why not have false + e_warning for strict_types=0 and fatal error for strict_types=1 ?
Doing function
random_int()
: int { ...How's this connected to
strict_types
? It's not.If people use this function without reading documentation, they will also use other things without documentation like database queries without binding/escaping, inject html without escaping, etc.
Having core functions suddenly throw exceptions causes many problems in the code structure.How are these things connected? How does this create any issues in any existing code structure? This RFC affects only two new functions introduced in PHP 7.
I think there are a lot of security problems if people ignore return values, e.g. password comparison, user lookup in database, lookups for permissions, etc.
You compare an edge case, where these two functions currently return false instead of throwing an exception to fail closed, to functions with an expected
true|false
return value.This change is especially important, because these functions may be used in a way like this, as already mentioned in the previous discussions:
for ($i = 0; $i < 10; $i++) {
$result .= $values[random_int(0, 10)];
}
It's simply far too easy to make mistakes in security relevant code.
Regards, Niklas
How's this connected to
strict_types
? It's not.
consider this code:
declare(strict_types=0);
ini_set('display_errors', '1');
function get_random_int(): int {
return false;
}
echo get_random_int();
and then use strict_types=1
How are these things connected? How does this create any issues in any existing code structure? This RFC affects only two new functions introduced in PHP 7.
People will switch their code from mt_rand()
to random_int()
. So you'll need try-catch in places where you normally not use try-catch.
for ($i = 0; $i < 10; $i++) {
$result .= $values[random_int(0, 10)];
}
Even correct return values of random_int()
might create bad passwords.
So I propose to have a function in core which tests the strength of the password:
$password = '';
for ($i = 0; $i < 10; $i++) {
$password .= $characters[random_int(0, 30)];
}
if (password_strength($password) < PHP_PASSWORD_STRONG) {
throw new Exception("password not strong enough");
}
Regards
Thomas
consider this code:
declare(strict_types=0);
ini_set('display_errors', '1');function get_random_int(): int {
return false;
}
echo get_random_int();and then use strict_types=1
So you're implying that in case of returning false with strict_types turned on that would just throw a TypeError? That would be a bug in PHP. If you were saying that in this case it should throw a different kind of error (the one that was proposed by Anthony initially) then it wouldn't make sense again because then you're creating a straight relation between having strict_types mode turned on and random_int()
throwing exception about not getting a reliable source of entropy.
Even correct return values of
random_int()
might create bad passwords.
So I propose to have a function in core which tests the strength of the password:$password = '';
for ($i = 0; $i < 10; $i++) {
$password .= $characters[random_int(0, 30)];
}
if (password_strength($password) < PHP_PASSWORD_STRONG) {
throw new Exception("password not strong enough");
}
I don't think it's a good idea, a language delivers features that user would have a hard time implementing themselves, a small blocks from which you can build whatever you want. A function that checks if a string contains alpha-numeric symbols as well as punctuation is pretty easy to implement in user land.
PS sorry Thomas, I sent it to you personally, not to ML
Nikita Nefedov wrote on 23.08.2015 18:27:
consider this code:
declare(strict_types=0);
ini_set('display_errors', '1');function get_random_int(): int {
return false;
}
echo get_random_int();and then use strict_types=1
So you're implying that in case of returning false with strict_types turned on
that would just throw a TypeError? That would be a bug in PHP. If you were
saying that in this case it should throw a different kind of error (the one
that was proposed by Anthony initially) then it wouldn't make sense again
because then you're creating a straight relation between having strict_types
mode turned on andrandom_int()
throwing exception about not getting a reliable
source of entropy.Even correct return values of
random_int()
might create bad passwords.
So I propose to have a function in core which tests the strength of the
password:$password = '';
for ($i = 0; $i < 10; $i++) {
$password .= $characters[random_int(0, 30)];
}
if (password_strength($password) < PHP_PASSWORD_STRONG) {
throw new Exception("password not strong enough");
}I don't think it's a good idea, a language delivers features that user would
have a hard time implementing themselves, a small blocks from which you can
build whatever you want. A function that checks if a string contains
alpha-numeric symbols as well as punctuation is pretty easy to implement in
user land.
If it would be so easy to implement this function, there would be no problems with weak passwords in PHP software.
You can also implement fopen('/dev/urandom','rb') in userland without having random_int()
.
Regards
Thomas
Nikita Nefedov wrote on 23.08.2015 18:27:
consider this code:
declare(strict_types=0);
ini_set('display_errors', '1');function get_random_int(): int {
return false;
}
echo get_random_int();and then use strict_types=1
So you're implying that in case of returning false with strict_types turned on
that would just throw a TypeError? That would be a bug in PHP. If you were
saying that in this case it should throw a different kind of error (the one
that was proposed by Anthony initially) then it wouldn't make sense again
because then you're creating a straight relation between having strict_types
mode turned on andrandom_int()
throwing exception about not getting a reliable
source of entropy.Even correct return values of
random_int()
might create bad passwords.
So I propose to have a function in core which tests the strength of the
password:$password = '';
for ($i = 0; $i < 10; $i++) {
$password .= $characters[random_int(0, 30)];
}
if (password_strength($password) < PHP_PASSWORD_STRONG) {
throw new Exception("password not strong enough");
}I don't think it's a good idea, a language delivers features that user would
have a hard time implementing themselves, a small blocks from which you can
build whatever you want. A function that checks if a string contains
alpha-numeric symbols as well as punctuation is pretty easy to implement in
user land.If it would be so easy to implement this function, there would be no problems with weak passwords in PHP software.
You can also implement fopen('/dev/urandom','rb') in userland without havingrandom_int()
.Regards
Thomas--
Hi Thomas,
There is no /dev/urandom on the Windows operating system, so that
system is not universal.
I invite you to look at random_compat and see the effort that has gone
into writing a congruent feature using only PHP 5 features.
https://github.com/paragonie/random_compat
Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises <https://paragonie.com
There is no /dev/urandom on the Windows operating system, so that
system is not universal.
For tge record, all version of crypto safe RNG (at least mcrypt and
openssl) use the windows crypto API since some time already.
I did not check the code in random_int but I suppose or hope it does (using
the PHP wrapper around it), so any improvement will be available in all
related areas.