Re: [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