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…


-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: </pipermail/attachments/20080920/54042d6f/attachment.pgp>