Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Received-SPF: neutral (asf.osuosl.org: local policy)
Message-ID: <443FD924.7030904@ukf.net>
Date: Fri, 14 Apr 2006 18:17:24 +0100
From: Max Bowsher <maxb1@ukf.net>
Reply-To: dev@apr.apache.org
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
 rv:1.7.12) Gecko/20050923 Thunderbird/1.0.7
MIME-Version: 1.0
To: rse@engelschall.com
CC: dev@apr.apache.org
Subject: Re: APR-util UUID generator broken
References: <20060414082149.GA78261@engelschall.com>
 <443F75F2.9040502@ukf.net> <20060414111128.GA11785@engelschall.com>
In-Reply-To: <20060414111128.GA11785@engelschall.com>
OpenPGP: id=C0F2C580
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ralf S. Engelschall wrote:
> On Fri, Apr 14, 2006, Max Bowsher wrote:
> 
>> [...]
>>> ...I've reviewed the UUID generator in APR-util. It unfortunately is
>>> totally broken and generates neither valid (format) nor useful (content)
>>> RFC4122 UUIDs.
>> REMOVING CC to dev@subversion, as this is entirely an APR issue.
> 
> Please keep in mind that Subversion ships with a _copy_ of APR (and seem
> to not very often upgrade their copy) and hence they have to fix it in
> their copy (or upgrade once your released a fixed APR version ;-), too.

I'm a Subversion committer, so I can say with complete certainty that
your understanding of the Subversion release process with respect to APR
is wildly inaccurate.

Subversion bundles the latest released version of APR 0.9.x. We do not
do anything as nastily inelegant as keeping a local copy that is allowed
to get stale.

>> Thankyou for taking the time to hunt down bugs, but you could have saved
>> yourself a lot of time by just checking trunk. Both of the above are
>> already fixed.
>>
>> I did not propose a backport at the time I fixed the bugs, but I take it
>> from your considerable action on this topic that you would like one. I
>> guess that's fine - the change does not have compatibility issues that I
>> am aware of.
> 
> Yes, please backport those fixes to your APR 1.2 branch as you yesterday
> even released APR 1.2.7 (from that branch, yes, I know) which still
> contains the same buggy UUID generator. As long as those fixes are not
> part of an official APR release the generated bogus UUIDs are still
> spreading around...

OK, I'll backport it. Just out of curiosity, is there any problem caused
by the weird UUIDs?

Should I be considering backporting it to 0.9.x as well? (Anyone using
Apache 2.0.x must continue to use APR 0.9.x, and 0.9.x will remain the
default bundled with Subversion for the forseeable future.)

>>> 3. OPTIMIZATION: for generating random content the local
>>>    get_system_time() function (which is based on apr_time_now()) is used
>>>    which time-adjusts for the UUID vs Unix Epoch time. For generating
>>>    random bytes it is fully sufficient to just use plain apr_time_now().
>>> Index: apr-util-1.2.6/crypto/getuuid.c
>>> --- apr-util-1.2.6/crypto/getuuid.c.orig	2005-02-04 21:45:35 +0100
>>> +++ apr-util-1.2.6/crypto/getuuid.c	2006-04-04 19:49:37 +0200
>>> @@ -131,7 +131,7 @@
>>>
>>>      /* crap. this isn't crypto quality, but it will be Good Enough */
>>>
>>> -    get_system_time(&time_now);
>>> +    time_now = apr_time_now();
>>>      srand((unsigned int)(((time_now >> 32) ^ time_now) & 0xffffffff));
>>>
>>>      return rand() & 0x0FFFF;
>> This seems sensible to me.
>>
>> Could someone give a second opinion that this is fine, and isn't going
>> to impact the randomness quality?
> 
> The difference between your get_system_time() and apr_time_now() is
> nothing more than the time offset between Unix epoch and the one used
> by UUIDs. Adding a value doesn't change the randomness _quality_ of an
> integer at all (it still is as bad or good as it was before)...

Naturally. I was mainly wondering about the multiplication combined with
the 64bit->32bit folding.

Max.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)

iD8DBQFEP9kkfFNSmcDyxYARAqh+AJ93cOJVfkQMswxSnlPmeJJWHqiEqgCgyuBn
foXI18vEFbX4TeF2mU9jAOU=
=uF3X
-----END PGP SIGNATURE-----