pg_stat_ssl additions
During discussions of alternative SSL implementations, contrib/sslinfo
is usually mentioned as something that something needs to be done about.
I've looked into adapting some functionality from sslinfo into the
pg_stat_ssl view. These two facilities have a lot of overlap but seem
mostly oblivious to each other.
The attached patch series
- Adds a documentation link from sslinfo to pg_stat_ssl.
- Adds tests under src/test/ssl/ for the pg_stat_ssl view.
- Changes pg_stat_ssl.clientdn to be null if there is no client
certificate (as documented, but not implemented). (bug fix)
- 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".)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v1-0001-doc-Add-link-from-sslinfo-to-pg_stat_ssl.patchtext/plain; charset=UTF-8; name=v1-0001-doc-Add-link-from-sslinfo-to-pg_stat_ssl.patch; x-mac-creator=0; x-mac-type=0Download
From 9b039b0e111e5a782f8737e8d66f144e84efd1e4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 2 Oct 2018 12:17:22 +0200
Subject: [PATCH v1 1/4] doc: Add link from sslinfo to pg_stat_ssl
---
doc/src/sgml/sslinfo.sgml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/doc/src/sgml/sslinfo.sgml b/doc/src/sgml/sslinfo.sgml
index cda09aaafd..0fde0fc10e 100644
--- a/doc/src/sgml/sslinfo.sgml
+++ b/doc/src/sgml/sslinfo.sgml
@@ -14,6 +14,11 @@ <title>sslinfo</title>
will return NULL) if the current connection does not use SSL.
</para>
+ <para>
+ Some of the information available through this module can also be obtained
+ using the built-in system view <xref linkend="pg-stat-ssl-view"/>.
+ </para>
+
<para>
This extension won't build at all unless the installation was
configured with <literal>--with-openssl</literal>.
base-commit: a7a1b44516e7db89104c06440f759c2831fcb0ff
--
2.19.1
v1-0002-Add-tests-for-pg_stat_ssl-system-view.patchtext/plain; charset=UTF-8; name=v1-0002-Add-tests-for-pg_stat_ssl-system-view.patch; x-mac-creator=0; x-mac-type=0Download
From a770f60016effd9448ac7a55fba9fd26639b68e4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 21 Aug 2018 21:11:36 +0200
Subject: [PATCH v1 2/4] Add tests for pg_stat_ssl system view
---
src/test/ssl/t/001_ssltests.pl | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 2b875a3c95..52bda0b955 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -8,7 +8,7 @@
if ($ENV{with_openssl} eq 'yes')
{
- plan tests => 65;
+ plan tests => 71;
}
else
{
@@ -309,6 +309,16 @@
qr/SSL error/,
"does not connect with client-side CRL");
+# pg_stat_ssl
+command_like([
+ 'psql', '-X', '-A', '-F', ',', '-P', 'null=_null_',
+ '-d', $common_connstr,
+ '-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,$}mx,
+ 'pg_stat_ssl view without client certificate');
+
### Server-side tests.
###
### Test certificate authorization.
@@ -331,6 +341,16 @@
"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
"certificate authorization succeeds with correct client cert");
+# pg_stat_ssl
+command_like([
+ 'psql', '-X', '-A', '-F', ',', '-P', 'null=_null_',
+ '-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,
+ 'pg_stat_ssl with client certificate');
+
# client key with wrong permissions
test_connect_fails(
$common_connstr,
--
2.19.1
v1-0003-Fix-pg_stat_ssl.clientdn.patchtext/plain; charset=UTF-8; name=v1-0003-Fix-pg_stat_ssl.clientdn.patch; x-mac-creator=0; x-mac-type=0Download
From df7fcbc7c86c56d2663de733eb83617bbed80979 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 15 Oct 2018 15:28:30 +0200
Subject: [PATCH v1 3/4] Fix pg_stat_ssl.clientdn
Return null if there is no client certificate. This is how it has
always been documented, but in reality it returned an empty string.
---
src/backend/utils/adt/pgstatfuncs.c | 5 ++++-
src/test/ssl/t/001_ssltests.pl | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index e95e347184..565ca19074 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -652,7 +652,10 @@ 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);
- values[23] = CStringGetTextDatum(beentry->st_sslstatus->ssl_clientdn);
+ if (beentry->st_sslstatus->ssl_clientdn[0])
+ values[23] = CStringGetTextDatum(beentry->st_sslstatus->ssl_clientdn);
+ else
+ nulls[23] = true;
}
else
{
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 52bda0b955..5f04b5590b 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -316,7 +316,7 @@
'-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,$}mx,
+ ^\d+,t,TLSv[\d.]+,\w+,\d+,f,_null_$}mx,
'pg_stat_ssl view without client certificate');
### Server-side tests.
--
2.19.1
v1-0004-Add-more-columns-to-pg_stat_ssl.patchtext/plain; charset=UTF-8; name=v1-0004-Add-more-columns-to-pg_stat_ssl.patch; x-mac-creator=0; x-mac-type=0Download
From 0cbdf4a65acc646e9436f9821bdacdc671eae865 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 15 Oct 2018 15:39:54 +0200
Subject: [PATCH v1 4/4] Add more columns to pg_stat_ssl
Add columns clientserial and issuerdn to pg_stat_ssl. These allow
uniquely identifying the client certificate.
---
doc/src/sgml/monitoring.sgml | 18 ++++++++++++++++-
src/backend/catalog/system_views.sql | 4 +++-
src/backend/libpq/be-secure-openssl.c | 29 +++++++++++++++++++++++++++
src/backend/postmaster/pgstat.c | 2 ++
src/backend/utils/adt/pgstatfuncs.c | 18 +++++++++++++++--
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, 95 insertions(+), 18 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 0484cfa77a..ae8baf6b51 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -2198,9 +2198,25 @@ <title><structname>pg_stat_ssl</structname> View</title>
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>clientserial</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>issuerdn</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>clientdn</structfield>.</entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index a03b005f73..2a45b866f6 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -781,7 +781,9 @@ CREATE VIEW pg_stat_ssl AS
S.sslcipher AS cipher,
S.sslbits AS bits,
S.sslcompression AS compression,
- S.sslclientdn AS clientdn
+ S.sslclientdn AS clientdn,
+ S.sslclientserial AS clientserial,
+ S.sslissuerdn AS issuerdn
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 6a576572bb..e17bbfa6d7 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -1105,6 +1105,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 8a5b2b3b42..9acd9bd41b 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -2912,6 +2912,8 @@ pgstat_bestart(void)
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_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 565ca19074..feba490b68 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);
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 cff58ed2d8..63493d8298 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4996,9 +4996,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,sslclientdn,sslclientserial,sslissuerdn}',
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 eb8bba4ed8..d0c40bb60d 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -266,6 +266,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 d59c24ae23..ae7bfb9ade 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -949,15 +949,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_clientdn[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 735dd37acf..0df8bdfd55 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, sslclientdn, sslclientserial, sslissuerdn)
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,
@@ -1862,7 +1862,7 @@ pg_stat_replication| SELECT s.pid,
w.replay_lag,
w.sync_priority,
w.sync_state
- 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, sslclientdn, sslclientserial, sslissuerdn)
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) ON ((s.pid = w.pid)))
LEFT JOIN pg_authid u ON ((s.usesysid = u.oid)));
pg_stat_ssl| SELECT s.pid,
@@ -1871,8 +1871,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.sslclientdn AS clientdn,
+ s.sslclientserial AS clientserial,
+ s.sslissuerdn AS issuerdn
+ 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, sslclientserial, sslissuerdn);
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 5f04b5590b..9d51d4a743 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -315,8 +315,8 @@
'-d', $common_connstr,
'-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,clientdn,clientserial,issuerdn\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,clientdn,clientserial,issuerdn\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.19.1
Hello.
At Thu, 18 Oct 2018 00:05:15 +0200, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in <398754d8-6bb5-c5cf-e7b8-22e5f0983caf@2ndquadrant.com>
During discussions of alternative SSL implementations, contrib/sslinfo
is usually mentioned as something that something needs to be done about.
I've looked into adapting some functionality from sslinfo into the
pg_stat_ssl view. These two facilities have a lot of overlap but seem
mostly oblivious to each other.The attached patch series
- Adds a documentation link from sslinfo to pg_stat_ssl.
- Adds tests under src/test/ssl/ for the pg_stat_ssl view.
With this change the feature mapping between the two is as
follows.
ssl_is_used() : .ssl
ssl_version() : .version
ssl_cipher() : .cipher
(none) : .bits
(none) : .compression
ssl_client_cert_present() : .clientdn IS NOT NULL
ssl_client_serial() : .clientserial
ssl_client_dn() : .clientdn
ssl_issuer_dn() : .issuerdn
ssl_client_dn_field() : (none)
ssl_issuer_field() : (none)
ssl_extension_info() : (none)
# I couldn't create x509 v3 certs for uncertain reasons..
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.
I don't look through all the similar points but it might be
better fix it.
- 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.)
- 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?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
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
On Thu, Oct 18, 2018 at 11:05 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
- Adds tests under src/test/ssl/ for the pg_stat_ssl view.
Hi Peter,
Your new tests fail when run by cfbot (Ubuntu), and also on my Debian
buster machine, but pass on my FreeBSD 13-CURRENT box. I think the
problem is that you match cipher names with \w+, but some cipher names
sometimes (on some library versions?) have hyphens in them instead of
underscores.
https://travis-ci.org/postgresql-cfbot/postgresql/builds/459620336
--
Thomas Munro
http://www.enterprisedb.com
On 27/11/2018 00:29, Thomas Munro wrote:
On Thu, Oct 18, 2018 at 11:05 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:- Adds tests under src/test/ssl/ for the pg_stat_ssl view.
Hi Peter,
Your new tests fail when run by cfbot (Ubuntu), and also on my Debian
buster machine, but pass on my FreeBSD 13-CURRENT box. I think the
problem is that you match cipher names with \w+, but some cipher names
sometimes (on some library versions?) have hyphens in them instead of
underscores.
Right, this is a TLSv1.3 vs earlier thing. Updated patches attached.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v2-0001-doc-Add-link-from-sslinfo-to-pg_stat_ssl.patchtext/plain; charset=UTF-8; name=v2-0001-doc-Add-link-from-sslinfo-to-pg_stat_ssl.patch; x-mac-creator=0; x-mac-type=0Download
From cdef9395275d88d8d4ac718c6e20f2decc5f7f5f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 2 Oct 2018 12:17:22 +0200
Subject: [PATCH v2 1/4] doc: Add link from sslinfo to pg_stat_ssl
---
doc/src/sgml/sslinfo.sgml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/doc/src/sgml/sslinfo.sgml b/doc/src/sgml/sslinfo.sgml
index cda09aaafd..0fde0fc10e 100644
--- a/doc/src/sgml/sslinfo.sgml
+++ b/doc/src/sgml/sslinfo.sgml
@@ -14,6 +14,11 @@ <title>sslinfo</title>
will return NULL) if the current connection does not use SSL.
</para>
+ <para>
+ Some of the information available through this module can also be obtained
+ using the built-in system view <xref linkend="pg-stat-ssl-view"/>.
+ </para>
+
<para>
This extension won't build at all unless the installation was
configured with <literal>--with-openssl</literal>.
base-commit: 0f9cdd7dca694d487ab663d463b308919f591c02
--
2.19.1
v2-0002-Add-tests-for-pg_stat_ssl-system-view.patchtext/plain; charset=UTF-8; name=v2-0002-Add-tests-for-pg_stat_ssl-system-view.patch; x-mac-creator=0; x-mac-type=0Download
From 23592face6b3061f4abae6716523f3b45469b40b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 21 Aug 2018 21:11:36 +0200
Subject: [PATCH v2 2/4] Add tests for pg_stat_ssl system view
---
src/test/ssl/t/001_ssltests.pl | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 2b875a3c95..2bcbb1b42a 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -8,7 +8,7 @@
if ($ENV{with_openssl} eq 'yes')
{
- plan tests => 65;
+ plan tests => 71;
}
else
{
@@ -309,6 +309,16 @@
qr/SSL error/,
"does not connect with client-side CRL");
+# pg_stat_ssl
+command_like([
+ 'psql', '-X', '-A', '-F', ',', '-P', 'null=_null_',
+ '-d', $common_connstr,
+ '-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,$}mx,
+ 'pg_stat_ssl view without client certificate');
+
### Server-side tests.
###
### Test certificate authorization.
@@ -331,6 +341,16 @@
"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
"certificate authorization succeeds with correct client cert");
+# pg_stat_ssl
+command_like([
+ 'psql', '-X', '-A', '-F', ',', '-P', 'null=_null_',
+ '-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,
+ 'pg_stat_ssl with client certificate');
+
# client key with wrong permissions
test_connect_fails(
$common_connstr,
--
2.19.1
v2-0003-Fix-pg_stat_ssl.clientdn.patchtext/plain; charset=UTF-8; name=v2-0003-Fix-pg_stat_ssl.clientdn.patch; x-mac-creator=0; x-mac-type=0Download
From 3c02ddaa301ac7fc82ac54f43f3a706fbc877a51 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 15 Oct 2018 15:28:30 +0200
Subject: [PATCH v2 3/4] Fix pg_stat_ssl.clientdn
Return null if there is no client certificate. This is how it has
always been documented, but in reality it returned an empty string.
---
src/backend/utils/adt/pgstatfuncs.c | 5 ++++-
src/test/ssl/t/001_ssltests.pl | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index f955f1912a..5178e16a0c 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -652,7 +652,10 @@ 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);
- values[23] = CStringGetTextDatum(beentry->st_sslstatus->ssl_clientdn);
+ if (beentry->st_sslstatus->ssl_clientdn[0])
+ values[23] = CStringGetTextDatum(beentry->st_sslstatus->ssl_clientdn);
+ else
+ nulls[23] = true;
}
else
{
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 2bcbb1b42a..ff7a20307f 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -316,7 +316,7 @@
'-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,$}mx,
+ ^\d+,t,TLSv[\d.]+,[\w-]+,\d+,f,_null_$}mx,
'pg_stat_ssl view without client certificate');
### Server-side tests.
--
2.19.1
v2-0004-Add-more-columns-to-pg_stat_ssl.patchtext/plain; charset=UTF-8; name=v2-0004-Add-more-columns-to-pg_stat_ssl.patch; x-mac-creator=0; x-mac-type=0Download
From 7dd02e6843f035f312a6e65f1dc62300eb730025 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 15 Oct 2018 15:39:54 +0200
Subject: [PATCH v2 4/4] Add more columns to pg_stat_ssl
Add columns clientserial and issuerdn to pg_stat_ssl. These allow
uniquely identifying the client certificate.
---
doc/src/sgml/monitoring.sgml | 18 ++++++++++++++++-
src/backend/catalog/system_views.sql | 4 +++-
src/backend/libpq/be-secure-openssl.c | 29 +++++++++++++++++++++++++++
src/backend/postmaster/pgstat.c | 2 ++
src/backend/utils/adt/pgstatfuncs.c | 18 +++++++++++++++--
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, 95 insertions(+), 18 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 7aada14417..b384589113 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -2206,9 +2206,25 @@ <title><structname>pg_stat_ssl</structname> View</title>
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>clientserial</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>issuerdn</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>clientdn</structfield>.</entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 715995dd88..3340fce2c6 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -781,7 +781,9 @@ CREATE VIEW pg_stat_ssl AS
S.sslcipher AS cipher,
S.sslbits AS bits,
S.sslcompression AS compression,
- S.sslclientdn AS clientdn
+ S.sslclientdn AS clientdn,
+ S.sslclientserial AS clientserial,
+ S.sslissuerdn AS issuerdn
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 6955d7230c..378a63b279 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 8676088e57..ea4b10139c 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -2908,6 +2908,8 @@ pgstat_bestart(void)
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_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 5178e16a0c..cc2351d09e 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);
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 034a41eb55..89bd35ebe4 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5005,9 +5005,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,sslclientdn,sslclientserial,sslissuerdn}',
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 b2c59df54e..6e599e9fcf 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 f1c10d16b8..83823a4cec 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_clientdn[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 735dd37acf..0df8bdfd55 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, sslclientdn, sslclientserial, sslissuerdn)
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,
@@ -1862,7 +1862,7 @@ pg_stat_replication| SELECT s.pid,
w.replay_lag,
w.sync_priority,
w.sync_state
- 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, sslclientdn, sslclientserial, sslissuerdn)
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) ON ((s.pid = w.pid)))
LEFT JOIN pg_authid u ON ((s.usesysid = u.oid)));
pg_stat_ssl| SELECT s.pid,
@@ -1871,8 +1871,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.sslclientdn AS clientdn,
+ s.sslclientserial AS clientserial,
+ s.sslissuerdn AS issuerdn
+ 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, sslclientserial, sslissuerdn);
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 ff7a20307f..ec7073f340 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -315,8 +315,8 @@
'-d', $common_connstr,
'-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,clientdn,clientserial,issuerdn\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,clientdn,clientserial,issuerdn\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.19.1
On 20/11/2018 22:41, Peter Eisentraut wrote:
- 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.
Any thoughts from others about whether to rename clientdn to client_dn
to allow better naming of the new fields?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Nov 28, 2018 at 06:31:59PM +0100, Peter Eisentraut wrote:
On 20/11/2018 22:41, Peter Eisentraut wrote:
- 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.
Any thoughts from others about whether to rename clientdn to client_dn
to allow better naming of the new fields?
Makes sense. The SSL acronyms can get very complex.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +
Greetings,
* Bruce Momjian (bruce@momjian.us) wrote:
On Wed, Nov 28, 2018 at 06:31:59PM +0100, Peter Eisentraut wrote:
On 20/11/2018 22:41, Peter Eisentraut wrote:
- 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.
Any thoughts from others about whether to rename clientdn to client_dn
to allow better naming of the new fields?Makes sense. The SSL acronyms can get very complex.
Agreed.
Thanks!
Stephen
Bruce Momjian <bruce@momjian.us> writes:
On Wed, Nov 28, 2018 at 06:31:59PM +0100, Peter Eisentraut wrote:
Any thoughts from others about whether to rename clientdn to client_dn
to allow better naming of the new fields?
Makes sense. The SSL acronyms can get very complex.
+1. It seems unlikely to me that there are very many applications out
there that have references to this view, so we can probably get away
with rationalizing the field names.
regards, tom lane
As we do make significant(?) use of the ssl-ish stuff - though not of the views - should I weigh in?
We do make some not-insignificant use of the sslinfo data, but I see little issue with adding underscores. In fact, ssl-ville is replete with underscores anyway.
Further, I’m not sure exposing details about Cert Issuer, etc. to non-privileged users is much of an issue. For the most part, in most use cases, ‘users’ should/would want to know what entity is the issuer. If we’re talking about client certs, most of this is readily readable anyway, no?
More from PostgreSQL == better.
Lou Picciano
PS - How you guys doin’? It’s been a while.
Show quoted text
On Nov 28, 2018, at 4:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Bruce Momjian <bruce@momjian.us> writes:
On Wed, Nov 28, 2018 at 06:31:59PM +0100, Peter Eisentraut wrote:
Any thoughts from others about whether to rename clientdn to client_dn
to allow better naming of the new fields?Makes sense. The SSL acronyms can get very complex.
+1. It seems unlikely to me that there are very many applications out
there that have references to this view, so we can probably get away
with rationalizing the field names.regards, tom lane
On 29/11/2018 01:27, Lou Picciano wrote:
Further, I’m not sure exposing details about Cert Issuer, etc. to
non-privileged users is much of an issue. For the most part, in most use
cases, ‘users’ should//would/ want to know what entity is the issuer. If
we’re talking about client certs, most of this is readily readable
anyway, no?
The debate is whether an unprivileged user should be able to read the
SSL information of *other* users' connections.
My opinion is no.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Updated patch with the renamed columns.
On 29/11/2018 14:49, Peter Eisentraut wrote:
On 29/11/2018 01:27, Lou Picciano wrote:
Further, I’m not sure exposing details about Cert Issuer, etc. to
non-privileged users is much of an issue. For the most part, in most use
cases, ‘users’ should//would/ want to know what entity is the issuer. If
we’re talking about client certs, most of this is readily readable
anyway, no?The debate is whether an unprivileged user should be able to read the
SSL information of *other* users' connections.My opinion is no.
I propose to address this as a separate patch later on.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v3-0001-doc-Add-link-from-sslinfo-to-pg_stat_ssl.patchtext/plain; charset=UTF-8; name=v3-0001-doc-Add-link-from-sslinfo-to-pg_stat_ssl.patch; x-mac-creator=0; x-mac-type=0Download
From 645b3e7d00de03bbf55d772bb8cabd4708aed684 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 2 Oct 2018 12:17:22 +0200
Subject: [PATCH v3 1/4] doc: Add link from sslinfo to pg_stat_ssl
---
doc/src/sgml/sslinfo.sgml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/doc/src/sgml/sslinfo.sgml b/doc/src/sgml/sslinfo.sgml
index cda09aaafd..0fde0fc10e 100644
--- a/doc/src/sgml/sslinfo.sgml
+++ b/doc/src/sgml/sslinfo.sgml
@@ -14,6 +14,11 @@ <title>sslinfo</title>
will return NULL) if the current connection does not use SSL.
</para>
+ <para>
+ Some of the information available through this module can also be obtained
+ using the built-in system view <xref linkend="pg-stat-ssl-view"/>.
+ </para>
+
<para>
This extension won't build at all unless the installation was
configured with <literal>--with-openssl</literal>.
base-commit: 7d4524aed3a4720086b2914ec22f1bbbc857ac44
--
2.19.2
v3-0002-Add-tests-for-pg_stat_ssl-system-view.patchtext/plain; charset=UTF-8; name=v3-0002-Add-tests-for-pg_stat_ssl-system-view.patch; x-mac-creator=0; x-mac-type=0Download
From 67730a3130709d3a262b073d023fb4f75f54232f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 21 Aug 2018 21:11:36 +0200
Subject: [PATCH v3 2/4] Add tests for pg_stat_ssl system view
---
src/test/ssl/t/001_ssltests.pl | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 2b875a3c95..2bcbb1b42a 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -8,7 +8,7 @@
if ($ENV{with_openssl} eq 'yes')
{
- plan tests => 65;
+ plan tests => 71;
}
else
{
@@ -309,6 +309,16 @@
qr/SSL error/,
"does not connect with client-side CRL");
+# pg_stat_ssl
+command_like([
+ 'psql', '-X', '-A', '-F', ',', '-P', 'null=_null_',
+ '-d', $common_connstr,
+ '-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,$}mx,
+ 'pg_stat_ssl view without client certificate');
+
### Server-side tests.
###
### Test certificate authorization.
@@ -331,6 +341,16 @@
"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
"certificate authorization succeeds with correct client cert");
+# pg_stat_ssl
+command_like([
+ 'psql', '-X', '-A', '-F', ',', '-P', 'null=_null_',
+ '-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,
+ 'pg_stat_ssl with client certificate');
+
# client key with wrong permissions
test_connect_fails(
$common_connstr,
--
2.19.2
v3-0003-Fix-pg_stat_ssl.clientdn.patchtext/plain; charset=UTF-8; name=v3-0003-Fix-pg_stat_ssl.clientdn.patch; x-mac-creator=0; x-mac-type=0Download
From b830a4c99561ac73ca0f5446758dd4a9827e51bb Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 15 Oct 2018 15:28:30 +0200
Subject: [PATCH v3 3/4] Fix pg_stat_ssl.clientdn
Return null if there is no client certificate. This is how it has
always been documented, but in reality it returned an empty string.
---
src/backend/utils/adt/pgstatfuncs.c | 5 ++++-
src/test/ssl/t/001_ssltests.pl | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index f955f1912a..5178e16a0c 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -652,7 +652,10 @@ 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);
- values[23] = CStringGetTextDatum(beentry->st_sslstatus->ssl_clientdn);
+ if (beentry->st_sslstatus->ssl_clientdn[0])
+ values[23] = CStringGetTextDatum(beentry->st_sslstatus->ssl_clientdn);
+ else
+ nulls[23] = true;
}
else
{
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 2bcbb1b42a..ff7a20307f 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -316,7 +316,7 @@
'-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,$}mx,
+ ^\d+,t,TLSv[\d.]+,[\w-]+,\d+,f,_null_$}mx,
'pg_stat_ssl view without client certificate');
### Server-side tests.
--
2.19.2
v3-0004-Add-more-columns-to-pg_stat_ssl.patchtext/plain; charset=UTF-8; name=v3-0004-Add-more-columns-to-pg_stat_ssl.patch; x-mac-creator=0; x-mac-type=0Download
From d4c15d6d185a69167e81d0f35f9cda7744cf3cdc Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Sat, 1 Dec 2018 14:37:25 +0100
Subject: [PATCH v3 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.
---
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 7aada14417..416f06f798 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -2200,15 +2200,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 715995dd88..d24f5e36a2 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -781,7 +781,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 6955d7230c..378a63b279 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 8676088e57..7d2878d73f 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -2907,7 +2907,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 5178e16a0c..d1dc193ee2 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 034a41eb55..17aa0b9375 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5005,9 +5005,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 b2c59df54e..6e599e9fcf 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 f1c10d16b8..5a527592de 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 735dd37acf..e8aec89166 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,
@@ -1862,7 +1862,7 @@ pg_stat_replication| SELECT s.pid,
w.replay_lag,
w.sync_priority,
w.sync_state
- 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) ON ((s.pid = w.pid)))
LEFT JOIN pg_authid u ON ((s.usesysid = u.oid)));
pg_stat_ssl| SELECT s.pid,
@@ -1871,8 +1871,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 ff7a20307f..7ee39ffa01 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -315,8 +315,8 @@
'-d', $common_connstr,
'-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.19.2
Updated patches for some merge conflicts
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v4-0001-doc-Add-link-from-sslinfo-to-pg_stat_ssl.patchtext/plain; charset=UTF-8; name=v4-0001-doc-Add-link-from-sslinfo-to-pg_stat_ssl.patch; x-mac-creator=0; x-mac-type=0Download
From 296d8d19cc49c2e7a6e8489a6d21cd6a182f2686 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 2 Oct 2018 12:17:22 +0200
Subject: [PATCH v4 1/4] doc: Add link from sslinfo to pg_stat_ssl
---
doc/src/sgml/sslinfo.sgml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/doc/src/sgml/sslinfo.sgml b/doc/src/sgml/sslinfo.sgml
index cda09aaafd..0fde0fc10e 100644
--- a/doc/src/sgml/sslinfo.sgml
+++ b/doc/src/sgml/sslinfo.sgml
@@ -14,6 +14,11 @@ <title>sslinfo</title>
will return NULL) if the current connection does not use SSL.
</para>
+ <para>
+ Some of the information available through this module can also be obtained
+ using the built-in system view <xref linkend="pg-stat-ssl-view"/>.
+ </para>
+
<para>
This extension won't build at all unless the installation was
configured with <literal>--with-openssl</literal>.
base-commit: 7170268efd511cb43bee49cd7963216a3f228648
--
2.20.1
v4-0002-Add-tests-for-pg_stat_ssl-system-view.patchtext/plain; charset=UTF-8; name=v4-0002-Add-tests-for-pg_stat_ssl-system-view.patch; x-mac-creator=0; x-mac-type=0Download
From a4b7d9b25eab6d677e388860b4fee0f94c2fb93e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 21 Aug 2018 21:11:36 +0200
Subject: [PATCH v4 2/4] Add tests for pg_stat_ssl system view
---
src/test/ssl/t/001_ssltests.pl | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 2b875a3c95..2bcbb1b42a 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -8,7 +8,7 @@
if ($ENV{with_openssl} eq 'yes')
{
- plan tests => 65;
+ plan tests => 71;
}
else
{
@@ -309,6 +309,16 @@
qr/SSL error/,
"does not connect with client-side CRL");
+# pg_stat_ssl
+command_like([
+ 'psql', '-X', '-A', '-F', ',', '-P', 'null=_null_',
+ '-d', $common_connstr,
+ '-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,$}mx,
+ 'pg_stat_ssl view without client certificate');
+
### Server-side tests.
###
### Test certificate authorization.
@@ -331,6 +341,16 @@
"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
"certificate authorization succeeds with correct client cert");
+# pg_stat_ssl
+command_like([
+ 'psql', '-X', '-A', '-F', ',', '-P', 'null=_null_',
+ '-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,
+ 'pg_stat_ssl with client certificate');
+
# client key with wrong permissions
test_connect_fails(
$common_connstr,
--
2.20.1
v4-0003-Fix-pg_stat_ssl.clientdn.patchtext/plain; charset=UTF-8; name=v4-0003-Fix-pg_stat_ssl.clientdn.patch; x-mac-creator=0; x-mac-type=0Download
From 12ac9f7e068ae6a762edff0e2c9706a6e8528986 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 15 Oct 2018 15:28:30 +0200
Subject: [PATCH v4 3/4] Fix pg_stat_ssl.clientdn
Return null if there is no client certificate. This is how it has
always been documented, but in reality it returned an empty string.
---
src/backend/utils/adt/pgstatfuncs.c | 5 ++++-
src/test/ssl/t/001_ssltests.pl | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 053bb73863..20ebcfbcdf 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -652,7 +652,10 @@ 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);
- values[23] = CStringGetTextDatum(beentry->st_sslstatus->ssl_clientdn);
+ if (beentry->st_sslstatus->ssl_clientdn[0])
+ values[23] = CStringGetTextDatum(beentry->st_sslstatus->ssl_clientdn);
+ else
+ nulls[23] = true;
}
else
{
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 2bcbb1b42a..ff7a20307f 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -316,7 +316,7 @@
'-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,$}mx,
+ ^\d+,t,TLSv[\d.]+,[\w-]+,\d+,f,_null_$}mx,
'pg_stat_ssl view without client certificate');
### Server-side tests.
--
2.20.1
v4-0004-Add-more-columns-to-pg_stat_ssl.patchtext/plain; charset=UTF-8; name=v4-0004-Add-more-columns-to-pg_stat_ssl.patch; x-mac-creator=0; x-mac-type=0Download
From 970b02167443f4222e632e92692a1dd8eb63df78 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Sat, 1 Dec 2018 14:37:25 +0100
Subject: [PATCH v4 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.
---
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 d50351547f..f1e9af23fa 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -2907,7 +2907,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 3233888e35..8246732ad3 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 313ca5f3c3..54c9c7120f 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 ff7a20307f..7ee39ffa01 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -315,8 +315,8 @@
'-d', $common_connstr,
'-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
Hello, thank you for the new version.
At Fri, 4 Jan 2019 00:25:57 +0100, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in <3f8a7a73-cebe-016f-49e3-853733f8baf2@2ndquadrant.com>
Updated patches for some merge conflicts
0001: Seems perfect to me.
0002:
The test 54-56 of 001_ssltest.pl failed, which succeeded before
applying 0002. Seems to need to use another user.
# Failed test 'pg_stat_ssl view without client certificate: no stderr'
# at t/001_ssltests.pl line 313.
# got: 'psql: SSL error: certificate verify failed
# FATAL: no pg_hba.conf entry for host "127.0.0.1", user "ssltestuser", database "trustdb", SSL off
# '
If this is not specific to my environment, the connevcion string
at line 313 of 001_ssltests.pl needs sslrootcert setting (, which
is feeded to test_connect_ok/fails() via $connstr, not via
$common_connstr).
0003: Looks fine to me.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Sorry, I left a garbage.
At Mon, 28 Jan 2019 17:14:00 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20190128.171400.111796002.horiguchi.kyotaro@lab.ntt.co.jp>
Hello, thank you for the new version.
At Fri, 4 Jan 2019 00:25:57 +0100, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in <3f8a7a73-cebe-016f-49e3-853733f8baf2@2ndquadrant.com>
Updated patches for some merge conflicts
0001: Seems perfect to me.
0002:
The test 54-56 of 001_ssltest.pl failed, which succeeded before
applying 0002. Seems to need to use another user.
"Seems to need to use another user." is useles leftover.
# Failed test 'pg_stat_ssl view without client certificate: no stderr'
# at t/001_ssltests.pl line 313.
# got: 'psql: SSL error: certificate verify failed
# FATAL: no pg_hba.conf entry for host "127.0.0.1", user "ssltestuser", database "trustdb", SSL off
# 'If this is not specific to my environment, the connevcion string
at line 313 of 001_ssltests.pl needs sslrootcert setting (, which
is feeded to test_connect_ok/fails() via $connstr, not via
$common_connstr).0003: Looks fine to me.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On 28/01/2019 09:14, Kyotaro HORIGUCHI wrote:
0002:
The test 54-56 of 001_ssltest.pl failed, which succeeded before
applying 0002. Seems to need to use another user.# Failed test 'pg_stat_ssl view without client certificate: no stderr'
# at t/001_ssltests.pl line 313.
# got: 'psql: SSL error: certificate verify failed
# FATAL: no pg_hba.conf entry for host "127.0.0.1", user "ssltestuser", database "trustdb", SSL off
# 'If this is not specific to my environment, the connevcion string
at line 313 of 001_ssltests.pl needs sslrootcert setting (, which
is feeded to test_connect_ok/fails() via $connstr, not via
$common_connstr).
This is strange. The tests work for me, and also on the cfbot. The
pg_hba.conf method is "trust", and there is nothing that should make it
do certificate verification for this test. Do you have have any PGSSL*
environment variables set perhaps? An interesting OpenSSL version or
configuration perhaps?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
At Mon, 28 Jan 2019 14:53:43 +0100, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in <24783370-5acd-e0f3-8eb7-7f42ff2a026d@2ndquadrant.com>
On 28/01/2019 09:14, Kyotaro HORIGUCHI wrote:
0002:
The test 54-56 of 001_ssltest.pl failed, which succeeded before
applying 0002. Seems to need to use another user.# Failed test 'pg_stat_ssl view without client certificate: no stderr'
# at t/001_ssltests.pl line 313.
# got: 'psql: SSL error: certificate verify failed
# FATAL: no pg_hba.conf entry for host "127.0.0.1", user "ssltestuser", database "trustdb", SSL off
# 'If this is not specific to my environment, the connevcion string
at line 313 of 001_ssltests.pl needs sslrootcert setting (, which
is feeded to test_connect_ok/fails() via $connstr, not via
$common_connstr).This is strange. The tests work for me, and also on the cfbot. The
Agreed. It seemed so also to me.
pg_hba.conf method is "trust", and there is nothing that should make it
do certificate verification for this test. Do you have have any PGSSL*
environment variables set perhaps? An interesting OpenSSL version or
configuration perhaps?
Some further investigation told me that the file
~/.postgresql/root.cert was the culprit.
When initializing SSL context, it picks up the root certificate
from my home directory, not in test installation and I had one
there. It is not based on $HOME but pwent so it is unchangeable
(and it is the right design for the purpose).
sslcert, sslkey, sslrootcert and sslcrl are in the same
characteristic so they should be set to invalid value (namely
"invalid") if not used.
The attached diff file on top of 0002 adds a new variable
$def_connstr for the properties above and some other variables,
then uses it as the first part of $common_connstr.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
dont_use_default_cert_files.patchtext/x-patch; charset=us-asciiDownload
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 2bcbb1b42a..aa0692cc47 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -25,6 +25,13 @@ my $SERVERHOSTADDR = '127.0.0.1';
# Allocation of base connection string shared among multiple tests.
my $common_connstr;
+# ssl-related properties may defautly set to the files in the users'
+# environment. Explicitly provide them a value so that they don't
+# point a valid file accidentially. Some other common properties are
+# set here together.
+# Attach this at the head of $common_connstr.
+my $def_connstr = "user=ssltestuser dbname=trustdb sslcert=invalid sslkey=invalid sslrootcert=invalid sslcrl=invalid ";
+
# The client's private key must not be world-readable, so take a copy
# of the key stored in the code tree and update its permissions.
copy("ssl/client.key", "ssl/client_tmp.key");
@@ -93,7 +100,7 @@ note "running client tests";
switch_server_cert($node, 'server-cn-only');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
+ $def_connstr."hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
# The server should not accept non-SSL connections.
test_connect_fails(
@@ -185,7 +192,7 @@ test_connect_ok(
# Check that connecting with verify-full fails, when the hostname doesn't
# match the hostname in the server's certificate.
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
+ $def_connstr."sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
test_connect_ok(
$common_connstr,
@@ -205,7 +212,7 @@ test_connect_fails(
switch_server_cert($node, 'server-multiple-alt-names');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
+ $def_connstr."sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
test_connect_ok(
$common_connstr,
@@ -236,7 +243,7 @@ test_connect_fails(
switch_server_cert($node, 'server-single-alt-name');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
+ $def_connstr."sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
test_connect_ok(
$common_connstr,
@@ -260,7 +267,7 @@ test_connect_fails(
switch_server_cert($node, 'server-cn-and-alt-names');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
+ $def_connstr."sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
test_connect_ok(
$common_connstr,
@@ -280,7 +287,7 @@ test_connect_fails(
# not a very sensible certificate, but libpq should handle it gracefully.
switch_server_cert($node, 'server-no-names');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
+ $def_connstr."sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
test_connect_ok(
$common_connstr,
@@ -296,7 +303,7 @@ test_connect_fails(
switch_server_cert($node, 'server-revoked');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
+ $def_connstr."hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
# Without the CRL, succeeds. With it, fails.
test_connect_ok(
@@ -326,7 +333,7 @@ command_like([
note "running server tests";
$common_connstr =
- "sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR";
+ $def_connstr."sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR";
# no client cert
test_connect_fails(
@@ -376,7 +383,7 @@ test_connect_fails(
# intermediate client_ca.crt is provided by client, and isn't in server's ssl_ca_file
switch_server_cert($node, 'server-cn-only', 'root_ca');
$common_connstr =
- "user=ssltestuser dbname=certdb sslkey=ssl/client_tmp.key sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
+ $def_connstr."dbname=certdb sslkey=ssl/client_tmp.key sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
test_connect_ok(
$common_connstr,
On Tue, Jan 29, 2019 at 12:18:29PM +0900, Kyotaro HORIGUCHI wrote:
At Mon, 28 Jan 2019 14:53:43 +0100, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in <24783370-5acd-e0f3-8eb7-7f42ff2a026d@2ndquadrant.com>
This is strange. The tests work for me, and also on the cfbot. The
Agreed. It seemed so also to me.
The tests look sane to me for what it's worth.
When initializing SSL context, it picks up the root certificate
from my home directory, not in test installation and I had one
there. It is not based on $HOME but pwent so it is unchangeable
(and it is the right design for the purpose).
Oops. I agree that the tests ought to be as much isolated as
possible, without optionally fetching things depending on the user
environment.
+# ssl-related properties may defautly set to the files in the users' +# environment. Explicitly provide them a value so that they don't +# point a valid file accidentially. Some other common properties are +# set here together. +# Attach this at the head of $common_connstr. +my $def_connstr = "user=ssltestuser dbname=trustdb sslcert=invalid sslkey=invalid sslrootcert=invalid sslcrl=invalid "; +
Maybe it is better to just put that in test_connect_ok and
test_connect_fails for only the SSL parameters? I don't see much
point in enforcing dbname and user.
# The server should not accept non-SSL connections. test_connect_fails( @@ -185,7 +192,7 @@ test_connect_ok( # Check that connecting with verify-full fails, when the hostname doesn't # match the hostname in the server's certificate. $common_connstr = - "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR"; + $def_connstr."sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
I think this is bad, inconsistent style. Adding the variable within
the quoted section is fine, as much as moving SERVERHOSTADDR out. But
mixing styles is not.
--
Michael
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
Attachments:
v5-0004-Add-more-columns-to-pg_stat_ssl.patchtext/plain; charset=UTF-8; name=v5-0004-Add-more-columns-to-pg_stat_ssl.patch; x-mac-creator=0; x-mac-type=0Download
From 5168e09ae3ee0c61cdd40ababeee7a6e45bdcb01 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@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.kyotaro@lab.ntt.co.jp>
Discussion: https://www.postgresql.org/message-id/flat/398754d8-6bb5-c5cf-e7b8-22e5f0983caf@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
At Tue, 29 Jan 2019 13:50:17 +0900, Michael Paquier <michael@paquier.xyz> wrote in <20190129045017.GC3121@paquier.xyz>
On Tue, Jan 29, 2019 at 12:18:29PM +0900, Kyotaro HORIGUCHI wrote:
At Mon, 28 Jan 2019 14:53:43 +0100, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in <24783370-5acd-e0f3-8eb7-7f42ff2a026d@2ndquadrant.com>
This is strange. The tests work for me, and also on the cfbot. The
Agreed. It seemed so also to me.
The tests look sane to me for what it's worth.
When initializing SSL context, it picks up the root certificate
from my home directory, not in test installation and I had one
there. It is not based on $HOME but pwent so it is unchangeable
(and it is the right design for the purpose).Oops. I agree that the tests ought to be as much isolated as
possible, without optionally fetching things depending on the user
environment.+# ssl-related properties may defautly set to the files in the users' +# environment. Explicitly provide them a value so that they don't +# point a valid file accidentially. Some other common properties are +# set here together. +# Attach this at the head of $common_connstr. +my $def_connstr = "user=ssltestuser dbname=trustdb sslcert=invalid sslkey=invalid sslrootcert=invalid sslcrl=invalid "; +Maybe it is better to just put that in test_connect_ok and
test_connect_fails for only the SSL parameters? I don't see much
point in enforcing dbname and user.
Hmm. It is the first thing I did for the issue. Anyway..
Of course dbname and user are not necessary to fix it. Ok, I
attached two patches. The first applies on the current master and
prevent psql from loading ssl-related files from out of testing
environment. The second applies on the 0002 patch and prevent the
patch from loading a wrong sslrootcert.
# The server should not accept non-SSL connections. test_connect_fails( @@ -185,7 +192,7 @@ test_connect_ok( # Check that connecting with verify-full fails, when the hostname doesn't # match the hostname in the server's certificate. $common_connstr = - "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR"; + $def_connstr."sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";I think this is bad, inconsistent style. Adding the variable within
the quoted section is fine, as much as moving SERVERHOSTADDR out. But
mixing styles is not.
I Understood. Thanks for the suggestion.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
0001-Prevent-from-loading-ssl-files-out-of-testing-enviro.patchtext/x-patch; charset=us-asciiDownload
From a3cb52ba165074db1d2910faa8e58030d84fa7c9 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Tue, 29 Jan 2019 21:06:58 +0900
Subject: [PATCH 1/4] Prevent from loading ssl files out of testing
environment.
SSL test script can let psql load SSL-related files out of the testing
environemnt. Fix it by explicitly setting sslcert, sslrootcert and
sslcrl when they are not used. sslkey doesn't need the fix because it
is loaded only when sslcert is loadded.
---
src/test/ssl/t/001_ssltests.pl | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 2b875a3c95..114541590d 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -93,7 +93,7 @@ note "running client tests";
switch_server_cert($node, 'server-cn-only');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
+ "user=ssltestuser dbname=trustdb sslcert=invalid sslcrl=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
# The server should not accept non-SSL connections.
test_connect_fails(
@@ -185,7 +185,7 @@ test_connect_ok(
# Check that connecting with verify-full fails, when the hostname doesn't
# match the hostname in the server's certificate.
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
+ "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt sslcrl=invalid hostaddr=$SERVERHOSTADDR";
test_connect_ok(
$common_connstr,
@@ -205,7 +205,7 @@ test_connect_fails(
switch_server_cert($node, 'server-multiple-alt-names');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
+ "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt sslcrl=invalid hostaddr=$SERVERHOSTADDR sslmode=verify-full";
test_connect_ok(
$common_connstr,
@@ -236,7 +236,7 @@ test_connect_fails(
switch_server_cert($node, 'server-single-alt-name');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
+ "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt sslcrl=invalid hostaddr=$SERVERHOSTADDR sslmode=verify-full";
test_connect_ok(
$common_connstr,
@@ -260,7 +260,7 @@ test_connect_fails(
switch_server_cert($node, 'server-cn-and-alt-names');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR sslmode=verify-full";
+ "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt sslcrl=invalid hostaddr=$SERVERHOSTADDR sslmode=verify-full";
test_connect_ok(
$common_connstr,
@@ -280,7 +280,7 @@ test_connect_fails(
# not a very sensible certificate, but libpq should handle it gracefully.
switch_server_cert($node, 'server-no-names');
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
+ "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt sslcrl=invalid hostaddr=$SERVERHOSTADDR";
test_connect_ok(
$common_connstr,
@@ -301,7 +301,7 @@ $common_connstr =
# Without the CRL, succeeds. With it, fails.
test_connect_ok(
$common_connstr,
- "sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca",
+ "sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=invalid",
"connects without client-side CRL");
test_connect_fails(
$common_connstr,
@@ -316,7 +316,7 @@ test_connect_fails(
note "running server tests";
$common_connstr =
- "sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR";
+ "sslrootcert=ssl/root+server_ca.crt sslcrl=invalid sslmode=require dbname=certdb hostaddr=$SERVERHOSTADDR";
# no client cert
test_connect_fails(
@@ -356,7 +356,7 @@ test_connect_fails(
# intermediate client_ca.crt is provided by client, and isn't in server's ssl_ca_file
switch_server_cert($node, 'server-cn-only', 'root_ca');
$common_connstr =
- "user=ssltestuser dbname=certdb sslkey=ssl/client_tmp.key sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
+ "user=ssltestuser dbname=certdb sslkey=ssl/client_tmp.key sslrootcert=ssl/root+server_ca.crt sslcrl=invalid hostaddr=$SERVERHOSTADDR";
test_connect_ok(
$common_connstr,
--
2.16.3
0002-Don-t-load-wrong-sslrootcert.patchtext/x-patch; charset=us-asciiDownload
From 47ab02f905fa43479c8e44e89b857b311e175363 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Tue, 29 Jan 2019 21:00:29 +0900
Subject: [PATCH 4/4] Don't load wrong sslrootcert
Fixes the pg_stat_ssl patch so that 0002 don't load sslrootcert out of
testing environment.
---
src/test/ssl/t/001_ssltests.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index dea25073b5..7bf9834964 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -312,7 +312,7 @@ test_connect_fails(
# pg_stat_ssl
command_like([
'psql', '-X', '-A', '-F', ',', '-P', 'null=_null_',
- '-d', $common_connstr,
+ '-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
--
2.16.3
On 29/01/2019 13:11, Peter Eisentraut wrote:
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.
I have committed the rest of this.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services