You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Bojan Smojver <bo...@rexursive.com> on 2008/04/28 08:34:19 UTC

PR #44881

Does anyone mind if I apply the fix mentioned in the bug report:

https://issues.apache.org/bugzilla/show_bug.cgi?id=44881

-- 
Bojan


Re: PR #44881

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Joe Orton wrote:
> 
> Given the lack of such a guarantee, nobody would presume the data is 
> suitable for cryptographic use, e.g. private keys.  So I think it's 
> right to make it fast at the expense of strength, and it should prefer 
> /dev/urandom over /dev/random.  (In Fedora we've been building APR to 
> use /dev/urandom forever, FWIW)

Then I suggest we change the defaults, if this is what people (probably
other distributions as well) use in *practice*.

> I proposed a new API something like Lucian describes above, way back 
> when: http://markmail.org/message/f7on762ulztbmocr

:)

This _ex() api can certainly still be added to 1.4 and given some clues
to pick from one of several sources.

> In retrospect, I don't think it's a good idea for APR to venture further 
> into this domain without a thorough review of what different randomness 
> sources are available on different OSes, what are the common 
> denominators, etc. The previous effort at providing something more 
> general here is completely unused (apr/random) and been a waste of space 
> AFAICT.

Well, some general level of randomness is necessary, it's certainly one of
many portability problems to be solved.

Given that in reality OS distributors have changed the preference in order
to attain non-blocking behavior, and this is not used by us for crypto,
let's move forward with the urandom patch, eh?

Bill

Re: PR #44881

Posted by Graham Leggett <mi...@sharp.fm>.
Joe Orton wrote:

> In retrospect, I don't think it's a good idea for APR to venture further 
> into this domain without a thorough review of what different randomness 
> sources are available on different OSes, what are the common 
> denominators, etc. The previous effort at providing something more 
> general here is completely unused (apr/random) and been a waste of space 
> AFAICT.

Tomcat advertises itself as offering "Secure session ID generation by 
default on all platforms (platforms other than Linux required random 
number generation using a configured entropy)" when APR is enabled 
within Tomcat.

They don't mention exactly which part of APR they are using to do this, 
but if it is apr/random, then it is being used.

What would be useful is a function which would return true if random 
number generation is crypto safe on that platform. At the very least, 
the user of the library gets no surprises as to the quality of the 
numbers they get.

Regards,
Graham
--

Re: PR #44881

Posted by Joe Orton <jo...@redhat.com>.
On Mon, Apr 28, 2008 at 06:48:43PM +0300, Lucian Adrian Grijincu wrote:
> Wouldn't adding a new function be more suitable?
> 
> http://apr.apache.org/docs/apr/1.2/group__apr__random.html:
> apr_generate_random_bytes says it will "Generate random bytes". This
> says nothing about the "pseudo-" vs. "true-" randomness of the
> generated array.
> 
> apr_generate_random_bytes_ex with an extra "flags" field seems a better way:
> * APR_RANDOM_TRUE - a true random source, return an error if no true
> random source is found on the system.
> * APR_RANDOM_PSEUDO - a pseudo

This is a complicated subject, and I'm not an expert, but...

The Linux /dev/random vs /dev/urandom distinction is not the same as 
"true random" vs "pseudo-random".

Linux /dev/random provides pseudo-random data with a guarantee of high 
entropy; that's not the same thing as "true" randomness.  The closest we 
can get to "true" randomness is the use of a hardware RNG, which draws 
from some physical source of entropy like thermal noise.  Such RNGs are 
exposed directly on some platforms, but hardware is not that common.

Currently apr_generate_random_bytes() provides no API guarantee on the 
"strength" (level of entropy) of the random data returned; nor any 
guarantee on whether it blocks.

Given the lack of such a guarantee, nobody would presume the data is 
suitable for cryptographic use, e.g. private keys.  So I think it's 
right to make it fast at the expense of strength, and it should prefer 
/dev/urandom over /dev/random.  (In Fedora we've been building APR to 
use /dev/urandom forever, FWIW)

I proposed a new API something like Lucian describes above, way back 
when: http://markmail.org/message/f7on762ulztbmocr

In retrospect, I don't think it's a good idea for APR to venture further 
into this domain without a thorough review of what different randomness 
sources are available on different OSes, what are the common 
denominators, etc. The previous effort at providing something more 
general here is completely unused (apr/random) and been a waste of space 
AFAICT.

joe

Re: PR #44881

Posted by Lucian Adrian Grijincu <lu...@gmail.com>.
On Tue, Apr 29, 2008 at 1:33 AM, William A. Rowe, Jr.
<wr...@rowe-clan.net> wrote:
> Lucian Adrian Grijincu wrote:
> > Wouldn't adding a new function be more suitable?
seems somebody already did all the work and committed it to trunk :)
http://apr.apache.org/docs/apr/trunk/group__apr__random.html

