Re: [PHP-DEV] Core functions throwing exceptions in PHP7
PHP 7.0 RC1 was just tagged. Shouldn't this be a relatively high priority to fix/decide so we don't end up with behavior that can't be fixed until PHP 8.0? On Sat, Aug 1, 2015 at 6:16 PM Scott Arciszewski sc...@paragonie.com wrote: On Sat, Aug 1, 2015 at 6:37 AM, Nikita Popov nikita@gmail.com wrote: tl;dr: This should definitely throw, but I'm as yet unclear as to *what* it should throw. My gut says zpp should throw Error, length/min/max errors should throw Error and the randomness-not-available condition should throw Exception. Hi Nikita, This sounds reasonable to me. As I've said before, I don't have any real preference for which is thrown. If Aaron (author of the Throwable RFC that passed unanimously) wants to chime in, I'd almost certainly go with whatever you, him, and Anthony could agree on. Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises https://paragonie.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Sat, Aug 1, 2015 at 5:50 PM, Niklas Keller m...@kelunik.com wrote: You always assume the developer just wants to fallback to something different. You can't detect the environment btw. because it could just fail because of too many open file descriptors. This is not my assumption, but others. They argue that they would like to fallback to alternative method when error happened. (which I think it's not best way to do) If they don't, E_RECOVERABLE_ERROR is enough. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Sat, Aug 1, 2015 at 6:37 AM, Nikita Popov nikita@gmail.com wrote: tl;dr: This should definitely throw, but I'm as yet unclear as to *what* it should throw. My gut says zpp should throw Error, length/min/max errors should throw Error and the randomness-not-available condition should throw Exception. Hi Nikita, This sounds reasonable to me. As I've said before, I don't have any real preference for which is thrown. If Aaron (author of the Throwable RFC that passed unanimously) wants to chime in, I'd almost certainly go with whatever you, him, and Anthony could agree on. Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises https://paragonie.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Anthony, -Original Message- From: Anthony Ferrara [mailto:ircmax...@gmail.com] Sent: Saturday, August 1, 2015 2:34 AM To: Ferenc Kovacs tyr...@gmail.com Cc: Sammy Kaye Powers m...@sammyk.me; internals@lists.php.net; Nikita Popov ni...@php.net Subject: Re: [PHP-DEV] Core functions throwing exceptions in PHP7 Ferenc, On Jul 31, 2015 6:34 PM, Ferenc Kovacs tyr...@gmail.com wrote: On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 I would vote for E_WARNING and return false. This can be wrapped in an oop wrapper in userland if somebody prefers and exception but would still keep the procedural style as first class citizen. Plus this would be consistent with other security/crypto related errors like mcrypt_encrypt() getting an invalid key/iv Nikita, Anthony what do you think? Strong -1 on this. The consistency point is something to consider, but there are three major differences between password_* + mcrypt_* and this case. First, the majority of mcrypt and password functions are single use. Meaning they are called once and not inside of loops. Random_int on the other hand will be used in loops and in direct combination with other functions. Second, the target audience is different. Mcrypt targets those that know something about what they are doing. The api is designed that way. Don't get me wrong, a lot of users have no idea how to use the APIs properly. But the api is designed such that you need to know it well to use it effectively. This is a massive fundamental problem with how crypto code was implemented in php. Things have been changing lately though. Apis are being designed with end users in mind. Password_hash was one of these, but others are on their way as well. In general, the difference is putting the onus of correct usage on the designer rather than the implementor. Make it harder to screw up than to do it safe. Third and lastly, they were built in different times. Had exceptions been acceptable when I wrote the password functions, believe me I would have used them. But they weren't just not an option, they were strictly verboten. Today things are very different with 7. IMHO an exception is the right way to error here. The problems Scott raises are too big and have too much potential impact to just leave to the implementors. Instead, let's follow the trend of not handicapping what's possible. But instead making it harder to do something wrong than to do it right. After all, we are not discussing whether an error is appropriate here. We are discussing whether to fail in a way that forces the user to admit there was failure, or fail in a way they they can miss (leading to insecurity)... If I'm allowed to notice, what is still not being discussed here is how exceptions are to be thrown in functions. Checking the wiki every day anticipating to find a RFC draft, but hopeless :) There's still no overall understanding where and how Error vs. Exception are to be meant. There is still no point about how to proceed with the legacy code a way not producing a parallel universe of nice vs ugly code. Still no mention which cases are there, which cases would be acceptable and which wouldn’t. Et cetera. There's only an discussion of one case which doesn't build the whole picture. Having at least this in mind, the procedure of how this case is being thrusted appears to be running deficient, while not the main thrust. If functions are supposed to be failing - so be. ATM it could be done PHP5 way, might it want to fail softly or rudely. But due to above, IMHO we're not ready to start throwing exceptions in functions today, so in 7.0. Regards Anatol
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
2015-08-01 1:43 GMT+02:00 Yasuo Ohgaki yohg...@ohgaki.net: Hi Niklas, On Sat, Aug 1, 2015 at 8:27 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: They should totally be handled. You need to catch the error and throw a defined exception, otherwise your public API will break if you choose to use another internal implementation. Additionally, you seem to assume that the library doesn't have to do things like cleanups in such a case. My thought is based on Design by Contract (Contract programming). When parameter or environment does not satisfy contract, contract error should be resulted in program/process termination. Fixing inappropriate parameter or environment is not library/framework author's responsibility, but the developer's. i.e. Caller(function/programmer/system admin) has the responsibility that satisfies parameter/environment requirement. If requirement is not met, it's perfectly OK for library/framework to raise fatal errors/exceptions. e.g. You need PHP 5.6 or greater error. I'll be more specific for CSPRNG not available error. If a author would like to handle the error and fallback to non crypt safe RNG, he/she should detect environment and execute alternative code for the environment. Hi Yasuo, You always assume the developer just wants to fallback to something different. You can't detect the environment btw. because it could just fail because of too many open file descriptors. Catching exception and fallback to non crypt safe RNG is not optimal way for handling unsatisfactory environment. IMHO. If we need function that checks environment, we are better to provide one rather than let users to use exception. This is damn insecure and far away from not optimal. As said, checking the environment before executing the function isn't safe, and no, exceptions would always be the better way here. Regards, Niklas Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Sat, Aug 1, 2015 at 12:34 AM, Ferenc Kovacs tyr...@gmail.com wrote: On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 I would vote for E_WARNING and return false. This can be wrapped in an oop wrapper in userland if somebody prefers and exception but would still keep the procedural style as first class citizen. Plus this would be consistent with other security/crypto related errors like mcrypt_encrypt() getting an invalid key/iv Nikita, Anthony what do you think? Hey Ferenc, I agree with the opinion of Anthony and Scott, in that this should throw some kind of exception. I don't think it is realistic to expect users to check the return value of random-number generation functions for false. Heck, I probably wouldn't bother with the check myself, given how this error condition is relatively unlikely (malconfigured system or very high load). This is exactly the kind of failure for which exceptions are ideal, because it's not something the programmer can (or at least should) handle locally -- this is something that needs to get handled at a high level, likely just with your standard error page. Regarding how this function differs from other existing crypto functions, the main difference is of temporal nature. mcrypt_encrypt exists since the very beginning of PHP 4 and I making the invalid key/IV checks that were introduced in PHP 5.6 throw would have been a somewhat drastic change for this function, in my opinion. On the other hand, these CRPRNG functions are being introduces in PHP 7. Our attitude towards exceptions has changed significantly in the meantime: Not only were they actually introduced in the first place (PHP 4 had no exceptions), in PHP 7 a lot of core functionality has been changed to throw. As of PHP 7 throwing an exception is the default mode for handling a new engine failure. Of course, this does not immediately apply to standard library functions. But as Stas pointed out, given the aforementioned developments, continuing to deny the use of exceptions in functions categorically is no longer reasonable. It may be unclear as yet under which circumstances precisely a function should throw an exception rather than a warning and of what type that exception ought to be, but it is clear that library functions will start throwing in the foreseeable future (and for that matter, they have already started, see for example intdiv). The only question I see here, and, as Anatol mentions, this is a general question that needs to be discussed before we start wildly throwing exceptions, is whether Error or Exception ought to be used. From what I gather, there exist essentially two interpretations of this matter: 1) Error is to be used in cases where an error is attributable to programmer mistake. 2) Error signifies a failure condition that the programmer is discouraged (and unlikely to want) to handle. It should only be dealt with at the top level. Depending on which interpretation is used, a different type would be appropriate in this particular case. Under the first interpretation we should throw an Exception, as this is not a programmer mistake. Instead it is either a malconfigured system or some sort of IO failure. Under the second interpretation we should throw an Error, as this is not a condition that can be reasonable handled. The first interpretation matches our current LogicException/RuntimeException split. It would essentially make Error the new LogicException and Exception the new RuntimeException. Looking at other error conditions, random_bytes() also errors if the length is = 0. This is clearly a programmer mistake and also not something you should catch, so this should be an Error. random_int() has similar restrictions on min/max, this is once again a clear
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Sat, Aug 1, 2015 at 2:00 AM, Scott Arciszewski sc...@paragonie.com
wrote:
On Fri, Jul 31, 2015 at 6:34 PM, Ferenc Kovacs tyr...@gmail.com wrote:
On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers m...@sammyk.me
wrote:
Hello lovely PHP nerds,
There are two open PR's for PHP7 to modify the behavior of the
CSPRNG's:
https://github.com/php/php-src/pull/1397 (main discussion)
https://github.com/php/php-src/pull/1398
Currently the random_*() functions will issue a warning and return
false if
a good source of random cannot be found. This is a potential security
hole
in the event the RNG fails and returns false which gets evaluated as 0
in a
cryptographic context.
To prevent this exploit the proposed behavior will throw an Exception
when
the RNG fails or certain argument validation fails. This also gives the
developer a graceful way to fall back to an alternate CSPRNG.
Since the core functions in PHP don't throw Exceptions, there is
debate on
whether or not this change should be implemented. Some say the CSPRNG's
should get a special pass since they will be relied on for
cryptography. If
we can't throw Exceptions, there were suggestions of raising a fatal
error
if the RNG fails.
I think the argument can be boiled down to consistency vs security.
We'd
love to hear your feedback to decide what we should do in this
context. :)
Thanks,
Sammy Kaye Powers
sammyk.me
Chicago, IL 60604
I would vote for E_WARNING and return false.
This can be wrapped in an oop wrapper in userland if somebody prefers and
exception but would still keep the procedural style as first class
citizen.
Plus this would be consistent with other security/crypto related errors
like mcrypt_encrypt() getting an invalid key/iv
Nikita, Anthony what do you think?
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Your vote is for apps to be insecure by default.
my vote is pragmatic.
This can be wrapped in an oop wrapper in userland if somebody prefers and
exception but would still keep the procedural style as first class
citizen.
Nobody's going to do that though. The end result is going to be less
security because of a cargo cult devotion to consistency.
if you want to change the status quo you have to show a decent case how
this is different than any other crypto related function.
you can't because it isn't.
This should be secure by default. The most secure way for an RNG to
fail is to interrupt the application. This means:
* E_ERROR
* throw new Exception (or a subclass)
* throw new Error (or a subclass)
for some applications this is the best way, for some other applications
it's none, because they have another source of entropy, or because they
want to retry or fail gracefully.
fatal errors should be used when there are no way to continue the
execution, for this function it isn't always the case.
Exceptions and Errors have the advantage that a developer who wants to
go out of their way to handle them can simply do this:
function randomPassword($length, $alphabet =
'abcdefghijklmnopqrstuvwxyz')
{
$sizeOfAlphabetMinusOne = strlen($alphabet) - 1;
try {
for ($i = 0; $i $length; ++$i) {
$password .= $alphabet[random_int(0,
$sizeOfAlphabetMinusOne)];
}
} catch (Error $e) {
return $this-framework-stylizedErrorMessage(RNG failure
message here);
}
return $password;
}
Care to guess what returning false will do for $password?
I don't have to guess, it is clear, I also understand where are you coming
from, but I think your suggestion is too strict and it is fairly trivial to
show similar misuse for any feature yet we have to draw a line somewhere as
we can't save the developer from him/herself always.
Any cryptography-related implementation needs to fail closed, not fail
open.
yet every entropy source function used by random_bytes() returns failure on
error instead of calling exit.
developer application library
your library shouldn't guess about the intentions of the applications, nor
should the application guess about the intentions of the developer.
you create clean contracts and educate your developers to use those
properly.
By raising E_WARNING and returning false, you are placing an extra
responsibility on the developer.
not extra, but see above.
Or as Daniel J. Bernstein would put it, YOU ARE BLAMING THE IMPLEMENTOR.
Ask any competent application security expert, they'll back me up.
Don't enforce insecure defaults just because it's more consistent.
https://en.wikipedia.org/wiki/No_true_Scotsman
Consistency is important, sure, but security is MORE important.
clear contract is more important and having consistency saves the developer
from the surprises which can be a good source of bugs.
Also, death to libmcrypt:
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Ferenc, On Jul 31, 2015 6:34 PM, Ferenc Kovacs tyr...@gmail.com wrote: On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 I would vote for E_WARNING and return false. This can be wrapped in an oop wrapper in userland if somebody prefers and exception but would still keep the procedural style as first class citizen. Plus this would be consistent with other security/crypto related errors like mcrypt_encrypt() getting an invalid key/iv Nikita, Anthony what do you think? Strong -1 on this. The consistency point is something to consider, but there are three major differences between password_* + mcrypt_* and this case. First, the majority of mcrypt and password functions are single use. Meaning they are called once and not inside of loops. Random_int on the other hand will be used in loops and in direct combination with other functions. Second, the target audience is different. Mcrypt targets those that know something about what they are doing. The api is designed that way. Don't get me wrong, a lot of users have no idea how to use the APIs properly. But the api is designed such that you need to know it well to use it effectively. This is a massive fundamental problem with how crypto code was implemented in php. Things have been changing lately though. Apis are being designed with end users in mind. Password_hash was one of these, but others are on their way as well. In general, the difference is putting the onus of correct usage on the designer rather than the implementor. Make it harder to screw up than to do it safe. Third and lastly, they were built in different times. Had exceptions been acceptable when I wrote the password functions, believe me I would have used them. But they weren't just not an option, they were strictly verboten. Today things are very different with 7. IMHO an exception is the right way to error here. The problems Scott raises are too big and have too much potential impact to just leave to the implementors. Instead, let's follow the trend of not handicapping what's possible. But instead making it harder to do something wrong than to do it right. After all, we are not discussing whether an error is appropriate here. We are discussing whether to fail in a way that forces the user to admit there was failure, or fail in a way they they can miss (leading to insecurity)... My $0.02 at least Anthony -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Fri, Jul 31, 2015 at 6:34 PM, Ferenc Kovacs tyr...@gmail.com wrote:
On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers m...@sammyk.me wrote:
Hello lovely PHP nerds,
There are two open PR's for PHP7 to modify the behavior of the CSPRNG's:
https://github.com/php/php-src/pull/1397 (main discussion)
https://github.com/php/php-src/pull/1398
Currently the random_*() functions will issue a warning and return false if
a good source of random cannot be found. This is a potential security hole
in the event the RNG fails and returns false which gets evaluated as 0 in a
cryptographic context.
To prevent this exploit the proposed behavior will throw an Exception when
the RNG fails or certain argument validation fails. This also gives the
developer a graceful way to fall back to an alternate CSPRNG.
Since the core functions in PHP don't throw Exceptions, there is debate on
whether or not this change should be implemented. Some say the CSPRNG's
should get a special pass since they will be relied on for cryptography. If
we can't throw Exceptions, there were suggestions of raising a fatal error
if the RNG fails.
I think the argument can be boiled down to consistency vs security. We'd
love to hear your feedback to decide what we should do in this context. :)
Thanks,
Sammy Kaye Powers
sammyk.me
Chicago, IL 60604
I would vote for E_WARNING and return false.
This can be wrapped in an oop wrapper in userland if somebody prefers and
exception but would still keep the procedural style as first class citizen.
Plus this would be consistent with other security/crypto related errors
like mcrypt_encrypt() getting an invalid key/iv
Nikita, Anthony what do you think?
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Your vote is for apps to be insecure by default.
This can be wrapped in an oop wrapper in userland if somebody prefers and
exception but would still keep the procedural style as first class citizen.
Nobody's going to do that though. The end result is going to be less
security because of a cargo cult devotion to consistency.
This should be secure by default. The most secure way for an RNG to
fail is to interrupt the application. This means:
* E_ERROR
* throw new Exception (or a subclass)
* throw new Error (or a subclass)
Exceptions and Errors have the advantage that a developer who wants to
go out of their way to handle them can simply do this:
function randomPassword($length, $alphabet = 'abcdefghijklmnopqrstuvwxyz')
{
$sizeOfAlphabetMinusOne = strlen($alphabet) - 1;
try {
for ($i = 0; $i $length; ++$i) {
$password .= $alphabet[random_int(0, $sizeOfAlphabetMinusOne)];
}
} catch (Error $e) {
return $this-framework-stylizedErrorMessage(RNG failure
message here);
}
return $password;
}
Care to guess what returning false will do for $password?
Any cryptography-related implementation needs to fail closed, not fail open.
By raising E_WARNING and returning false, you are placing an extra
responsibility on the developer.
Or as Daniel J. Bernstein would put it, YOU ARE BLAMING THE IMPLEMENTOR.
Ask any competent application security expert, they'll back me up.
Don't enforce insecure defaults just because it's more consistent.
Consistency is important, sure, but security is MORE important.
Also, death to libmcrypt:
https://paragonie.com/blog/2015/05/if-you-re-typing-word-mcrypt-into-your-code-you-re-doing-it-wrong
Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises https://paragonie.com
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Fri, Jul 31, 2015 at 8:23 PM, Ferenc Kovacs tyr...@gmail.com wrote:
On Sat, Aug 1, 2015 at 2:00 AM, Scott Arciszewski sc...@paragonie.com
wrote:
On Fri, Jul 31, 2015 at 6:34 PM, Ferenc Kovacs tyr...@gmail.com wrote:
On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers m...@sammyk.me
wrote:
Hello lovely PHP nerds,
There are two open PR's for PHP7 to modify the behavior of the
CSPRNG's:
https://github.com/php/php-src/pull/1397 (main discussion)
https://github.com/php/php-src/pull/1398
Currently the random_*() functions will issue a warning and return
false if
a good source of random cannot be found. This is a potential security
hole
in the event the RNG fails and returns false which gets evaluated as 0
in a
cryptographic context.
To prevent this exploit the proposed behavior will throw an Exception
when
the RNG fails or certain argument validation fails. This also gives
the
developer a graceful way to fall back to an alternate CSPRNG.
Since the core functions in PHP don't throw Exceptions, there is
debate on
whether or not this change should be implemented. Some say the
CSPRNG's
should get a special pass since they will be relied on for
cryptography. If
we can't throw Exceptions, there were suggestions of raising a fatal
error
if the RNG fails.
I think the argument can be boiled down to consistency vs security.
We'd
love to hear your feedback to decide what we should do in this
context. :)
Thanks,
Sammy Kaye Powers
sammyk.me
Chicago, IL 60604
I would vote for E_WARNING and return false.
This can be wrapped in an oop wrapper in userland if somebody prefers
and
exception but would still keep the procedural style as first class
citizen.
Plus this would be consistent with other security/crypto related errors
like mcrypt_encrypt() getting an invalid key/iv
Nikita, Anthony what do you think?
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Your vote is for apps to be insecure by default.
my vote is pragmatic.
This can be wrapped in an oop wrapper in userland if somebody prefers
and
exception but would still keep the procedural style as first class
citizen.
Nobody's going to do that though. The end result is going to be less
security because of a cargo cult devotion to consistency.
if you want to change the status quo you have to show a decent case how this
is different than any other crypto related function.
you can't because it isn't.
This should be secure by default. The most secure way for an RNG to
fail is to interrupt the application. This means:
* E_ERROR
* throw new Exception (or a subclass)
* throw new Error (or a subclass)
for some applications this is the best way, for some other applications it's
none, because they have another source of entropy, or because they want to
retry or fail gracefully.
fatal errors should be used when there are no way to continue the execution,
for this function it isn't always the case.
Exceptions and Errors have the advantage that a developer who wants to
go out of their way to handle them can simply do this:
function randomPassword($length, $alphabet =
'abcdefghijklmnopqrstuvwxyz')
{
$sizeOfAlphabetMinusOne = strlen($alphabet) - 1;
try {
for ($i = 0; $i $length; ++$i) {
$password .= $alphabet[random_int(0,
$sizeOfAlphabetMinusOne)];
}
} catch (Error $e) {
return $this-framework-stylizedErrorMessage(RNG failure
message here);
}
return $password;
}
Care to guess what returning false will do for $password?
I don't have to guess, it is clear, I also understand where are you coming
from, but I think your suggestion is too strict and it is fairly trivial to
show similar misuse for any feature yet we have to draw a line somewhere as
we can't save the developer from him/herself always.
Any cryptography-related implementation needs to fail closed, not fail
open.
yet every entropy source function used by random_bytes() returns failure on
error instead of calling exit.
developer application library
your library shouldn't guess about the intentions of the applications, nor
should the application guess about the intentions of the developer.
you create clean contracts and educate your developers to use those
properly.
By raising E_WARNING and returning false, you are placing an extra
responsibility on the developer.
not extra, but see above.
Or as Daniel J. Bernstein would put it, YOU ARE BLAMING THE IMPLEMENTOR.
Ask any competent application security expert, they'll back me up.
Don't enforce insecure defaults just because it's more consistent.
https://en.wikipedia.org/wiki/No_true_Scotsman
Consistency is important, sure, but security is MORE important.
clear contract is more important and having consistency saves
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Fri, Jul 31, 2015 at 7:20 PM, Niklas Keller m...@kelunik.com wrote: Using set_error_handler isn't handling errors gracefully. Well, it's better than E_ERROR, but then libraries can't handle those errors gracefully, because the user might override its error handler by setting an own handler. Now I see what do you mean by gracefully. TL;DR; It's app developer jobs to handle these fatal errors. Most fatal errors shouldn't be recovered by library. e.g. Fallback to non CSPRNG when CSPRNG is not available. IMO, library authors do not have to worry about errors like session's E_RECOVERABLE_ERROR or CSPRNG not available. These are fatal errors by its nature and app developer or system administrator job to handle them. We cannot blindly assume such fatal errors never happen in production apps, so we are better to give users a chance to return Nice error page/message when it happened. Anyway, we are better to move errors to exceptions at once and force all modules including 3rd party module to raise exceptions. i.e. Make php_error_docref() raise exceptions. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
2015-07-31 23:36 GMT+02:00 Yasuo Ohgaki yohg...@ohgaki.net: Hi Niklas, On Fri, Jul 31, 2015 at 7:20 PM, Niklas Keller m...@kelunik.com wrote: Using set_error_handler isn't handling errors gracefully. Well, it's better than E_ERROR, but then libraries can't handle those errors gracefully, because the user might override its error handler by setting an own handler. Now I see what do you mean by gracefully. TL;DR; It's app developer jobs to handle these fatal errors. Nope. Most fatal errors shouldn't be recovered by library. e.g. Fallback to non CSPRNG when CSPRNG is not available. They should totally be handled. You need to catch the error and throw a defined exception, otherwise your public API will break if you choose to use another internal implementation. Additionally, you seem to assume that the library doesn't have to do things like cleanups in such a case. Regards, Niklas IMO, library authors do not have to worry about errors like session's E_RECOVERABLE_ERROR or CSPRNG not available. These are fatal errors by its nature and app developer or system administrator job to handle them. We cannot blindly assume such fatal errors never happen in production apps, so we are better to give users a chance to return Nice error page/message when it happened. Anyway, we are better to move errors to exceptions at once and force all modules including 3rd party module to raise exceptions. i.e. Make php_error_docref() raise exceptions. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Scott, On Fri, Jul 31, 2015 at 6:33 AM, Scott Arciszewski sc...@paragonie.com wrote: On Jul 30, 2015 2:27 PM, Niklas Keller m...@kelunik.com wrote: I prefer Exception, too, because it's I/O related. @Scott: You can open votes on everything, doesn't matter, just create a page with a vote. I just don't know where to put it in the wiki, because it's not a RFC. Regards, Niklas I'm not sure how to do that. I have a noncritical security patch I intend to write for random_bytes() that I would like to submit as a PR but first I'd like to see what the resolution will be re: Exceptions. Also, merge conflicts aren't fun. ;) If there's anything I can do to get those two merged faster, please let me know. You can have RFC vote options like - Raise Error/Exception when CSPRNG is not available (Choice: Yes/No) - Use error(E_RECOVERABLE_ERROR) or exception(Whatever exception prefered) (Choice: Error/Exception) BTW, I prefer exception, but consistency is important also. My vote will be Yes and Error. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Sat, Aug 1, 2015 at 7:20 AM, Niklas Keller m...@kelunik.com wrote: 2015-07-31 23:36 GMT+02:00 Yasuo Ohgaki yohg...@ohgaki.net: Hi Niklas, On Fri, Jul 31, 2015 at 7:20 PM, Niklas Keller m...@kelunik.com wrote: Using set_error_handler isn't handling errors gracefully. Well, it's better than E_ERROR, but then libraries can't handle those errors gracefully, because the user might override its error handler by setting an own handler. Now I see what do you mean by gracefully. TL;DR; It's app developer jobs to handle these fatal errors. Nope. Most fatal errors shouldn't be recovered by library. e.g. Fallback to non CSPRNG when CSPRNG is not available. They should totally be handled. You need to catch the error and throw a defined exception, otherwise your public API will break if you choose to use another internal implementation. Additionally, you seem to assume that the library doesn't have to do things like cleanups in such a case. My thought is based on Design by Contract (Contract programming). When parameter or environment does not satisfy contract, contract error should be resulted in program/process termination. Fixing inappropriate parameter or environment is not library/framework author's responsibility, but the developer's. i.e. Caller(function/programmer/system admin) has the responsibility that satisfies parameter/environment requirement. If requirement is not met, it's perfectly OK for library/framework to raise fatal errors/exceptions. e.g. You need PHP 5.6 or greater error. Handling these fatal errors in a library/framework make code complex unnecessarily. More complex code has more chances to have bugs including security related bugs. Library/framework may simply raise error/exception telling users It's impossible to work. PHP is general programing language, so we have to consider long life applications such as standalone apps. I fully agree that exception is far easier for handling errors properly and keep app running. However, making randon_*() a special function does not worth it. IMHO. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Sat, Aug 1, 2015 at 8:27 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: They should totally be handled. You need to catch the error and throw a defined exception, otherwise your public API will break if you choose to use another internal implementation. Additionally, you seem to assume that the library doesn't have to do things like cleanups in such a case. My thought is based on Design by Contract (Contract programming). When parameter or environment does not satisfy contract, contract error should be resulted in program/process termination. Fixing inappropriate parameter or environment is not library/framework author's responsibility, but the developer's. i.e. Caller(function/programmer/system admin) has the responsibility that satisfies parameter/environment requirement. If requirement is not met, it's perfectly OK for library/framework to raise fatal errors/exceptions. e.g. You need PHP 5.6 or greater error. I'll be more specific for CSPRNG not available error. If a author would like to handle the error and fallback to non crypt safe RNG, he/she should detect environment and execute alternative code for the environment. Catching exception and fallback to non crypt safe RNG is not optimal way for handling unsatisfactory environment. IMHO. If we need function that checks environment, we are better to provide one rather than let users to use exception. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 I would vote for E_WARNING and return false. This can be wrapped in an oop wrapper in userland if somebody prefers and exception but would still keep the procedural style as first class citizen. Plus this would be consistent with other security/crypto related errors like mcrypt_encrypt() getting an invalid key/iv Nikita, Anthony what do you think? -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Fri, Jul 31, 2015 at 5:12 PM, Niklas Keller m...@kelunik.com wrote: I think the question is more whether Exception or Error (class, not E_RECOVERABLE_ERROR), to allow handling it gracefully. E_WARNING is too weak for CSPRNG not available. It's fatal error. I agree fatal errors should be able to handle gracefully if it is possible. That's the reason why I prepared a patch that changes session E_ERROR to E_RECOVERABLE_ERROR. https://github.com/php/php-src/compare/master...yohgaki:master-e_recoverable_error?expand=1 It's inspired by this thread :) Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Using set_error_handler isn't handling errors gracefully. Well, it's better than E_ERROR, but then libraries can't handle those errors gracefully, because the user might override its error handler by setting an own handler. Regards, Niklas 2015-07-31 11:46 GMT+02:00 Yasuo Ohgaki yohg...@ohgaki.net: Hi Niklas, On Fri, Jul 31, 2015 at 5:12 PM, Niklas Keller m...@kelunik.com wrote: I think the question is more whether Exception or Error (class, not E_RECOVERABLE_ERROR), to allow handling it gracefully. E_WARNING is too weak for CSPRNG not available. It's fatal error. I agree fatal errors should be able to handle gracefully if it is possible. That's the reason why I prepared a patch that changes session E_ERROR to E_RECOVERABLE_ERROR. https://github.com/php/php-src/compare/master...yohgaki:master-e_recoverable_error?expand=1 It's inspired by this thread :) Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Jul 30, 2015 2:27 PM, Niklas Keller m...@kelunik.com wrote: I prefer Exception, too, because it's I/O related. @Scott: You can open votes on everything, doesn't matter, just create a page with a vote. I just don't know where to put it in the wiki, because it's not a RFC. Regards, Niklas I'm not sure how to do that. I have a noncritical security patch I intend to write for random_bytes() that I would like to submit as a PR but first I'd like to see what the resolution will be re: Exceptions. Also, merge conflicts aren't fun. ;) If there's anything I can do to get those two merged faster, please let me know. Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises https://paragonie.com
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Mon, Jul 27, 2015 at 2:03 PM, Anthony Ferrara ircmax...@gmail.com wrote:
Rowan,
This is certainly some people's concern, but Anatol has raised a subtly
different consistency-related point, which is this:
Since we have no policy for what kinds of Throwable should be emitted in
what circumstance, throwing anything in this function sets a precedent which
will have to be incorporated in any future plan.
Assuming nobody is fundamentally against ever adding Throwables to core
functions, there are a minimum number of questions that need to be agreed
before adding the first one:
- when should we inherit from Error and when from Exception?
IMHO, Errors signify programmer error, where Exceptions signify
unknown or runtime errors. Meaning that an Error should always be a
problem with your code, but an Exception could be a systems problem, a
user problem or a problem in your code.
While that's slightly off-topic to this discussion, it frames which
type random_* would throw pretty clearly (Exception).
- is it ever OK to throw a plain Error or Exception (thus forcing users into
the otherwise bad practice of catching those base classes)?
For now, I think that's a good practice. It doesn't constrain us from
sub-typing down the road (7.1, etc), but it also lets us build the
support in today.
For example, if we throw Exception, in 7.1 we could make it
php\RandomException in 7.1 without issue (all we need to get right is
the hierarchy parent).
- if not throwing the base class, how specific should sub-classes be? (i.e.
a framework for defining the hierarchy, not necessarily the hierarchy
itself)
I think this is something that should be RFC'd for 7.1. I don't think
that limits us here though.
If we can get agreement on those points in time for 7.0, fine, but time is
very tight, and the window for such discussions has theoretically closed...
I think the only real agreement we need is Error vs Exception. If we
can agree on one of those, we can do the rest in 7.1 without worrying
about BC...
Anthony
I'm fine with either Error or Exception. I'd prefer Exception (easier
to write a sane backport for PHP 5.6) but I leave this decision in the
hands of others.
/**
* Slightly insane PHP 5 backport but it works
*/
class Error extends Exception { } // Done!
Does anybody feel particularly strong about one or the other?
If so, should we set up a vote somewhere? (I don't vote karma on RFCs
etc. so I don't know if the existing infrastructure would work.)
If not, can we get PR 1397 1398 merged? :)
Regards,
Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises https://paragonie.com
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
2015-07-30 19:12 GMT+02:00 Scott Arciszewski sc...@paragonie.com:
On Mon, Jul 27, 2015 at 2:03 PM, Anthony Ferrara ircmax...@gmail.com
wrote:
Rowan,
This is certainly some people's concern, but Anatol has raised a subtly
different consistency-related point, which is this:
Since we have no policy for what kinds of Throwable should be emitted in
what circumstance, throwing anything in this function sets a precedent
which
will have to be incorporated in any future plan.
Assuming nobody is fundamentally against ever adding Throwables to core
functions, there are a minimum number of questions that need to be
agreed
before adding the first one:
- when should we inherit from Error and when from Exception?
IMHO, Errors signify programmer error, where Exceptions signify
unknown or runtime errors. Meaning that an Error should always be a
problem with your code, but an Exception could be a systems problem, a
user problem or a problem in your code.
While that's slightly off-topic to this discussion, it frames which
type random_* would throw pretty clearly (Exception).
- is it ever OK to throw a plain Error or Exception (thus forcing users
into
the otherwise bad practice of catching those base classes)?
For now, I think that's a good practice. It doesn't constrain us from
sub-typing down the road (7.1, etc), but it also lets us build the
support in today.
For example, if we throw Exception, in 7.1 we could make it
php\RandomException in 7.1 without issue (all we need to get right is
the hierarchy parent).
- if not throwing the base class, how specific should sub-classes be?
(i.e.
a framework for defining the hierarchy, not necessarily the hierarchy
itself)
I think this is something that should be RFC'd for 7.1. I don't think
that limits us here though.
If we can get agreement on those points in time for 7.0, fine, but time
is
very tight, and the window for such discussions has theoretically
closed...
I think the only real agreement we need is Error vs Exception. If we
can agree on one of those, we can do the rest in 7.1 without worrying
about BC...
Anthony
I'm fine with either Error or Exception. I'd prefer Exception (easier
to write a sane backport for PHP 5.6) but I leave this decision in the
hands of others.
/**
* Slightly insane PHP 5 backport but it works
*/
class Error extends Exception { } // Done!
Does anybody feel particularly strong about one or the other?
If so, should we set up a vote somewhere? (I don't vote karma on RFCs
etc. so I don't know if the existing infrastructure would work.)
If not, can we get PR 1397 1398 merged? :)
Regards,
Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises https://paragonie.com
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
I prefer Exception, too, because it's I/O related.
@Scott: You can open votes on everything, doesn't matter, just create a
page with a vote.
I just don't know where to put it in the wiki, because it's not a RFC.
Regards, Niklas
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Scott Arciszewski wrote on 27/07/2015 07:57: My only concern is that, we have a fixed timetable for the 7.0.0 release. It's certainly a good idea to develop a cogent strategy before moving forward, but I worry that bikeshedding will get involved and we won't make the deadline. I propose that, if a better strategy cannot be presented in time for RC1, consistency should take a back seat to security. Fail hard, throw an Error, deal with the details later. I think taking on board Anatol's concerns about proper planning and consistency, and yours about short timescales, the reasonable solution is to create as small a possibility for future BC as possible, while maintaining the fail hard policy. My suggestion would therefore be to raise an E_ERROR for this case in 7.0. Unlike an E_RECOVERABLE_ERROR or any kind of Throwable, there is no way that code can be written to explicitly handle that case, it will simply halt execution. As such, there are no limits to what we can introduce later, other than that unmodified code should continue to fail hard. This buys us time to put together a cogent policy for what if any built-in functions can make use of Throwables in the 7.x release series, with a likely result of throwing some class of Error for this function in 7.1. Regards, -- Rowan Collins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Mon, Jul 27, 2015 at 10:27 AM, Rowan Collins rowan.coll...@gmail.com wrote: Scott Arciszewski wrote on 27/07/2015 07:57: My only concern is that, we have a fixed timetable for the 7.0.0 release. It's certainly a good idea to develop a cogent strategy before moving forward, but I worry that bikeshedding will get involved and we won't make the deadline. I propose that, if a better strategy cannot be presented in time for RC1, consistency should take a back seat to security. Fail hard, throw an Error, deal with the details later. I think taking on board Anatol's concerns about proper planning and consistency, and yours about short timescales, the reasonable solution is to create as small a possibility for future BC as possible, while maintaining the fail hard policy. My suggestion would therefore be to raise an E_ERROR for this case in 7.0. Unlike an E_RECOVERABLE_ERROR or any kind of Throwable, there is no way that code can be written to explicitly handle that case, it will simply halt execution. As such, there are no limits to what we can introduce later, other than that unmodified code should continue to fail hard. This buys us time to put together a cogent policy for what if any built-in functions can make use of Throwables in the 7.x release series, with a likely result of throwing some class of Error for this function in 7.1. Regards, -- Rowan Collins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php The problem with fatal errors is that This function can fail irrecoverably outside of your control isn't going to encourage adoption. Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Scott Arciszewski wrote on 27/07/2015 18:35: I understand that some of the Internals contributors are allergic to special cases. There's some validity to wanting the language to be predictable and consistent across the board, because that might be a UX concern in and of itself. (What do you mean this one function has to be wrapped in try-catch blocks when the others can be prefixed with @ to silence errors? I'm telling PHPSadness!) This is certainly some people's concern, but Anatol has raised a subtly different consistency-related point, which is this: Since we have no policy for what kinds of Throwable should be emitted in what circumstance, throwing anything in this function sets a precedent which will have to be incorporated in any future plan. Assuming nobody is fundamentally against ever adding Throwables to core functions, there are a minimum number of questions that need to be agreed before adding the first one: - when should we inherit from Error and when from Exception? - is it ever OK to throw a plain Error or Exception (thus forcing users into the otherwise bad practice of catching those base classes)? - if not throwing the base class, how specific should sub-classes be? (i.e. a framework for defining the hierarchy, not necessarily the hierarchy itself) If we can get agreement on those points in time for 7.0, fine, but time is very tight, and the window for such discussions has theoretically closed... Regards, -- Rowan Collins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Mon, Jul 27, 2015 at 1:04 PM, Rowan Collins rowan.coll...@gmail.com wrote: Scott Arciszewski wrote on 27/07/2015 15:45: The problem with fatal errors is that This function can fail irrecoverably outside of your control isn't going to encourage adoption. Well... rats! ;) It seems whatever we do here is going to carry some kind of risk / downside. :( Aye, unfortunately, we need to decide what our priorities are. I've said my piece before, but it bears repeating: If you are going to offer a Cryptographically Secure Pseudo-Random Number Generator feature, you need it to be secure first and foremost. An E_ERROR satisfies this requirement (it fails closed), but it's a horrible UX decision. There's an old trope that security and usability are opposites. This isn't necessarily true. I understand that some of the Internals contributors are allergic to special cases. There's some validity to wanting the language to be predictable and consistent across the board, because that might be a UX concern in and of itself. (What do you mean this one function has to be wrapped in try-catch blocks when the others can be prefixed with @ to silence errors? I'm telling PHPSadness!) However, there's already a precedent with a core function throwing a Throwable (i.e. intdiv()), so making a special case for random_bytes() and random_int() doesn't introduce an unprecedented inconsistency. My priorities can be summed up as follows: 1. Don't fail open. 2. Don't make developers want to avoid a secure RNG like the plague. 3. Although it hasn't come up yet: Don't scare or mislead developers when documenting these features. Or in other words: Security = User Friendly Consistency with the rest of the language features Your priorities might be different. That's okay, monocultures lead to fragility. But the CSPRNG is a security feature and security should come first. Any deviation from this priority, in the context of a security feature, is a high caliber foot-bullet. I strongly recommend anyone who argues against Exceptions consider how their proposal will affect security and the adoption of the new API in their response. I'm open to change and being proven wrong (i.e. with Errors not meaning E_ERROR earlier in this thread), but don't cripple a security feature because I want consistency at any cost. Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises https://paragonie.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Rowan, This is certainly some people's concern, but Anatol has raised a subtly different consistency-related point, which is this: Since we have no policy for what kinds of Throwable should be emitted in what circumstance, throwing anything in this function sets a precedent which will have to be incorporated in any future plan. Assuming nobody is fundamentally against ever adding Throwables to core functions, there are a minimum number of questions that need to be agreed before adding the first one: - when should we inherit from Error and when from Exception? IMHO, Errors signify programmer error, where Exceptions signify unknown or runtime errors. Meaning that an Error should always be a problem with your code, but an Exception could be a systems problem, a user problem or a problem in your code. While that's slightly off-topic to this discussion, it frames which type random_* would throw pretty clearly (Exception). - is it ever OK to throw a plain Error or Exception (thus forcing users into the otherwise bad practice of catching those base classes)? For now, I think that's a good practice. It doesn't constrain us from sub-typing down the road (7.1, etc), but it also lets us build the support in today. For example, if we throw Exception, in 7.1 we could make it php\RandomException in 7.1 without issue (all we need to get right is the hierarchy parent). - if not throwing the base class, how specific should sub-classes be? (i.e. a framework for defining the hierarchy, not necessarily the hierarchy itself) I think this is something that should be RFC'd for 7.1. I don't think that limits us here though. If we can get agreement on those points in time for 7.0, fine, but time is very tight, and the window for such discussions has theoretically closed... I think the only real agreement we need is Error vs Exception. If we can agree on one of those, we can do the rest in 7.1 without worrying about BC... Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Scott Arciszewski wrote on 27/07/2015 15:45: The problem with fatal errors is that This function can fail irrecoverably outside of your control isn't going to encourage adoption. Well... rats! ;) It seems whatever we do here is going to carry some kind of risk / downside. :( -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Aaron, -Original Message- From: Aaron Piotrowski [mailto:aa...@icicle.io] Sent: Monday, July 27, 2015 5:32 AM To: Sammy Kaye Powers m...@sammyk.me; Scott Arciszewski sc...@paragonie.com Cc: Anatol Belski anatol@belski.net; la...@garfieldtech.com; Jakub Kubíček kelerest...@gmail.com; Stanislav Malyshev smalys...@gmail.com; sc...@paragonie.com; rowan.coll...@gmail.com; pierre@gmail.com; Dean Eigenmann dean.eigenm...@icloud.com; Yasuo Ohgaki yohg...@ohgaki.net; PHP Internals internals@lists.php.net Subject: Re: [PHP-DEV] Core functions throwing exceptions in PHP7 I must have overlooked a detail here. According to https://github.com/tpunt/PHP7-Reference#throwable-interface there are Throwables called Error, as a separate designation from an exception. I didn't see this in the engine exceptions RFC, so I was unaware that was even a thing. In this case, yes, as long as you can wrap it in try/catch blocks, SecurityError which extends Error and/or implements Throwable is an excellent suggestion. Previously, I thought the suggestion was to stick to triggering errors (E_ERROR, E_RECOVERABLE_ERROR, etc.). Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises https://paragonie.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php I believe some were suggesting triggering an E_ERROR, though most E_ERRORs in the engine have been replaced with thrown Error exceptions, so I think using E_ERROR in this case would be inappropriate. As I suggested in my prior email, I think throwing an instance of Error would be appropriate when the functions random_bytes() and random_int() fail. There are several conditions that already cause the engine to throw an Error (or subclass thereof): (1)-method(); // Throws Error declare(strict_types=1); array_map(1, 1); // Throws TypeError require 'file-with- parse-error.php'; // Throws ParseError eval($a[ = 1;); // Throws ParseError 1 -1; // Throws ArithmeticError intdiv(1, 0); // Throws DivisionByZeroError 1 % 0; // Throws DivisionByZeroError Of particular interest in the above examples is intdiv(), an internal function that can throw an instance of Error if the denominator is zero. I propose that random_bytes() and random_int() should throw an instance of Error if the parameters are not as expected or if generating a random number fails. (To avoid further debate about a subclass, the function should throw just a plain instance of Error, it can always be subclassed later without BC concerns). random_bytes() and random_int() failing closed is very important to prevent misguided or lazy programmers from using false in place of a random value. A return of false can easily be overlooked and unintentionally be cast to a zero or empty string. A thrown instance of Error must be purposely caught and ignored to produce the same behavior. As Larry pointed out, it is a very common error for programmers to not do a strict check using === against false when calling strpos(). Does anyone have a strong objection to the above proposal? If not, then I think Sammy should update his PRs to throw an Error so they can be merged before the next beta release. Yes, there is an objection at least on my side. As we was discussing the PR on the github page, the primary goal of the suggestion to move the discussion to the intarnals list was to create an approach about exceptions in functions. This seems not be the case here. It is not the question whether exceptions should be thrown in functions - it's almost obvious that they should now after it's the engine behavior. But it is a huge question of the consistency with anything else. That was also the reason why me and Kalle have changed our minds about your PR converting fatals in the functions. The only case where exception is thrown in a function is intdiv() now. But also as mentioned elsewhere - it's something tightly coupled with the behavior of div/mod in the engine. IMHO it is not building any base for randomly adding exceptions elsewhere. Neither I don't see as a base that the CSPRNG functions are new. Neither the argument we can change it later to xyz. The main concern is still not addressed, and even wasn't started to be addressed. It is for nothing to have new nice functions with nice and correct behavior while leaving the old ugly functions ugly. IMO we should stop building special cases and move to the conceptualization as first. The more special cases exist, the harder it will be in the future to bring the behaviors inline without BC breaks. Till now I haven't seen even any gentle hint about such conceptualization work going on, no RFC page still exists, but it's being continued pushing on a special case. Solving an immediate issue might be tactically justifiable, but without a strategical thinking will lead to even a bigger issue. It is not something
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
My only concern is that, we have a fixed timetable for the 7.0.0 release. It's certainly a good idea to develop a cogent strategy before moving forward, but I worry that bikeshedding will get involved and we won't make the deadline. I propose that, if a better strategy cannot be presented in time for RC1, consistency should take a back seat to security. Fail hard, throw an Error, deal with the details later. Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises On Mon, Jul 27, 2015 at 2:53 AM, Anatol Belski anatol@belski.net wrote: Hi Aaron, -Original Message- From: Aaron Piotrowski [mailto:aa...@icicle.io] Sent: Monday, July 27, 2015 5:32 AM To: Sammy Kaye Powers m...@sammyk.me; Scott Arciszewski sc...@paragonie.com Cc: Anatol Belski anatol@belski.net; la...@garfieldtech.com; Jakub Kubíček kelerest...@gmail.com; Stanislav Malyshev smalys...@gmail.com; sc...@paragonie.com; rowan.coll...@gmail.com; pierre@gmail.com; Dean Eigenmann dean.eigenm...@icloud.com; Yasuo Ohgaki yohg...@ohgaki.net; PHP Internals internals@lists.php.net Subject: Re: [PHP-DEV] Core functions throwing exceptions in PHP7 I must have overlooked a detail here. According to https://github.com/tpunt/PHP7-Reference#throwable-interface there are Throwables called Error, as a separate designation from an exception. I didn't see this in the engine exceptions RFC, so I was unaware that was even a thing. In this case, yes, as long as you can wrap it in try/catch blocks, SecurityError which extends Error and/or implements Throwable is an excellent suggestion. Previously, I thought the suggestion was to stick to triggering errors (E_ERROR, E_RECOVERABLE_ERROR, etc.). Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises https://paragonie.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php I believe some were suggesting triggering an E_ERROR, though most E_ERRORs in the engine have been replaced with thrown Error exceptions, so I think using E_ERROR in this case would be inappropriate. As I suggested in my prior email, I think throwing an instance of Error would be appropriate when the functions random_bytes() and random_int() fail. There are several conditions that already cause the engine to throw an Error (or subclass thereof): (1)-method(); // Throws Error declare(strict_types=1); array_map(1, 1); // Throws TypeError require 'file-with- parse-error.php'; // Throws ParseError eval($a[ = 1;); // Throws ParseError 1 -1; // Throws ArithmeticError intdiv(1, 0); // Throws DivisionByZeroError 1 % 0; // Throws DivisionByZeroError Of particular interest in the above examples is intdiv(), an internal function that can throw an instance of Error if the denominator is zero. I propose that random_bytes() and random_int() should throw an instance of Error if the parameters are not as expected or if generating a random number fails. (To avoid further debate about a subclass, the function should throw just a plain instance of Error, it can always be subclassed later without BC concerns). random_bytes() and random_int() failing closed is very important to prevent misguided or lazy programmers from using false in place of a random value. A return of false can easily be overlooked and unintentionally be cast to a zero or empty string. A thrown instance of Error must be purposely caught and ignored to produce the same behavior. As Larry pointed out, it is a very common error for programmers to not do a strict check using === against false when calling strpos(). Does anyone have a strong objection to the above proposal? If not, then I think Sammy should update his PRs to throw an Error so they can be merged before the next beta release. Yes, there is an objection at least on my side. As we was discussing the PR on the github page, the primary goal of the suggestion to move the discussion to the intarnals list was to create an approach about exceptions in functions. This seems not be the case here. It is not the question whether exceptions should be thrown in functions - it's almost obvious that they should now after it's the engine behavior. But it is a huge question of the consistency with anything else. That was also the reason why me and Kalle have changed our minds about your PR converting fatals in the functions. The only case where exception is thrown in a function is intdiv() now. But also as mentioned elsewhere - it's something tightly coupled with the behavior of div/mod in the engine. IMHO it is not building any base for randomly adding exceptions elsewhere. Neither I don't see as a base that the CSPRNG functions are new. Neither the argument we can change it later to xyz. The main concern is still not addressed, and even wasn't started to be addressed. It is for nothing to have new nice functions
RE: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Scott, -Original Message- From: Scott Arciszewski [mailto:sc...@paragonie.com] Sent: Monday, July 27, 2015 8:58 AM To: Anatol Belski anatol@belski.net Cc: Aaron Piotrowski aa...@icicle.io; Sammy Kaye Powers m...@sammyk.me; Larry Garfield la...@garfieldtech.com; Jakub Kubíček kelerest...@gmail.com; Stanislav Malyshev smalys...@gmail.com; rowan.coll...@gmail.com; Pierre Joye pierre@gmail.com; Dean Eigenmann dean.eigenm...@icloud.com; Yasuo Ohgaki yohg...@ohgaki.net; PHP Internals internals@lists.php.net Subject: Re: [PHP-DEV] Core functions throwing exceptions in PHP7 My only concern is that, we have a fixed timetable for the 7.0.0 release. It's certainly a good idea to develop a cogent strategy before moving forward, but I worry that bikeshedding will get involved and we won't make the deadline. Clear, it is not something arising overnight. And bikeshedding is good because it reveals many potential issues. Clear, it needs time. RC1 might come after beta3 (or might not) which is roughly 1 month from now on. It is in your guys hands. I propose that, if a better strategy cannot be presented in time for RC1, consistency should take a back seat to security. Fail hard, throw an Error, deal with the details later. Clear, it can't be complete and accepted/declined in such a short period (though much time is lost discussing only this one case). That's why I wrote literally about a rough draft. But it also can't arise if no one cares. Then yes - we end up in a situation like A over B or B over C without really knowing the consequences. Cheers Anatol -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On 07/26/2015 01:36 PM, Jakub Kubíček wrote:
Hi Larry!
2015-07-26 1:29 GMT+02:00 Larry Garfield la...@garfieldtech.com:
Another point here is that 0 is a perfectly legitimate random number in many
cases, so callers would need to do a === check, not just a boolean check.
What boolean check are you talkin' about? I've never seen a code
using e.g. strpos() like follows:
?php
if(!is_bool($pos = strpos('foobar', 'baz'))) {
// we are correct, use $pos' value somewhere
} else {
// strpos() produced a boolean, thus no 'baz' found in 'foobar'
}
?
Rather it is most frequently being done like below:
?php
if(FALSE !== $pos = strpos('foobar', 'baz')) {
// we are correct, use $pos' value somewhere
} else {
// strpos() produced a boolean, thus no 'baz' found in 'foobar'
}
?
I think in both cases you do a kind of boolean check.
Yes, I am referring to the second, or variations therein. When using
strpos() you need to do a strict check against FALSE (===) to avoid a
legit 0 getting misinterpreted. It's also a very commonly forgotten
check, especially among newer devs that haven't been burned by it a few
times.
My point is that when talking about crypto-related functions, relying on
people to get burned a few times and then remember to do a === check
against FALSE is a horrible, horrible idea, hence random_int() should
fail much harder, as is being suggested here (be that via E_FATAL,
exception, error throw, or whatever, as long as the execution does NOT
continue). That is, I am agreeing with the proposal to have it die
hard. I moderately prefer the throw SecurityError approach as it's not
a user-input error but a system config error, but anything that prevents
execution from continuing is acceptable security-wise.
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Larry!
2015-07-26 1:29 GMT+02:00 Larry Garfield la...@garfieldtech.com:
Another point here is that 0 is a perfectly legitimate random number in many
cases, so callers would need to do a === check, not just a boolean check.
What boolean check are you talkin' about? I've never seen a code
using e.g. strpos() like follows:
?php
if(!is_bool($pos = strpos('foobar', 'baz'))) {
// we are correct, use $pos' value somewhere
} else {
// strpos() produced a boolean, thus no 'baz' found in 'foobar'
}
?
Rather it is most frequently being done like below:
?php
if(FALSE !== $pos = strpos('foobar', 'baz')) {
// we are correct, use $pos' value somewhere
} else {
// strpos() produced a boolean, thus no 'baz' found in 'foobar'
}
?
I think in both cases you do a kind of boolean check.
Especially as we're talking not about a user error but a your system is not
setup
right so it will never work situation, as I understand it.
So, I generally agree with this approach. It is a better thing to
always fail closed if your system isn't set up, e.g. missing some
required things, to work properly. It's the same if your code uses
some vendor extension not included in the core by default -- it can
not work work properly without having this extension available.
I say +1 for raising a E_ERROR on random_int()/random_bytes() fail.
Best regards,
Kubo2, Jakub Kubíček
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi,
-Original Message-
From: Larry Garfield [mailto:la...@garfieldtech.com]
Sent: Sunday, July 26, 2015 9:38 PM
To: internals@lists.php.net
Subject: Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On 07/26/2015 01:36 PM, Jakub Kubíček wrote:
Hi Larry!
2015-07-26 1:29 GMT+02:00 Larry Garfield la...@garfieldtech.com:
Another point here is that 0 is a perfectly legitimate random number
in many cases, so callers would need to do a === check, not just a boolean
check.
What boolean check are you talkin' about? I've never seen a code
using e.g. strpos() like follows:
?php
if(!is_bool($pos = strpos('foobar', 'baz'))) {
// we are correct, use $pos' value somewhere } else {
// strpos() produced a boolean, thus no 'baz' found in 'foobar'
}
?
Rather it is most frequently being done like below:
?php
if(FALSE !== $pos = strpos('foobar', 'baz')) {
// we are correct, use $pos' value somewhere } else {
// strpos() produced a boolean, thus no 'baz' found in 'foobar'
}
?
I think in both cases you do a kind of boolean check.
Yes, I am referring to the second, or variations therein. When using
strpos() you need to do a strict check against FALSE (===) to avoid a legit 0
getting misinterpreted. It's also a very commonly forgotten check, especially
among newer devs that haven't been burned by it a few times.
My point is that when talking about crypto-related functions, relying on
people
to get burned a few times and then remember to do a === check against FALSE is
a horrible, horrible idea, hence random_int() should fail much harder, as is
being
suggested here (be that via E_FATAL, exception, error throw, or whatever, as
long as the execution does NOT continue). That is, I am agreeing with the
proposal to have it die hard. I moderately prefer the throw SecurityError
approach as it's not a user-input error but a system config error, but
anything
that prevents execution from continuing is acceptable security-wise.
When implementing a security related code, people that are tired or asleep
should stay home :) But going seriously - what it is about is changing the
paradigm. IMHO this kind of thing should not be done making a special case.
Since Trowable is a good base, there is a need on a strategy.
The keyword about the exception hierarchy is not seldom to hear it this
regard nowadays. So besides SecurityError, what is with IOError, MemoryError,
other common cases? Also be aware and prepared that many cases can't currently
be served by exceptions, the very low level errors. But basically what I would
refer to is a concept catching the common cases, so then
- we have a set of rules about exceptions in functions
- those rules produce consistent behaviors
- the case by case basis argument is eliminated by them in 99% of cases
Regards
Anatol
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
I must have overlooked a detail here. According to https://github.com/tpunt/PHP7-Reference#throwable-interface there are Throwables called Error, as a separate designation from an exception. I didn't see this in the engine exceptions RFC, so I was unaware that was even a thing. In this case, yes, as long as you can wrap it in try/catch blocks, SecurityError which extends Error and/or implements Throwable is an excellent suggestion. Previously, I thought the suggestion was to stick to triggering errors (E_ERROR, E_RECOVERABLE_ERROR, etc.). Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises https://paragonie.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php I believe some were suggesting triggering an E_ERROR, though most E_ERRORs in the engine have been replaced with thrown Error exceptions, so I think using E_ERROR in this case would be inappropriate. As I suggested in my prior email, I think throwing an instance of Error would be appropriate when the functions random_bytes() and random_int() fail. There are several conditions that already cause the engine to throw an Error (or subclass thereof): (1)-method(); // Throws Error declare(strict_types=1); array_map(1, 1); // Throws TypeError require 'file-with-parse-error.php'; // Throws ParseError eval($a[ = 1;); // Throws ParseError 1 -1; // Throws ArithmeticError intdiv(1, 0); // Throws DivisionByZeroError 1 % 0; // Throws DivisionByZeroError Of particular interest in the above examples is intdiv(), an internal function that can throw an instance of Error if the denominator is zero. I propose that random_bytes() and random_int() should throw an instance of Error if the parameters are not as expected or if generating a random number fails. (To avoid further debate about a subclass, the function should throw just a plain instance of Error, it can always be subclassed later without BC concerns). random_bytes() and random_int() failing closed is very important to prevent misguided or lazy programmers from using false in place of a random value. A return of false can easily be overlooked and unintentionally be cast to a zero or empty string. A thrown instance of Error must be purposely caught and ignored to produce the same behavior. As Larry pointed out, it is a very common error for programmers to not do a strict check using === against false when calling strpos(). Does anyone have a strong objection to the above proposal? If not, then I think Sammy should update his PRs to throw an Error so they can be merged before the next beta release. Aaron Piotrowski -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
+1 for Error On Jul 26, 2015 11:32 PM, Aaron Piotrowski aa...@icicle.io wrote: I must have overlooked a detail here. According to https://github.com/tpunt/PHP7-Reference#throwable-interface there are Throwables called Error, as a separate designation from an exception. I didn't see this in the engine exceptions RFC, so I was unaware that was even a thing. In this case, yes, as long as you can wrap it in try/catch blocks, SecurityError which extends Error and/or implements Throwable is an excellent suggestion. Previously, I thought the suggestion was to stick to triggering errors (E_ERROR, E_RECOVERABLE_ERROR, etc.). Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises https://paragonie.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php I believe some were suggesting triggering an E_ERROR, though most E_ERRORs in the engine have been replaced with thrown Error exceptions, so I think using E_ERROR in this case would be inappropriate. As I suggested in my prior email, I think throwing an instance of Error would be appropriate when the functions random_bytes() and random_int() fail. There are several conditions that already cause the engine to throw an Error (or subclass thereof): (1)-method(); // Throws Error declare(strict_types=1); array_map(1, 1); // Throws TypeError require 'file-with-parse-error.php'; // Throws ParseError eval($a[ = 1;); // Throws ParseError 1 -1; // Throws ArithmeticError intdiv(1, 0); // Throws DivisionByZeroError 1 % 0; // Throws DivisionByZeroError Of particular interest in the above examples is intdiv(), an internal function that can throw an instance of Error if the denominator is zero. I propose that random_bytes() and random_int() should throw an instance of Error if the parameters are not as expected or if generating a random number fails. (To avoid further debate about a subclass, the function should throw just a plain instance of Error, it can always be subclassed later without BC concerns). random_bytes() and random_int() failing closed is very important to prevent misguided or lazy programmers from using false in place of a random value. A return of false can easily be overlooked and unintentionally be cast to a zero or empty string. A thrown instance of Error must be purposely caught and ignored to produce the same behavior. As Larry pointed out, it is a very common error for programmers to not do a strict check using === against false when calling strpos(). Does anyone have a strong objection to the above proposal? If not, then I think Sammy should update his PRs to throw an Error so they can be merged before the next beta release. Aaron Piotrowski
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On 07/24/2015 06:58 PM, Stanislav Malyshev wrote: Hi! On the surface, this sounds like a good thing. Although, I question that if a user is not checking $result === false, then will they end up just wrapping this in an empty try/catch so their code does not fail? There is a mechanism to detect the error now. True, but not checking for false is an error of omission, and an easy one to make (How can random fail? It's just creating random numbers! I won't bother to check), Another point here is that 0 is a perfectly legitimate random number in many cases, so callers would need to do a === check, not just a boolean check. How many people already forget to do that for strpos() on a daily basis? Too many. :-) Whatever the exact mechanism, a break hard so you can't continue approach seems far better in this case than a return false and hope the user knows what they're doing, even if the latter is currently more common in PHP itself. Especially as we're talking not about a user error but a your system is not setup right so it will never work situation, as I understand it. --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Thu, Jul 23, 2015 at 1:26 PM, Aaron Piotrowski aa...@icicle.io wrote: On Jul 14, 2015, at 4:04 PM, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 How about instead of throwing an instance of Exception, the random_*() functions throw an instance of Error on failure. A subclass of Error, such as SecurityError could also be added. As it is unlikely that the failure of these functions to generate a random value could be handled at runtime, throwing an instance of Error makes the most sense imho. Many internal functions can result in an instance of Error being thrown, particularly with declare(strict_types=1). So those looking for consistency can be satisfied that other internal functions already can behave similarly, and those looking to fail closed can be satisfied that an exception will be thrown if securely generating a random value fails. Aaron Piotrowski I must have overlooked a detail here. According to https://github.com/tpunt/PHP7-Reference#throwable-interface there are Throwables called Error, as a separate designation from an exception. I didn't see this in the engine exceptions RFC, so I was unaware that was even a thing. In this case, yes, as long as you can wrap it in try/catch blocks, SecurityError which extends Error and/or implements Throwable is an excellent suggestion. Previously, I thought the suggestion was to stick to triggering errors (E_ERROR, E_RECOVERABLE_ERROR, etc.). Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises https://paragonie.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi! Given that the *engine itself* now throws exceptions, and we have bundled extensions which throw exceptions, the blanket ban on exceptions in core functions seems increasingly out-dated. This particular I agree. I think once we agreed we want to convert from fatals to exceptions, we need to recognize exceptions as a first-class citizen. Which means using it in internal functions is OK. Now, there is a big gap between E_WARNING on one side and E_ERROR/exception on the other side. Whether particular function crosses this gap or not is a case-by-case discussion, but we can not use exceptions in core should no longer be a part of it. too quickly. I'm also a big fan of proper exception hierarchies - I've seen so many times that you should never *catch* the base Exception class, so *throwing* a base Exception seems very wrong to me. Agreed. Maybe we should have something like RuntimeException or Java? That is pretty generic too, but at least not Exception. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi! On the surface, this sounds like a good thing. Although, I question that if a user is not checking $result === false, then will they end up just wrapping this in an empty try/catch so their code does not fail? There is a mechanism to detect the error now. True, but not checking for false is an error of omission, and an easy one to make (How can random fail? It's just creating random numbers! I won't bother to check), while wrapping in try/catch has to be done explicitly - and can be easily caught on review, even often with automatic tools, and people can be trained not to do empty try/catches (it is much easier to train for do not do this specific thing than always do these 1000 things in 1000 different special cases). I question why the cryptographic functions would not force an integer to be passed. Those should not accept a boolean and evaluate it as false. I am not sure what functions you are talking about though. Maybe 3rd party user land code? Accepting a boolean in those cases is a bug in that code IMO. Given PHP is a weakly-typed language, I don't thing you can rely on this kind of type checking. While in many cases we can tell people just check for errors or accept the fallout, when we come to security I think the price is too high. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi! 1. E_WARNING - fail open, possibly cause security problems for the user 2. E_ERROR - fail closed, but make graceful handling a pain in the neck Can't you just catch Error just as you would catch an Exception? Of course, it's less clean than specialized exception but I don't see why it's not an option. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Jul 14, 2015, at 4:04 PM, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 How about instead of throwing an instance of Exception, the random_*() functions throw an instance of Error on failure. A subclass of Error, such as SecurityError could also be added. As it is unlikely that the failure of these functions to generate a random value could be handled at runtime, throwing an instance of Error makes the most sense imho. Many internal functions can result in an instance of Error being thrown, particularly with declare(strict_types=1). So those looking for consistency can be satisfied that other internal functions already can behave similarly, and those looking to fail closed can be satisfied that an exception will be thrown if securely generating a random value fails. Aaron Piotrowski -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Brian Moon wrote on 16/07/2015 17:19:
On the surface, this sounds like a good thing. Although, I question
that if a user is not checking $result === false, then will they end
up just wrapping this in an empty try/catch so their code does not
fail? There is a mechanism to detect the error now.
I question why the cryptographic functions would not force an integer
to be passed. Those should not accept a boolean and evaluate it as
false. I am not sure what functions you are talking about though.
Maybe 3rd party user land code? Accepting a boolean in those cases is
a bug in that code IMO.
Scott provided an example elsewhere in the thread:
$max = strlen($alphabet) - 1;
for ($i = 0; $i 32; ++$i) {
$password .= $alphabet[random_int(0, $max)];
}
That demonstrates both a situation where booleans can't be excluded, and
where you'd have to try pretty hard to silently catch the exception in a
way that left your code broken. I suppose you could forget to check if
the generated password is empty, but that doesn't seem all that likely.
If you've gone to the trouble of putting a try block in, you're at least
aware that the function CAN fail.
Regards,
--
Rowan Collins
[IMSoP]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On 7/14/15 16:04 , Sammy Kaye Powers wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. On the surface, this sounds like a good thing. Although, I question that if a user is not checking $result === false, then will they end up just wrapping this in an empty try/catch so their code does not fail? There is a mechanism to detect the error now. I question why the cryptographic functions would not force an integer to be passed. Those should not accept a boolean and evaluate it as false. I am not sure what functions you are talking about though. Maybe 3rd party user land code? Accepting a boolean in those cases is a bug in that code IMO. -- Brian. http://brian.moonspot.net/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Sammy, On Wed, Jul 15, 2015 at 6:04 AM, Sammy Kaye Powers m...@sammyk.me wrote: There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) I prefer exception rather than error. However, I would not like to see exception in some functions. It's whether we use exception for builtin functions or not. I understand the risk, but users should handle all errors properly to be secure anyway. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Sammy Kaye Powers wrote on 14/07/2015 22:04: Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. Given that the *engine itself* now throws exceptions, and we have bundled extensions which throw exceptions, the blanket ban on exceptions in core functions seems increasingly out-dated. This particular function is in ext/standard, which is about as core as you can get, but it is a brand new function with no BC considerations and an explicit mandate to be as secure as possible. To me, using an exception here makes perfect sense, but I'm not sure how to formulate a policy that allows that without opening the floodgates too quickly. I'm also a big fan of proper exception hierarchies - I've seen so many times that you should never *catch* the base Exception class, so *throwing* a base Exception seems very wrong to me. Regards, -- Rowan Collins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Wed, Jul 15, 2015 at 6:57 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote:
Hi Scott,
On Wed, Jul 15, 2015 at 7:19 PM, Scott Arciszewski sc...@paragonie.com
wrote:
On Wed, Jul 15, 2015 at 4:27 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote:
Hi Sammy,
On Wed, Jul 15, 2015 at 6:04 AM, Sammy Kaye Powers m...@sammyk.me wrote:
There are two open PR's for PHP7 to modify the behavior of the
CSPRNG's:
https://github.com/php/php-src/pull/1397 (main discussion)
https://github.com/php/php-src/pull/1398
Currently the random_*() functions will issue a warning and return
false if
a good source of random cannot be found. This is a potential security
hole
in the event the RNG fails and returns false which gets evaluated as 0
in a
cryptographic context.
To prevent this exploit the proposed behavior will throw an Exception
when
the RNG fails or certain argument validation fails. This also gives the
developer a graceful way to fall back to an alternate CSPRNG.
Since the core functions in PHP don't throw Exceptions, there is debate
on
whether or not this change should be implemented. Some say the CSPRNG's
should get a special pass since they will be relied on for
cryptography. If
we can't throw Exceptions, there were suggestions of raising a fatal
error
if the RNG fails.
I think the argument can be boiled down to consistency vs security.
We'd
love to hear your feedback to decide what we should do in this context.
:)
I prefer exception rather than error.
However, I would not like to see exception in some functions.
It's whether we use exception for builtin functions or not.
I understand the risk, but users should handle all errors properly
to be secure anyway.
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
-1 I emphatically disagree.
I understand the risk, but users should handle all errors properly
to be secure anyway.
Recommended reading:
http://cr.yp.to/talks/2015.01.07/slides-djb-20150107-a4.pdf
---
Instead of placing all the burden on the user to add code to their
application to check for errors, lest their application fail open and
operate with zero entropy, which is what you've proposed, I say we
throw an exception if the random number generator fails. This means
that, in the default case, their application will fail closed (i.e.
not continue processing with zero entropy), but the developer can
still intuitively handle the exception if they want to add code to
fail gracefully. Try-catch blocks are far less messy than overriding
the error handler.
Failing open is a terrible idea. You're going to have people who write
code like this:
$max = strlen($alphabet) - 1;
for ($i = 0; $i 32; ++$i) {
$password .= $alphabet[random_int(0, $max)];
}
How do I know this? Because people already write code like that with
mt_rand().
If a random number generator failure occurs, instead of generating a
predictable password, we ought to abort. If the developer wants to
handle this situation, they ought to do this.
try {
$max = strlen($alphabet) - 1;
for ($i = 0; $i 32; ++$i) {
$password .= $alphabet[random_int(0, $max)];
}
} catch (Exception $e) {
$this-template('error', 'Could not safely generate a passphrase!
:(');
}
In the case of a negligent developer, the uncaught exception is
indistinguishable from raising a fatal error. Either way prevents the
script from proceeding with zero entropy.
TL;DR we have three options:
1. E_WARNING - fail open, possibly cause security problems for the user
2. E_ERROR - fail closed, but make graceful handling a pain in the neck
3. Exception - fail closed by default, developer can write their own
graceful failure code if they so choose, would currently be an
exception to the rule
1 is bad, 2 is less bad, I think 3 is ideal. Security continuity.
The basis of my thought is user must write code in a way that would
never raise errors under normal conditions and user must stop execution
by any errors because they are unexpected.
To do that, user should have custom error handler always to achieve
graceful exit just like handling uncaught exceptions.
i.e. User should have something like following always.
?php
set_error_handler(function ($severity, $message, $file, $line) {
throw new ErrorException($message, 0, $severity, $file, $line);
});
function exception_handler($exception) {
echo Uncaught exception: , $exception-getMessage(), \n;
}
set_exception_handler('exception_handler');
?
Server side web app is simple request/response app basically.
There is no point to continue execution when unexpected occurs.
Users should have something similar to above code anyway.
By the way, I agree that random_*() errors are critical. So I don't
mind to raise E_RECOVERABLE_ERROR from them. IMO, E_RECOVERABLE_ERROR
is better. The issue with E_ERROR is
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Yasuo Ohgaki wrote on 15/07/2015 12:20: Engine exception is a little different. The main motivation for engine exception is to give a chance to users for graceful program termination. Functions can achieve the objective by E_RECOVERABLE_ERROR mostly. This is simply incorrect, as demonstrated by conversion of existing E_RECOVERABLE_ERROR states in the engine to Throwable, and nullified RFCs which would have, had EngineExceptions not passed, convert engine-issued E_ERRORs into E_RECOVERABLE_ERROR. If we are going to adopt exception for functions, it would be better to have switch that convert all errors to exceptions. Strongly disagree. A runtime switch would be a horrible consistency nightmare for userland, and many of the existing functions which desperately need better error-handling (e.g. file I/O) don't issue an error at the moment anyway. The only reason (and it's a massive reason, don't get me wrong) not to start throwing exceptions all over the core is backwards compatibility. New functions do not, by definition, have anything to be backwards compatible with. So my suggestion is, to come up with some reasonable definition of new area of functionality where we can do things in a way which everyone seems to agree is basically better, i.e. throw Exceptions. - The rule shouldn't apply for new variations or close cousins of existing functions (e.g. the json_decode_to() that was briefly discussed should use similar conventions to json_decode() if it were implemented), only in cases like this where we're adding a new set of functions (the manual puts random_bytes and random_int in their own book: http://php.net/manual/en/book.csprng.php). - It also shouldn't be used for routine errors (like File Not Found), only in place of conditions which can justify a fatal error - both for the intuitive reason that an uncaught exception *is* a fatal error, and the procedural reason that we may want to decide a policy for routine error handling at a later date. At some point, we should revisit the general error-handling strategy of both the language and the core library, but 7.0 having been somewhat derailed (in my opinion) by the endless debates about scalar type hints, it will have to wait until 8.0 before we can do anything radical to existing functions. Regards, -- Rowan Collins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Wed, Jul 15, 2015 at 4:27 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote:
Hi Sammy,
On Wed, Jul 15, 2015 at 6:04 AM, Sammy Kaye Powers m...@sammyk.me wrote:
There are two open PR's for PHP7 to modify the behavior of the CSPRNG's:
https://github.com/php/php-src/pull/1397 (main discussion)
https://github.com/php/php-src/pull/1398
Currently the random_*() functions will issue a warning and return false if
a good source of random cannot be found. This is a potential security hole
in the event the RNG fails and returns false which gets evaluated as 0 in a
cryptographic context.
To prevent this exploit the proposed behavior will throw an Exception when
the RNG fails or certain argument validation fails. This also gives the
developer a graceful way to fall back to an alternate CSPRNG.
Since the core functions in PHP don't throw Exceptions, there is debate on
whether or not this change should be implemented. Some say the CSPRNG's
should get a special pass since they will be relied on for cryptography. If
we can't throw Exceptions, there were suggestions of raising a fatal error
if the RNG fails.
I think the argument can be boiled down to consistency vs security. We'd
love to hear your feedback to decide what we should do in this context. :)
I prefer exception rather than error.
However, I would not like to see exception in some functions.
It's whether we use exception for builtin functions or not.
I understand the risk, but users should handle all errors properly
to be secure anyway.
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
-1 I emphatically disagree.
I understand the risk, but users should handle all errors properly
to be secure anyway.
Recommended reading: http://cr.yp.to/talks/2015.01.07/slides-djb-20150107-a4.pdf
---
Instead of placing all the burden on the user to add code to their
application to check for errors, lest their application fail open and
operate with zero entropy, which is what you've proposed, I say we
throw an exception if the random number generator fails. This means
that, in the default case, their application will fail closed (i.e.
not continue processing with zero entropy), but the developer can
still intuitively handle the exception if they want to add code to
fail gracefully. Try-catch blocks are far less messy than overriding
the error handler.
Failing open is a terrible idea. You're going to have people who write
code like this:
$max = strlen($alphabet) - 1;
for ($i = 0; $i 32; ++$i) {
$password .= $alphabet[random_int(0, $max)];
}
How do I know this? Because people already write code like that with mt_rand().
If a random number generator failure occurs, instead of generating a
predictable password, we ought to abort. If the developer wants to
handle this situation, they ought to do this.
try {
$max = strlen($alphabet) - 1;
for ($i = 0; $i 32; ++$i) {
$password .= $alphabet[random_int(0, $max)];
}
} catch (Exception $e) {
$this-template('error', 'Could not safely generate a passphrase! :(');
}
In the case of a negligent developer, the uncaught exception is
indistinguishable from raising a fatal error. Either way prevents the
script from proceeding with zero entropy.
TL;DR we have three options:
1. E_WARNING - fail open, possibly cause security problems for the user
2. E_ERROR - fail closed, but make graceful handling a pain in the neck
3. Exception - fail closed by default, developer can write their own
graceful failure code if they so choose, would currently be an
exception to the rule
1 is bad, 2 is less bad, I think 3 is ideal. Security continuity.
Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises https://paragonie.com
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Scott,
On Wed, Jul 15, 2015 at 7:19 PM, Scott Arciszewski sc...@paragonie.com
wrote:
On Wed, Jul 15, 2015 at 4:27 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote:
Hi Sammy,
On Wed, Jul 15, 2015 at 6:04 AM, Sammy Kaye Powers m...@sammyk.me wrote:
There are two open PR's for PHP7 to modify the behavior of the CSPRNG's:
https://github.com/php/php-src/pull/1397 (main discussion)
https://github.com/php/php-src/pull/1398
Currently the random_*() functions will issue a warning and return
false if
a good source of random cannot be found. This is a potential security
hole
in the event the RNG fails and returns false which gets evaluated as 0
in a
cryptographic context.
To prevent this exploit the proposed behavior will throw an Exception
when
the RNG fails or certain argument validation fails. This also gives the
developer a graceful way to fall back to an alternate CSPRNG.
Since the core functions in PHP don't throw Exceptions, there is debate
on
whether or not this change should be implemented. Some say the CSPRNG's
should get a special pass since they will be relied on for
cryptography. If
we can't throw Exceptions, there were suggestions of raising a fatal
error
if the RNG fails.
I think the argument can be boiled down to consistency vs security. We'd
love to hear your feedback to decide what we should do in this context.
:)
I prefer exception rather than error.
However, I would not like to see exception in some functions.
It's whether we use exception for builtin functions or not.
I understand the risk, but users should handle all errors properly
to be secure anyway.
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
-1 I emphatically disagree.
I understand the risk, but users should handle all errors properly
to be secure anyway.
Recommended reading:
http://cr.yp.to/talks/2015.01.07/slides-djb-20150107-a4.pdf
---
Instead of placing all the burden on the user to add code to their
application to check for errors, lest their application fail open and
operate with zero entropy, which is what you've proposed, I say we
throw an exception if the random number generator fails. This means
that, in the default case, their application will fail closed (i.e.
not continue processing with zero entropy), but the developer can
still intuitively handle the exception if they want to add code to
fail gracefully. Try-catch blocks are far less messy than overriding
the error handler.
Failing open is a terrible idea. You're going to have people who write
code like this:
$max = strlen($alphabet) - 1;
for ($i = 0; $i 32; ++$i) {
$password .= $alphabet[random_int(0, $max)];
}
How do I know this? Because people already write code like that with
mt_rand().
If a random number generator failure occurs, instead of generating a
predictable password, we ought to abort. If the developer wants to
handle this situation, they ought to do this.
try {
$max = strlen($alphabet) - 1;
for ($i = 0; $i 32; ++$i) {
$password .= $alphabet[random_int(0, $max)];
}
} catch (Exception $e) {
$this-template('error', 'Could not safely generate a passphrase!
:(');
}
In the case of a negligent developer, the uncaught exception is
indistinguishable from raising a fatal error. Either way prevents the
script from proceeding with zero entropy.
TL;DR we have three options:
1. E_WARNING - fail open, possibly cause security problems for the user
2. E_ERROR - fail closed, but make graceful handling a pain in the neck
3. Exception - fail closed by default, developer can write their own
graceful failure code if they so choose, would currently be an
exception to the rule
1 is bad, 2 is less bad, I think 3 is ideal. Security continuity.
The basis of my thought is user must write code in a way that would
never raise errors under normal conditions and user must stop execution
by any errors because they are unexpected.
To do that, user should have custom error handler always to achieve
graceful exit just like handling uncaught exceptions.
i.e. User should have something like following always.
?php
set_error_handler(function ($severity, $message, $file, $line) {
throw new ErrorException($message, 0, $severity, $file, $line);
});
function exception_handler($exception) {
echo Uncaught exception: , $exception-getMessage(), \n;
}
set_exception_handler('exception_handler');
?
Server side web app is simple request/response app basically.
There is no point to continue execution when unexpected occurs.
Users should have something similar to above code anyway.
By the way, I agree that random_*() errors are critical. So I don't
mind to raise E_RECOVERABLE_ERROR from them. IMO, E_RECOVERABLE_ERROR
is better. The issue with E_ERROR is graceful exit. E_RECOVERABLE_ERROR
resolves
the issue.
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Rowan, On Wed, Jul 15, 2015 at 7:57 PM, Rowan Collins rowan.coll...@gmail.com wrote: Sammy Kaye Powers wrote on 14/07/2015 22:04: Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. Given that the *engine itself* now throws exceptions, and we have bundled extensions which throw exceptions, the blanket ban on exceptions in core functions seems increasingly out-dated. This particular function is in ext/standard, which is about as core as you can get, but it is a brand new function with no BC considerations and an explicit mandate to be as secure as possible. To me, using an exception here makes perfect sense, but I'm not sure how to formulate a policy that allows that without opening the floodgates too quickly. I'm also a big fan of proper exception hierarchies - I've seen so many times that you should never *catch* the base Exception class, so *throwing* a base Exception seems very wrong to me. As I wrote earlier, I prefer exceptions rather than errors. Engine exception is a little different. The main motivation for engine exception is to give a chance to users for graceful program termination. Functions can achieve the objective by E_RECOVERABLE_ERROR mostly. I don't want to see one function raise exception and the other raise error. It's should be avoided inconsistency. IMHO. If we are going to adopt exception for functions, it would be better to have switch that convert all errors to exceptions. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
try {
random_*();
} catch (SomeException $e) {
// Something
}
... is far cleaner than telling developers to override the error
handler for one specific function.
That said, I won't oppose E_RECOVERABLE_ERROR or E_ERROR if that's
what the rest of the Internals team settles on. Just don't expect me
to be quiet when, in future versions, we transition back to an
Exception. Failing closed is my only priority here.
Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises
On Wed, Jul 15, 2015 at 7:27 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote:
Hi Scott,
On Wed, Jul 15, 2015 at 8:20 PM, Scott Arciszewski sc...@paragonie.com
wrote:
At what point do we stop blaming the developers for not knowing how to
use our badly designed features, and accept responsibility for
exposing an API that is hostile towards simple, efficient, and correct
implementations?
I fully agree with this.
Isn't E_RECOVERABLE_ERROR enough for random_*()?
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Thu, Jul 16, 2015 at 4:58 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: I understand many of us dislike INI switch nor runtime switch that changes behavior. However, it's impossible to move to exception at once because we have no control for users' code including 3rd party modules. Well, ini settiing remains ugly. Code will have to take care of it and this is not going to be a good thing. IMHO. Rather than adding exception adopted functions gradually, having the switch is better for in the long run. We may deprecate/remove the switch 10 years later or more. BTW, I'm thinking to have ModulenameException/ModulenameFunctionException or like rather than simply calling/converting errors to ErrorException when exception is enabled for functions. I am not sure I like this idea. I would prefer to define namespace and rely on standard names (when possible) and per extension namespace instead. I also agree to actually deal with exceptions globally at once instead of incremental additions in more or less random places, functions or methods. Cheers, -- Pierre @pierrejoye | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Rowan, On Wed, Jul 15, 2015 at 9:00 PM, Rowan Collins rowan.coll...@gmail.com wrote: If we are going to adopt exception for functions, it would be better to have switch that convert all errors to exceptions. Strongly disagree. A runtime switch would be a horrible consistency nightmare for userland, and many of the existing functions which desperately need better error-handling (e.g. file I/O) don't issue an error at the moment anyway. The only reason (and it's a massive reason, don't get me wrong) not to start throwing exceptions all over the core is backwards compatibility. New functions do not, by definition, have anything to be backwards compatible with. So my suggestion is, to come up with some reasonable definition of new area of functionality where we can do things in a way which everyone seems to agree is basically better, i.e. throw Exceptions. - The rule shouldn't apply for new variations or close cousins of existing functions (e.g. the json_decode_to() that was briefly discussed should use similar conventions to json_decode() if it were implemented), only in cases like this where we're adding a new set of functions (the manual puts random_bytes and random_int in their own book: http://php.net/manual/en/book.csprng.php). - It also shouldn't be used for routine errors (like File Not Found), only in place of conditions which can justify a fatal error - both for the intuitive reason that an uncaught exception *is* a fatal error, and the procedural reason that we may want to decide a policy for routine error handling at a later date. At some point, we should revisit the general error-handling strategy of both the language and the core library, but 7.0 having been somewhat derailed (in my opinion) by the endless debates about scalar type hints, it will have to wait until 8.0 before we can do anything radical to existing functions. I prefer exception also. I agree with your discussion in general. The only difference is how PHP module functions should adopt exceptions. When PHP adopted naming standard, we didn't enforce new naming standard for all functions. As a result, we have module_func_name and modulefuncname still. The same thing will happen for exception/error. I would like to avoid such situation (at least for modules provided with PHP). i.e. One function raises exception, another raises error. I understand many of us dislike INI switch nor runtime switch that changes behavior. However, it's impossible to move to exception at once because we have no control for users' code including 3rd party modules. IMHO. Rather than adding exception adopted functions gradually, having the switch is better for in the long run. We may deprecate/remove the switch 10 years later or more. BTW, I'm thinking to have ModulenameException/ModulenameFunctionException or like rather than simply calling/converting errors to ErrorException when exception is enabled for functions. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] Core functions throwing exceptions in PHP7
Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Would a PHP Error not work in this case? Or would the error then be interpreted as the result? On 14 Jul 2015, at 23:04, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
On Tue, Jul 14, 2015 at 5:10 PM, Dean Eigenmann dean.eigenm...@icloud.com wrote: Would a PHP Error not work in this case? Or would the error then be interpreted as the result? On 14 Jul 2015, at 23:04, Sammy Kaye Powers m...@sammyk.me wrote: Hello lovely PHP nerds, There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: https://github.com/php/php-src/pull/1397 (main discussion) https://github.com/php/php-src/pull/1398 Currently the random_*() functions will issue a warning and return false if a good source of random cannot be found. This is a potential security hole in the event the RNG fails and returns false which gets evaluated as 0 in a cryptographic context. To prevent this exploit the proposed behavior will throw an Exception when the RNG fails or certain argument validation fails. This also gives the developer a graceful way to fall back to an alternate CSPRNG. Since the core functions in PHP don't throw Exceptions, there is debate on whether or not this change should be implemented. Some say the CSPRNG's should get a special pass since they will be relied on for cryptography. If we can't throw Exceptions, there were suggestions of raising a fatal error if the RNG fails. I think the argument can be boiled down to consistency vs security. We'd love to hear your feedback to decide what we should do in this context. :) Thanks, Sammy Kaye Powers sammyk.me Chicago, IL 60604 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php How can a developer gracefully and simply handle a fatal error? Recommending that developers mess with set_error_handler() for one very specific function seems like a bad design choice in my opinion. An uncaught exception should kill the script, a caught exception can let the developer decide how to proceed. I'd vote in favor of Exceptions (which, by default, will fail closed but can be handled gracefully through try-catch blocks) rather than E_WARNING (which fails open) or E_ERROR (which doesn't allow for graceful handling). Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
