https://redmine.lighttpd.net/https://redmine.lighttpd.net/favicon.ico?13667327412022-01-02T09:59:40Zlighty labsLighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=129532022-01-02T09:59:40Zdevnexen
<ul><li><strong>File</strong> <a href="/attachments/2151">0001-rand-macOs-handling-update.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/2151/0001-rand-macOs-handling-update.patch">0001-rand-macOs-handling-update.patch</a> added</li></ul> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=129542022-01-02T10:34:00Zgstrauss
<ul><li><strong>Category</strong> set to <i>core</i></li><li><strong>Priority</strong> changed from <i>Normal</i> to <i>Low</i></li></ul><p>Citations and references, please.</p>
<p>If lighttpd is linked against any crypto library, you can see that lighttpd prefers interfaces from those crypto libraries to obtain random bytes.</p>
<p>lighttpd uses random bytes in a very small number of places (besides the TLS modules), and if lighttpd is not built against any crypto libraries or with the Nettle library, then the remaining algorithms shipped with lighttpd for historical compatibility are weak (MD5 and SHA1).</p>
<p>I agree with the part of the patch which adds some missing defines to config.h.cmake, but please provide some justification about the benefits of adding your proposed code to src/rand.c. Improving arc4random is not much of an improvement if you were reaching the point in the code which falls back to use arc4random.</p> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=129552022-01-02T10:49:28Zstbuehler
<ul></ul><p>I think it falls back to <code>random()</code>...</p>
<p>The justification should include what the failure conditions of <code>CCRandomGenerateBytes</code> are - does it just return an error if it thinks the entropy was bad, but generated output anyway? Because falling back to <code>random()</code> sounds worse in that case.</p> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=129562022-01-02T11:04:31Zdevnexen
<ul></ul><p>Fair enough</p>
<p>[[<a class="external" href="https://opensource.apple.com/source/Libc/Libc-1439.40.11/gen/FreeBSD/arc4random.c.auto.html">https://opensource.apple.com/source/Libc/Libc-1439.40.11/gen/FreeBSD/arc4random.c.auto.html</a>]]</p>
<p>[[<a class="external" href="https://opensource.apple.com/source/xnu/xnu-2050.18.24/EXTERNAL_HEADERS/corecrypto/ccrng.h.auto.html">https://opensource.apple.com/source/xnu/xnu-2050.18.24/EXTERNAL_HEADERS/corecrypto/ccrng.h.auto.html</a>]]</p>
<p>here what CCRandomGenerateBytes can possibly returns</p>
<p>[[<a class="external" href="https://opensource.apple.com/source/CommonCrypto/CommonCrypto-60074/include/CommonCryptoError.h.auto.html">https://opensource.apple.com/source/CommonCrypto/CommonCrypto-60074/include/CommonCryptoError.h.auto.html</a>]]</p> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=129572022-01-02T11:08:44Zdevnexen
<ul></ul><p>pb with arc4random you cannot know if it failed. otherwise we can always open /dev/urandom but might be heavyweight for this part of the code I think.</p> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=129582022-01-02T12:36:48Zstbuehler
<ul></ul><p><code>CCRandomGenerateBytes()</code> seems to be based on <code>ccrng</code> (which is also used by the <code>arc4random</code> code): <a class="external" href="https://www.niap-ccevs.org/Documents_and_Guidance/view_td.cfm?TD=0510">https://www.niap-ccevs.org/Documents_and_Guidance/view_td.cfm?TD=0510</a>.</p>
<p>It seems <code>ccrng</code> is basically a function pointer: <code>int (*generate)(struct ccrng_state *rng, unsigned long outlen, void *out)</code>; although it returns an int the return value is unspecified in the header; can't blame anyone for not handling it - hopefully the function can't fail in the first place.</p>
<p>I think the only error <code>CCRandomGenerateBytes()</code> can reasonably return is <code>kCCMemoryFailure</code> - when the initial allocation of <code>ccrng()</code> fails (<code>arc4_init()</code> simply crashes in this case).</p>
<p>So... unless we care <code>arc4random</code> crashes the process on OOM I don't think there is anything to worry about; this crash should only happen in the initial call to <code>li_rand_init()</code>, not after <code>fork()</code>.</p>
<p>(Also I wonder whether a workaround should use the underlying <code>ccrng</code> interface directly instead of <code>CCRandomGenerateBytes</code>)</p> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=129592022-01-02T12:52:56Zdevnexen
<ul></ul><p>this function pointer returns 0 on success, now the vast majority of time it does not fail indeed and the buffers at play here are fairly small as well.</p>
<p>However if only the cmake fixes part is committed that would be already something, I will be fine if this is not entirely accepted.</p>
<p>About your last remark, I m not 100% sure if this is necessarily not risky (for ex they might deprecate it for a better api one day without notice as the ccrng api is found buggy or not up to the standards, as this api is not public exposed one (they are in private headers) generally it s for a reasonable intent).</p> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=129612022-01-02T21:13:38Zgstrauss
<ul></ul><p>I consider the following <strong>untested</strong> patch to be better encapsulated and easier to read. Only because it is encapsulated will I consider it as a marginal improvement over <code>arc4random()</code>, since the patch checks if <code>CCRandomGenerateBytes()</code> fails, which should not happen on a functioning system. With this patch, on systems with <code>CCRandomGenerateBytes()</code>, HAVE_ARC4RANDOM_BUF is manually undefined with <code>#undef</code> since <code>arc4random()</code> on those systems uses (the equivalent of) <code>CCRandomGenerateBytes()</code> under the hood, but without checking the function return status.<br /><pre>
--- a/src/rand.c
+++ b/src/rand.c
@@ -81,6 +81,13 @@
#ifdef RNDGETENTCNT
#include <sys/ioctl.h>
#endif
+#if defined(__APPLE__) && defined(__MACH__)
+#if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 101000 /* OS X 10.10+ */
+#undef HAVE_ARC4RANDOM_BUF
+#define HAVE_CCRANDOMGENERATEBYTES
+#include <CommonCrypto/CommonRandom.h>
+#endif
+#endif
/* Take some reasonable steps to attempt to *seed* random number generators with
* cryptographically random data. Some of these initialization routines may
@@ -239,6 +246,11 @@ static void li_rand_init (void)
if (1 == li_rand_device_bytes((unsigned char *)xsubi, (int)sizeof(xsubi))) {
u = ((unsigned int)xsubi[0] << 16) | xsubi[1];
}
+ #ifdef HAVE_CCRANDOMGENERATEBYTES
+ else if (CCRandomGenerateBytes(xsubi, sizeof(xsubi)) == kCCSuccess
+ && CCRandomGenerateBytes(&u, sizeof(u)) == kCCSuccess) {
+ }
+ #endif
else {
#ifdef HAVE_ARC4RANDOM_BUF
u = arc4random();
@@ -373,6 +385,11 @@ int li_rand_pseudo (void)
if (SECSuccess == PK11_GenerateRandom((unsigned char *)&i, sizeof(i)))
return i;
#endif
+ #ifdef HAVE_CCRANDOMGENERATEBYTES
+ int i;
+ if (CCRandomGenerateBytes(&i, sizeof(i)) == kCCSuccess)
+ return i;
+ #endif
#ifdef HAVE_ARC4RANDOM_BUF
return (int)arc4random();
#elif defined(__COVERITY__)
</pre></p>
<p><a class="external" href="https://opensource.apple.com/source/CommonCrypto/CommonCrypto-60074/include/CommonCryptoError.h.auto.html">https://opensource.apple.com/source/CommonCrypto/CommonCrypto-60074/include/CommonCryptoError.h.auto.html</a><br /><a class="external" href="https://opensource.apple.com/source/CommonCrypto/CommonCrypto-60092.50.5/include/CommonRandom.h.auto.html">https://opensource.apple.com/source/CommonCrypto/CommonCrypto-60092.50.5/include/CommonRandom.h.auto.html</a><br /><a class="external" href="https://opensource.apple.com/source/CommonCrypto/CommonCrypto-60092.50.5/lib/CommonRandom.c.auto.html">https://opensource.apple.com/source/CommonCrypto/CommonCrypto-60092.50.5/lib/CommonRandom.c.auto.html</a><br /><a class="external" href="https://opensource.apple.com/source/Libc/Libc-1439.40.11/gen/FreeBSD/arc4random.c.auto.html">https://opensource.apple.com/source/Libc/Libc-1439.40.11/gen/FreeBSD/arc4random.c.auto.html</a><br /><a class="external" href="https://opensource.apple.com/source/xnu/xnu-2050.18.24/EXTERNAL_HEADERS/corecrypto/ccrng.h.auto.html">https://opensource.apple.com/source/xnu/xnu-2050.18.24/EXTERNAL_HEADERS/corecrypto/ccrng.h.auto.html</a><br /><a class="external" href="https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/random.3.html">https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/random.3.html</a><br /><a class="external" href="https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/arc4random.3.html">https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/arc4random.3.html</a></p>
<hr />
<p>The above patch does not address missing defines in src/config.h.cmake. That should be a separate patch.</p> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=129622022-01-02T23:09:12Zdevnexen
<ul></ul><p>your patch works for me.</p> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=129632022-01-03T07:25:06Zgstrauss
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Fixed</i></li></ul><p>Applied in changeset <a class="changeset" title="[core] CCRandomGenerateBytes() for rand on macOS (fixes #3129) (thx devnexen) x-ref: "rand ma..." href="https://redmine.lighttpd.net/projects/lighttpd/repository/14/revisions/1334dd4ad52f8685948066af4798446af6b3e2da">1334dd4ad52f8685948066af4798446af6b3e2da</a>.</p> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=129642022-01-03T07:27:08Zgstrauss
<ul><li><strong>Target version</strong> changed from <i>1.4.xx</i> to <i>1.4.64</i></li></ul> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=130662022-01-21T18:52:36Zgstrauss
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-5 priority-4 priority-default closed" href="/issues/3140">Bug #3140</a>: lighttpd build failure on macOS 10.13 and earlier</i> added</li></ul> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=130682022-01-21T18:58:24Zdevnexen
<ul></ul><p>Adding this line</p>
<pre><code class="c syntaxhl" data-language="c"><span class="cp">#include</span> <span class="cpf"><CommonCrypto/CommonCryptoError.h></span><span class="cp">
</span></code></pre>
<p>on top of</p>
<pre><code class="c syntaxhl" data-language="c"><span class="cp">#include</span> <span class="cpf"><CommonCrypto/CommonRandom.h></span><span class="cp">
</span></code></pre>
<p>should solve it. not needed in later releases of macOs.</p> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=130692022-01-21T18:58:54Zdevnexen
<ul></ul><p>work on all releases at least supporting this api.</p> Lighttpd - Feature #3129: rand macOs case handling updatehttps://redmine.lighttpd.net/issues/3129?journal_id=130702022-01-21T19:00:35Zdevnexen
<ul></ul><p>that s how I solved it in openssl back then <a class="external" href="https://github.com/openssl/openssl/commit/c023d98dcf2ba1cc30f545ae54d0e037e80a8794#diff-a07c97f62a965dd59eb44daa4891efa6304d32ae1e3bf1a9f901caf10645072f">https://github.com/openssl/openssl/commit/c023d98dcf2ba1cc30f545ae54d0e037e80a8794#diff-a07c97f62a965dd59eb44daa4891efa6304d32ae1e3bf1a9f901caf10645072f</a></p>
<p>I forgot about it.</p>