agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedFrom: Paul Kim <mok03127@gmail.com>
To: jacob.champion@enterprisedb.com
Cc: reshkekirill@gmail.com
Cc: ayushtiwari.slg01@gmail.com
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: REVOKE's CASCADE protection doesn't work with INHERITed table owners
Date: Wed, 30 Sep 2026 13:32:14 +0900
Message-ID: <179074273441.6180.2515281464552050662@mail.gmail.com> (raw)
In-Reply-To: <CALdSSPhR+oy9cmV_Ltx+Vu7ihVRQSDW93R3pWCdgcAwJJeJ14Q@mail.gmail.com>
References: <CAOYmi+=KTLd+XsEP=TDiZ48iVf-CEc7JrZd5uhWPYWKEfOgyyQ@mail.gmail.com>
<CAOYmi+n_hw=SC5V1i3BmqfZPfPBRaUSJc+BeOXEKDwRue+WYrg@mail.gmail.com>
<CAJTYsWXbrewUuiMekCmah3V3fA+ZvcASyV6CJgaUQ2Wc-M0MLw@mail.gmail.com>
<CAOYmi+nns1VaW6W7kTJBTCTYVSfacJaZKyj-qxRuquT7082+WA@mail.gmail.com>
<CAJTYsWUvyQchDAA6y2a9YdLcApG=ccArpsbr77FeNZyx40bnmQ@mail.gmail.com>
<CALdSSPhR+oy9cmV_Ltx+Vu7ihVRQSDW93R3pWCdgcAwJJeJ14Q@mail.gmail.com>
Hi,
I reproduced the superuser case on PostgreSQL 18.4 and master
(5e592904025). Starting as a superuser:
CREATE ROLE o;
CREATE ROLE g;
CREATE ROLE r;
GRANT CREATE ON SCHEMA public TO o;
SET SESSION AUTHORIZATION o;
CREATE TABLE t (a int);
GRANT SELECT ON t TO g WITH GRANT OPTION;
SET SESSION AUTHORIZATION g;
GRANT SELECT ON t TO r;
RESET SESSION AUTHORIZATION;
ALTER ROLE g SUPERUSER;
REVOKE GRANT OPTION FOR SELECT ON t FROM g; -- no error
ALTER ROLE g NOSUPERUSER;
The REVOKE succeeds and leaves r=r/g in relacl, although g no longer
has the grant option. pg_dump emits the dependent grant under
SET SESSION AUTHORIZATION g. Restoring into a fresh database reports
"WARNING: no privileges were granted" and leaves r without SELECT.
With your patch, REVOKE fails with "dependent privileges exist";
with CASCADE, it removes r's grant.
Without GRANTED BY, a superuser's GRANT is recorded under the object
owner, which is why the sequence above has g grant before it is
promoted. On master, GRANTED BY g can also record the grant under g
while it is already a superuser.
I added a test for this case next to the atest4_groupowned tests.
Reverting just the acl.c change makes it fail: REVOKE succeeds, and
r keeps SELECT after CASCADE. The attached 0001 is your v1 rebased
onto master, with no changes to the code or tests; 0002 adds the
superuser test. Please feel free to fold 0002 into your patch.
I labeled them v2 only so the two apply as a set; renumber as you
like.
The pair applies with git am on REL_16_STABLE through REL_18_STABLE.
On 14 and 15, the acl.c prototype hunk needs git am -3. The core
regression suite passes on all five branches, and on master.
This sequence does not self-grant, so it does not exercise the
check_circularity issue discussed earlier in the thread.
Regards,
Paul Kim
Attachments:
[text/x-patch] v2-0001-Prevent-broken-grant-chains-when-indirect-grant-o.patch (10.0K, ../179074273441.6180.2515281464552050662@mail.gmail.com/2-v2-0001-Prevent-broken-grant-chains-when-indirect-grant-o.patch)
download | inline diff:
From 0a811de40f3b4bb7fe5d4e43ea2f583a5275c0f6 Mon Sep 17 00:00:00 2001
From: Jacob Champion <jacob.champion@enterprisedb.com>
Date: Thu, 25 Jun 2026 11:18:20 -0700
Subject: [PATCH v2 1/2] Prevent broken grant chains when indirect grant
options are held
recursive_revoke() is meant to protect against cases where a grantor
loses their grant option after issuing dependent grants to other roles.
In that case, the user must specify CASCADE to prune the dependent ACLs.
However, if a grantor happens to hold grant options (or table ownership)
indirectly, whether via INHERIT or SUPERUSER, recursive_revoke()
short-circuits and allows the REVOKE to break the grant chain. Symptoms
include general user confusion as well as problems during dump/restore,
which cannot recreate the original state.
To fix, switch from aclmask() to aclmask_direct().
Discussion: https://postgr.es/m/CAOYmi%2B%3DKTLd%2BXsEP%3DTDiZ48iVf-CEc7JrZd5uhWPYWKEfOgyyQ%40mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/adt/acl.c | 19 +++--
src/test/regress/expected/privileges.out | 88 ++++++++++++++++++++++++
src/test/regress/sql/privileges.sql | 44 ++++++++++++
3 files changed, 147 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 913d715a8c8..8bcda719e4c 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -102,6 +102,8 @@ static void check_circularity(const Acl *old_acl, const AclItem *mod_aip,
Oid ownerId);
static Acl *recursive_revoke(Acl *acl, Oid grantee, AclMode revoke_privs,
Oid ownerId, DropBehavior behavior);
+static AclMode aclmask_direct(const Acl *acl, Oid roleid, Oid ownerId,
+ AclMode mask, AclMaskHow how);
static AclMode convert_any_priv_string(text *priv_type_text,
const priv_map *privileges);
@@ -1349,10 +1351,19 @@ recursive_revoke(Acl *acl,
if (grantee == ownerId)
return acl;
- /* The grantee might still have some grant options via another grantor */
- still_has = aclmask(acl, grantee, ownerId,
- ACL_GRANT_OPTION_FOR(revoke_privs),
- ACLMASK_ALL);
+ /*
+ * The grantee might still have some grant options via another grantor,
+ * keeping the chain intact for those privileges. And if _all_ of the
+ * remaining privileges can still be granted by any role on the chain,
+ * we're done.
+ *
+ * Indirectly held grant options cannot keep the chain alive, so consult
+ * aclmask_direct() rather than aclmask(). (Otherwise, for example, a
+ * chain could be broken wherever the grantee was a superuser).
+ */
+ still_has = aclmask_direct(acl, grantee, ownerId,
+ ACL_GRANT_OPTION_FOR(revoke_privs),
+ ACLMASK_ALL);
revoke_privs &= ~ACL_OPTION_TO_PRIVS(still_has);
if (revoke_privs == ACL_NO_RIGHTS)
return acl;
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 9b7153c9cd5..4de9d749574 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1885,6 +1885,8 @@ CREATE TABLE atest4 (a int);
GRANT SELECT ON atest4 TO regress_priv_user2 WITH GRANT OPTION;
GRANT UPDATE ON atest4 TO regress_priv_user2;
GRANT SELECT ON atest4 TO GROUP regress_priv_group1 WITH GRANT OPTION;
+GRANT SELECT ON atest4 TO PUBLIC WITH GRANT OPTION; -- fail
+ERROR: grant options can only be granted to roles
SET SESSION AUTHORIZATION regress_priv_user2;
GRANT SELECT ON atest4 TO regress_priv_user3;
GRANT UPDATE ON atest4 TO regress_priv_user3; -- fail
@@ -1919,6 +1921,91 @@ SELECT has_table_privilege('regress_priv_user1', 'atest4', 'SELECT WITH GRANT OP
t
(1 row)
+-- Test GRANT OPTION chains where the intermediate grantor has the privileges of
+-- the owner (regress_priv_group1 -> regress_priv_user4 -> regress_priv_user2)
+-- or inherits the option (regress_priv_group2 -> regress_priv_user1 ->
+-- regress_priv_user3).
+-- pin the group membership that we rely on below
+SELECT pg_has_role('regress_priv_user4', 'regress_priv_group1', 'MEMBER'),
+ pg_has_role('regress_priv_user1', 'regress_priv_group2', 'MEMBER');
+ pg_has_role | pg_has_role
+-------------+-------------
+ t | t
+(1 row)
+
+SET SESSION AUTHORIZATION regress_priv_group1;
+CREATE TABLE atest4_groupowned (a int);
+GRANT SELECT ON atest4_groupowned TO regress_priv_user4 WITH GRANT OPTION;
+GRANT UPDATE ON atest4_groupowned TO regress_priv_group2 WITH GRANT OPTION;
+GRANT UPDATE ON atest4_groupowned TO regress_priv_user1 WITH GRANT OPTION;
+SET SESSION AUTHORIZATION regress_priv_user4;
+GRANT SELECT ON atest4_groupowned TO regress_priv_user2;
+SET SESSION AUTHORIZATION regress_priv_user1;
+GRANT UPDATE ON atest4_groupowned TO regress_priv_user3;
+SET SESSION AUTHORIZATION regress_priv_group1;
+REVOKE SELECT ON atest4_groupowned FROM regress_priv_user4; -- fail
+ERROR: dependent privileges exist
+HINT: Use CASCADE to revoke them too.
+SELECT has_table_privilege('regress_priv_user2', 'atest4_groupowned', 'SELECT'); -- true
+ has_table_privilege
+---------------------
+ t
+(1 row)
+
+SELECT has_table_privilege('regress_priv_user4', 'atest4_groupowned', 'SELECT'); -- true
+ has_table_privilege
+---------------------
+ t
+(1 row)
+
+REVOKE SELECT ON atest4_groupowned FROM regress_priv_user4 CASCADE; -- ok
+SELECT has_table_privilege('regress_priv_user2', 'atest4_groupowned', 'SELECT'); -- false
+ has_table_privilege
+---------------------
+ f
+(1 row)
+
+SELECT has_table_privilege('regress_priv_user4', 'atest4_groupowned', 'SELECT'); -- still true (inherited)
+ has_table_privilege
+---------------------
+ t
+(1 row)
+
+REVOKE UPDATE ON atest4_groupowned FROM regress_priv_user1; -- fail
+ERROR: dependent privileges exist
+HINT: Use CASCADE to revoke them too.
+SELECT has_table_privilege('regress_priv_user3', 'atest4_groupowned', 'UPDATE'); -- true
+ has_table_privilege
+---------------------
+ t
+(1 row)
+
+SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- true
+ has_table_privilege
+---------------------
+ t
+(1 row)
+
+REVOKE UPDATE ON atest4_groupowned FROM regress_priv_user1 CASCADE; -- ok
+SELECT has_table_privilege('regress_priv_user3', 'atest4_groupowned', 'UPDATE'); -- false
+ has_table_privilege
+---------------------
+ f
+(1 row)
+
+SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- still true (inherited)
+ has_table_privilege
+---------------------
+ t
+(1 row)
+
+REVOKE UPDATE ON atest4_groupowned FROM regress_priv_group2 RESTRICT; -- ok
+SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- false
+ has_table_privilege
+---------------------
+ f
+(1 row)
+
-- security-restricted operations
\c -
CREATE ROLE regress_sro_user;
@@ -3228,6 +3315,7 @@ DROP TABLE atest1;
DROP TABLE atest2;
DROP TABLE atest3;
DROP TABLE atest4;
+DROP TABLE atest4_groupowned;
DROP TABLE atest5;
DROP TABLE atest6;
DROP TABLE atestc;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 0113dfa5dab..c34f5196fe5 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -1226,6 +1226,7 @@ CREATE TABLE atest4 (a int);
GRANT SELECT ON atest4 TO regress_priv_user2 WITH GRANT OPTION;
GRANT UPDATE ON atest4 TO regress_priv_user2;
GRANT SELECT ON atest4 TO GROUP regress_priv_group1 WITH GRANT OPTION;
+GRANT SELECT ON atest4 TO PUBLIC WITH GRANT OPTION; -- fail
SET SESSION AUTHORIZATION regress_priv_user2;
@@ -1243,6 +1244,48 @@ SELECT has_table_privilege('regress_priv_user3', 'atest4', 'SELECT'); -- false
SELECT has_table_privilege('regress_priv_user1', 'atest4', 'SELECT WITH GRANT OPTION'); -- true
+-- Test GRANT OPTION chains where the intermediate grantor has the privileges of
+-- the owner (regress_priv_group1 -> regress_priv_user4 -> regress_priv_user2)
+-- or inherits the option (regress_priv_group2 -> regress_priv_user1 ->
+-- regress_priv_user3).
+
+-- pin the group membership that we rely on below
+SELECT pg_has_role('regress_priv_user4', 'regress_priv_group1', 'MEMBER'),
+ pg_has_role('regress_priv_user1', 'regress_priv_group2', 'MEMBER');
+
+SET SESSION AUTHORIZATION regress_priv_group1;
+
+CREATE TABLE atest4_groupowned (a int);
+
+GRANT SELECT ON atest4_groupowned TO regress_priv_user4 WITH GRANT OPTION;
+GRANT UPDATE ON atest4_groupowned TO regress_priv_group2 WITH GRANT OPTION;
+GRANT UPDATE ON atest4_groupowned TO regress_priv_user1 WITH GRANT OPTION;
+
+SET SESSION AUTHORIZATION regress_priv_user4;
+
+GRANT SELECT ON atest4_groupowned TO regress_priv_user2;
+
+SET SESSION AUTHORIZATION regress_priv_user1;
+
+GRANT UPDATE ON atest4_groupowned TO regress_priv_user3;
+
+SET SESSION AUTHORIZATION regress_priv_group1;
+
+REVOKE SELECT ON atest4_groupowned FROM regress_priv_user4; -- fail
+SELECT has_table_privilege('regress_priv_user2', 'atest4_groupowned', 'SELECT'); -- true
+SELECT has_table_privilege('regress_priv_user4', 'atest4_groupowned', 'SELECT'); -- true
+REVOKE SELECT ON atest4_groupowned FROM regress_priv_user4 CASCADE; -- ok
+SELECT has_table_privilege('regress_priv_user2', 'atest4_groupowned', 'SELECT'); -- false
+SELECT has_table_privilege('regress_priv_user4', 'atest4_groupowned', 'SELECT'); -- still true (inherited)
+
+REVOKE UPDATE ON atest4_groupowned FROM regress_priv_user1; -- fail
+SELECT has_table_privilege('regress_priv_user3', 'atest4_groupowned', 'UPDATE'); -- true
+SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- true
+REVOKE UPDATE ON atest4_groupowned FROM regress_priv_user1 CASCADE; -- ok
+SELECT has_table_privilege('regress_priv_user3', 'atest4_groupowned', 'UPDATE'); -- false
+SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- still true (inherited)
+REVOKE UPDATE ON atest4_groupowned FROM regress_priv_group2 RESTRICT; -- ok
+SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- false
-- security-restricted operations
\c -
@@ -1918,6 +1961,7 @@ DROP TABLE atest1;
DROP TABLE atest2;
DROP TABLE atest3;
DROP TABLE atest4;
+DROP TABLE atest4_groupowned;
DROP TABLE atest5;
DROP TABLE atest6;
DROP TABLE atestc;
--
2.50.1 (Apple Git-155)
[text/x-patch] v2-0002-Test-grant-option-chains-through-a-grantor-that-b.patch (4.0K, ../179074273441.6180.2515281464552050662@mail.gmail.com/3-v2-0002-Test-grant-option-chains-through-a-grantor-that-b.patch)
download | inline diff:
From a9993f9a4ff90264aa10cb747d83ca58a4538fed Mon Sep 17 00:00:00 2001
From: Paul Kim <mok03127@gmail.com>
Date: Mon, 28 Sep 2026 10:17:34 +0900
Subject: [PATCH v2 2/2] Test grant option chains through a grantor that became
superuser
The previous commit tests inherited ownership and grant options.
Add coverage for an intermediate grantor that becomes a superuser
after issuing a grant.
Without the fix, revoking that role's grant option succeeds and leaves
its dependent grant in the ACL. After the role loses SUPERUSER,
dump/restore loses the dependent grant with a warning. Check that
RESTRICT (the default) rejects the revocation and CASCADE removes
the dependent grant.
Discussion: https://postgr.es/m/CAOYmi%2B%3DKTLd%2BXsEP%3DTDiZ48iVf-CEc7JrZd5uhWPYWKEfOgyyQ%40mail.gmail.com
---
src/test/regress/expected/privileges.out | 31 ++++++++++++++++++++++++
src/test/regress/sql/privileges.sql | 29 ++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 4de9d749574..b6276b30d15 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -2006,6 +2006,37 @@ SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE');
f
(1 row)
+-- Same, but the intermediate grantor has become a superuser since issuing its
+-- grant (regress_priv_user1 -> regress_priv_su -> regress_priv_user2).
+RESET SESSION AUTHORIZATION;
+CREATE ROLE regress_priv_su;
+SET SESSION AUTHORIZATION regress_priv_user1;
+CREATE TABLE atest4_su (a int);
+GRANT SELECT ON atest4_su TO regress_priv_su WITH GRANT OPTION;
+SET SESSION AUTHORIZATION regress_priv_su;
+GRANT SELECT ON atest4_su TO regress_priv_user2;
+RESET SESSION AUTHORIZATION;
+ALTER ROLE regress_priv_su SUPERUSER;
+SET SESSION AUTHORIZATION regress_priv_user1;
+REVOKE GRANT OPTION FOR SELECT ON atest4_su FROM regress_priv_su; -- fail
+ERROR: dependent privileges exist
+HINT: Use CASCADE to revoke them too.
+SELECT has_table_privilege('regress_priv_user2', 'atest4_su', 'SELECT'); -- true
+ has_table_privilege
+---------------------
+ t
+(1 row)
+
+REVOKE GRANT OPTION FOR SELECT ON atest4_su FROM regress_priv_su CASCADE; -- ok
+SELECT has_table_privilege('regress_priv_user2', 'atest4_su', 'SELECT'); -- false
+ has_table_privilege
+---------------------
+ f
+(1 row)
+
+RESET SESSION AUTHORIZATION;
+DROP TABLE atest4_su;
+DROP ROLE regress_priv_su;
-- security-restricted operations
\c -
CREATE ROLE regress_sro_user;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index c34f5196fe5..a05aae34b6b 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -1287,6 +1287,35 @@ SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE');
REVOKE UPDATE ON atest4_groupowned FROM regress_priv_group2 RESTRICT; -- ok
SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- false
+-- Same, but the intermediate grantor has become a superuser since issuing its
+-- grant (regress_priv_user1 -> regress_priv_su -> regress_priv_user2).
+RESET SESSION AUTHORIZATION;
+CREATE ROLE regress_priv_su;
+
+SET SESSION AUTHORIZATION regress_priv_user1;
+
+CREATE TABLE atest4_su (a int);
+
+GRANT SELECT ON atest4_su TO regress_priv_su WITH GRANT OPTION;
+
+SET SESSION AUTHORIZATION regress_priv_su;
+
+GRANT SELECT ON atest4_su TO regress_priv_user2;
+
+RESET SESSION AUTHORIZATION;
+ALTER ROLE regress_priv_su SUPERUSER;
+
+SET SESSION AUTHORIZATION regress_priv_user1;
+
+REVOKE GRANT OPTION FOR SELECT ON atest4_su FROM regress_priv_su; -- fail
+SELECT has_table_privilege('regress_priv_user2', 'atest4_su', 'SELECT'); -- true
+REVOKE GRANT OPTION FOR SELECT ON atest4_su FROM regress_priv_su CASCADE; -- ok
+SELECT has_table_privilege('regress_priv_user2', 'atest4_su', 'SELECT'); -- false
+
+RESET SESSION AUTHORIZATION;
+DROP TABLE atest4_su;
+DROP ROLE regress_priv_su;
+
-- security-restricted operations
\c -
CREATE ROLE regress_sro_user;
--
2.50.1 (Apple Git-155)
view thread (7+ messages)
Message-ID: <179074273441.6180.2515281464552050662@mail.gmail.com>
Permalink: ../179074273441.6180.2515281464552050662@mail.gmail.com/
Also on: postgresql.org/message-id/179074273441.6180.2515281464552050662@mail.gmail.com
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: mok03127@gmail.com, jacob.champion@enterprisedb.com, reshkekirill@gmail.com, ayushtiwari.slg01@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: REVOKE's CASCADE protection doesn't work with INHERITed table owners
In-Reply-To: <179074273441.6180.2515281464552050662@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox