Skip to content

Memory leak when using client certificate auth with rehandshake and OCSP

Hi,

Description of problem:

When using GNUTLS to perform an HTTPS request with client certificate authentication, valgrind reports a memory leak issue in _gnutls_recv_server_certificate_status.

There is no issue when the API is not asking for a rehandshake.

Version of gnutls used:

3.6.15 but also in earlier versions.

Distributor of gnutls (e.g., Ubuntu, Fedora, RHEL)

Ubuntu

How reproducible:

Steps to Reproduce:

  • The server API is enforcing rehandshake after the data has been sent by the client
  • Use the http client example with TLS and certificate authentication
  • the client is set up with either: gnutls_init(GNUTLS_CLIENT | GNUTLS_AUTO_REAUTH) or gnutls_init(GNUTLS_CLIENT | GNUTLS_NONBLOCK) and a step for rehandshake after write such as:
int ret = gnutls_record_recv(session, front.data(), front.size());
                if (ret == GNUTLS_E_REHANDSHAKE && is_safe_renegotiation_enabled())
                        ret = gnutls_handshake(session);
                 // ... other steps
                }
  • valgrind --leak-check=full -v ./https_client

Actual results:

valgrind --leak-check=full -v ./https_client

==9160== HEAP SUMMARY:
==9160==     in use at exit: 1,762 bytes in 2 blocks
==9160==   total heap usage: 15,866 allocs, 15,864 frees, 1,701,175 bytes allocated
==9160== 
==9160== Searching for pointers to 2 not-freed blocks
==9160== Checked 314,840 bytes
==9160== 
==9160== 1,762 (16 direct, 1,746 indirect) bytes in 1 blocks are definitely lost in loss record 2 of 2
==9160==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9160==    by 0x4F44612: _gnutls_recv_server_certificate_status (status_request.c:497)
==9160==    by 0x4E84914: handshake_client (handshake.c:3011)
==9160==    by 0x4E84914: gnutls_handshake (handshake.c:2780)

Expected results:

On rehandshake a malloc is performed in status_request.c in _gnutls_recv_server_certificate_status:
info->raw_ocsp_list = gnutls_malloc(sizeof(gnutls_datum_t));

Whether or not it is already allocated. A check should be performed first and free or remove the previous allocated resources.

Fix:

  • A quick fix I tested and removing all the memory leak for this case is as follow:

status_request.c -> _gnutls_recv_server_certificate_status

                // A handshake was already performed
                if (info->raw_ocsp_list != NULL)
		{
			for(int i=0;i<info->nocsp;i++)
				gnutls_free(info->raw_ocsp_list[i].data);
			gnutls_free(info->raw_ocsp_list);
		}

		info->raw_ocsp_list = gnutls_malloc(sizeof(gnutls_datum_t));

Thanks

Edited by remiolivier