SSL information view
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.
You can find it out today through libpq (the PQgetssl functions), the
psql banner, or contrib/sslinfo. All of them are client side (the
sslinfo module runs on the server, but it's just SQL functions that
can show information about the current connection, nothing that can be
used to inspect other connections).
I recently put together a quick hack
(https://github.com/mhagander/pg_sslstatus) that exposes a view with
this information, but it's definitely hacky, and it really is
functionality that we should include in core. Thus, I'll provide a
version of that hack for 9.5.
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?
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?
--
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
Magnus Hagander <magnus@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
On Sat, Jul 12, 2014 at 4:36 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@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
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
On Sun, Jul 13, 2014 at 10:32 PM, Stefan Kaltenbrunner
<stefan@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
On 07/13/2014 10:35 PM, Magnus Hagander wrote:
On Sun, Jul 13, 2014 at 10:32 PM, Stefan Kaltenbrunner
<stefan@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
On Mon, Jul 14, 2014 at 7:54 PM, Stefan Kaltenbrunner
<stefan@kaltenbrunner.cc> wrote:
On 07/13/2014 10:35 PM, Magnus Hagander wrote:
On Sun, Jul 13, 2014 at 10:32 PM, Stefan Kaltenbrunner
<stefan@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
--On 12. Juli 2014 15:08:01 +0200 Magnus Hagander <magnus@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
On Mon, Jul 21, 2014 at 5:24 PM, Bernd Helmle <mailings@oopsware.de> wrote:
--On 12. Juli 2014 15:08:01 +0200 Magnus Hagander <magnus@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/
Attachments:
pg_stat_ssl.patchtext/x-patch; charset=US-ASCII; name=pg_stat_ssl.patchDownload+299-0
On Tue, Nov 11, 2014 at 1:43 AM, Magnus Hagander <magnus@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
On Tue, Nov 11, 2014 at 1:04 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Tue, Nov 11, 2014 at 1:43 AM, Magnus Hagander <magnus@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/
Attachments:
pg_stat_ssl.patchtext/x-patch; charset=US-ASCII; name=pg_stat_ssl.patchDownload+384-0
Magnus Hagander <magnus@hagander.net> writes:
You should add it to the next CF for proper tracking, there are already many
patches in the queue waiting for reviews :)Absolutely - I just wanted those that were already involved in the
thread to get a chance to look at it early :) didn't want to submit it
until it was complete.Which it is now - attached is a new version that includes docs.
Here's my review of the patch:
* Applies to the current HEAD, no failed hunks.
* Compiles and works as advertised.
* Docs included.
The following catches my eye:
psql (9.5devel)
SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384, bits: 256, compression: off)
Type "help" for help.
postgres=# select * from pg_stat_ssl;
pid | ssl | bits | compression | version | cipher | clientdn
------+-----+------+-------------+---------+-----------------------------+----------
1343 | t | 256 | f | TLSv1.2 | ECDHE-RSA-AES256-GCM-SHA384 |
(1 row)
I think the order of details in the psql prompt makes more sense,
because it puts more important details first.
I suggest we reorder the view columns, while also renaming 'version' to
'protocol' (especially since we have another patch in the works that
adds a GUC named 'ssl_protocols'):
pid, ssl, protocol, cipher, bits, compression, clientdn.
Next, this one:
+ be_tls_get_cipher(Port *port, char *ptr, size_t len)
+ {
+ if (port->ssl)
+ strlcpy(ptr, SSL_get_cipher(port->ssl), NAMEDATALEN);should be this:
+ strlcpy(ptr, SSL_get_cipher(port->ssl), len);
The same goes for this one:
+ be_tls_get_peerdn_name(Port *port, char *ptr, size_t len)
+ {
+ if (port->peer)
+ strlcpy(ptr, X509_NAME_to_cstring(X509_get_subject_name(port->peer)), NAMEDATALEN);The NAMEDATALEN constant is passed in the calling code in
pgstat_bestart().
Other than that, looks good to me.
--
Alex
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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;
bool st_ssl_compression;
char st_ssl_version[NAMEDATALEN]; /* MUST be null-terminated */
char st_ssl_cipher[NAMEDATALEN]; /* MUST be null-terminated */
char st_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
On 11/19/14 7:36 AM, Magnus Hagander wrote:
+ <row> + <entry><structname>pg_stat_ssl</><indexterm><primary>pg_stat_ssl</primary></indexterm></entry> + <entry>One 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
On Mon, Jan 5, 2015 at 9:56 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 11/19/14 7:36 AM, Magnus Hagander wrote:
+ <row>
+<entry><structname>pg_stat_ssl</><indexterm><primary>pg_stat_ssl</primary></indexterm></entry>
+ <entry>One 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.
Good point.
We should make contrib/sslinfo a wrapper around this view as much as
possible.
Agreed - but let's do that as a separate patch.
Is it useful to include rows for sessions not using SSL?
I think so, mainly because it makes things "more obvious" that you are
querying it the right way. Sure you could do a LEFT JOIN from
pg_stat_activity and a CASE to get the same results, but that makes it a
lot less in-your-face.
Should we perpetuate the "ssl"-naming? Is there a more general term?
tls? :)
We call it ssl everywhere else, I think being consistent is important.
Will this work for non-OpenSSL implementations?
Yes, it uses (and declares new) the new internal APIs that Heikki created.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Where are we on this patch? No new version has been provided and there
have been comments provided by Heikki here
(5491E547.4040705@vmware.com) and by Alexei here
(87ppbqz00h.fsf@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
On Tue, Feb 3, 2015 at 6:42 AM, Michael Paquier <michael.paquier@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.4040705@vmware.com) and by Alexei here
(87ppbqz00h.fsf@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/
On Tue, Feb 3, 2015 at 9:36 PM, Magnus Hagander <magnus@hagander.net> wrote:
On Tue, Feb 3, 2015 at 6:42 AM, Michael Paquier <michael.paquier@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.4040705@vmware.com) and by Alexei here
(87ppbqz00h.fsf@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
On Fri, Feb 13, 2015 at 9:07 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
On Tue, Feb 3, 2015 at 9:36 PM, Magnus Hagander <magnus@hagander.net>
wrote:On Tue, Feb 3, 2015 at 6:42 AM, Michael Paquier <
michael.paquier@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.4040705@vmware.com) and by Alexei here
(87ppbqz00h.fsf@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/
On Fri, Feb 13, 2015 at 5:31 PM, Magnus Hagander <magnus@hagander.net>wrote:
On Fri, Feb 13, 2015 at 9:07 AM, Michael Paquier
<michael.paquier@gmail.com>wrote: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
