DROP OWNED BY fails to drop privileges granted by non-owners (was Re: [GENERAL] Bug, Feature, or what else?)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Adrian Klaver <adrian(dot)klaver(at)gmail(dot)com>
Cc: Andreas Kretschmer <andreas(at)a-kretschmer(dot)de>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgreSQL(dot)org
Subject: DROP OWNED BY fails to drop privileges granted by non-owners (was Re: [GENERAL] Bug, Feature, or what else?)
Date: 2013-02-08 17:09:29
Message-ID: 19080.1360343369@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-general pgsql-hackers

Adrian Klaver <adrian(dot)klaver(at)gmail(dot)com> writes:
> On 02/08/2013 08:14 AM, Tom Lane wrote:
>> Of course, postgres has other options besides that, of which "DROP OWNED
>> BY ak02" is probably the most appropriate here. Or if you really want
>> to get rid of just that grant, SET ROLE TO akretschmer01 and revoke.

> The DROP OWNED was tried further up the thread and did not seem to work:

Huh. You're right, here is a complete test case:

regression=# create schema s1;
cCREATE SCHEMA
regression=# create user u1;
CREATE ROLE
regression=# create user u2;
CREATE ROLE
regression=# grant all on schema s1 to u1 with grant option;
GRANT
regression=# \c - u1
You are now connected to database "regression" as user "u1".
regression=> grant all on schema s1 to u2;
GRANT
regression=> \c - postgres
You are now connected to database "regression" as user "postgres".
regression=# \dn+ s1
List of schemas
Name | Owner | Access privileges | Description
------+----------+----------------------+-------------
s1 | postgres | postgres=UC/postgres+|
| | u1=U*C*/postgres +|
| | u2=UC/u1 |
(1 row)

regression=# drop user u2; -- expect failure here
ERROR: role "u2" cannot be dropped because some objects depend on it
DETAIL: privileges for schema s1
regression=# drop owned by u2;
DROP OWNED
regression=# drop user u2; -- failure here is wrong
ERROR: role "u2" cannot be dropped because some objects depend on it
DETAIL: privileges for schema s1
regression=# \dn+ s1
List of schemas
Name | Owner | Access privileges | Description
------+----------+----------------------+-------------
s1 | postgres | postgres=UC/postgres+|
| | u1=U*C*/postgres +|
| | u2=UC/u1 |
(1 row)

I believe the problem is that DROP OWNED for privileges is implemented
by calling REVOKE. As noted upthread, when a superuser does REVOKE,
it's executed as though the object owner did the REVOKE, so only
privileges granted directly by the object owner go away. In this
particular example, "DROP OWNED BY u1" makes the grant to u1 go away,
and then the grant to u2 goes away via cascade ... but "DROP OWNED BY
u2" fails to accomplish anything at all, because postgres never granted
anything directly to u2.

We haven't seen this reported before, probably because the use of
GRANT OPTIONS isn't very common, but AFAICS it's been wrong since
the invention of DROP OWNED.

It looks to me like DropOwnedObjects doesn't actually insist on
superuserness to do DROP OWNED, only ability to become the role,
which means that DROP OWNED BY is completely broken for privileges
if executed by a non-superuser; the only privileges it would remove
would be those granted by the current user to the target user.
I'm not really sure what the desirable behavior would be in such a
case though. Ordinary users can't revoke privileges granted *to*
them, only privileges granted *by* them. So it's not necessarily
the case that a non-superuser should be able to make all privileges
granted to a target role go away, even if he's allowed to become
the target role and thereby drop objects that it owns. I wonder
how sensible it is really to allow DROP OWNED to non-superusers.

regards, tom lane

In response to

Responses

Browse pgsql-general by date

  From Date Subject
Next Message Andreas Kretschmer 2013-02-08 17:37:45 Re: Bug, Feature, or what else?
Previous Message Adrian Klaver 2013-02-08 17:01:44 Re: Bug, Feature, or what else?

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2013-02-08 17:21:37 Time for an autoconf update
Previous Message Jeff Janes 2013-02-08 17:08:52 Re: Vacuum/visibility is busted