>On 12/18/2014 06:29 PM, Theo de Raadt wrote:
>>> On 12/18/2014 12:14 AM, Theo de Raadt wrote:
>>>>> getentropy*.c: "cannot call abort() because some systems have unsafe
>>>>> corefiles"
>>>>> arc4random.c: "if(_rs_allocate(...) == -1) abort();"
>>>>>
>>>>> Am I missing any difference between the two cases?
>>>>> (brain,cvsweb,google+gmane weren't any help)
>>>>
>>>> Policy.
>>>>
>>>> getentropy() should eventually be a lower level intrinsic.  It is an
>>>> emulation of a system call on another system (ie. OpenBSD).
>>>>
>>>> arc4random() is written on top of this, it is a standard library
>>>> routine.
>>>>
>>>> For example.  Say you wrote a replacement library that had open() and
>>>> fopen().  The open should not fatally abort -- it's job is to return
>>>> errors.  That is firmly specified -- go read the open() manual page to
>>>> get the picture.  But the upper level function, what it does is less
>>>> refined and more flexible implementation wise.
>>>>
>>> Agreed - I spent too much time trying to figure it out and too little
>>> writing a clear question, sorry, let me retry :)
>>>
>>> Getentropy has no "internal" errors - if it fails  it's the caller's
>>> fault, otherwise it works 100% of the time, period.
>>> Arc4random is designed to always work, and does so by being based on a
>>> function that never fails; cvsweb mentions a dumb user could make it
>>> (actually sysctl back then) fail with systrace, which used to be
>>> ignored, now results in _getentropy_fail(), ie the system's equivalent
>>> of a SIGKILL(*).
>>>
>>> So arc4random (used for ) always works, unless:
>>> 1. getentropy fails => _getentropy_fail()
>>> 2. mmap fails => abort() => "some systems have unsafe corefiles"
>>> (I know #2 is unlikely to fail since it's a small amount of memory;
>>> assume Murphy's law)
>>>
>>> Given that arc4random is the base for anything random in libressl, how
>>> is abort potentially unsafe for #2 but not for #1? If someone can peek
>>> at the dump they won't see the keystream but any data belonging to the
>>> application will still be there - or not?
>>
>> I explained that.
>>
>Uh? Either we are misunderstanding each other or I'm missing something 
>really really stupid here.
>
>= TLDR =
>
>*in arc4random.c*, why is it not ok to abort() when getentropy() fails 
>but it's ok if _rs_allocate() fails?
>
>
>= Painfully detailed =
>
>Your answer boils down to: getentropy, real or emulated, is a system 
>call, so it can never ever abort; arc4random is a function, if it finds 
>it convenient to abort, it will abort.
>
>That's a perfectly fine answer to a question like "what's wrong with 
>abort() in getentropy_*.c?" - if the problem is that my question sounded 
>like that even the 2nd time, sorry!
>
>But the comment doesn't say "because aborting __in_getentropy__ is an 
>awful idea" (policy), it says it can't abort "because some systems have 
>unsafe corefiles": that is a property of bad systems independent of who 
>is calling abort, and its logical consequence should be "just in case, 
>never abort()".
>Looking at the rest of the code, abort() is only called in verify(1) and 
>as a result of failed assertions (OPENSSL_assert(), or the "default:" 
>case in some switch statements where it's not supposed to happen, which 
>is morally an assertion).
>
>
>In case I'm missing something really obvious, sorry again.
>
>
>
>From lory.fu...@infinito.it Thu Dec 18 15:19:07 2014
>Delivered-To: dera...@cvs.openbsd.org
>X-Virus-Scanned: amavisd-new at milter-3
>X-Spam-Flag: NO
>X-Spam-Score: -0.999
>X-Spam-Level: 
>X-Spam-Status: No, score=-0.999 tagged_above=-1 required=5
>       tests=[ALL_TRUSTED=-1, FREEMAIL_FROM=0.001] autolearn=no
>Date: Thu, 18 Dec 2014 23:18:58 +0100
>From: Lorenzo <lory.fu...@infinito.it>
>Reply-To: lory.fu...@infinito.it
>User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 
>Icedove/31.3.0
>MIME-Version: 1.0
>To: Theo de Raadt <dera...@cvs.openbsd.org>
>CC: tech@openbsd.org
>Subject: Re: [nitpicking] abort in arc4random?
>References: <201412181729.sbihtcva034...@mx-2.infinito.ipnext.it>
>In-Reply-To: <201412181729.sbihtcva034...@mx-2.infinito.ipnext.it>
>Content-Type: text/plain; charset=utf-8; format=flowed
>Content-Transfer-Encoding: 7bit
>
>On 12/18/2014 06:29 PM, Theo de Raadt wrote:
>>> On 12/18/2014 12:14 AM, Theo de Raadt wrote:
>>>>> getentropy*.c: "cannot call abort() because some systems have unsafe
>>>>> corefiles"
>>>>> arc4random.c: "if(_rs_allocate(...) == -1) abort();"
>>>>>
>>>>> Am I missing any difference between the two cases?
>>>>> (brain,cvsweb,google+gmane weren't any help)
>>>>
>>>> Policy.
>>>>
>>>> getentropy() should eventually be a lower level intrinsic.  It is an
>>>> emulation of a system call on another system (ie. OpenBSD).
>>>>
>>>> arc4random() is written on top of this, it is a standard library
>>>> routine.
>>>>
>>>> For example.  Say you wrote a replacement library that had open() and
>>>> fopen().  The open should not fatally abort -- it's job is to return
>>>> errors.  That is firmly specified -- go read the open() manual page to
>>>> get the picture.  But the upper level function, what it does is less
>>>> refined and more flexible implementation wise.
>>>>
>>> Agreed - I spent too much time trying to figure it out and too little
>>> writing a clear question, sorry, let me retry :)
>>>
>>> Getentropy has no "internal" errors - if it fails  it's the caller's
>>> fault, otherwise it works 100% of the time, period.
>>> Arc4random is designed to always work, and does so by being based on a
>>> function that never fails; cvsweb mentions a dumb user could make it
>>> (actually sysctl back then) fail with systrace, which used to be
>>> ignored, now results in _getentropy_fail(), ie the system's equivalent
>>> of a SIGKILL(*).
>>>
>>> So arc4random (used for ) always works, unless:
>>> 1. getentropy fails => _getentropy_fail()
>>> 2. mmap fails => abort() => "some systems have unsafe corefiles"
>>> (I know #2 is unlikely to fail since it's a small amount of memory;
>>> assume Murphy's law)
>>>
>>> Given that arc4random is the base for anything random in libressl, how
>>> is abort potentially unsafe for #2 but not for #1? If someone can peek
>>> at the dump they won't see the keystream but any data belonging to the
>>> application will still be there - or not?
>>
>> I explained that.
>>
>Uh? Either we are misunderstanding each other or I'm missing something 
>really really stupid here.
>
>= TLDR =
>
>*in arc4random.c*, why is it not ok to abort() when getentropy() fails 
>but it's ok if _rs_allocate() fails?
>
>= Painfully detailed =
>
>Your answer boils down to: getentropy, real or emulated, is a system 
>call, so it can never ever abort; arc4random is a function, if it finds 
>it convenient to abort, it will abort.
>
>That's a perfectly fine answer to a question like "what's wrong with 
>abort() in getentropy_*.c?" - if the problem is that my question sounded 
>like that even the 2nd time, sorry!
>
>But the comment doesn't say "because aborting __in_getentropy__ is an 
>awful idea" (policy), it says it can't abort "because some systems have 
>unsafe corefiles": that is a property of bad systems independent of who 
>is calling abort, and its logical consequence should be "just in case, 
>never abort()".
>Looking at the rest of the code, abort() is only called in verify(1) and 
>as a result of failed assertions (OPENSSL_assert(), or the "default:" 
>case in some switch statements where it's not supposed to happen, which 
>is morally an assertion).

The comment says, AS A WHOLE:

        /*
         * Entropy collection via /dev/urandom and sysctl have failed.
         *
         * No other API exists for collecting entropy.  See the large
         * comment block above.
         *
         * We have very few options:
         *     - Even syslog_r is unsafe to call at this low level, so
         *       there is no way to alert the user or program.
         *     - Cannot call abort() because some systems have unsafe
         *       corefiles.
         *     - Could raise(SIGKILL) resulting in silent program termination.
         *     - Return EIO, to hint that arc4random's stir function
         *       should raise(SIGKILL)
         *     - Do the best under the circumstances....
         *
         * This code path exists to bring light to the issue that Linux
         * does not provide a failsafe API for entropy collection.
         *
         * We hope this demonstrates that Linux should either retain their
         * sysctl ABI, or consider providing a new failsafe API which
         * works in a chroot or when file descriptors are exhausted.
         */

It is a list of reasons for why this API is designed like this.  You
are nitpicking about a comment which does not stand alone.

Reply via email to