Hi Stephen,
> > I have no problem adding it to this ROLE, but we'd have to amend the
> > doc for default-roles to reflect that SELECT for this view is also
> > granted to `pg_read_all_stats`.
>
> I agree in general that pg_monitor shouldn't have privileges granted
> directly to it. If this needs a new default role, that's an option, but
> it seems like it'd make sense to be part of pg_read_all_stats to me, so
> amending the docs looks reasonable from here.
Good, that's more or less what I had in mind.
Here goes v2 of the patch, now there are 4 files (I could have
squashed the docs with the code changes, but hey, that'll be easy to
merge if needed :-) )
I did some fiddling to Michaels doc proposal, but it says basically the same.
Not 100% happy with the change to user-manag.sgml, but ok enough to send.
I also added an entry to the commitfest so we can track this there as well.
Regards,
--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From 87c00782c691f2c6c13768cd6d96b19de69cab16 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Marqu=C3=A9s?=
<martin.marq...@2ndquadrant.com>
Date: Tue, 2 Jun 2020 10:44:42 -0300
Subject: [PATCH v2 1/4] Access to pg_replication_origin_status view has
restricted access only for superusers due to it using
pg_show_replication_origin_status(), and all replication origin functions
requiring superuser rights.
This patch removes the check for superuser priviledges when executing
replication origin functions, which is hardcoded, and instead rely on
ACL checks.
This patch will also revoke execution of such functions from PUBLIC
---
src/backend/catalog/system_views.sql | 16 ++++++++++++++++
src/backend/replication/logical/origin.c | 5 -----
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 56420bbc9d6..6658f0c2eb2 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1469,6 +1469,22 @@ REVOKE EXECUTE ON FUNCTION pg_stat_file(text,boolean) FROM public;
REVOKE EXECUTE ON FUNCTION pg_ls_dir(text) FROM public;
REVOKE EXECUTE ON FUNCTION pg_ls_dir(text,boolean,boolean) FROM public;
+--
+-- Permision to execute Replication Origin functions should be revoked from public
+--
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_advance(text, pg_lsn) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_create(text) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_drop(text) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_oid(text) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_progress(text, boolean) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_session_is_setup() FROM public;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_session_progress(boolean) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_session_reset() FROM public;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_session_setup(text) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_xact_reset() FROM public;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_xact_setup(pg_lsn, timestamp with time zone) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_show_replication_origin_status() FROM public;
+
--
-- We also set up some things as accessible to standard roles.
--
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index 981d60f135d..1f223daf21f 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -182,11 +182,6 @@ static ReplicationState *session_replication_state = NULL;
static void
replorigin_check_prerequisites(bool check_slots, bool recoveryOK)
{
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("only superusers can query or manipulate replication origins")));
-
if (check_slots && max_replication_slots == 0)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
--
2.21.3
From 55068efd0da51efe56a5123e56572e36b9d5f623 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Marqu=C3=A9s?=
<martin.marq...@2ndquadrant.com>
Date: Tue, 2 Jun 2020 13:05:10 -0300
Subject: [PATCH v2 3/4] Change replication origin function documenation to
reflect that now none superusers could be granted execution of these
functions.
---
doc/src/sgml/func.sgml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 7c06afd3eaf..cd5c985911f 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -24617,7 +24617,9 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
<xref linkend="streaming-replication-slots"/>, and
<xref linkend="replication-origins"/>
for information about the underlying features.
- Use of functions for replication origin is restricted to superusers.
+ Use of functions for replication origin is by default only allowed
+ to the superuser, but may be permitted to other users by using the
+ GRANT command.
Use of functions for replication slots is restricted to superusers
and users having <literal>REPLICATION</literal> privilege.
</para>
--
2.21.3
From c6f0705ae82793584344141b630253682e051406 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Marqu=C3=A9s?=
<martin.marq...@2ndquadrant.com>
Date: Tue, 2 Jun 2020 12:43:02 -0300
Subject: [PATCH v2 2/4] We want the monitoring role `pg_read_all_stats` to be
able to SELECT from `pg_replication_origin_status`.
This will help so output diagnostics and monitoring tools can be able to
gather replication origins status without requiring superuser privileges
---
src/backend/catalog/system_views.sql | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 6658f0c2eb2..995bb5663fc 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1497,3 +1497,5 @@ GRANT EXECUTE ON FUNCTION pg_ls_tmpdir(oid) TO pg_monitor;
GRANT pg_read_all_settings TO pg_monitor;
GRANT pg_read_all_stats TO pg_monitor;
GRANT pg_stat_scan_tables TO pg_monitor;
+
+GRANT SELECT ON pg_replication_origin_status TO pg_read_all_stats;
--
2.21.3
From f7f6c198d2a29b608399713e0d10156bd9ebfeb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Marqu=C3=A9s?=
<martin.marq...@2ndquadrant.com>
Date: Tue, 2 Jun 2020 13:06:49 -0300
Subject: [PATCH v2 4/4] Apply changes to the documentation to reflect that now
pg_read_all_stats also has SELECT privileges on pg_replication_origin_status
---
doc/src/sgml/user-manag.sgml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index 829decd8839..88b38bc15d5 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -524,8 +524,8 @@ DROP ROLE doomed_role;
</row>
<row>
<entry>pg_read_all_stats</entry>
- <entry>Read all pg_stat_* views and use various statistics related extensions,
- even those normally visible only to superusers.</entry>
+ <entry>Read all pg_stat_* and pg_replication_origin_status views, and use various statistics
+ related extensions, even those normally visible only to superusers.</entry>
</row>
<row>
<entry>pg_stat_scan_tables</entry>
--
2.21.3
