Hi Kyotaro-san,
> Sorry for not mentioning it at that time, but about the following diff:
>
> +GRANT SELECT ON pg_replication_origin_status TO pg_read_all_stats;
>
> system_views.sql already has a REVOKE command on the view. We should
> put the above just below the REVOKE command.
>
> I'm not sure where to put the GRANT on
> pg_show_replication_origin_status(), but maybe it also should be at
> the same place.
Yes, I agree that it makes the revoking/granting easier to read if
it's grouped by objects, or groups of objects.
Done.
> In the previous comment, one point I meant is that the "to the
> superuser" should be "to superusers", because a PostgreSQL server
> (cluster) can define multiple superusers. Another is that "permitted
> to other users by using the GRANT command." might be obscure for
> users. In this regard I found a more specific description in the same
> file:
OK, now I understand what you were saying. :-)
> Computes the total disk space used by the database with the specified
> name or OID. To use this function, you must
> have <literal>CONNECT</literal> privilege on the specified database
> (which is granted by default) or be a member of
> the <literal>pg_read_all_stats</literal> role.
>
> So, as the result it would be like the following: (Note that, as you
> know, I'm not good at this kind of task..)
>
> Use of functions for replication origin is restricted to superusers.
> Use for these functions may be permitted to other users by granting
> <literal>EXECUTE<literal> privilege on the functions.
>
> And in regard to the view, granting privileges on both the view and
> function to individual user is not practical so we should mention only
> granting pg_read_all_stats to users, like the attached patch.
I did some re-writing of the doc, which is pretty close to what you
proposed above.
> By the way, the attachements of your mail are out-of-order. I'm not
> sure that that does something bad, though.
That's likely Gmail giving them random order when you attach multiple
files all at once.
New patches attached.
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 v4 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 64bafe6be9bfea8baae0806fe9416394ff321e57 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Marqu=C3=A9s?=
<martin.marq...@2ndquadrant.com>
Date: Thu, 4 Jun 2020 08:44:20 -0300
Subject: [PATCH v4 2/4] We want the monitoring role `pg_read_all_stats` to be
able to SELECT from `pg_replication_origin_status`. We also need to give this
role EXECUTE privileges for `pg_show_replication_origin_status()`.
---
src/backend/catalog/system_views.sql | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 6658f0c2eb2..9949b4316f5 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1123,6 +1123,9 @@ CREATE VIEW pg_replication_origin_status AS
REVOKE ALL ON pg_replication_origin_status FROM public;
+GRANT SELECT ON pg_replication_origin_status TO pg_read_all_stats;
+
+
-- All columns of pg_subscription except subconninfo are readable.
REVOKE ALL ON pg_subscription FROM public;
GRANT SELECT (subdbid, subname, subowner, subenabled, subslotname, subpublications)
@@ -1485,6 +1488,8 @@ 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;
+GRANT EXECUTE ON FUNCTION pg_show_replication_origin_status() TO pg_read_all_stats;
+
--
-- We also set up some things as accessible to standard roles.
--
--
2.21.3
From 7a26662f4a451fa3bafb5425b94251cc7f08d4ab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Marqu=C3=A9s?=
<martin.marq...@2ndquadrant.com>
Date: Thu, 4 Jun 2020 09:04:36 -0300
Subject: [PATCH v4 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 | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 7c06afd3eaf..e1a820dfbdd 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -24617,7 +24617,10 @@ 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 allowed only
+ to superusers.
+ To use these functions, you must have <literal>EXECUTE<literal>
+ privilege on them.
Use of functions for replication slots is restricted to superusers
and users having <literal>REPLICATION</literal> privilege.
</para>
--
2.21.3
From 344f4e04aa0772520deff5d9e311d6c91ea63cbd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Marqu=C3=A9s?=
<martin.marq...@2ndquadrant.com>
Date: Thu, 4 Jun 2020 09:05:39 -0300
Subject: [PATCH v4 4/4] Apply changes to the documentation to reflect that now
pg_read_all_stats a lso 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
