Re: SSL information view

Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>
>> You should add it to the next CF for proper tracking, there are already many
>> patches in the queue waiting for reviews :)
>
> Absolutely - I just wanted those that were already involved in the
> thread to get a chance to look at it early :) didn't want to submit it
> until it was complete.
>
> Which it is now - attached is a new version that includes docs.

Here's my review of the patch:

* Applies to the current HEAD, no failed hunks.
* Compiles and works as advertised.
* Docs included.

The following catches my eye:

psql (9.5devel)
SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384, bits: 256, compression: off)
Type "help" for help.

postgres=# select * from pg_stat_ssl;
pid | ssl | bits | compression | version | cipher | clientdn
------+-----+------+-------------+---------+-----------------------------+----------
1343 | t | 256 | f | TLSv1.2 | ECDHE-RSA-AES256-GCM-SHA384 |
(1 row)

I think the order of details in the psql prompt makes more sense,
because it puts more important details first.

I suggest we reorder the view columns, while also renaming 'version' to
'protocol' (especially since we have another patch in the works that
adds a GUC named 'ssl_protocols'):

pid, ssl, protocol, cipher, bits, compression, clientdn.

Next, this one:

+ be_tls_get_cipher(Port *port, char *ptr, size_t len)
+ {
+ if (port->ssl)
+ strlcpy(ptr, SSL_get_cipher(port->ssl), NAMEDATALEN);

should be this:

+ strlcpy(ptr, SSL_get_cipher(port->ssl), len);

The same goes for this one:

+ be_tls_get_peerdn_name(Port *port, char *ptr, size_t len)
+ {
+ if (port->peer)
+ strlcpy(ptr, X509_NAME_to_cstring(X509_get_subject_name(port->peer)), NAMEDATALEN);

The NAMEDATALEN constant is passed in the calling code in
pgstat_bestart().

Other than that, looks good to me.

--
Alex