Re: [RFC] [Discuss] Random Functions Throwing Exceptions in PHP 7.0.0

php.internals

Unnamed Person

11 years ago
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 > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >
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

Scott Arciszewski

11 years ago
On Sat, Aug 22, 2015 at 8:33 PM, Thomas Bley <mails@thomasbley.de> wrote:
> 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 >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: http://www.php.net/unsub.php >> > > 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 > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >
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: 1. 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. 2. 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: 1. It fails closed. 2. 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>

Larry Garfield

11 years ago
On 08/22/2015 07:33 PM, Thomas Bley wrote:
> 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 >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: http://www.php.net/unsub.php >>
*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

Unnamed Person

11 years ago
Larry Garfield wrote on 23.08.2015 18:19:
> On 08/22/2015 07:33 PM, Thomas Bley wrote: >> 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 >>> >>> -- >>> PHP Internals - PHP Runtime Development Mailing List >>> To unsubscribe, visit: http://www.php.net/unsub.php >>> > > *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 >
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

Niklas Keller

11 years ago
> > 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(). >
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