Re: [PATCH] Log details for client certificate failures

Peter Eisentraut <peter.eisentraut@enterprisedb.com>

From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Jacob Champion <jchampion@timescale.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2022-07-14T20:12:33Z
Lists: pgsql-hackers

Attachments

On 13.07.22 01:06, Jacob Champion wrote:
>> I had to read up on this "ex_data" API.  Interesting.  But I'm wondering
>> a bit about how the life cycle of these objects is managed.  What
>> happens if the allocated error string is deallocated before its
>> containing object?  Or vice versa?
> 
> Yeah, I'm currently leaning heavily on the lack of any memory context
> switches here. And I end up leaking out a pointer to the stale stack
> of be_tls_open_server(), which is gross -- it works since there are no
> other clients, but that could probably come back to bite us.
> 
> The ex_data API exposes optional callbacks for new/dup/free (I'm
> currently setting those to NULL), so we can run custom code whenever
> the SSL* is destroyed. If you'd rather the data have the same lifetime
> of the SSL* object, we can switch to malloc/strdup/free (or even
> OPENSSL_strdup() in later versions). But since we don't have any use
> for the ex_data outside of this function, maybe we should just clear
> it before we return, rather than carrying it around.

Concretely, I was thinking like the attached top-up patch.

The other way can surely be made to work somehow, but this seems much 
simpler and with fewer questions about the details.

Commits

  1. Fix tiny memory leaks

  2. Don't reflect unescaped cert data to the logs

  3. pg_clean_ascii(): escape bytes rather than lose them

  4. Log details for client certificate failures