[gnutls-dev] Re: bug in _gnutls_pkcs1_rsa_encrypt

[Robey Pointer]

Simon Josefsson wrote:

>
>>Attached is a patch against 1.0.19 which moves the 3-random-byte fetch 
>>outside the loop, and adds a mask inside the GMAX so that only the lower 
>>8 bits count.
>>    
>>
>
>Whatever the intention was, IMHO, the logic was convoluted, I have now
>installed code which says:
>
>	if ((ret =
>	     _gnutls_get_random(ps, psize, GNUTLS_STRONG_RANDOM)) < 0) {
>	    gnutls_assert();
>	    gnutls_afree(edata);
>	    return ret;
>	}
>	for (i = 0; i < psize; i++)
>	  while (ps[i] == 0) {
>	    if ((ret =
>		 _gnutls_get_random(&ps[i], 1, GNUTLS_STRONG_RANDOM)) < 0) {
>		gnutls_assert();
>		gnutls_afree(edata);
>		return ret;
>	    }
>	  }
>	}
>
>It seems easier to argue for correctness of the above.
>

I agree that this is a lot more appropriate, and it looks correct to 
me.  Werner's version (in a different post) also looks solid (and 
appears to be optimized for making as few calls to _gnutls_get_random as 
possible, in case that's an issue).  I just wasn't sure what the 
original intent of the convoluted code was. :)


>As this is
>important code, more eyes on it would be appreciated.  One might have
>qualms about a possible infinite loop.  I looked at PKCS#1 1.5 type 2
>padding in OpenSSL, and it also loop until non-zero random data is
>generated.  Nettle just replace 0 with 1.
>

The one case that will generate an infinite loop is where the PRNG is 
spewing out a stream of all-zeros.  I think this is a case where you 
would rather lock up anyway. ;)  But yeah, I don't think the loop should 
be an issue.

Thanks!

robey