>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.