>  Interesting thought, keep in mind the other half of the issue is the number
>  of times we consume generate_random_bytes ourselves from other functions,
>  you'll have to suggest which should be pseudo, which should be truly random
>  and which should be configurable.
find . | xargs grep apr_generate_
tells (beside tests) only of apr-util/crypto/getuuid.c and I couldn't
see where a *true* random number was needed.
These things considered, for 1.2.x DEV_RANDOM can be defined to a
lower quality random number generator
if one is available.
-    for f in /dev/arandom /dev/random /dev/urandom; do
+    for f in /dev/arandom /dev/urandom /dev/random; do
both arandom and urandom are pseudo-random number generators and
should be tested first.


a bit of ugliness I stumbled upon:
http://svn.apache.org/viewvc/apr/apr-util/trunk/crypto/getuuid.c?view=markup

/* true_random -- generate a crypto-quality random number. */
static int true_random(void)
{
    apr_uint64_t time_now;

#if APR_HAS_RANDOM
    unsigned char buf[2];

    if (apr_generate_random_bytes(buf, 2) == APR_SUCCESS) {
        return (buf[0] << 8) | buf[1];
    }
#endif

    /* crap. this isn't crypto quality, but it will be Good Enough */

    time_now = apr_time_now();
    srand((unsigned int)(((time_now >> 32) ^ time_now) & 0xffffffff));

    return rand() & 0x0FFFF;
}


the true_random function, which should "generate a crypto-quality random number"
falls back on "rand()"
  man 3 rand
  The  rand()  function  returns  a  pseudo-random  integer ...

