On 20/11/2018 09:31, Kyotaro HORIGUCHI wrote: > Comparing the two codes in pgstatfuncs.c and sslinfo.c, It seems > that ssl_client_serial() can be largely simplified using > be_tls_get_peer_serial(). X509_NAME_to_text() can be simplified > using X509_NAME_to_cstring(). But this would be another patch. > > By the way, though it is not by this patch, X509_NAME_to_cstring > doesn't handle NID_undef from OBJ_obj2nid() and NULL from > OBJ_nid2ln(). The latter case feeds NULL to BIO_printf() . I'm > not sure it can actually happen.
Yes, that seems problematic, at least in theory. >> - Changes pg_stat_ssl.clientdn to be null if there is no client >> certificate (as documented, but not implemented). (bug fix) > > This reveals DN, serial and issuer DN of the cert to > non-superusres. pg_stat_activity hides at least > client_addr. Aren't they needed to be hidden? (This will change > the existing behavior.) Yes. Arguably, nothing in this view other than your own session should be seen by normal users. Thoughts from others? >> - Adds new fields to pg_stat_ssl: issuerdn and clientserial. These >> allow uniquely identifying the client certificate. AFAICT, these are >> the most interesting pieces of information provided by sslinfo but not >> in pg_stat_ssl. (I don't like the underscore-free naming of these >> fields, but it matches the existing "clientdn".) > > clientdn, clientserial, issuerdn are the fields about client > certificates. Only the last one omits prefixing "client". But > "clientissuerdn" seems somewhat rotten... Counldn't we rename > clientdn to client_dn? I'd prefer renaming as well, but some people might not like that. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
