Re: [HACKERS] SSL information view
On Fri, Apr 10, 2015 at 2:14 PM, Michael Paquier michael.paqu...@gmail.com
wrote:
On Fri, Apr 10, 2015 at 6:39 PM, Magnus Hagander wrote:
+typedef struct PgBackendSSLStatus
+{
+/* Information about SSL connection */
+int ssl_bits;
+boolssl_compression;
+charssl_version[NAMEDATALEN]; /* MUST be
null-terminated */
+charssl_cipher[NAMEDATALEN]; /* MUST be
null-terminated */
+charssl_clientdn[NAMEDATALEN]; /* MUST be
null-terminated */
+} PgBackendSSLStatus;
git diff is showing in red here, spaces should be replaced with a tab.
Ugh. Fixed. One too many copy/pastes I think.
You forgot one here:
+/* Information about SSL connection */
In other news, I have now fixed my git to show these things to be again. It
used to do that, but I broke it :)
Thanks!
Except for those style comments (feel free to ignore them), I tested
the patch and it is doing what it claims. As I don't have more
comments, let's switch that to Ready for Committer then...
Ok. Thanks - and patch applied!
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Re: [HACKERS] SSL information view
On Thu, Apr 9, 2015 at 3:20 PM, Andres Freund and...@anarazel.de wrote: Hi, On 2015-04-09 13:31:55 +0200, Magnus Hagander wrote: + row + entrystructnamepg_stat_ssl/indextermprimarypg_stat_ssl/primary/indexterm/entry + entryOne row per connection (regular and replication), showing information about +SSL used on this connection. +See xref linkend=pg-stat-ssl-view for details. + /entry + /row + I kinda wonder why this even separate from pg_stat_activity, at least from the POV of the function gathering the result. This way you have to join between pg_stat_activity and pg_stat_ssl which will mean that the two don't necessarily correspond to each other. To keep from cluttering pg_stat_activity for the majority of users who are the ones not actually using SSL. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] SSL information view
On 2015-04-09 15:56:00 +0200, Magnus Hagander wrote: On Thu, Apr 9, 2015 at 3:20 PM, Andres Freund and...@anarazel.de wrote: Hi, On 2015-04-09 13:31:55 +0200, Magnus Hagander wrote: + row + entrystructnamepg_stat_ssl/indextermprimarypg_stat_ssl/primary/indexterm/entry + entryOne row per connection (regular and replication), showing information about +SSL used on this connection. +See xref linkend=pg-stat-ssl-view for details. + /entry + /row + I kinda wonder why this even separate from pg_stat_activity, at least from the POV of the function gathering the result. This way you have to join between pg_stat_activity and pg_stat_ssl which will mean that the two don't necessarily correspond to each other. To keep from cluttering pg_stat_activity for the majority of users who are the ones not actually using SSL. I'm not sure that's actually a problem. But even if, it seems a bit better to return the data for both views from one SRF and just define the views differently. That way there's a way to query without the danger of matching the wrong rows between pg_stat_activity stat_ssl due to pid reuse. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL information view
Hi, On 2015-04-09 13:31:55 +0200, Magnus Hagander wrote: + row + entrystructnamepg_stat_ssl/indextermprimarypg_stat_ssl/primary/indexterm/entry + entryOne row per connection (regular and replication), showing information about +SSL used on this connection. +See xref linkend=pg-stat-ssl-view for details. + /entry + /row + I kinda wonder why this even separate from pg_stat_activity, at least from the POV of the function gathering the result. This way you have to join between pg_stat_activity and pg_stat_ssl which will mean that the two don't necessarily correspond to each other. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL information view
On Wed, Dec 17, 2014 at 9:19 PM, Heikki Linnakangas hlinnakan...@vmware.com
wrote:
On 11/19/2014 02:36 PM, Magnus Hagander wrote:
+ /* Create or attach to the shared SSL status buffers */
+ size = mul_size(NAMEDATALEN, MaxBackends);
+ BackendSslVersionBuffer = (char *)
+ ShmemInitStruct(Backend SSL Version Buffer, size,
found);
+
+ if (!found)
+ {
+ MemSet(BackendSslVersionBuffer, 0, size);
+
+ /* Initialize st_ssl_version pointers. */
+ buffer = BackendSslVersionBuffer;
+ for (i = 0; i MaxBackends; i++)
+ {
+ BackendStatusArray[i].st_ssl_version = buffer;
+ buffer += NAMEDATALEN;
+ }
+ }
+
+ size = mul_size(NAMEDATALEN, MaxBackends);
+ BackendSslCipherBuffer = (char *)
+ ShmemInitStruct(Backend SSL Cipher Buffer, size,
found);
+
+ if (!found)
+ {
+ MemSet(BackendSslCipherBuffer, 0, size);
+
+ /* Initialize st_ssl_cipher pointers. */
+ buffer = BackendSslCipherBuffer;
+ for (i = 0; i MaxBackends; i++)
+ {
+ BackendStatusArray[i].st_ssl_cipher = buffer;
+ buffer += NAMEDATALEN;
+ }
+ }
+
+ size = mul_size(NAMEDATALEN, MaxBackends);
+ BackendSslClientDNBuffer = (char *)
+ ShmemInitStruct(Backend SSL Client DN Buffer, size,
found);
+
+ if (!found)
+ {
+ MemSet(BackendSslClientDNBuffer, 0, size);
+
+ /* Initialize st_ssl_clientdn pointers. */
+ buffer = BackendSslClientDNBuffer;
+ for (i = 0; i MaxBackends; i++)
+ {
+ BackendStatusArray[i].st_ssl_clientdn = buffer;
+ buffer += NAMEDATALEN;
+ }
+ }
This pattern gets a bit tedious. We do that already for application_names,
client hostnames, and activity status but this adds three more such
strings. Why are these not just regular char arrays in PgBackendStatus
struct, anyway? The activity status is not, because its size is
configurable with the pgstat_track_activity_query_size GUC, but all those
other things are fixed-size.
Also, it would be nice if you didn't allocate the memory for all those SSL
strings, when SSL is disabled altogether. Perhaps put the SSL-related
information into a separate struct:
struct
{
/* Information about SSL connection */
int st_ssl_bits;
boolst_ssl_compression;
charst_ssl_version[NAMEDATALEN]; /* MUST be
null-terminated */
charst_ssl_cipher[NAMEDATALEN]; /* MUST be
null-terminated */
charst_ssl_clientdn[NAMEDATALEN]; /* MUST be
null-terminated */
} PgBackendSSLStatus;
Those structs could be allocated like you allocate the string buffers now,
with a pointer to that struct from PgBackendStatus. When SSL is disabled,
the structs are not allocated and the pointers in PgBackendStatus structs
are NULL.
Finally, I found time to do this. PFA a new version of this patch.
It takes into account the changes suggested by Heikki and Alex (minus the
renaming of fields - I think that's a separate thing to do, and we should
stick to existing naming conventions for now - but I changed the order of
the fields). Also the documentation changes suggested by Peter (but still
not the contrib/sslinfo part, as that should be a separate patch - but I
can look at that once we agree on this one). And resolves the inevitable
oid conflict for a patch that's been delayed that long.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
*** a/doc/src/sgml/monitoring.sgml
--- b/doc/src/sgml/monitoring.sgml
***
*** 300,305 postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser
--- 300,313
/entry
/row
+ row
+ entrystructnamepg_stat_ssl/indextermprimarypg_stat_ssl/primary/indexterm/entry
+ entryOne row per connection (regular and replication), showing information about
+SSL used on this connection.
+See xref linkend=pg-stat-ssl-view for details.
+ /entry
+ /row
+
/tbody
/tgroup
/table
***
*** 825,830 postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser
--- 833,907
listed; no information is available about downstream standby servers.
/para
+ table id=pg-stat-ssl-view xreflabel=pg_stat_ssl
+titlestructnamepg_stat_ssl/structname View/title
+tgroup cols=3
+ thead
+ row
+ entryColumn/entry
+ entryType/entry
+ entryDescription/entry
+ /row
+ /thead
+
+tbody
+ row
+
Re: [HACKERS] SSL information view
On Thu, Apr 9, 2015 at 5:46 PM, Andres Freund and...@anarazel.de wrote: On 2015-04-09 15:56:00 +0200, Magnus Hagander wrote: On Thu, Apr 9, 2015 at 3:20 PM, Andres Freund and...@anarazel.de wrote: Hi, On 2015-04-09 13:31:55 +0200, Magnus Hagander wrote: + row + entrystructnamepg_stat_ssl/indextermprimarypg_stat_ssl/primary/indexterm/entry + entryOne row per connection (regular and replication), showing information about +SSL used on this connection. +See xref linkend=pg-stat-ssl-view for details. + /entry + /row + I kinda wonder why this even separate from pg_stat_activity, at least from the POV of the function gathering the result. This way you have to join between pg_stat_activity and pg_stat_ssl which will mean that the two don't necessarily correspond to each other. To keep from cluttering pg_stat_activity for the majority of users who are the ones not actually using SSL. I'm not sure that's actually a problem. But even if, it seems a bit better to return the data for both views from one SRF and just define the views differently. That way there's a way to query without the danger of matching the wrong rows between pg_stat_activity stat_ssl due to pid reuse. Ah, now I see your point. Attached is a patch which does this, at least the way I think you meant. And also fixes a nasty crash bug in the previous one that for some reason my testing missed (can't set a pointer to null and then expect to use it later, no... When it's in shared memory, it survives into a new backend.) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ *** a/doc/src/sgml/monitoring.sgml --- b/doc/src/sgml/monitoring.sgml *** *** 300,305 postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser --- 300,313 /entry /row + row + entrystructnamepg_stat_ssl/indextermprimarypg_stat_ssl/primary/indexterm/entry + entryOne row per connection (regular and replication), showing information about +SSL used on this connection. +See xref linkend=pg-stat-ssl-view for details. + /entry + /row + /tbody /tgroup /table *** *** 825,830 postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser --- 833,907 listed; no information is available about downstream standby servers. /para + table id=pg-stat-ssl-view xreflabel=pg_stat_ssl +titlestructnamepg_stat_ssl/structname View/title +tgroup cols=3 + thead + row + entryColumn/entry + entryType/entry + entryDescription/entry + /row + /thead + +tbody + row + entrystructfieldpid//entry + entrytypeinteger//entry + entryProcess ID of a backend or WAL sender process/entry + /row + row + entrystructfieldssl//entry + entrytypeboolean//entry + entryTrue if SSL is used on this connection/entry + /row + row + entrystructfieldversion//entry + entrytypetext//entry + entryVersion of SSL in use, or NULL if SSL is not in use + on this connection/entry + /row + row + entrystructfieldcipher//entry + entrytypetext//entry + entryName of SSL cipher in use, or NULL if SSL is not in use + on this connection/entry + /row + row + entrystructfieldbits//entry + entrytypeinteger//entry + entryNumber of bits in the encryption algorithm used, or NULL + if SSL is not used on this connection/entry + /row + row + entrystructfieldcompression//entry + entrytypeboolean//entry + entryTrue if SSL compression is in use, false if not, + or NULL if SSL is not in use on this connection/entry + /row + row + entrystructfieldclientdn//entry + entrytypetext//entry + entryDistinguished 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 symbolNAMEDATALEN/symbol (64 characters + in a standard build) + /entry + /row +/tbody +/tgroup + /table + + para +The structnamepg_stat_ssl/structname view will contain one row per +backend or WAL sender process, showing statistics about SSL usage on +this connection. It can be joined to structnamepg_stat_activity/structname +or structnamepg_stat_replication/structname on the +structfieldpid/structfield column to get more details about the +connection. + /para + table id=pg-stat-archiver-view xreflabel=pg_stat_archiver titlestructnamepg_stat_archiver/structname View/title *** a/src/backend/catalog/system_views.sql --- b/src/backend/catalog/system_views.sql *** *** 646,651 CREATE VIEW pg_stat_replication AS ---
Re: [HACKERS] SSL information view
On Fri, Feb 13, 2015 at 9:07 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Tue, Feb 3, 2015 at 9:36 PM, Magnus Hagander mag...@hagander.net wrote: On Tue, Feb 3, 2015 at 6:42 AM, Michael Paquier michael.paqu...@gmail.com wrote: Where are we on this patch? No new version has been provided and there have been comments provided by Heikki here (5491e547.4040...@vmware.com) and by Alexei here (87ppbqz00h@commandprompt.com). Yeah, it's on my shame list. I'm definitely planning to get a new version in before the next CF and to try to work quicker with it that time to finish it off on time. Thanks for the reminder! Magnus, are you planning to work on this item of your shame list soon? Could you clarify the status of this patch? I do, and I hope to work on it over the next week or two. However, feel free to bump it to the next CF -- I'll pick it up halfway in between if necessary. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] SSL information view
On Tue, Feb 3, 2015 at 9:36 PM, Magnus Hagander mag...@hagander.net wrote: On Tue, Feb 3, 2015 at 6:42 AM, Michael Paquier michael.paqu...@gmail.com wrote: Where are we on this patch? No new version has been provided and there have been comments provided by Heikki here (5491e547.4040...@vmware.com) and by Alexei here (87ppbqz00h@commandprompt.com). Yeah, it's on my shame list. I'm definitely planning to get a new version in before the next CF and to try to work quicker with it that time to finish it off on time. Thanks for the reminder! Magnus, are you planning to work on this item of your shame list soon? Could you clarify the status of this patch? -- Michael
Re: [HACKERS] SSL information view
On Fri, Feb 13, 2015 at 5:31 PM, Magnus Hagander mag...@hagander.netwrote: On Fri, Feb 13, 2015 at 9:07 AM, Michael Paquier michael.paqu...@gmail.comwrote: Magnus, are you planning to work on this item of your shame list soon? Could you clarify the status of this patch? I do, and I hope to work on it over the next week or two. However, feel free to bump it to the next CF -- I'll pick it up halfway in between if necessary. OK, I moved it to 2015-02 with the same status Waiting on Author. -- Michael
Re: [HACKERS] SSL information view
On Tue, Feb 3, 2015 at 6:42 AM, Michael Paquier michael.paqu...@gmail.com wrote: Where are we on this patch? No new version has been provided and there have been comments provided by Heikki here (5491e547.4040...@vmware.com) and by Alexei here (87ppbqz00h@commandprompt.com). Yeah, it's on my shame list. I'm definitely planning to get a new version in before the next CF and to try to work quicker with it that time to finish it off on time. Thanks for the reminder! -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] SSL information view
Where are we on this patch? No new version has been provided and there have been comments provided by Heikki here (5491e547.4040...@vmware.com) and by Alexei here (87ppbqz00h@commandprompt.com). -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL information view
On 11/19/14 7:36 AM, Magnus Hagander wrote: + row + entrystructnamepg_stat_ssl/indextermprimarypg_stat_ssl/primary/indexterm/entry + entryOne row per connection (regular and replication), showing statistics about +SSL used on this connection. +See xref linkend=pg-stat-ssl-view for details. + /entry + /row + It doesn't really show statistics. It shows information or data. We should make contrib/sslinfo a wrapper around this view as much as possible. Is it useful to include rows for sessions not using SSL? Should we perpetuate the ssl-naming? Is there a more general term? Will this work for non-OpenSSL implementations? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL information view
On 11/19/2014 02:36 PM, Magnus Hagander wrote:
+ /* Create or attach to the shared SSL status buffers */
+ size = mul_size(NAMEDATALEN, MaxBackends);
+ BackendSslVersionBuffer = (char *)
+ ShmemInitStruct(Backend SSL Version Buffer, size, found);
+
+ if (!found)
+ {
+ MemSet(BackendSslVersionBuffer, 0, size);
+
+ /* Initialize st_ssl_version pointers. */
+ buffer = BackendSslVersionBuffer;
+ for (i = 0; i MaxBackends; i++)
+ {
+ BackendStatusArray[i].st_ssl_version = buffer;
+ buffer += NAMEDATALEN;
+ }
+ }
+
+ size = mul_size(NAMEDATALEN, MaxBackends);
+ BackendSslCipherBuffer = (char *)
+ ShmemInitStruct(Backend SSL Cipher Buffer, size, found);
+
+ if (!found)
+ {
+ MemSet(BackendSslCipherBuffer, 0, size);
+
+ /* Initialize st_ssl_cipher pointers. */
+ buffer = BackendSslCipherBuffer;
+ for (i = 0; i MaxBackends; i++)
+ {
+ BackendStatusArray[i].st_ssl_cipher = buffer;
+ buffer += NAMEDATALEN;
+ }
+ }
+
+ size = mul_size(NAMEDATALEN, MaxBackends);
+ BackendSslClientDNBuffer = (char *)
+ ShmemInitStruct(Backend SSL Client DN Buffer, size, found);
+
+ if (!found)
+ {
+ MemSet(BackendSslClientDNBuffer, 0, size);
+
+ /* Initialize st_ssl_clientdn pointers. */
+ buffer = BackendSslClientDNBuffer;
+ for (i = 0; i MaxBackends; i++)
+ {
+ BackendStatusArray[i].st_ssl_clientdn = buffer;
+ buffer += NAMEDATALEN;
+ }
+ }
This pattern gets a bit tedious. We do that already for
application_names, client hostnames, and activity status but this adds
three more such strings. Why are these not just regular char arrays in
PgBackendStatus struct, anyway? The activity status is not, because its
size is configurable with the pgstat_track_activity_query_size GUC, but
all those other things are fixed-size.
Also, it would be nice if you didn't allocate the memory for all those
SSL strings, when SSL is disabled altogether. Perhaps put the
SSL-related information into a separate struct:
struct
{
/* Information about SSL connection */
int st_ssl_bits;
boolst_ssl_compression;
charst_ssl_version[NAMEDATALEN]; /* MUST be
null-terminated */
charst_ssl_cipher[NAMEDATALEN]; /* MUST be
null-terminated */
charst_ssl_clientdn[NAMEDATALEN]; /* MUST be
null-terminated */
} PgBackendSSLStatus;
Those structs could be allocated like you allocate the string buffers
now, with a pointer to that struct from PgBackendStatus. When SSL is
disabled, the structs are not allocated and the pointers in
PgBackendStatus structs are NULL.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL information view
Magnus Hagander mag...@hagander.net writes:
You should add it to the next CF for proper tracking, there are already many
patches in the queue waiting for reviews :)
Absolutely - I just wanted those that were already involved in the
thread to get a chance to look at it early :) didn't want to submit it
until it was complete.
Which it is now - attached is a new version that includes docs.
Here's my review of the patch:
* Applies to the current HEAD, no failed hunks.
* Compiles and works as advertised.
* Docs included.
The following catches my eye:
psql (9.5devel)
SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384, bits:
256, compression: off)
Type help for help.
postgres=# select * from pg_stat_ssl;
pid | ssl | bits | compression | version | cipher|
clientdn
--+-+--+-+-+-+--
1343 | t | 256 | f | TLSv1.2 | ECDHE-RSA-AES256-GCM-SHA384 |
(1 row)
I think the order of details in the psql prompt makes more sense,
because it puts more important details first.
I suggest we reorder the view columns, while also renaming 'version' to
'protocol' (especially since we have another patch in the works that
adds a GUC named 'ssl_protocols'):
pid, ssl, protocol, cipher, bits, compression, clientdn.
Next, this one:
+ be_tls_get_cipher(Port *port, char *ptr, size_t len)
+ {
+ if (port-ssl)
+ strlcpy(ptr, SSL_get_cipher(port-ssl), NAMEDATALEN);
should be this:
+ strlcpy(ptr, SSL_get_cipher(port-ssl), len);
The same goes for this one:
+ be_tls_get_peerdn_name(Port *port, char *ptr, size_t len)
+ {
+ if (port-peer)
+ strlcpy(ptr,
X509_NAME_to_cstring(X509_get_subject_name(port-peer)), NAMEDATALEN);
The NAMEDATALEN constant is passed in the calling code in
pgstat_bestart().
Other than that, looks good to me.
--
Alex
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL information view
On Tue, Nov 11, 2014 at 1:04 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Tue, Nov 11, 2014 at 1:43 AM, Magnus Hagander mag...@hagander.net wrote: Right now it just truncates the dn at NAMEDATALEN - so treating it the same as we do with hostnames. My guess is this is not a big problem because in the case of long DNs, most of the time the important stuff is at the beginning anyway... (And it's not like it's actually used for authentication, in which case it would of course be a problem). You should add it to the next CF for proper tracking, there are already many patches in the queue waiting for reviews :) Absolutely - I just wanted those that were already involved in the thread to get a chance to look at it early :) didn't want to submit it until it was complete. Which it is now - attached is a new version that includes docs. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ *** a/doc/src/sgml/monitoring.sgml --- b/doc/src/sgml/monitoring.sgml *** *** 300,305 postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser --- 300,313 /entry /row + row + entrystructnamepg_stat_ssl/indextermprimarypg_stat_ssl/primary/indexterm/entry + entryOne row per connection (regular and replication), showing statistics about +SSL used on this connection. +See xref linkend=pg-stat-ssl-view for details. + /entry + /row + /tbody /tgroup /table *** *** 825,830 postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: ser --- 833,907 listed; no information is available about downstream standby servers. /para + table id=pg-stat-ssl-view xreflabel=pg_stat_ssl +titlestructnamepg_stat_ssl/structname View/title +tgroup cols=3 + thead + row + entryColumn/entry + entryType/entry + entryDescription/entry + /row + /thead + +tbody + row + entrystructfieldpid//entry + entrytypeinteger//entry + entryProcess ID of a backend or WAL sender process/entry + /row + row + entrystructfieldssl//entry + entrytypeboolean//entry + entryTrue if SSL is used on this connection/entry + /row + row + entrystructfieldbits//entry + entrytypeinteger//entry + entryNumber of bits in the encryption algorithm used, or NULL + if SSL is not used on this connection/entry + /row + row + entrystructfieldcompression//entry + entrytypeboolean//entry + entryTrue if SSL compression is in use, false if not, + or NULL if SSL is not in use on this connection/entry + /row + row + entrystructfieldversion//entry + entrytypetext//entry + entryVersion of SSL in use, or NULL if SSL is not in use + on this connection/entry + /row + row + entrystructfieldcipher//entry + entrytypetext//entry + entryName of SSL cipher in use, or NULL if SSL is not in use + on this connection/entry + /row + row + entrystructfieldclientdn//entry + entrytypetext//entry + entryDistinguished 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 symbolNAMEDATALEN/symbol (64 characters + in a standard build) + /entry + /row +/tbody +/tgroup + /table + + para +The structnamepg_stat_ssl/structname view will contain one row per +backend or WAL sender process, showing statistics about SSL usage on +this connection. It can be joined to structnamepg_stat_activity/structname +or structnamepg_stat_replication/structname on the +structfieldpid/structfield column to get more details about the +connection. + /para + table id=pg-stat-archiver-view xreflabel=pg_stat_archiver titlestructnamepg_stat_archiver/structname View/title *** a/src/backend/catalog/system_views.sql --- b/src/backend/catalog/system_views.sql *** *** 648,653 CREATE VIEW pg_stat_replication AS --- 648,664 WHERE S.usesysid = U.oid AND S.pid = W.pid; + CREATE VIEW pg_stat_ssl AS + SELECT + I.pid, + I.ssl, + I.bits, + I.compression, + I.version, + I.cipher, + I.clientdn + FROM pg_stat_get_sslstatus() AS I; + CREATE VIEW pg_replication_slots AS SELECT L.slot_name, *** a/src/backend/libpq/be-secure-openssl.c --- b/src/backend/libpq/be-secure-openssl.c *** *** 88,93 static void info_cb(const SSL *ssl, int type, int args); --- 88,95 static void initialize_ecdh(void); static const char *SSLerrmessage(void); + static char *X509_NAME_to_cstring(X509_NAME *name); + /* are we in the middle
Re: [HACKERS] SSL information view
On Mon, Jul 21, 2014 at 5:24 PM, Bernd Helmle maili...@oopsware.de wrote:
--On 12. Juli 2014 15:08:01 +0200 Magnus Hagander mag...@hagander.net
wrote:
Before doing that, however, I'd like to ask for opinions :) The hack
currently exposes a separate view that you can join to
pg_stat_activity (or pg_stat_replication) on the pid -- this is sort
of the same way that pg_stat_replication works in the first place. Do
we want something similar to that for a builtin SSL view as well, or
do we want to include the fields directly in pg_stat_activity and
pg_stat_replication?
I've heard more than once the wish to get this information without
contrib..especially for the SSL version used (client and server likewise).
So ++1 for this feature.
I'd vote for a special view, that will keep the information into a single
place and someone can easily join extra information together.
Here's a patch that implements that.
Docs are currently ont included because I'm waiting for the
restructuring of tha section to be done (started by me in a separate
thread) first, but the code is there for review.
Right now it just truncates the dn at NAMEDATALEN - so treating it the
same as we do with hostnames. My guess is this is not a big problem
because in the case of long DNs, most of the time the important stuff
is at the beginning anyway... (And it's not like it's actually used
for authentication, in which case it would of course be a problem).
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
***
*** 648,653 CREATE VIEW pg_stat_replication AS
--- 648,664
WHERE S.usesysid = U.oid AND
S.pid = W.pid;
+ CREATE VIEW pg_stat_ssl AS
+ SELECT
+ I.pid,
+ I.ssl,
+ I.bits,
+ I.compression,
+ I.version,
+ I.cipher,
+ I.clientdn
+ FROM pg_stat_get_sslstatus() AS I;
+
CREATE VIEW pg_replication_slots AS
SELECT
L.slot_name,
*** a/src/backend/libpq/be-secure-openssl.c
--- b/src/backend/libpq/be-secure-openssl.c
***
*** 88,93 static void info_cb(const SSL *ssl, int type, int args);
--- 88,95
static void initialize_ecdh(void);
static const char *SSLerrmessage(void);
+ static char *X509_NAME_to_cstring(X509_NAME *name);
+
/* are we in the middle of a renegotiation? */
static bool in_ssl_renegotiation = false;
***
*** 1053,1055 SSLerrmessage(void)
--- 1055,1159
snprintf(errbuf, sizeof(errbuf), _(SSL error code %lu), errcode);
return errbuf;
}
+
+ /*
+ * Return information about the SSL connection
+ */
+ int
+ be_tls_get_cipher_bits(Port *port)
+ {
+ int bits;
+
+ if (port-ssl)
+ {
+ SSL_get_cipher_bits(port-ssl, bits);
+ return bits;
+ }
+ else
+ return 0;
+ }
+
+ bool
+ be_tls_get_compression(Port *port)
+ {
+ if (port-ssl)
+ return (SSL_get_current_compression(port-ssl) != NULL);
+ else
+ return false;
+ }
+
+ void
+ be_tls_get_version(Port *port, char *ptr, size_t len)
+ {
+ if (port-ssl)
+ strlcpy(ptr, SSL_get_version(port-ssl), len);
+ else
+ ptr[0] = '\0';
+ }
+
+ void
+ be_tls_get_cipher(Port *port, char *ptr, size_t len)
+ {
+ if (port-ssl)
+ strlcpy(ptr, SSL_get_cipher(port-ssl), NAMEDATALEN);
+ else
+ ptr[0] = '\0';
+ }
+
+ void
+ be_tls_get_peerdn_name(Port *port, char *ptr, size_t len)
+ {
+ if (port-peer)
+ strlcpy(ptr, X509_NAME_to_cstring(X509_get_subject_name(port-peer)), NAMEDATALEN);
+ else
+ ptr[0] = '\0';
+ }
+
+ /*
+ * Convert an X509 subject name to a cstring.
+ *
+ */
+ static char *
+ X509_NAME_to_cstring(X509_NAME *name)
+ {
+ BIO *membuf = BIO_new(BIO_s_mem());
+ int i,
+ nid,
+ count = X509_NAME_entry_count(name);
+ X509_NAME_ENTRY *e;
+ ASN1_STRING *v;
+ const char *field_name;
+ size_t size;
+ char nullterm;
+ char *sp;
+ char *dp;
+ char *result;
+
+ (void) BIO_set_close(membuf, BIO_CLOSE);
+ for (i = 0; i count; i++)
+ {
+ e = X509_NAME_get_entry(name, i);
+ nid = OBJ_obj2nid(X509_NAME_ENTRY_get_object(e));
+ v = X509_NAME_ENTRY_get_data(e);
+ field_name = OBJ_nid2sn(nid);
+ if (!field_name)
+ field_name = OBJ_nid2ln(nid);
+ BIO_printf(membuf, /%s=, field_name);
+ ASN1_STRING_print_ex(membuf, v,
+ ((ASN1_STRFLGS_RFC2253 ~ASN1_STRFLGS_ESC_MSB)
+ | ASN1_STRFLGS_UTF8_CONVERT));
+ }
+
+ /* ensure null termination of the BIO's content */
+ nullterm = '\0';
+ BIO_write(membuf, nullterm, 1);
+ size = BIO_get_mem_data(membuf, sp);
+ dp = pg_any_to_server(sp, size - 1, PG_UTF8);
+
+ result = pstrdup(dp);
+ if (dp != sp)
+ pfree(dp);
+ BIO_free(membuf);
+
+ return result;
+ }
*** a/src/backend/postmaster/pgstat.c
--- b/src/backend/postmaster/pgstat.c
***
*** 2386,2391 pgstat_fetch_global(void)
--- 2386,2394
Re: [HACKERS] SSL information view
On Tue, Nov 11, 2014 at 1:43 AM, Magnus Hagander mag...@hagander.net wrote: Right now it just truncates the dn at NAMEDATALEN - so treating it the same as we do with hostnames. My guess is this is not a big problem because in the case of long DNs, most of the time the important stuff is at the beginning anyway... (And it's not like it's actually used for authentication, in which case it would of course be a problem). You should add it to the next CF for proper tracking, there are already many patches in the queue waiting for reviews :) -- Michael
Re: [HACKERS] SSL information view
--On 12. Juli 2014 15:08:01 +0200 Magnus Hagander mag...@hagander.net wrote: Before doing that, however, I'd like to ask for opinions :) The hack currently exposes a separate view that you can join to pg_stat_activity (or pg_stat_replication) on the pid -- this is sort of the same way that pg_stat_replication works in the first place. Do we want something similar to that for a builtin SSL view as well, or do we want to include the fields directly in pg_stat_activity and pg_stat_replication? I've heard more than once the wish to get this information without contrib..especially for the SSL version used (client and server likewise). So ++1 for this feature. I'd vote for a special view, that will keep the information into a single place and someone can easily join extra information together. -- Thanks Bernd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL information view
On Mon, Jul 14, 2014 at 7:54 PM, Stefan Kaltenbrunner ste...@kaltenbrunner.cc wrote: On 07/13/2014 10:35 PM, Magnus Hagander wrote: On Sun, Jul 13, 2014 at 10:32 PM, Stefan Kaltenbrunner ste...@kaltenbrunner.cc wrote: On 07/12/2014 03:08 PM, Magnus Hagander wrote: As an administrator, I find that you fairly often want to know what your current connections are actually using as SSL parameters, and there is currently no other way than gdb to find that out - something we definitely should fix. Yeah that would be handy - however I often wish to be able to figure that out based on the logfile as well, any chance of getting these into connection-logging/log_line_prefix as well? We do already log some of it if you have enabled log_connections - protocol and cipher. Anything else in particular you'd be looking for - compression info? DN mostly, not sure I care about compression info... Compression fitted more neatly in with the format that was there now. I wonder if we shuold add a DETAIL field on that error message that has the DN in case there is a client certificate. Would that make sense? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL information view
On 07/13/2014 10:35 PM, Magnus Hagander wrote: On Sun, Jul 13, 2014 at 10:32 PM, Stefan Kaltenbrunner ste...@kaltenbrunner.cc wrote: On 07/12/2014 03:08 PM, Magnus Hagander wrote: As an administrator, I find that you fairly often want to know what your current connections are actually using as SSL parameters, and there is currently no other way than gdb to find that out - something we definitely should fix. Yeah that would be handy - however I often wish to be able to figure that out based on the logfile as well, any chance of getting these into connection-logging/log_line_prefix as well? We do already log some of it if you have enabled log_connections - protocol and cipher. Anything else in particular you'd be looking for - compression info? DN mostly, not sure I care about compression info... Stefan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL information view
On 07/12/2014 03:08 PM, Magnus Hagander wrote: As an administrator, I find that you fairly often want to know what your current connections are actually using as SSL parameters, and there is currently no other way than gdb to find that out - something we definitely should fix. Yeah that would be handy - however I often wish to be able to figure that out based on the logfile as well, any chance of getting these into connection-logging/log_line_prefix as well? Stefan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL information view
On Sun, Jul 13, 2014 at 10:32 PM, Stefan Kaltenbrunner ste...@kaltenbrunner.cc wrote: On 07/12/2014 03:08 PM, Magnus Hagander wrote: As an administrator, I find that you fairly often want to know what your current connections are actually using as SSL parameters, and there is currently no other way than gdb to find that out - something we definitely should fix. Yeah that would be handy - however I often wish to be able to figure that out based on the logfile as well, any chance of getting these into connection-logging/log_line_prefix as well? We do already log some of it if you have enabled log_connections - protocol and cipher. Anything else in particular you'd be looking for - compression info? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL information view
Magnus Hagander mag...@hagander.net writes: As an administrator, I find that you fairly often want to know what your current connections are actually using as SSL parameters, and there is currently no other way than gdb to find that out - something we definitely should fix. I'm wondering whether it's such a great idea that everybody can see everybody else's client DN. Other than that, no objection to the concept. Second, I was planning to implement it by adding fields to PgBackendStatus and thus to BackendStatusArray, booleans directly in the struct and strings similar to how we track for example hostnames. Anybody see a problem with that? Space in that array is at a premium, and again the client DN seems problematic, in that it's not short and has no clear upper bound. If you were to drop the DN from the proposed view then I'd be fine with this. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSL information view
On Sat, Jul 12, 2014 at 4:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: As an administrator, I find that you fairly often want to know what your current connections are actually using as SSL parameters, and there is currently no other way than gdb to find that out - something we definitely should fix. I'm wondering whether it's such a great idea that everybody can see everybody else's client DN. Other than that, no objection to the concept. I was thinking that's mostly the equivalent of a username, which we do let everybody see in pg_stat_activity. Second, I was planning to implement it by adding fields to PgBackendStatus and thus to BackendStatusArray, booleans directly in the struct and strings similar to how we track for example hostnames. Anybody see a problem with that? Space in that array is at a premium, and again the client DN seems problematic, in that it's not short and has no clear upper bound. If you were to drop the DN from the proposed view then I'd be fine with this. The text fields, like hostname, are tracked in separate parts of shared memory with just a pointer in the main array - I assume that's why, and was planning to do the same. We'd have to cap the length oft he DN at something though (and document as such), to make it fixed length. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
