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
