Magnus Hagander <magnus@hagander.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
