Horton

Re: [PATCH] Log details for client certificate failures

Jacob Champion <jchampion@timescale.com>

View thread View on postgr.es
From: Jacob Champion <jchampion@timescale.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2022-07-21T23:29:35Z
Lists: pgsql-hackers

Attachments

On Wed, Jul 20, 2022 at 3:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Jacob Champion <jchampion@timescale.com> writes:
> > I'm currently hardcoding an elevel of ERROR on the new guc_strdup()s,
> > because that seems to be a common case for the check hooks.
>
> Really?  That's almost certainly NOT okay.  As an example, if you
> have a problem with a new value loaded from postgresql.conf during
> SIGHUP processing, throwing ERROR will cause the postmaster to exit.

v4 attempts to fix this by letting the check hooks pass
MCXT_ALLOC_NO_OOM to pg_clean_ascii(). (It's ignored in the frontend,
which just mallocs.)

> I wouldn't be too surprised if there are isolated cases where people
> didn't understand what they were doing and wrote that, but that
> needs to be fixed not emulated.

I might be missing something, but in guc.c at least it appears to be
the rule and not the exception.

Thanks,
--Jacob

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