restrict pg_stat_ssl to superuser?
As discussed in [0]</messages/by-id/398754d8-6bb5-c5cf-e7b8-22e5f0983caf@2ndquadrant.com>, should we restrict access to pg_stat_ssl to
superusers (and an appropriate pg_ role)?
If so, is there anything in that view that should be made available to
non-superusers? If not, then we could perhaps do this via a simple
permission change instead of going the route of blanking out individual
columns.
[0]: </messages/by-id/398754d8-6bb5-c5cf-e7b8-22e5f0983caf@2ndquadrant.com>
</messages/by-id/398754d8-6bb5-c5cf-e7b8-22e5f0983caf@2ndquadrant.com>
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Feb 07, 2019 at 09:30:38AM +0100, Peter Eisentraut wrote:
If so, is there anything in that view that should be made available to
non-superusers? If not, then we could perhaps do this via a simple
permission change instead of going the route of blanking out individual
columns.
Hm. It looks sensible to move to a per-permission approach for that
view. Now, pg_stat_get_activity() is not really actually restricted,
and would still return the information on direct calls, so the idea
would be to split the SSL-related data into its own function?
--
Michael
On 2019-02-12 07:40, Michael Paquier wrote:
On Thu, Feb 07, 2019 at 09:30:38AM +0100, Peter Eisentraut wrote:
If so, is there anything in that view that should be made available to
non-superusers? If not, then we could perhaps do this via a simple
permission change instead of going the route of blanking out individual
columns.Hm. It looks sensible to move to a per-permission approach for that
view. Now, pg_stat_get_activity() is not really actually restricted,
and would still return the information on direct calls, so the idea
would be to split the SSL-related data into its own function?
We could remove default privileges from pg_stat_get_activity(). Would
that be a problem?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Feb 15, 2019 at 02:04:59PM +0100, Peter Eisentraut wrote:
We could remove default privileges from pg_stat_get_activity(). Would
that be a problem?
I don't think so, still I am wondering about the impact that this
could have for monitoring tools calling it directly as we document
it..
--
Michael
On 2019-02-18 04:58, Michael Paquier wrote:
On Fri, Feb 15, 2019 at 02:04:59PM +0100, Peter Eisentraut wrote:
We could remove default privileges from pg_stat_get_activity(). Would
that be a problem?I don't think so, still I am wondering about the impact that this
could have for monitoring tools calling it directly as we document
it..
Actually, this approach isn't going to work anyway, because function
permissions in a view are checked as the current user, not the view owner.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Feb 7, 2019 at 3:30 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
As discussed in [0], should we restrict access to pg_stat_ssl to
superusers (and an appropriate pg_ role)?If so, is there anything in that view that should be made available to
non-superusers? If not, then we could perhaps do this via a simple
permission change instead of going the route of blanking out individual
columns.
Shouldn't unprivileged users be able to see their own entries, or
entries for roles which they could assume via SET ROLE?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 2019-02-19 16:57, Peter Eisentraut wrote:
On 2019-02-18 04:58, Michael Paquier wrote:
On Fri, Feb 15, 2019 at 02:04:59PM +0100, Peter Eisentraut wrote:
We could remove default privileges from pg_stat_get_activity(). Would
that be a problem?I don't think so, still I am wondering about the impact that this
could have for monitoring tools calling it directly as we document
it..Actually, this approach isn't going to work anyway, because function
permissions in a view are checked as the current user, not the view owner.
So here is a patch doing it the "normal" way of nulling out all the rows
the user shouldn't see.
I haven't found any documentation of these access restrictions in the
context of pg_stat_activity. Is there something that I'm not seeing or
something that should be added?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v1-0001-Hide-other-user-s-pg_stat_ssl-rows.patchtext/plain; charset=UTF-8; name=v1-0001-Hide-other-user-s-pg_stat_ssl-rows.patch; x-mac-creator=0; x-mac-type=0Download
From e164cc0d7fbd8d1e6de23219eed3dd038f99e557 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 20 Feb 2019 11:38:44 +0100
Subject: [PATCH v1] Hide other user's pg_stat_ssl rows
Change pg_stat_ssl so that an unprivileged user can only see their own
rows; other rows will be all null. This makes the behavior consistent
with pg_stat_activity, where information about where the connection
came from is also restricted.
---
src/backend/utils/adt/pgstatfuncs.c | 73 ++++++++++++++++-------------
1 file changed, 41 insertions(+), 32 deletions(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index b6ba856ebe..69f7265779 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -645,38 +645,6 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
else
nulls[16] = true;
- if (beentry->st_ssl)
- {
- values[18] = BoolGetDatum(true); /* ssl */
- values[19] = CStringGetTextDatum(beentry->st_sslstatus->ssl_version);
- 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_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] = nulls[24] = nulls[25] = true;
- }
-
/* Values only available to role member or pg_read_all_stats */
if (has_privs_of_role(GetUserId(), beentry->st_userid) ||
is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS))
@@ -854,6 +822,39 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
else
values[17] =
CStringGetTextDatum(pgstat_get_backend_desc(beentry->st_backendType));
+
+ /* SSL information */
+ if (beentry->st_ssl)
+ {
+ values[18] = BoolGetDatum(true); /* ssl */
+ values[19] = CStringGetTextDatum(beentry->st_sslstatus->ssl_version);
+ 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_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] = nulls[24] = nulls[25] = true;
+ }
}
else
{
@@ -870,6 +871,14 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
nulls[13] = true;
nulls[14] = true;
nulls[17] = true;
+ nulls[18] = true;
+ nulls[19] = true;
+ nulls[20] = true;
+ nulls[21] = true;
+ nulls[22] = true;
+ nulls[23] = true;
+ nulls[24] = true;
+ nulls[25] = true;
}
tuplestore_putvalues(tupstore, tupdesc, values, nulls);
base-commit: 56fadbedbd2f697400b89e7b767cfa4ec67932d6
--
2.20.1
On Wed, Feb 20, 2019 at 11:51:08AM +0100, Peter Eisentraut wrote:
So here is a patch doing it the "normal" way of nulling out all the rows
the user shouldn't see.
That looks fine to me.
I haven't found any documentation of these access restrictions in the
context of pg_stat_activity. Is there something that I'm not seeing or
something that should be added?
Yes, there is nothing. I agree that it would be good to mention that
some fields are set to NULL and only visible to superusers or members of
pg_read_all_stats with a note for pg_stat_activity and
pg_stat_get_activity(). Here is an idea:
"Backend and SSL information are restricted to superusers and members
of the <literal>pg_read_all_stats</literal> role. Access may be
granted to others using <command>GRANT</command>.
Getting that back-patched would be nice where pg_read_all_stats was
introduced.
--
Michael
On 2019-02-21 09:11, Michael Paquier wrote:
On Wed, Feb 20, 2019 at 11:51:08AM +0100, Peter Eisentraut wrote:
So here is a patch doing it the "normal" way of nulling out all the rows
the user shouldn't see.That looks fine to me.
Committed, thanks.
I haven't found any documentation of these access restrictions in the
context of pg_stat_activity. Is there something that I'm not seeing or
something that should be added?Yes, there is nothing. I agree that it would be good to mention that
some fields are set to NULL and only visible to superusers or members of
pg_read_all_stats with a note for pg_stat_activity and
pg_stat_get_activity(). Here is an idea:
"Backend and SSL information are restricted to superusers and members
of the <literal>pg_read_all_stats</literal> role. Access may be
granted to others using <command>GRANT</command>.Getting that back-patched would be nice where pg_read_all_stats was
introduced.
I added something. I don't know if it's worth backpatching. This
situation goes back all the way to when pg_stat_activity was added.
pg_read_all_stats does have documentation, it's just not linked from
everywhere.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services