2.5.7 gnutls_x509_privkey_generate() returns GNUTLS_E_INVALID_REQUEST

[Simon Josefsson]

Sam Varshavchik <mrsam at courier-mta.com> writes:

> Simon Josefsson writes:
>
>> Sam Varshavchik <mrsam at courier-mta.com> writes:
>>
>>> The following short test program runs when compiled against
>>> 2.4.0. Compiling it against 2.5.7 causes it to report a
>>> GNUTLS_E_INVALID_REQUEST from the second call to
>>> gnutls_x509_privkey_generate().
>>
>> I can't reproduce this, adding this somewhere:
>>
>>   printf ("vers %s %s\n", LIBGNUTLS_VERSION, gnutls_check_version (NULL));
>>
>> Does print 2.5.7 for both, confirming that I really use 2.5.7.  So it
>> seems something else is required to reproduce this.  Can you try to
>> debug gnutls_x509_privkey_generate and see what happens?
>>
>> Does 'certtool -p' trigger the same problem for you?
>
> The bug seems to be easy to spot. I think this is it:
>
> int
> gnutls_x509_privkey_generate (gnutls_x509_privkey_t key,
>                              gnutls_pk_algorithm_t algo, unsigned int bits,
>                              unsigned int flags)
> {
>  int ret;
>  unsigned int params_len;
>
> // . . .
>
>    ret = _gnutls_rsa_generate_params (key→params, &params_len, bits);
>
> This goes into:
>
> static int
> _generate_params (int algo, bigint_t *resarr, unsigned int *resarr_len,
>                  int bits)
>
> // . . .
>
>  if (resarr && resarr_len && *resarr_len > params.params_nr)
>                              ===========
>
> Looks like *resarr_len points to uninitialized memory at this
> point. gnutls_x509_privkey_generate() never initialized params_len, as
> far as I can tell.

Thanks for analysis, I guess it broke during the crypto.h conversion.
How about this patch?

diff --git a/lib/x509/privkey.c b/lib/x509/privkey.c
index 82408c6..e5e6de3 100644
--- a/lib/x509/privkey.c
+++ b/lib/x509/privkey.c
@@ -1316,7 +1316,7 @@ gnutls_x509_privkey_generate (gnutls_x509_privkey_t key,
 			      unsigned int flags)
 {
   int ret;
-  unsigned int params_len;
+  unsigned int params_len = MAX_PRIV_PARAMS_SIZE;
   unsigned int i;
 
   if (key == NULL)

Nikos, do you think this is correct?

/Simon