pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedmissing privilege check after not-null constraint rework
3+ messages / 1 participants
[nested] [flat]
Commits from this thread:
2023-09-07 ac22a9545c Move privilege check to the right place
* missing privilege check after not-null constraint rework
@ 2023-09-05 17:44 Alvaro Herrera <alvherre@alvh.no-ip.org>
2023-09-05 17:45 ` Re: missing privilege check after not-null constraint rework Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 1 reply; 3+ messages in thread
From: Alvaro Herrera @ 2023-09-05 17:44 UTC (permalink / raw)
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Here's a fix to move the privilege check on constraint dropping from
ATExecDropConstraint to dropconstraint_internal. The former doesn't
recurse anymore, so there's no point in doing that or in fact even
having the 'recursing' argument anymore.
This fixes the following test case
CREATE ROLE alice;
CREATE ROLE bob;
GRANT ALL ON SCHEMA PUBLIC to alice, bob;
GRANT alice TO bob;
SET ROLE alice;
CREATE TABLE parent (a int NOT NULL);
SET ROLE bob;
CREATE TABLE child () INHERITS (parent);
At this point, bob owns the child table, to which alice has no access.
But alice can do this:
ALTER TABLE parent ALTER a DROP NOT NULL;
which is undesirable, because it removes the NOT NULL constraint from
table child, which is owned by bob.
Alternatively, we could say that Alice is allowed to drop the constraint
on her table, and that we should react by marking the constraint on
Bob's child table as 'islocal' instead of removing it. Now, I'm pretty
sure we don't really care one bit about this case, and the reason is
this: we seem to have no tests for mixed-ownership table hierarchies.
If we did care, we would have some, and this bug would not have occurred
in the first place. Besides, nobody likes legacy inheritance anyway.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"La persona que no quería pecar / estaba obligada a sentarse
en duras y empinadas sillas / desprovistas, por cierto
de blandos atenuantes" (Patricio Vogel)
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: missing privilege check after not-null constraint rework
2023-09-05 17:44 missing privilege check after not-null constraint rework Alvaro Herrera <alvherre@alvh.no-ip.org>
@ 2023-09-05 17:45 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2023-09-07 11:02 ` Re: missing privilege check after not-null constraint rework Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 1 reply; 3+ messages in thread
From: Alvaro Herrera @ 2023-09-05 17:45 UTC (permalink / raw)
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
On 2023-Sep-05, Alvaro Herrera wrote:
> Here's a fix to move the privilege check on constraint dropping from
> ATExecDropConstraint to dropconstraint_internal.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"No renuncies a nada. No te aferres a nada."
Attachments:
0001-Move-privilege-check-to-the-right-place.patch (text/x-diff, 3.1K) download
From f58d4532d39bc39edc3841c1ee5c3e3e9e9d153a Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 5 Sep 2023 17:06:23 +0200
Subject: [PATCH] Move privilege check to the right place
Now that ATExecDropConstraint doesn't recurse anymore, there's no point
in testing the privileges during recursion there. Move the check to
dropconstraint_internal, which is the place where recursion occurs.
In passing, remove now-useless 'recursing' argument to
ATExecDropConstraint.
---
src/backend/commands/tablecmds.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 47c556669f..8a2c671b66 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -542,8 +542,7 @@ static void GetForeignKeyCheckTriggers(Relation trigrel,
Oid *insertTriggerOid,
Oid *updateTriggerOid);
static void ATExecDropConstraint(Relation rel, const char *constrName,
- DropBehavior behavior,
- bool recurse, bool recursing,
+ DropBehavior behavior, bool recurse,
bool missing_ok, LOCKMODE lockmode);
static ObjectAddress dropconstraint_internal(Relation rel,
HeapTuple constraintTup, DropBehavior behavior,
@@ -5236,7 +5235,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
break;
case AT_DropConstraint: /* DROP CONSTRAINT */
ATExecDropConstraint(rel, cmd->name, cmd->behavior,
- cmd->recurse, false,
+ cmd->recurse,
cmd->missing_ok, lockmode);
break;
case AT_AlterColumnType: /* ALTER COLUMN TYPE */
@@ -12263,8 +12262,7 @@ createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid,
*/
static void
ATExecDropConstraint(Relation rel, const char *constrName,
- DropBehavior behavior,
- bool recurse, bool recursing,
+ DropBehavior behavior, bool recurse,
bool missing_ok, LOCKMODE lockmode)
{
Relation conrel;
@@ -12273,10 +12271,6 @@ ATExecDropConstraint(Relation rel, const char *constrName,
HeapTuple tuple;
bool found = false;
- /* At top level, permission check was done in ATPrepCmd, else do it */
- if (recursing)
- ATSimplePermissions(AT_DropConstraint, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
-
conrel = table_open(ConstraintRelationId, RowExclusiveLock);
/*
@@ -12302,7 +12296,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
{
List *readyRels = NIL;
- dropconstraint_internal(rel, tuple, behavior, recurse, recursing,
+ dropconstraint_internal(rel, tuple, behavior, recurse, false,
missing_ok, &readyRels, lockmode);
found = true;
}
@@ -12353,6 +12347,10 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha
return InvalidObjectAddress;
*readyRels = lappend_oid(*readyRels, RelationGetRelid(rel));
+ /* At top level, permission check was done in ATPrepCmd, else do it */
+ if (recursing)
+ ATSimplePermissions(AT_DropConstraint, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+
conrel = table_open(ConstraintRelationId, RowExclusiveLock);
con = (Form_pg_constraint) GETSTRUCT(constraintTup);
--
2.30.2
* Re: missing privilege check after not-null constraint rework
2023-09-05 17:44 missing privilege check after not-null constraint rework Alvaro Herrera <alvherre@alvh.no-ip.org>
2023-09-05 17:45 ` Re: missing privilege check after not-null constraint rework Alvaro Herrera <alvherre@alvh.no-ip.org>
@ 2023-09-07 11:02 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 3+ messages in thread
From: Alvaro Herrera @ 2023-09-07 11:02 UTC (permalink / raw)
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
On 2023-Sep-05, Alvaro Herrera wrote:
> On 2023-Sep-05, Alvaro Herrera wrote:
>
> > Here's a fix to move the privilege check on constraint dropping from
> > ATExecDropConstraint to dropconstraint_internal.
I have pushed this. It's just a fixup for an embarrasing bug in
b0e96f311985.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Pensar que el espectro que vemos es ilusorio no lo despoja de espanto,
sólo le suma el nuevo terror de la locura" (Perelandra, C.S. Lewis)
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-09-07 11:02 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-09-05 17:44 missing privilege check after not-null constraint rework Alvaro Herrera <alvherre@alvh.no-ip.org>
2023-09-05 17:45 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2023-09-07 11:02 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox