From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: restrict pg_stat_ssl to superuser?
Date: Wed, 20 Feb 2019 11:51:08 +0100
Message-ID: <3018acd9-e5d8-1e85-5ed7-47276cd77569@2ndquadrant.com> (raw)
In-Reply-To: <1d1bf3a7-955e-ce6b-de99-b900136ca789@2ndquadrant.com>
References: <63117976-d02c-c8e2-3aef-caa31a5ab8d3@2ndquadrant.com>
<20190212064007.GN1475@paquier.xyz>
<71319f2d-a073-0499-4562-6003e026307e@2ndquadrant.com>
<20190218035813.GH15532@paquier.xyz>
<1d1bf3a7-955e-ce6b-de99-b900136ca789@2ndquadrant.com>
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.cindex 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.cindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter.eisentraut@2ndquadrant.com, michael@paquier.xyz
Subject: Re: restrict pg_stat_ssl to superuser?
In-Reply-To: <3018acd9-e5d8-1e85-5ed7-47276cd77569@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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