missing GRANT on pg_subscription columns

Started by Euler Taveiraover 4 years ago5 messages

euler@eulerto.com

over 4 years ago

2 attachment(s)

Hi,

I was checking the GRANT on pg_subscription and noticed that the command is not
correct. There is a comment that says "All columns of pg_subscription except
subconninfo are readable". However, there are columns that aren't included: oid
and subsynccommit. It seems an oversight in the commits 6f236e1eb8c and
887227a1cc8.

There are monitoring tools and data collectors that aren't using a
superuser to read catalog information (I usually recommend using pg_monitor).
Hence, you cannot join pg_subscription with relations such as
pg_subscription_rel or pg_stat_subscription because column oid has no
column-level privilege. I'm attaching a patch to fix it (indeed, 2 patches
because of additional columns for v14). We should add instructions in the minor
version release notes too.

This issue was reported by Israel Barth.

--
Euler Taveira
EDB https://www.enterprisedb.com/

Attachments:

grant-pg-subscription.patchtext/x-patch; name=grant-pg-subscription.patchDownload

From 64b138b9974934f689e57fc34d370424b2a348a9 Mon Sep 17 00:00:00 2001
From: Euler Taveira <euler.taveira@enterprisedb.com>
Date: Mon, 31 May 2021 19:40:36 -0300
Subject: [PATCH] Grant read privilege to additional pg_subscription columns

pg_subscription should be read by PUBLIC except subconninfo column.
Documentation is correct but the GRANT command is not. Columns oid and
subsynccommit don't have the right privileges. It seems an oversight in
the commits 6f236e1eb8c and 887227a1cc8. The current behavior prohibits
joins between pg_subscription and related tables (pg_subscription_rel
and pg_stat_subscription) for non-superusers.
---
 src/backend/catalog/system_views.sql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 5c84d758bb..5088e7f1d5 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1254,5 +1254,5 @@ REVOKE ALL ON pg_replication_origin_status FROM public;
 
 -- All columns of pg_subscription except subconninfo are readable.
 REVOKE ALL ON pg_subscription FROM public;
-GRANT SELECT (subdbid, subname, subowner, subenabled, subbinary, substream, subslotname, subpublications)
+GRANT SELECT (oid, subdbid, subname, subowner, subenabled, subbinary, substream, subslotname, subsynccommit, subpublications)
     ON pg_subscription TO public;
-- 
2.20.1

grant-pg-subscription-10-13.patchtext/x-patch; name=grant-pg-subscription-10-13.patchDownload

From d54165988c69a021962adaad6e2e31f80f4ad85c Mon Sep 17 00:00:00 2001
From: Euler Taveira <euler.taveira@enterprisedb.com>
Date: Tue, 1 Jun 2021 11:17:26 -0300
Subject: [PATCH] Grant read privilege to additional pg_subscription columns

pg_subscription should be read by PUBLIC except subconninfo column.
Documentation is correct but the GRANT command is not. Columns oid and
subsynccommit don't have the right privileges. It seems an oversight in
the commits 6f236e1eb8c and 887227a1cc8. The current behavior prohibits
joins between pg_subscription and related tables (pg_subscription_rel
and pg_stat_subscription) for non-superusers.
---
 src/backend/catalog/system_views.sql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 51d738cc42..eb363c9ade 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1122,7 +1122,7 @@ REVOKE ALL ON pg_replication_origin_status FROM public;
 
 -- All columns of pg_subscription except subconninfo are readable.
 REVOKE ALL ON pg_subscription FROM public;
-GRANT SELECT (subdbid, subname, subowner, subenabled, subslotname, subpublications)
+GRANT SELECT (oid, subdbid, subname, subowner, subenabled, subslotname, subsynccommit, subpublications)
     ON pg_subscription TO public;
 
 
-- 
2.20.1

Tom Lane

tgl@sss.pgh.pa.us

over 4 years ago

In reply to: Euler Taveira (#1)

Re: missing GRANT on pg_subscription columns

"Euler Taveira" <euler@eulerto.com> writes:

I was checking the GRANT on pg_subscription and noticed that the command is not
correct. There is a comment that says "All columns of pg_subscription except
subconninfo are readable". However, there are columns that aren't included: oid
and subsynccommit. It seems an oversight in the commits 6f236e1eb8c and
887227a1cc8.

Ugh.

There are monitoring tools and data collectors that aren't using a
superuser to read catalog information (I usually recommend using pg_monitor).
Hence, you cannot join pg_subscription with relations such as
pg_subscription_rel or pg_stat_subscription because column oid has no
column-level privilege. I'm attaching a patch to fix it (indeed, 2 patches
because of additional columns for v14). We should add instructions in the minor
version release notes too.

I agree with fixing this in HEAD. But given that this has been wrong
since v10 with zero previous complaints, I doubt that it is worth the
complication of trying to do something about it in the back branches.
Maybe we could just adjust the docs there, instead.