The code that uses `true_random' doesn't seem to need a crypto-quality
random number.
Shouldn't this be properly renamed as "get_pseudo_random_number()" or
something like it.

-- 
Lucian

Re: PR #44881

Posted by Bojan Smojver <bo...@rexursive.com>.
On Thu, 2008-05-01 at 18:05 -0500, William A. Rowe, Jr. wrote:

> So no, I would not change the manner that UUID's are generated to urandom.
> generate_random_bytes is defined to provide the greatest entropy we can
> obtain.  It is not, after all, generate_psuedorandom_bytes.

OK, thanks. I'll update the bug report.

-- 
Bojan


Re: PR #44881

Posted by Bojan Smojver <bo...@rexursive.com>.
On Fri, 2008-05-02 at 02:56 +0300, Lucian Adrian Grijincu wrote:

> 1. Some function in APR's uuid generator falls back to rand(3) if
> apr_generate_random_bytes returns an error. ...
> 2. E2fsprogs on which other major open source UUID generators are
> supposed to be based on (at least according to
> http://en.wikipedia.org/wiki/UUID) tries to open /dev/urandom first
> and /dev/random second. I don't know if posting the code snniplet here
> is apropiate (licensing reasons).

I was also under the impression that /dev/urandom is good enough for
most stuff and is generally used for things like this these days
(especially because /dev/random blocks). But, I'm not a crypto guy, so
I'll defer all that to someone that has better knowledge of the issues
involved.

-- 
Bojan


Re: PR #44881

Posted by Lucian Adrian Grijincu <lu...@gmail.com>.
On Fri, May 2, 2008 at 2:05 AM, William A. Rowe, Jr.
<wr...@rowe-clan.net> wrote:
>
>  As it turns out only UUID code is affected (on platforms which have no
>  native uuid generation function).  Note that predicting the next UUID is
>  a serious flaw when they are used as session identifiers, etc, and that the
>  native implementations all switched to strong crypto once this potential
>  flaw was identified.
>
>  So no, I would not change the manner that UUID's are generated to urandom.
>  generate_random_bytes is defined to provide the greatest entropy we can
>  obtain.  It is not, after all, generate_psuedorandom_bytes.

1. Some function in APR's uuid generator falls back to rand(3) if
apr_generate_random_bytes returns an error. ...
2. E2fsprogs on which other major open source UUID generators are
supposed to be based on (at least according to
http://en.wikipedia.org/wiki/UUID) tries to open /dev/urandom first
and /dev/random second. I don't know if posting the code snniplet here
is apropiate (licensing reasons).

-- 
Lucian

Re: PR #44881

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Bojan Smojver wrote:
> On Mon, 2008-04-28 at 17:33 -0500, William A. Rowe, Jr. wrote:
> 
>> Interesting thought, keep in mind the other half of the issue is the number
>> of times we consume generate_random_bytes ourselves from other functions,
>> you'll have to suggest which should be pseudo, which should be truly random
>> and which should be configurable.
> 
> Bill, are we doing something about this before 1.3.0 gets out?

As it turns out only UUID code is affected (on platforms which have no
native uuid generation function).  Note that predicting the next UUID is
a serious flaw when they are used as session identifiers, etc, and that the
native implementations all switched to strong crypto once this potential
flaw was identified.

So no, I would not change the manner that UUID's are generated to urandom.
generate_random_bytes is defined to provide the greatest entropy we can
obtain.  It is not, after all, generate_psuedorandom_bytes.

I think the _ex() flavor offering two alternatives plus blocking is a very
exciting new feature, but given that this only came up in the past week, and
that people have been waiting much longer for 1.3.0's new features, I just
don't see the reason to hold this up any longer.

If we want a shorter window to 1.4.0, we have to stop dragging out revision
bumps this long, eh?  Which means to move forward with 1.3.0 now and perhaps
we'll be no further than 1.3.3 when we are ready with the ssl, evp, and our
other substantial 1.4.0 enhancements.

Bill


Re: PR #44881

Posted by Bojan Smojver <bo...@rexursive.com>.
On Mon, 2008-04-28 at 17:33 -0500, William A. Rowe, Jr. wrote:

> Interesting thought, keep in mind the other half of the issue is the number
> of times we consume generate_random_bytes ourselves from other functions,
> you'll have to suggest which should be pseudo, which should be truly random
> and which should be configurable.

Bill, are we doing something about this before 1.3.0 gets out?

-- 
Bojan


Re: PR #44881

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Lucian Adrian Grijincu wrote:
> Wouldn't adding a new function be more suitable?
> 
> http://apr.apache.org/docs/apr/1.2/group__apr__random.html:
> apr_generate_random_bytes says it will "Generate random bytes". This
> says nothing about the "pseudo-" vs. "true-" randomness of the
> generated array.
> 
> apr_generate_random_bytes_ex with an extra "flags" field seems a better way:
> * APR_RANDOM_TRUE - a true random source, return an error if no true
> random source is found on the system.
> * APR_RANDOM_PSEUDO - a pseudo
> * we could provide a O_NONBLOCK like flag: if specified when using
> APR_RANDOM_TRUE and the source does not have enough bits we can return
> an error.

Interesting thought, keep in mind the other half of the issue is the number
of times we consume generate_random_bytes ourselves from other functions,
you'll have to suggest which should be pseudo, which should be truly random
and which should be configurable.

Re: PR #44881

Posted by Lucian Adrian Grijincu <lu...@gmail.com>.
Wouldn't adding a new function be more suitable?

http://apr.apache.org/docs/apr/1.2/group__apr__random.html:
apr_generate_random_bytes says it will "Generate random bytes". This
says nothing about the "pseudo-" vs. "true-" randomness of the
generated array.

apr_generate_random_bytes_ex with an extra "flags" field seems a better way:
* APR_RANDOM_TRUE - a true random source, return an error if no true
random source is found on the system.
* APR_RANDOM_PSEUDO - a pseudo
* we could provide a O_NONBLOCK like flag: if specified when using
APR_RANDOM_TRUE and the source does not have enough bits we can return
an error.



>  --- Comment #1 from Bojan Smojver <bo...@rexursive.com>  2008-04-27
> 23:29:28 PST ---
>  Suggested fix:
>
>  Index: configure.in
>  ===================================================================
>  --- configure.in        (revision 651703)
>  +++ configure.in        (working copy)
>  @@ -1955,7 +1955,7 @@
>    if test "$apr_devrandom" = "yes"; then
>      # /dev/random on OpenBSD doesn't provide random data, so
>      # prefer /dev/arandom, which does; see random(4).
>  -    for f in /dev/arandom /dev/random /dev/urandom; do
>  +    for f in /dev/arandom /dev/urandom /dev/random; do
>        if test -r $f; then
>          apr_devrandom=$f
>          rand=1
>

-- 
Lucian

Re: PR #44881

Posted by Bojan Smojver <bo...@rexursive.com>.
On Mon, 2008-04-28 at 10:12 -0500, William A. Rowe, Jr. wrote:

> It's a mailing list, please don't make people chase down a link (remember
> some of us read our email detached from the web).  Citing the patch and
> bug's subject is a bare minimum :)

OOPS, sorry :-(

BTW, the same thing was done in Tomcat's Java code, long, long time ago,
because /dev/random would block in exactly the same way.

-- 
Bojan


Re: PR #44881

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Bojan Smojver wrote:
> Does anyone mind if I apply the fix mentioned in the bug report:
> 
> https://issues.apache.org/bugzilla/show_bug.cgi?id=44881

It's a mailing list, please don't make people chase down a link (remember
some of us read our email detached from the web).  Citing the patch and
bug's subject is a bare minimum :)

I'd like to hear from Joe and Ben especially (and Ralf if he's around)
on this topic.

Bill