Re: 2.5.7 gnutls_x509_privkey_generate() returns GNUTLS_E_INVALID_REQUEST

[Sam Varshavchik]

Got some more leaks. All of these leaks came out when I ran valgrind on some 
test code that sets up a TLS session, then requests a rehandshake. Setting 
up a debugging environment, given the nature of the application, was rather 
difficult, I tried to find the source of the leak by eyeballing the code, I 
think I can see where the problem is.

==8519==    at 0x4A05174: calloc (vg_replace_malloc.c:397)
==8519==    by 0x317D24477E: _gnutls_copy_certificate_auth_info (auth_cert.c:81)
==8519==    by 0x317D2464E8: _gnutls_proc_x509_server_certificate 
(auth_cert.c:1022)
==8519==    by 0x317D247079: _gnutls_proc_cert_server_certificate 
(auth_cert.c:1257)
==8519==    by 0x317D233DE4: _gnutls_recv_server_certificate (gnutls_kx.c:719)
==8519==    by 0x317D22E6AD: _gnutls_handshake_client (gnutls_handshake.c:2359)
==8519==    by 0x317D22E268: gnutls_handshake (gnutls_handshake.c:2262)

I think this is a leak in _gnutls_copy_certificate_auth_info():

 if (ncerts == 0)
   {
     info->raw_certificate_list = NULL;
     info->ncerts = 0;
     return 0;
   }

 info->raw_certificate_list =
   gnutls_calloc (ncerts, sizeof (gnutls_datum_t));

I think what's happening is that on an initial handshake, the 
raw_certificate_list was allocated. On the rehandshake, the same code 
allocated a new list, and overwrote the previously allocated list, leaking 
it.

I think a check is needed here, before the if() statement, to deallocate any 
previously allocated list, from an earlier handshake.

==8519==    at 0x4A0739E: malloc (vg_replace_malloc.c:207)
==8519==    by 0x317D23AEA0: _gnutls_set_datum_m (gnutls_datum.c:80)
==8519==    by 0x317D242342: _gnutls_set_keys (gnutls_constate.c:130)
==8519==    by 0x317D242F90: _gnutls_set_write_keys (gnutls_constate.c:424)
==8519==    by 0x317D243AC4: _gnutls_write_connection_state_init 
(gnutls_constate.c:721)
==8519==    by 0x317D22ECF0: _gnutls_send_handshake_final 
(gnutls_handshake.c:2462)
==8519==    by 0x317D22F89C: _gnutls_handshake_common (gnutls_handshake.c:2675)
==8519==    by 0x317D22E2B1: gnutls_handshake (gnutls_handshake.c:2279)

I see that _gnutls_set_datum_m() allocates memory for the new datum object 
and doesn't check whether the datum object passed to it already points to a 
previous memory block that was, presumably, allocated earlier.

So, it looks to me like when _gnutls_set_keys():

     if (_gnutls_sset_datum
          (&session->cipher_specs.client_write_mac_secret,
           &key_block[pos], hash_size) < 0)
        {
          gnutls_free (key_block);
          return GNUTLS_E_MEMORY_ERROR;
        }

So, on a rehandshake, this would leak the memory that 
client_write_mac_secret points to, that was allocated during the initial 
handshake.

What I do not understand is why there's no corresponding leak for 
server_write_mac_secret datum object, that's allocated immediately following 
the above code.

==8519== 1,536 bytes in 6 blocks are definitely lost in loss record 33 of 35
==8519==    at 0x4A0739E: malloc (vg_replace_malloc.c:207)
==8519==    by 0x317D23E8D3: _gnutls_mpi_randomize (gnutls_mpi.c:51)
==8519==    by 0x317D23258F: gnutls_calc_dh_secret (gnutls_dh.c:65)
==8519==    by 0x317D254FB2: _gnutls_dh_common_print_server_kx 
(auth_dh_common.c:316)
==8519==    by 0x317D24A46D: gen_dhe_server_kx (auth_dhe.c:135)
==8519==    by 0x317D232F9C: _gnutls_send_server_kx_message (gnutls_kx.c:199)
==8519==    by 0x317D22F2EA: _gnutls_handshake_server (gnutls_handshake.c:2593)
==8519==    by 0x317D22E276: gnutls_handshake (gnutls_handshake.c:2266)

This does not appear to be rehandshake-related.

_gnutls_mpi_randomize() mallocs a buffer, but does not appear to free it, on 
its succesfull execution path, only on its error path.

pgp1iXkISTwqE.pgp
Description: PGP signature