Date: Tue, 9 Jun 2020 15:11:04 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: masahiko.sawada@2ndquadrant.com, martin@2ndquadrant.com,
	sfrost@snowman.net, pgsql-hackers@postgresql.org
Subject: Re: Read access for pg_monitor to pg_replication_origin_status view
Message-ID: <20200609061104.GD38342@paquier.xyz>
References: <20200604.161033.1845732016784727041.horikyota.ntt@gmail.com>
 <CAPdiE1wUt3WQZq30Wa1Xz0NM7rtVPZdK15PKRd0zS3ugZsCsow@mail.gmail.com>
 <CA+fd4k6ybp7M=GXKrJG0h4eg9XehmtGtKBxwwA9auHUuTNk+Kg@mail.gmail.com>
 <20200608.174456.238992210909952383.horikyota.ntt@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="7cm2iqirTL37Ot+N"
Content-Disposition: inline
In-Reply-To: <20200608.174456.238992210909952383.horikyota.ntt@gmail.com>
Precedence: bulk


--7cm2iqirTL37Ot+N
Content-Type: multipart/mixed; boundary="4Epv4kl9IRBfg3rk"
Content-Disposition: inline


--4Epv4kl9IRBfg3rk
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Jun 08, 2020 at 05:44:56PM +0900, Kyotaro Horiguchi wrote:
> Mmm. Right.

Yep.  I bumped on that myself.  I am not sure about 0002 and 0004 yet,
and IMO they are not mandatory pieces, but from what I can see in the
set 0001 and 0003 can just be squashed together to remove those
superuser checks, and no spots within the twelve functions calling
replorigin_check_prerequisites() are missing a REVOKE.  So something
like the attached could just happen first, no?  If the rights of
pg_read_all_stats need to be extended, it would always be possible to
do so once the attached is done with a custom script.

Also, why don't we use this occation to do the same thing for the
functions working on replication slots?  While we are looking at this
area, we may as well just do it.  Here is the set of functions that
would be involved:
- pg_create_physical_replication_slot
- pg_create_logical_replication_slot
- pg_replication_slot_advance
- pg_drop_replication_slot
- pg_copy_logical_replication_slot (3 functions)
- pg_copy_physical_replication_slot (2 functions)
--
Michael

--4Epv4kl9IRBfg3rk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="rep-origin-superuser-v6.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/sys=
tem_views.sql
index 56420bbc9d..5314e9348f 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1463,6 +1463,19 @@ REVOKE EXECUTE ON FUNCTION pg_read_binary_file(text)=
 FROM public;
 REVOKE EXECUTE ON FUNCTION pg_read_binary_file(text,bigint,bigint) FROM pu=
blic;
 REVOKE EXECUTE ON FUNCTION pg_read_binary_file(text,bigint,bigint,boolean)=
 FROM public;
=20
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_advance(text, pg_lsn) FRO=
M 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) F=
ROM public;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_session_is_setup() FROM p=
ublic;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_session_progress(boolean)=
 FROM public;
+REVOKE EXECUTE ON FUNCTION pg_replication_origin_session_reset() FROM publ=
ic;
+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, timest=
amp with time zone) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_show_replication_origin_status() FROM public;
+
 REVOKE EXECUTE ON FUNCTION pg_stat_file(text) FROM public;
 REVOKE EXECUTE ON FUNCTION pg_stat_file(text,boolean) FROM public;
=20
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replica=
tion/logical/origin.c
index 1ca4479605..bc50106070 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -182,11 +182,6 @@ static ReplicationState *session_replication_state =3D=
 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 =3D=3D 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index a8d57f4e39..b65aa28f34 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -24614,7 +24614,9 @@ postgres=3D# SELECT * FROM pg_walfile_name_offset(p=
g_stop_backup());
     <xref linkend=3D"streaming-replication-slots"/>, and
     <xref linkend=3D"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 only allowed to the
+    superuser by default, but may be allowed to other users by using the
+    <literal>GRANT</literal> command.
     Use of functions for replication slots is restricted to superusers
     and users having <literal>REPLICATION</literal> privilege.
    </para>

--4Epv4kl9IRBfg3rk--

--7cm2iqirTL37Ot+N
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAl7fJ/gACgkQnvQgOdby
QH367g//Zr+22X7AxDC0dGIglv1n7BfN/LUfb8wxNd1XWVDZzD+Fe8VwdIeMQ2KS
u/cUF6UMrsZYv6eNk7mOwPg8zth1RpI4hrmAObWt/RnZV8F4lVYJXHZwMfjRMLKd
iVoRNa0YMiVJcdkFI/UII0QQspvLsNFLfMzOtKdGrJ4YbGuNgMw0zUV6eyIQslzj
2YfNI7JecRmjFtOrmq1AxHKAVBVPVCou7A+xSwPX5huiAUesFyolMSUAsarooe81
1NR2OkI0F3lDczOakpySlMkMsVhS7UZ0Yd7CuQ1NNedl2qqnIVn6lT36ocP3IR1w
Um23YbpuDMAVfsR11OX77OibqiYIQ5QeZ/gdxV4gI8lNSQPKkp/bcet4mnLRiDdR
7Y5P3k70toD0u3aPy82TFrWHJyz2fsafu20VnzoI4DhV9zJ1RDR7pP4jP1w28vgv
sYQAAwHZXsfvvfrPH4xO+PqWxD9zLmuD/HW+mjRvWnobZ4v+kBXZ58Nbarso8TPs
GBoVyABNlSjmodKDLtplzZF2NLGtv3Q2e7xLkXxOQSXpVrnQ7Mb826QhiNxon6vJ
ugiLPSLxxKt05CPVndl9loJzaczcf8sryWQd3CYdYhMx2vskztpbNL/X4J9JiONN
8uYejFdM8ZaLNuzUNjFvJhX7v/v2PUELJHIU70Se/pEP3wxilm4=
=EUnC
-----END PGP SIGNATURE-----

--7cm2iqirTL37Ot+N--