On 29/01/2019 04:18, Kyotaro HORIGUCHI wrote: > Some further investigation told me that the file > ~/.postgresql/root.cert was the culprit.
OK, I could reproduce the problem and found a fix for it. Basically you need to specify sslrootcert in each test, and these new tests didn't do it. All other tests were OK, so a local fix was enough. I have committed 0001..0003 now. Attached is the latest version of 0004, about which I didn't not get an explicit comment in your last review message. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 5168e09ae3ee0c61cdd40ababeee7a6e45bdcb01 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Mon, 28 Jan 2019 14:40:04 +0100 Subject: [PATCH v5 4/4] Add more columns to pg_stat_ssl Add columns client_serial and issuer_dn to pg_stat_ssl. These allow uniquely identifying the client certificate. Rename the existing column clientdn to client_dn, to make the naming more consistent and easier to read. Reviewed-by: Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> Discussion: https://www.postgresql.org/message-id/flat/398754d8-6bb5-c5cf-e7b8-22e5f0983...@2ndquadrant.com/ --- doc/src/sgml/monitoring.sgml | 20 ++++++++++++++++-- src/backend/catalog/system_views.sql | 4 +++- src/backend/libpq/be-secure-openssl.c | 29 +++++++++++++++++++++++++++ src/backend/postmaster/pgstat.c | 4 +++- src/backend/utils/adt/pgstatfuncs.c | 22 ++++++++++++++++---- src/include/catalog/pg_proc.dat | 6 +++--- src/include/libpq/libpq-be.h | 2 ++ src/include/pgstat.h | 16 ++++++++++++--- src/test/regress/expected/rules.out | 10 +++++---- src/test/ssl/t/001_ssltests.pl | 8 ++++---- 10 files changed, 99 insertions(+), 22 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 60a85a7898..7a84f51340 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -2201,15 +2201,31 @@ <title><structname>pg_stat_ssl</structname> View</title> or NULL if SSL is not in use on this connection</entry> </row> <row> - <entry><structfield>clientdn</structfield></entry> + <entry><structfield>client_dn</structfield></entry> <entry><type>text</type></entry> <entry>Distinguished Name (DN) field from the client certificate used, or NULL if no client certificate was supplied or if SSL is not in use on this connection. This field is truncated if the DN field is longer than <symbol>NAMEDATALEN</symbol> (64 characters - in a standard build) + in a standard build). </entry> </row> + <row> + <entry><structfield>client_serial</structfield></entry> + <entry><type>numeric</type></entry> + <entry>Serial number of the client certificate, or NULL if no client + certificate was supplied or if SSL is not in use on this connection. The + combination of certificate serial number and certificate issuer uniquely + identifies a certificate (unless the issuer erroneously reuses serial + numbers).</entry> + </row> + <row> + <entry><structfield>issuer_dn</structfield></entry> + <entry><type>text</type></entry> + <entry>DN of the issuer of the client certificate, or NULL if no client + certificate was supplied or if SSL is not in use on this connection. + This field is truncated like <structfield>client_dn</structfield>.</entry> + </row> </tbody> </tgroup> </table> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index f4d9e9daf7..3e229c693c 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -782,7 +782,9 @@ CREATE VIEW pg_stat_ssl AS S.sslcipher AS cipher, S.sslbits AS bits, S.sslcompression AS compression, - S.sslclientdn AS clientdn + S.ssl_client_dn AS client_dn, + S.ssl_client_serial AS client_serial, + S.ssl_issuer_dn AS issuer_dn FROM pg_stat_get_activity(NULL) AS S; CREATE VIEW pg_replication_slots AS diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 789a975409..f619fe9639 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -1117,6 +1117,35 @@ be_tls_get_peerdn_name(Port *port, char *ptr, size_t len) ptr[0] = '\0'; } +void +be_tls_get_issuer_name(Port *port, char *ptr, size_t len) +{ + if (port->peer) + strlcpy(ptr, X509_NAME_to_cstring(X509_get_issuer_name(port->peer)), len); + else + ptr[0] = '\0'; +} + +void +be_tls_get_peer_serial(Port *port, char *ptr, size_t len) +{ + if (port->peer) + { + ASN1_INTEGER *serial; + BIGNUM *b; + char *decimal; + + serial = X509_get_serialNumber(port->peer); + b = ASN1_INTEGER_to_BN(serial, NULL); + decimal = BN_bn2dec(b); + BN_free(b); + strlcpy(ptr, decimal, len); + OPENSSL_free(decimal); + } + else + ptr[0] = '\0'; +} + #ifdef HAVE_X509_GET_SIGNATURE_NID char * be_tls_get_certificate_hash(Port *port, size_t *len) diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index 3b9e86f770..e92cd0c44e 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -2906,7 +2906,9 @@ pgstat_bestart(void) beentry->st_sslstatus->ssl_compression = be_tls_get_compression(MyProcPort); strlcpy(beentry->st_sslstatus->ssl_version, be_tls_get_version(MyProcPort), NAMEDATALEN); strlcpy(beentry->st_sslstatus->ssl_cipher, be_tls_get_cipher(MyProcPort), NAMEDATALEN); - be_tls_get_peerdn_name(MyProcPort, beentry->st_sslstatus->ssl_clientdn, NAMEDATALEN); + be_tls_get_peerdn_name(MyProcPort, beentry->st_sslstatus->ssl_client_dn, NAMEDATALEN); + be_tls_get_peer_serial(MyProcPort, beentry->st_sslstatus->ssl_client_serial, NAMEDATALEN); + be_tls_get_issuer_name(MyProcPort, beentry->st_sslstatus->ssl_issuer_dn, NAMEDATALEN); } else { diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 20ebcfbcdf..b6ba856ebe 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -541,7 +541,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS) Datum pg_stat_get_activity(PG_FUNCTION_ARGS) { -#define PG_STAT_GET_ACTIVITY_COLS 24 +#define PG_STAT_GET_ACTIVITY_COLS 26 int num_backends = pgstat_fetch_stat_numbackends(); int curr_backend; int pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0); @@ -652,15 +652,29 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) values[20] = CStringGetTextDatum(beentry->st_sslstatus->ssl_cipher); values[21] = Int32GetDatum(beentry->st_sslstatus->ssl_bits); values[22] = BoolGetDatum(beentry->st_sslstatus->ssl_compression); - if (beentry->st_sslstatus->ssl_clientdn[0]) - values[23] = CStringGetTextDatum(beentry->st_sslstatus->ssl_clientdn); + + if (beentry->st_sslstatus->ssl_client_dn[0]) + values[23] = CStringGetTextDatum(beentry->st_sslstatus->ssl_client_dn); else nulls[23] = true; + + if (beentry->st_sslstatus->ssl_client_serial[0]) + values[24] = DirectFunctionCall3(numeric_in, + CStringGetDatum(beentry->st_sslstatus->ssl_client_serial), + ObjectIdGetDatum(InvalidOid), + Int32GetDatum(-1)); + else + nulls[24] = true; + + if (beentry->st_sslstatus->ssl_issuer_dn[0]) + values[25] = CStringGetTextDatum(beentry->st_sslstatus->ssl_issuer_dn); + else + nulls[25] = true; } else { values[18] = BoolGetDatum(false); /* ssl */ - nulls[19] = nulls[20] = nulls[21] = nulls[22] = nulls[23] = true; + nulls[19] = nulls[20] = nulls[21] = nulls[22] = nulls[23] = nulls[24] = nulls[25] = true; } /* Values only available to role member or pg_read_all_stats */ diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 3ecc2e12c3..b8de13f03b 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5058,9 +5058,9 @@ proname => 'pg_stat_get_activity', prorows => '100', proisstrict => 'f', proretset => 't', provolatile => 's', proparallel => 'r', prorettype => 'record', proargtypes => 'int4', - proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,bool,text}', - proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,sslcompression,sslclientdn}', + proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,bool,text,numeric,text}', + proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,sslcompression,ssl_client_dn,ssl_client_serial,ssl_issuer_dn}', prosrc => 'pg_stat_get_activity' }, { oid => '3318', descr => 'statistics: information about progress of backends running maintenance command', diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 7570649b5b..2f76e3b1be 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -259,6 +259,8 @@ extern bool be_tls_get_compression(Port *port); extern const char *be_tls_get_version(Port *port); extern const char *be_tls_get_cipher(Port *port); extern void be_tls_get_peerdn_name(Port *port, char *ptr, size_t len); +extern void be_tls_get_issuer_name(Port *port, char *ptr, size_t len); +extern void be_tls_get_peer_serial(Port *port, char *ptr, size_t len); /* * Get the server certificate hash for SCRAM channel binding type diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 0ce79489da..88a75fb798 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -950,15 +950,25 @@ typedef enum ProgressCommandType * * For each backend, we keep the SSL status in a separate struct, that * is only filled in if SSL is enabled. + * + * All char arrays must be null-terminated. */ typedef struct PgBackendSSLStatus { /* Information about SSL connection */ int ssl_bits; bool ssl_compression; - char ssl_version[NAMEDATALEN]; /* MUST be null-terminated */ - char ssl_cipher[NAMEDATALEN]; /* MUST be null-terminated */ - char ssl_clientdn[NAMEDATALEN]; /* MUST be null-terminated */ + char ssl_version[NAMEDATALEN]; + char ssl_cipher[NAMEDATALEN]; + char ssl_client_dn[NAMEDATALEN]; + + /* + * serial number is max "20 octets" per RFC 5280, so this size should be + * fine + */ + char ssl_client_serial[NAMEDATALEN]; + + char ssl_issuer_dn[NAMEDATALEN]; } PgBackendSSLStatus; diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index e384cd2279..2c8e21baa7 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1731,7 +1731,7 @@ pg_stat_activity| SELECT s.datid, s.backend_xmin, s.query, s.backend_type - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, sslcompression, sslclientdn) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, sslcompression, ssl_client_dn, ssl_client_serial, ssl_issuer_dn) LEFT JOIN pg_database d ON ((s.datid = d.oid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_all_indexes| SELECT c.oid AS relid, @@ -1863,7 +1863,7 @@ pg_stat_replication| SELECT s.pid, w.sync_priority, w.sync_state, w.reply_time - FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, sslcompression, sslclientdn) + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, sslcompression, ssl_client_dn, ssl_client_serial, ssl_issuer_dn) JOIN pg_stat_get_wal_senders() w(pid, state, sent_lsn, write_lsn, flush_lsn, replay_lsn, write_lag, flush_lag, replay_lag, sync_priority, sync_state, reply_time) ON ((s.pid = w.pid))) LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_ssl| SELECT s.pid, @@ -1872,8 +1872,10 @@ pg_stat_ssl| SELECT s.pid, s.sslcipher AS cipher, s.sslbits AS bits, s.sslcompression AS compression, - s.sslclientdn AS clientdn - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, sslcompression, sslclientdn); + s.ssl_client_dn AS client_dn, + s.ssl_client_serial AS client_serial, + s.ssl_issuer_dn AS issuer_dn + FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, sslcompression, ssl_client_dn, ssl_client_serial, ssl_issuer_dn); pg_stat_subscription| SELECT su.oid AS subid, su.subname, st.pid, diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index bbddb8e2b7..915007805e 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -315,8 +315,8 @@ '-d', "$common_connstr sslrootcert=invalid", '-c', "SELECT * FROM pg_stat_ssl WHERE pid = pg_backend_pid()" ], - qr{^pid,ssl,version,cipher,bits,compression,clientdn\n - ^\d+,t,TLSv[\d.]+,[\w-]+,\d+,f,_null_$}mx, + qr{^pid,ssl,version,cipher,bits,compression,client_dn,client_serial,issuer_dn\n + ^\d+,t,TLSv[\d.]+,[\w-]+,\d+,f,_null_,_null_,_null_$}mx, 'pg_stat_ssl view without client certificate'); ### Server-side tests. @@ -347,8 +347,8 @@ '-d', "$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key", '-c', "SELECT * FROM pg_stat_ssl WHERE pid = pg_backend_pid()" ], - qr{^pid,ssl,version,cipher,bits,compression,clientdn\n - ^\d+,t,TLSv[\d.]+,[\w-]+,\d+,f,/CN=ssltestuser$}mx, + qr{^pid,ssl,version,cipher,bits,compression,client_dn,client_serial,issuer_dn\n + ^\d+,t,TLSv[\d.]+,[\w-]+,\d+,f,/CN=ssltestuser,1,\Q/CN=Test CA for PostgreSQL SSL regression test client certs\E$}mx, 'pg_stat_ssl with client certificate'); # client key with wrong permissions -- 2.20.1
