gnupg-0.9.1 -- bug in cipher/rndunix.c

[Steven Bakker]

Hi folks,

  I recently downloaded gnupg-0.9.1 and compiled it on my Solaris
7 box (gcc v2.8.1).  I ran "gpg --gen-key" and after entering my
pass phrase, gpg entered an infinite loop.  I managed to track
this down to gather_random() in cipher/rndunix.c.

Consider the (snipped) code below:

   672  static int
   673  gather_random( void (*add)(const void*, size_t, int), int requester,
   674                 size_t length, int level )
   675  {

	...

   703      while( length ) {

   	...

   732          n = msg.ndata;
   733          if( n > length )
   734              n = length;
   735          (*add)( msg.data, n, requester );
   736
   737          /* this is the trick how e cope with the goodness */
   738          length -= (ulong)n * goodness / 100;
   739      }

Now, if the remaining length is 1 (happened to me), the following
happens:

	msg.ndata == 500
	goodness  == 86
	length    == 1

   732          n = msg.ndata;

	goodness  == 86
	length    == 1
	n         == 500

   733          if( n > length )
   734              n = length;

	goodness  == 86
	length    == 1
	n         == 1

   735          (*add)( msg.data, n, requester );
   736
   737          /* this is the trick how e cope with the goodness */
   738          length -= (ulong)n * goodness / 100;

	(ulong)n * goodness / 100 == 0
	length == 1

   739      }

Result: infinite loop.
There are two ways around this:

-----
   737      {
   738		/* This is the trick how we cope with the goodness. */
   739 		ulong subtract = (ulong)n * goodness / 100;
   740          length -= subtract ? subtract : 1;
   741      }
-----

Or:

-----
   737      {
   738		/* This is the trick how we cope with the goodness. */
   739 		ulong subtract = (ulong)msg.ndata * goodness / 100;
   740          length -= subtract <= length ? subtract : length;
   741      }
-----

The second one tends to use less iterations (with high goodness and low
length, one iteration is usually enough), while the first one typically
takes more (in my case 4 versus 1).

I chose to use the first, on the assumption that more iterations give
me more randomness, but that's more a gut feeling than anything else.
Either way it works.

Lemme know if I'm completely off (or marginally right).

Cheers,
Steven