Anthony Ferrara wrote on 22.08.2015 21:58:
All,
I am putting a simple RFC up for discussion to make random_* throw
exceptions on failure in order to ensure we fail-closed.https://wiki.php.net/rfc/random-function-exceptions
Considering this topic has already been discussed, I intend to open
voting on this as soon as allowable. Given the voting policy specifies
2 weeks for language changes and 1 week for another, this is assumed
to require 1 week of "discussion".With that in mind, I intend to put this RFC up to vote on August 29/30th.
Thanks!
Anthony
--
Hi,
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.
Having false + E_WARNING
highlighted in the documentation with a yellow box and the Caution title should be enough.
For those who want exceptions can implement this in userland:
$rand = random_int(10,100);
if ($rand === false) {
throw new Exception('error ...');
}
// or write a wrapper like random_int_exception(...).
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.
Regards
Thomas
Anthony Ferrara wrote on 22.08.2015 21:58:
All,
I am putting a simple RFC up for discussion to make random_* throw
exceptions on failure in order to ensure we fail-closed.https://wiki.php.net/rfc/random-function-exceptions
Considering this topic has already been discussed, I intend to open
voting on this as soon as allowable. Given the voting policy specifies
2 weeks for language changes and 1 week for another, this is assumed
to require 1 week of "discussion".With that in mind, I intend to put this RFC up to vote on August 29/30th.
Thanks!
Anthony
--
Hi,
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.
Having false +
E_WARNING
highlighted in the documentation with a yellow box and the Caution title should be enough.For those who want exceptions can implement this in userland:
$rand = random_int(10,100);
if ($rand === false) {
throw new Exception('error ...');
}
// or write a wrapper like random_int_exception(...).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.Regards
Thomas--
Hi Thomas,
Your proposal effectively blames the user if they get it wrong and
burdens them with additional responsibilities. Increasing the
cognitive load of PHP developers will not result in a net gain for the
security of the applications they develop. I've made this argument
elsewhere in the course of this discussion.
Cryptography implementations should do everything reasonable to not
blame the user, and cryptographic primitives should do everything
reasonable to not blame the implementor.
Read this: http://cr.yp.to/talks/2015.01.07/slides-djb-20150107-a4.pdf
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.
Exceptions will only be thrown in exceptional circumstances:
- If they're using shared hosting, and a malicious script triggers a
file descriptor exhaustion condition on another hosting account (we're
assuming great sandboxing between customers), an exception will be
thrown rather than returning FALSE. - If the system is utterly incapable of generating random numbers.
An exception must either be handled (try-catch), or it by default
terminates script execution. This is the best of both worlds:
- It fails closed.
- It's easy to handle gracefully should the developer wish to do so.
Designing a CSPRNG that will fail closed, without unavoidably killing
their script (i.e. E_ERROR), will do far more to make PHP applications
secure than telling people they should RTFM.
Hackers have been saying RTFM since the ARPANET days, and yet most
people still don't. It's a losing battle. Let's patch what we can with
good design decisions.
TL;DR - the path forward is Throwable. Whether it's an Error object or
and Exception object (and when it throws which) is what's to be
discussed and voted on.
Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises <https://paragonie.com
Anthony Ferrara wrote on 22.08.2015 21:58:
All,
I am putting a simple RFC up for discussion to make random_* throw
exceptions on failure in order to ensure we fail-closed.https://wiki.php.net/rfc/random-function-exceptions
Considering this topic has already been discussed, I intend to open
voting on this as soon as allowable. Given the voting policy specifies
2 weeks for language changes and 1 week for another, this is assumed
to require 1 week of "discussion".With that in mind, I intend to put this RFC up to vote on August 29/30th.
Thanks!
Anthony
--
snip
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.Regards
Thomas
It's actually the other way around that is the compelling argument here.
We already know that developers use things without documentation, like
database queries without binding/escaping, inject HTML without escaping,
etc. These things happen all the time, despite extensive documentation
efforts to prevent them from doing so. Therefore, there is little
evidence to suggest that documenting "you must check the return value to
make sure it's not false", no matter how big, red, and flashing we made
it, will result in people actually doing so.
If we want these functions to be safely used, they need to be naively
safe to use. They simply won't be used safely otherwise, and these are
high-sensitivity functions (by design).
--Larry Garfield
Larry Garfield wrote on 23.08.2015 18:19:
Anthony Ferrara wrote on 22.08.2015 21:58:
All,
I am putting a simple RFC up for discussion to make random_* throw
exceptions on failure in order to ensure we fail-closed.https://wiki.php.net/rfc/random-function-exceptions
Considering this topic has already been discussed, I intend to open
voting on this as soon as allowable. Given the voting policy specifies
2 weeks for language changes and 1 week for another, this is assumed
to require 1 week of "discussion".With that in mind, I intend to put this RFC up to vote on August 29/30th.
Thanks!
Anthony
--
snip
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.Regards
ThomasIt's actually the other way around that is the compelling argument here.
We already know that developers use things without documentation, like
database queries without binding/escaping, inject HTML without escaping,
etc. These things happen all the time, despite extensive documentation
efforts to prevent them from doing so. Therefore, there is little
evidence to suggest that documenting "you must check the return value to
make sure it's not false", no matter how big, red, and flashing we made
it, will result in people actually doing so.If we want these functions to be safely used, they need to be naively
safe to use. They simply won't be used safely otherwise, and these are
high-sensitivity functions (by design).--Larry Garfield
I guess people continue to use rand()
or mt_rand()
if they skip the documentation.
Even frameworks which are advertised with 100% php7 compatibility use mt_rand()
.
Regards
Thomas
I guess people continue to use
rand()
ormt_rand()
if they skip the
documentation.
Even frameworks which are advertised with 100% php7 compatibility use
mt_rand()
.
There's nothing wrong with mt_rand()
in non-security contexts, it's still
there in PHP 7. If anyone is using mt_rand()
in security related code in
PHP 5, they're doing it wrong. It doesn't have anything to do with PHP 7
compatibility.
Regards, Niklas