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-07T09:50:08Z
Lists: pgsql-hackers
On 05.07.22 18:34, Jacob Champion wrote:
> On Fri, Jul 1, 2022 at 1:51 PM Jacob Champion <jchampion@timescale.com> wrote:
>> Sorry for the misunderstanding! v3 adds the Issuer to the logs as well.
> 
> Resending v3; I messed up the certificate diff with my gitconfig.

This patch looks pretty good to me.  Some minor details:

I looked into how you decode the serial number.  I have found some code 
elsewhere that passed the result of X509_get_serialNumber() directly to 
ASN1_INTEGER_set().  But I guess a serial number of maximum length 20 
octets wouldn't fit into a 32-bit long.  (There is 
ASN1_INTEGER_set_int64(), but that requires OpenSSL 1.1.0.)  Does that 
match your understanding?

For the detail string, I think we could do something like:

DETAIL:  Failed certificate data (unverified): subject '%s', serial 
number %s, issuer '%s'



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