pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedrestrict pg_stat_ssl to superuser?
9+ messages / 3 participants
[nested] [flat]
* restrict pg_stat_ssl to superuser?
@ 2019-02-07 08:30 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 2 replies; 9+ messages in thread
From: Peter Eisentraut @ 2019-02-07 08:30 UTC (permalink / raw)
To: pgsql-hackers
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.
[0]:
<https://www.postgresql.org/message-id/flat/398754d8-6bb5-c5cf-e7b8-22e5f0983caf@2ndquadrant.com;
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: restrict pg_stat_ssl to superuser?
@ 2019-02-12 06:40 Michael Paquier <michael@paquier.xyz>
parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
1 sibling, 1 reply; 9+ messages in thread
From: Michael Paquier @ 2019-02-12 06:40 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers
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
Attachments:
[application/pgp-signature] signature.asc (832B, ../../20190212064007.GN1475@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: restrict pg_stat_ssl to superuser?
@ 2019-02-15 13:04 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 9+ messages in thread
From: Peter Eisentraut @ 2019-02-15 13:04 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: restrict pg_stat_ssl to superuser?
@ 2019-02-18 03:58 Michael Paquier <michael@paquier.xyz>
parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 1 reply; 9+ messages in thread
From: Michael Paquier @ 2019-02-18 03:58 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers
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
Attachments:
[application/pgp-signature] signature.asc (832B, ../../20190218035813.GH15532@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: restrict pg_stat_ssl to superuser?
@ 2019-02-19 15:57 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 9+ messages in thread
From: Peter Eisentraut @ 2019-02-19 15:57 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: restrict pg_stat_ssl to superuser?
@ 2019-02-19 17:44 Robert Haas <robertmhaas@gmail.com>
parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
1 sibling, 0 replies; 9+ messages in thread
From: Robert Haas @ 2019-02-19 17:44 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: restrict pg_stat_ssl to superuser?
@ 2019-02-20 10:51 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 1 reply; 9+ messages in thread
From: Peter Eisentraut @ 2019-02-20 10:51 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers
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
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
Attachments:
[text/plain] v1-0001-Hide-other-user-s-pg_stat_ssl-rows.patch (3.9K, ../../3018acd9-e5d8-1e85-5ed7-47276cd77569@2ndquadrant.com/2-v1-0001-Hide-other-user-s-pg_stat_ssl-rows.patch)
download | inline diff:
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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: restrict pg_stat_ssl to superuser?
@ 2019-02-21 08:11 Michael Paquier <michael@paquier.xyz>
parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 1 reply; 9+ messages in thread
From: Michael Paquier @ 2019-02-21 08:11 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers
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
Attachments:
[application/pgp-signature] signature.asc (832B, ../../20190221081124.GS15532@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: restrict pg_stat_ssl to superuser?
@ 2019-02-21 18:56 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 9+ messages in thread
From: Peter Eisentraut @ 2019-02-21 18:56 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2019-02-21 18:56 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-02-07 08:30 restrict pg_stat_ssl to superuser? Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-02-12 06:40 ` Michael Paquier <michael@paquier.xyz>
2019-02-15 13:04 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-02-18 03:58 ` Michael Paquier <michael@paquier.xyz>
2019-02-19 15:57 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-02-20 10:51 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-02-21 08:11 ` Michael Paquier <michael@paquier.xyz>
2019-02-21 18:56 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-02-19 17:44 ` Robert Haas <robertmhaas@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox