Mailing List Archive

Gratuitous gcry_fast_random_poll (Was: Re: [GNUnet-developers] _gcry_ath_mutex_lock)
[.Also to gcrypt-devel since I believe this is partially libgcrypt's fault]

I figured it out. While the way that we're using libgcrypt should not require
any synchronization for this code, the code does in gcry_cipher_open the
completely gratuitous

/* If the application missed to call the random poll function, we do
it here to ensure that it is used once in a while. */
_gcry_fast_random_poll ();


Now, while creating a symcipher and doing some encryption with it should not
require locking, the random_poll operation does require locking. Now, GNUnet
does not do any locking here (since we assumed that no locking would be
required) -- which results in the assertion failure you describe below (given
concurrent use). Now, we can fix it by doing more (useless) locking in
GNUnet (will patch), but I'll CC this to libgcrypt-devel since I believe they
should avoid doing such calls that require locks on paths that one would
commonly not consider accessing the random-pool or other global
datastructures.

> On Monday 03 May 2004 08:05, Glenn McGrath wrote:
> > Ive uploaded debian packages of 0.6.2, however ive just realised i have
> > a problem with it, i get the following error.
> >
> > gnunetd: ath.c:181: _gcry_ath_mutex_lock: Assertion `*lock ==
> > ((ath_mutex_t) 0)' failed.
> >
> > I built the deb against libgcrypt-1.2.0, to doso i copied the files
> > src/util/hostkey_gcrypt.c and src/util/symcipher_gcrypt.c from cvs.
> >
> > The gcrypt function in question is ath_mutex_lock(), it tried tracking
> > it down using gdb but had no luck.
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
At Wed, 05 May 2004 19:35:53 +0200,
Werner Koch wrote:
> I don't know what you mean by locking though. If you are using a
> multi-threaded application you need to make sure that libgcrypt is
> porperly initialized - see the section in the manual.

The asserts in the code check if the proper lock/unlock calls are made
in the single-threaded case, too.

I put them in there for my own debugging at the time - in GPGME, where
the code originated, all locking happens internally, so an assertion
would show if internal to GPGME some locks were inproperly used.

If the locking interface is exported in gcrypt, and users are
responsible for their own locking, these assertions should probably be
removed.

Thanks,
Marcus
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
On Mon, 3 May 2004 23:52:52 -0500, Christian Grothoff said:

> concurrent use). Now, we can fix it by doing more (useless) locking in
> GNUnet (will patch), but I'll CC this to libgcrypt-devel since I believe they
> should avoid doing such calls that require locks on paths that one would
> commonly not consider accessing the random-pool or other global
> datastructures.

This call to fast_random_poll() is there on purpose as the comment
describes. Without that people will very likely forget to add a
couple of poll clals and we better make sure that they are at least
used from time to time.

I don't know what you mean by locking though. If you are using a
multi-threaded application you need to make sure that libgcrypt is
porperly initialized - see the section in the manual.

>> > gnunetd: ath.c:181: _gcry_ath_mutex_lock: Assertion `*lock ==
>> > ((ath_mutex_t) 0)' failed.

May it be that another library you link to uses pthreads and you don't
know about this. libpcsclite for example does this.


Werner
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
On Wed, 05 May 2004 19:35:34 +0200, Marcus Brinkmann said:

> The asserts in the code check if the proper lock/unlock calls are made
> in the single-threaded case, too.

Which is a Good Thing.

> If the locking interface is exported in gcrypt, and users are
> responsible for their own locking, these assertions should probably be
> removed.

Why? Calling ath_mutex_lock on a locked mutex is a severe error in
any case.

Werner
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
* Werner Koch:

> This call to fast_random_poll() is there on purpose as the comment
> describes. Without that people will very likely forget to add a
> couple of poll clals and we better make sure that they are at least
> used from time to time.

Sorry, those people won't be able to write decent crypto software
anyway. 8-/ Maybe it's better to check that the appropriate minimum
number of calls is being made and abort() if necessary.

Some fast_random_poll() call in GnuPG is also a bottleneck during
signature verification. (I think I've already reported that.)

--
Current mail filters: many dial-up/DSL/cable modem hosts, and the
following domains: atlas.cz, bigpond.com, di-ve.com, hotmail.com,
jumpy.it, libero.it, netscape.net, postino.it, simplesnet.pt,
tiscali.co.uk, tiscali.cz, tiscali.it, voila.fr, yahoo.com.
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
At Wed, 05 May 2004 20:36:09 +0200,
Werner Koch wrote:
>
> On Wed, 05 May 2004 19:35:34 +0200, Marcus Brinkmann said:
>
> > The asserts in the code check if the proper lock/unlock calls are made
> > in the single-threaded case, too.
>
> Which is a Good Thing.
>
> > If the locking interface is exported in gcrypt, and users are
> > responsible for their own locking, these assertions should probably be
> > removed.
>
> Why? Calling ath_mutex_lock on a locked mutex is a severe error in
> any case.

Yes, but compare it with pthread: you have error checking locks
(robust locks) and fast locks. And everybody just uses the fast locks
and never checks the error.

It's not a good thing if a library gives an assertion failure, unless
there really is an internal error. An error value returned would be
better, but nobody would check it. Oh well.

Marcus
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
On Wednesday 05 May 2004 21:57, Florian Weimer wrote:

> > This call to fast_random_poll() is there on purpose as the comment
> > describes. Without that people will very likely forget to add a
> > couple of poll clals and we better make sure that they are at least
> > used from time to time.
> Sorry, those people won't be able to write decent crypto software
> anyway. 8-/ Maybe it's better to check that the appropriate minimum
> number of calls is being made and abort() if necessary.
Well since this function is neither documented nor exported, you shouldn't
expect anyone to call it. An exported random pool initialization function
would help here.

--
Nikos Mavroyanopoulos
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
On Wednesday 05 May 2004 12:35, Werner Koch wrote:
> On Mon, 3 May 2004 23:52:52 -0500, Christian Grothoff said:
> > concurrent use). Now, we can fix it by doing more (useless) locking in
> > GNUnet (will patch), but I'll CC this to libgcrypt-devel since I believe
> > they should avoid doing such calls that require locks on paths that one
> > would commonly not consider accessing the random-pool or other global
> > datastructures.
>
> This call to fast_random_poll() is there on purpose as the comment
> describes. Without that people will very likely forget to add a
> couple of poll clals and we better make sure that they are at least
> used from time to time.

All I'm saying is leave that to the application. For example, I may not be
using any key generation from libgcrypt, so I definitely don't need any of
the PRNG code. And as others have remarked before, assertion failures are
bad anyway -- return error codes. If the client doesn't check them, then the
software is probably not worth worrying about anyway.

> I don't know what you mean by locking though. If you are using a
> multi-threaded application you need to make sure that libgcrypt is
> porperly initialized - see the section in the manual.

I'm aware of that discussion. The problem is that the code in question was
originally written for an earlier version of libgcrypt and had not been
adapted. Anyway, it should work just fine (with any version of libgcrypt) if
the client guarantees that only one thread enters libgcrypt at a time. Now,
that's a bit strong and could be relaxed to "one thread enters libgcrypt at a
time for operations that may require global state" -- and the call in
question requires global state (and sync) on a path where that's completely
counter-intuitive.

> >> > gnunetd: ath.c:181: _gcry_ath_mutex_lock: Assertion `*lock ==
> >> > ((ath_mutex_t) 0)' failed.
>
> May it be that another library you link to uses pthreads and you don't
> know about this. libpcsclite for example does this.

Oh, we're using pthreads, I've resolved the problem by putting some extra
locks around our libgcrypt calls (since I don't want to rely on a 'recent'
libgcrypt version). Nevertheless I find the random-poll is completely out of
place. It would take 2 lines of code for me to call it every n seconds or
something like that, which would be much more appropriate than this kind of
random (pun not intended) injection of the call. The code in question
initializes the symcipher millions of times but never ever creates a single
random key. Ideally, it would never even open /dev/random.

Christian
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
On Wed, 05 May 2004 21:13:11 +0200, Marcus Brinkmann said:

> Yes, but compare it with pthread: you have error checking locks
> (robust locks) and fast locks. And everybody just uses the fast locks
> and never checks the error.

Which is bad because you won't detect logical errors in your program.

> It's not a good thing if a library gives an assertion failure, unless
> there really is an internal error. An error value returned would be

It is an internal error. The library is used in a non-allowed way and
all shit may happen later. Be glad that it tells you that - even if
you don't want so. It is not different from free(3) which might abort
the process if it detects an invalid state of the heaps.

Werner
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
On Wed, 05 May 2004 20:57:37 +0200, Florian Weimer said:

> Sorry, those people won't be able to write decent crypto software
> anyway. 8-/ Maybe it's better to check that the appropriate minimum
> number of calls is being made and abort() if necessary.

As of now there is no published API do do this polling, so we need to
rely on the internal hacks.

> Some fast_random_poll() call in GnuPG is also a bottleneck during
> signature verification. (I think I've already reported that.)

Yes, I know.

Werner
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
On Wed, 05 May 2004 23:21:56 +0300, Nikos Mavroyanopoulos said:

> expect anyone to call it. An exported random pool initialization function
> would help here.

Its not about initializing the RNG but to update the internal pool of
the RNG from time to time. Doing this in the cipher and md open calls
seesm to be a good place and we are doing so for many years.


Ciao,

Werner
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
On Wed, 5 May 2004 16:10:09 -0500, Christian Grothoff said:

> All I'm saying is leave that to the application. For example, I may not be
> using any key generation from libgcrypt, so I definitely don't need any of

You may want to create a signature and thus you need random. There
are quite some palces where random numbers are required and so for the
avaerage application the current scheme works.

> the PRNG code. And as others have remarked before, assertion failures are
> bad anyway -- return error codes. If the client doesn't check them, then the

It is not always pissible to return error codes. Some functions are
expected to simply work. If they don;t, something is badly wrong with
their environment.

> originally written for an earlier version of libgcrypt and had not been
> adapted. Anyway, it should work just fine (with any version of libgcrypt) if
> the client guarantees that only one thread enters libgcrypt at a time. Now,

Obviously there is another thread using libgcrypt.

> time for operations that may require global state" -- and the call in
> question requires global state (and sync) on a path where that's completely
> counter-intuitive.

You should not make any assumption about the internal working of a
library. This may work for one version but maybe not for the next.
We have tried to hide all details but your are still looking inside.\
- don't do this.

> Oh, we're using pthreads, I've resolved the problem by putting some extra
> locks around our libgcrypt calls (since I don't want to rely on a 'recent'
> libgcrypt version). Nevertheless I find the random-poll is completely out of
There is only one stable release yet - all prior releases where
clearly marked as under development. Please change gnunet to properly
intialize libgcrypt and you are set.

> random (pun not intended) injection of the call. The code in question
> initializes the symcipher millions of times but never ever creates a single

If you make such heavy use of cipher_open, please change your
application to presever the context returned by cipher_open and use
gcry_cipher_reset before setting up a new key and IV. This will avoid
a lot of other overhead too.

Werner
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
[I assume, Nikos sent his mail accidently only to me].

On Fri, 07 May 2004 13:19:16 +0300, Nikos Mavroyanopoulos said:

> Seems fine then. Maybe removing the initialization part of this function
> might speed up things to programs that do not use the rnd (just hash
> or encrypt). So this will only update the pool if initialized. I don't know if
I have done these changes right now in the CVS and you or Christan
might want to look at it. If this works out, I will gop into 1.2.1.

The random number is now only initialzed when random numbers are
actually been requested or the new macro gcry_fast_random_poll() has
been used. The internal _gcry_fast_random_poll is a NOP as long as
the RNG has not been initialized - thus for simple application
/dev/random should not even be opened.

There is of course the drawback that we put less of possible entropy
into the pool but I don't consider this a serious problem because two
calls to the internal fast random poll function in a short time won't
make any difference as the current time and used resources will be
mostly identical (I assume that a cipher_open or md_open will
immediatley be followed by a request for random or vice versa). And
more important, good random for the RNG will either be sucked in from
/dev/random or a seed file at startup anyway.

If you have an application with a main processing loop or other points
in the code which are regulary called (say about every second) you
might already want to start adding this

#ifdef gcry_fast_random_poll
gcry_fast_random_poll ();
#endif

and as soon as you use compile against a newer libgcrypt version this
will be used. Don't use this in a timer tick handler - it would be
mostly pointless.



Salam-Shalom,

Werner
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
On Friday 07 May 2004 17:14, Werner Koch wrote:

> > Seems fine then. Maybe removing the initialization part of this function
> > might speed up things to programs that do not use the rnd (just hash
> > or encrypt). So this will only update the pool if initialized. I don't
> > know if
> I have done these changes right now in the CVS and you or Christan
> might want to look at it. If this works out, I will gop into 1.2.1.
> The random number is now only initialzed when random numbers are
> actually been requested or the new macro gcry_fast_random_poll() has
> been used. The internal _gcry_fast_random_poll is a NOP as long as
> the RNG has not been initialized - thus for simple application
> /dev/random should not even be opened.

I was just wondering how many times is /dev/random accessed? I
though that it opened only once in the initialization of the
random pool. I ask because I came across a server the used fork
for each client and initializes libgcrypt (and gnutls) on every child.
If /dev/random is accessed several times from each child then
/dev/random would be exhausted soon, thus the server's childs would
be blocked (gnutls calls several times the md_open, cipher_open as well
as the random functions).


> Salam-Shalom,
> Werner

--
Nikos Mavroyanopoulos
Re: Gratuitous gcry_fast_random_poll [ In reply to ]
On Sat, 08 May 2004 11:09:59 +0300, Nikos Mavroyanopoulos said:

> I was just wondering how many times is /dev/random accessed? I
> though that it opened only once in the initialization of the
> random pool. I ask because I came across a server the used fork

Exactly. The random pool will be filled up and unless you have a seed
file this will use /dev/random. A fork will reuse the same pool but
due to the two stage mixing the random won't match.

Werner