tlshd: Deinit output variable in __tlshd_config_get_privkey on error

[trchen1033]

Fixes a small memory leak where the output variable was constructed but not destructed on error exit.

Fixes: #156

[chucklever]

Thanks for tracking this down -- the leak is real. gnutls_privkey_init() allocates the key object, and a failed gnutls_privkey_import_x509_raw() leaves it allocated, so the early return false does drop it.

One thing to sort out before this lands: where the key gets freed on the normal path. These keys are owned by tlshd_x509_client_put_privkey() / tlshd_x509_server_put_privkey(), which unconditionally gnutls_privkey_deinit() the module-global keys. On the TLS handshake paths a get_privkey() failure jumps to the cleanup label (out_free_certs in client.c, out_deinit_session in server.c), which calls put_privkey(). So with this patch the same key is deinit'd twice -- once here, once in put_privkey() -- a double-free.

The leak actually bites on the QUIC paths, where the failure handler does a bare return and never reaches put_privkey(). Both behaviors are real; they just live on different call paths.

There's also a quieter case: tlshd_config_get_privkey() ignores the return value of the PQ-key call. If the PQ key file exists but fails to import while the regular key succeeds, the handshake proceeds, and this patch will have left the PQ key dangling for put_privkey() to deinit again later.

Nulling the pointer after freeing it covers all of these:

Suggested change

	gnutls_privkey_deinit(*privkey);
	gnutls_privkey_deinit(*privkey);
	*privkey = NULL;

That plugs the QUIC-path leak and makes the second deinit in put_privkey() a no-op. gnutls_privkey_deinit(NULL) is already safe and relied upon -- when get_certs() fails before get_privkey(), put_privkey() deinits the still-NULL globals on every handshake.

[trchen1033]

Oops, sorry for underestimating the extent of the problem and made a bad patch. I traced through the object lifetime this time and agree what you've suggested is the best way forward.

[trchen1033]

Yea, gnutls_pcert_deinit is not idempotent. Sad. :(

[chucklever]

Thanks -- this is correct. gnutls_privkey_init() allocates the key object and a failed gnutls_privkey_import_x509_raw() leaves it allocated, so deinit'ing the local before the early return plugs the leak without leaving a dangling global for put_privkey() to double-free. Routing the QUIC server path through err_config closes the bare-return leak there, and NULL-ing the globals after deinit keeps the second put a no-op.

The QUIC client path (tlshd_quic_client_set_x509_session()) has the same bare-return pattern; I'll handle that in a follow-up so this can go in.