Re: 2.5.7 gnutls_x509_privkey_generate() returns GNUTLS_E_INVALID_REQUEST

[Sam Varshavchik]

Nikos Mavrogiannopoulos writes:

> > --- 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?
>
> Yes, indeed!

No, even with this fix in place, this bug remains.

In _generate_params():

 if (resarr && resarr_len && *resarr_len > params.params_nr)

Now, *resarr_len is initialized to 6, however, for RSA, params.params_nr is 
also 6, so this test still fails, and GNUTLS_E_INVALID_REQUEST gets 
returned.

This line in gnutls_pk.c needs to be changed to:

 if (resarr && resarr_len && *resarr_len >= params.params_nr)


=======================================================================

Unfortunately, there appears to be more problems with private key functions 
in 2.5.7. I'm now getting segfaults in gnutls_x509_privkey_import(). My 
debugging results are as follows:

* I'm passing a DER-formatted DSA key for input to 
gnutls_x509_privkey_import().

* Execution reaches the following code in gnutls_x509_privkey_import():

     key->pk_algorithm = GNUTLS_PK_RSA;
     key->key = _gnutls_privkey_decode_pkcs1_rsa_key (&_data, key);
   
     if (key->key == NULL)
       {
         key->pk_algorithm = GNUTLS_PK_DSA;
         key->key = decode_dsa_key (&_data, key);
         if (key->key == NULL)
           gnutls_assert ();
       }

* _gnutls_privkey_decode_pkcs1_rsa_key() gets called, and tries to parse my 
DER-formatted DSA key:

 result = asn1_der_decoding (&pkey_asn, raw_key->data, raw_key->size, NULL);
  if (result != ASN1_SUCCESS)
   {
     gnutls_assert ();
     goto error;
   }

Here, asn1_der_decoding() fails, and the "goto error" branch gets taken:

error:
 asn1_delete_structure (&pkey_asn);
 _gnutls_mpi_release (&pk_params.params[0]);

Unfortunately, this function never got far enough to initialize 
pk_params.params, so the entire array is uninitialized memory, 
_gnutls_mpi_release dereferences a random memory address, and segfaults.

It looks to me like the temp_params local variable needs to be cleared.

I tried the following patch, and it seems to fix the segfaults:

--- lib/x509/privkey.c~ 2008-09-20 23:09:27.000000000 -0400
+++ lib/x509/privkey.c  2008-09-20 23:31:06.000000000 -0400
@@ -162,6 +162,8 @@
  pk_params.params = temp_params;
  pk_params.params_nr = RSA_PRIVATE_PARAMS;

+  memset(temp_params, 0, sizeof(temp_params));
+
  if ((result =
       asn1_create_element (_gnutls_get_gnutls_asn (),
                            "GNUTLS.RSAPrivateKey",


This fixes the segfaults, and my app runs, but valgrind complains about a 
couple of memory leaks, in the same general bits of code:

==3339== 240 bytes in 5 blocks are definitely lost in loss record 2 of 9
==3339==    at 0x4A0739E: malloc (vg_replace_malloc.c:207)
==3339==    by 0x4C689C4: wrap_gcry_pk_generate_params (pk-libgcrypt.c:773)
==3339==    by 0x4C57881: _generate_params (gnutls_pk.c:527)
==3339==    by 0x4C83591: gnutls_x509_privkey_generate (privkey.c:1354)

and

==3339== 5,886 (168 direct, 5,718 indirect) bytes in 3 blocks are definitely 
lost in loss record 8 of 9
==3339==    at 0x4A05174: calloc (vg_replace_malloc.c:397)
==3339==    by 0x3158C0A211: (within /usr/lib64/libtasn1.so.3.0.14)
==3339==    by 0x3158C0A3F2: (within /usr/lib64/libtasn1.so.3.0.14)
==3339==    by 0x3158C0A7BD: asn1_create_element (in 
/usr/lib64/libtasn1.so.3.0.14)
==3339==    by 0x4C829B2: _gnutls_asn1_encode_rsa (privkey.c:1075)
==3339==    by 0x4C83E0B: gnutls_x509_privkey_export (privkey.c:739)


I'll try to chase these down…

pgpZMUv83F5Vd.pgp
Description: PGP signature