Re: Feature request: Logging SSL connections

On Fri, Dec 06, 2013 at 11:43:55AM +0100, Dr. Andreas Kunert wrote:
> >>That seems useful. Do we need more information, like whether a client
> >>certificate was presented, or what ciphers were used?
> >
> >Yes, please show ciphersuite and TLS version too. Andreas, you can use my
> >recent \conninfo patch as template:
> >
> > https://github.com/markokr/postgres/commit/7d1b27ac74643abd15007cc4ec0b56ba92b39d90
> >
> >Also, please show the SSL level also for walsender connections. It's
> >quite important to know whether they are using SSL or not.
> >
> >But I think the 'bits' output is unnecessary, as it's cipher strength
> >is known by ciphersuite. Perhaps it can be removed from \conninfo too.
>
> A new patch is attached. I added the ciphersuite and TLS version
> like shown in your template (minus the 'bits' output). I also added
> the SSL information for walsender connections, but due to a missing
> test setup I cannot test that part.
>
> Anything else missing?

Functionally it's fine now, but I see few style problems:

- "if (port->ssl > 0)" is wrong, ->ssl is pointer. So use just
"if (port->ssl)".

- Deeper indentation would look nicer with braces.

- There are some duplicated message, could you restructure it so that
each message exists only once.

Something like this perhaps:

#if USE_SSL
if (port->ssl)
{
if (walsender) ..
else ..
}
else
#endif
...

--
marko