regards, tom lane

Amit Kapila

amit.kapila16@gmail.com

over 4 years ago

In reply to: Tom Lane (#2)

Re: missing GRANT on pg_subscription columns

On Thu, Jun 3, 2021 at 10:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Euler Taveira" <euler@eulerto.com> writes:

I was checking the GRANT on pg_subscription and noticed that the command is not
correct. There is a comment that says "All columns of pg_subscription except
subconninfo are readable". However, there are columns that aren't included: oid
and subsynccommit. It seems an oversight in the commits 6f236e1eb8c and
887227a1cc8.

Ugh.

There are monitoring tools and data collectors that aren't using a
superuser to read catalog information (I usually recommend using pg_monitor).
Hence, you cannot join pg_subscription with relations such as
pg_subscription_rel or pg_stat_subscription because column oid has no
column-level privilege. I'm attaching a patch to fix it (indeed, 2 patches
because of additional columns for v14). We should add instructions in the minor
version release notes too.

I agree with fixing this in HEAD. But given that this has been wrong
since v10 with zero previous complaints, I doubt that it is worth the
complication of trying to do something about it in the back branches.
Maybe we could just adjust the docs there, instead.

This sounds reasonable to me. Euler, can you provide the doc updates
for back-branches?

--
With Regards,
Amit Kapila.

vignesh C

vignesh21@gmail.com

over 4 years ago

In reply to: Amit Kapila (#3)

1 attachment(s)

Re: missing GRANT on pg_subscription columns

On Mon, Jun 7, 2021 at 2:38 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Thu, Jun 3, 2021 at 10:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Euler Taveira" <euler@eulerto.com> writes:

I was checking the GRANT on pg_subscription and noticed that the command is not
correct. There is a comment that says "All columns of pg_subscription except
subconninfo are readable". However, there are columns that aren't included: oid
and subsynccommit. It seems an oversight in the commits 6f236e1eb8c and
887227a1cc8.

Ugh.

There are monitoring tools and data collectors that aren't using a
superuser to read catalog information (I usually recommend using pg_monitor).
Hence, you cannot join pg_subscription with relations such as
pg_subscription_rel or pg_stat_subscription because column oid has no
column-level privilege. I'm attaching a patch to fix it (indeed, 2 patches
because of additional columns for v14). We should add instructions in the minor
version release notes too.

I agree with fixing this in HEAD. But given that this has been wrong
since v10 with zero previous complaints, I doubt that it is worth the
complication of trying to do something about it in the back branches.
Maybe we could just adjust the docs there, instead.

This sounds reasonable to me. Euler, can you provide the doc updates
for back-branches?

Attached patch has the documentation changes for the back-branches. As
there is no specific reason for this, I have just mentioned
"Additionally normal users can't access columns oid and
subsynccommit." The same patch applies till V10 branch.

Regards,
Vignesh

Attachments:

v1-0001-Documentation-for-normal-users-not-having-permiss.patchtext/x-patch; charset=US-ASCII; name=v1-0001-Documentation-for-normal-users-not-having-permiss.patchDownload

From 37f9fc48baa2c233d16ee8ac1e8547680cd05b04 Mon Sep 17 00:00:00 2001
From: vignesh <vignesh21@gmail.com>
Date: Mon, 28 Jun 2021 10:06:58 +0530
Subject: [PATCH v1] Documentation for normal users not having permission for
 columns oid and subsynccommit in pg_subscription catalog table.

Documentation for normal users not having permission for columns oid
and subsynccommit in pg_subscription catalog table.
---
 doc/src/sgml/catalogs.sgml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 4dff3f60a2..f6b5c2e562 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -7457,7 +7457,9 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
 
   <para>
    Access to the column <structfield>subconninfo</structfield> is revoked from
-   normal users, because it could contain plain-text passwords.
+   normal users, because it could contain plain-text passwords. Additionally
+   normal users can't access columns <structfield>oid</structfield> and
+   <structfield>subsynccommit</structfield>.
   </para>
 
   <table>
-- 
2.25.1

Amit Kapila

amit.kapila16@gmail.com

over 4 years ago

In reply to: vignesh C (#4)

Re: missing GRANT on pg_subscription columns

On Mon, Jun 28, 2021 at 11:02 AM vignesh C <vignesh21@gmail.com> wrote:

On Mon, Jun 7, 2021 at 2:38 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Thu, Jun 3, 2021 at 10:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Attached patch has the documentation changes for the back-branches. As
there is no specific reason for this, I have just mentioned
"Additionally normal users can't access columns oid and
subsynccommit." The same patch applies till V10 branch.

Thanks for the patch. Tom has already pushed the code as part of
commit 3590680b85, so I am not sure if it is still valuable to fix
docs in back-branches.

--
With Regards,
Amit Kapila.