Hi,
Took me a bit longer than expected, but here is a new version, now
with the idea of just removing the superuser() check and REVOKEing
execution of the functions from public. At the end I grant permission
to functions and the pg_replication_origin_status view.
I wonder now if I needed to GRANT execution of the functions. A grant
on the view should be enough.
I'll think about it.
El dom., 31 de may. de 2020 a la(s) 12:13, Martín Marqués
(mar...@2ndquadrant.com) escribió:
>
> Hi Michael,
>
> > Wouldn't it be just better to remove this hardcoded superuser check
> > and replace it with equivalent ACLs by default? The trick is to make
> > sure that any function calling replorigin_check_prerequisites() has
> > its execution correctly revoked from public. See for example
> > e79350fe.
>
> Looking at that, it seems a better solution. Let me wrap up a new
> patch, likely later today or early tomorrow as it's Sunday ;-)
>
> --
> Martín Marqués http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From 945cd1847d624da64e15e3bdbbabcf34a04f848f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Marqu=C3=A9s?=
<martin.marq...@2ndquadrant.com>
Date: Mon, 1 Jun 2020 08:39:54 -0300
Subject: [PATCH 1/2] Access to pg_replication_origin_status view was
restricted to superusers only due to it using
pg_show_replication_origin_status(), and that all replication origin
functions require superuser.
This patch removes the check for superuser, which is hardcoded, and relies on
ACL checks (we revoke execution on such functions)
---
src/backend/catalog/system_views.sql | 20 ++++++++++++++++++++
src/backend/replication/logical/origin.c | 5 -----
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 56420bbc9d6..a95f30ccaed 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1469,6 +1469,21 @@ 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 modification 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;
+
--
-- We also set up some things as accessible to standard roles.
--
@@ -1478,6 +1493,11 @@ GRANT EXECUTE ON FUNCTION pg_ls_archive_statusdir() TO pg_monitor;
GRANT EXECUTE ON FUNCTION pg_ls_tmpdir() TO pg_monitor;
GRANT EXECUTE ON FUNCTION pg_ls_tmpdir(oid) TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_replication_origin_progress(text, boolean) TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_replication_origin_session_progress(boolean) 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_monitor;
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 4016d5f817a202ea272eb3ef8a9f17dfec2b66ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Marqu=C3=A9s?=
<martin.marq...@2ndquadrant.com>
Date: Mon, 1 Jun 2020 14:46:11 -0300
Subject: [PATCH 2/2] Forgot to REVOKE permissions over
pg_show_replication_origin_status() to PUBLIC.
After testing, I uncommented the GRANT SELECT over the pg_replication_origin_status
---
src/backend/catalog/system_views.sql | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index a95f30ccaed..c16061f8f00 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1483,6 +1483,7 @@ 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.
@@ -1500,4 +1501,4 @@ 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_monitor;
+GRANT SELECT ON pg_replication_origin_status TO pg_monitor;
--
2.21.3
