Re: SSL information view

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