Adrian Klaver 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

Search Discussions

  • Adrian Klaver at Feb 8, 2013 at 5:52 pm

    On 02/08/2013 09:09 AM, Tom Lane wrote:
    Adrian Klaver <adrian.klaver@gmail.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.
    I am not sure I am following. Are we talking two different cases here?

    1) As mentioned in the first paragraph the case where running DROP OWNED
    as a supersuser does not work.

    2) A non-superuser running DROP OWNED and not having the necessary
    privileges.
    regards, tom lane

    --
    Adrian Klaver
    adrian.klaver@gmail.com
  • Tom Lane at Feb 8, 2013 at 6:09 pm

    Adrian Klaver writes:
    I am not sure I am following. Are we talking two different cases here?
    What I was pointing out was that the non-superuser case seems to be
    broken almost completely, whereas the superuser case is only broken
    if the object owner has given away some grant options and those have
    been exercised.

        regards, tom lane
  • Adrian Klaver at Feb 8, 2013 at 6:13 pm

    On 02/08/2013 10:09 AM, Tom Lane wrote:
    Adrian Klaver <adrian.klaver@gmail.com> writes:
    I am not sure I am following. Are we talking two different cases here?
    What I was pointing out was that the non-superuser case seems to be
    broken almost completely, whereas the superuser case is only broken
    if the object owner has given away some grant options and those have
    been exercised.
    Got it, thanks.
    regards, tom lane

    --
    Adrian Klaver
    adrian.klaver@gmail.com
  • Alvaro Herrera at Mar 22, 2013 at 11:12 pm

    Tom Lane escribió:

    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.
    So it seems.

    I have been mulling this over today, and it seems to me that one
    way to fix it would be to have ExecGrant_Objecttype() loop over all
    possible grantors when determining what to revoke when it's called by
    DROP OWNED. A proof-of-concept is below (RemoveRoleFromObjectACL would
    set istmt.all_grantors to true, whereas ExecuteGrantStmt leaves it as
    false and behaves as today).

    I am not sure about the restrict_and_check_grant() call I left out in
    the middle of the operation; it seems to me that if we limit DROP OWNED
    to be called only by a superuser, we can get away without that check.
    Or maybe we need a new version of that routine that will only apply the
    bitmask and not raise any error messages.

    The patch below is just for ExecGrant_Relation(), but as far as I can
    tell all the ExecGrant_Objecttype() routines do pretty much the same
    here, so I think it'd be better to refactor the select_best_grantor/
    restrict_and_check_grant/merge_acl_with_grant sequence into a common
    function, and then have that function apply the loop for all grantors.

    Thoughts?


    ***************
    *** 1891,1932 **** ExecGrant_Relation(InternalGrant *istmt)
                   AclObjectKind aclkind;

                   /* Determine ID to do the grant as, and available grant options */
    ! select_best_grantor(GetUserId(), this_privileges,
    ! old_acl, ownerId,
    ! &grantorId, &avail_goptions);
    !
    ! switch (pg_class_tuple->relkind)
                   {
    ! case RELKIND_SEQUENCE:
    ! aclkind = ACL_KIND_SEQUENCE;
    ! break;
    ! default:
    ! aclkind = ACL_KIND_CLASS;
    ! break;
                   }

    ! /*
    ! * Restrict the privileges to what we can actually grant, and emit
    ! * the standards-mandated warning and error messages.
    ! */
    ! this_privileges =
    ! restrict_and_check_grant(istmt->is_grant, avail_goptions,
    ! istmt->all_privs, this_privileges,
    ! relOid, grantorId, aclkind,
    ! NameStr(pg_class_tuple->relname),
    ! 0, NULL);

    ! /*
    ! * Generate new ACL.
    ! */
    ! new_acl = merge_acl_with_grant(old_acl,
    ! istmt->is_grant,
    ! istmt->grant_option,
    ! istmt->behavior,
    ! istmt->grantees,
    ! this_privileges,
    ! grantorId,
    ! ownerId);

                   /*
                    * We need the members of both old and new ACLs so we can correct
    --- 1895,1962 ----
                   AclObjectKind aclkind;

                   /* Determine ID to do the grant as, and available grant options */
    ! if (!istmt->all_grantors)
                   {
    ! select_best_grantor(GetUserId(), this_privileges,
    ! old_acl, ownerId,
    ! &grantorId, &avail_goptions);
    !
    ! switch (pg_class_tuple->relkind)
    ! {
    ! case RELKIND_SEQUENCE:
    ! aclkind = ACL_KIND_SEQUENCE;
    ! break;
    ! default:
    ! aclkind = ACL_KIND_CLASS;
    ! break;
    ! }
    !
    ! /*
    ! * Restrict the privileges to what we can actually grant, and emit
    ! * the standards-mandated warning and error messages.
    ! */
    ! this_privileges =
    ! restrict_and_check_grant(istmt->is_grant, avail_goptions,
    ! istmt->all_privs, this_privileges,
    ! relOid, grantorId, aclkind,
    ! NameStr(pg_class_tuple->relname),
    ! 0, NULL);
    !
    ! /*
    ! * Generate new ACL.
    ! */
    ! new_acl = merge_acl_with_grant(old_acl,
    ! istmt->is_grant,
    ! istmt->grant_option,
    ! istmt->behavior,
    ! istmt->grantees,
    ! this_privileges,
    ! grantorId,
    ! ownerId);
                   }
    + else
    + {
    + int ngrantors;
    + Oid *grantors;
    + int i;
    +
    + ngrantors = aclgrantors(old_acl, istmt->grantees, &grantors);

    ! new_acl = aclcopy(old_acl);
    ! for (i = 0; i < ngrantors; i++)
    ! {
    ! Oid grantorId = grantors[i];

    ! new_acl = merge_acl_with_grant(new_acl,
    ! istmt->is_grant,
    ! istmt->grant_option,
    ! istmt->behavior,
    ! istmt->grantees,
    ! this_privileges,
    ! grantorId,
    ! ownerId);
    ! }
    ! }

                   /*
                    * We need the members of both old and new ACLs so we can correct


    --
    Álvaro Herrera http://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Training & Services
  • Alvaro Herrera at Mar 25, 2013 at 4:52 pm

    Tom Lane escribió:

    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.
    Well, moreover it fails completely with "permission denied" when tried,
    so yeah it is broken today. Now, I don't necessarily think we should
    remove the capability completely, as it seems useful. Consider a
    database where the superuser has created various department roles; admin
    privileges are given to certain users (bosses?) for each of such
    department roles. Such admins can add and remove other users from their
    roles at will, via grant and revoke; it seems useful to be able to
    remove all privileges from a certain user without going through objects
    one by one. Example:

    create user alice;
    create user bob;

    create role arts;

    create role arts_boss;
    grant arts to arts_boss with admin option;
    grant arts_boss to bob;

    create schema s_arts authorization arts_boss;
    revoke create on schema s_arts from public;

    \c - bob
    set role arts_boss;
    grant arts to alice;
    create table s_arts.animations (id int, description text);
    grant all on s_arts.animations to arts;
    grant update on s_arts.animations to alice;

    Here, Bob can GRANT and REVOKE specific things to Alice; but it doesn't
    work for Bob to "DROP OWNED BY alice" (fails with "permission denied to
    drop objects"), when in fact it does make sense that it would revoke
    those privileges -- but not those that alice might have gotten via a
    different chain of command, say on role "building" on which Charlie is
    admin.

    Alternatively we might say this is just gilding the lily and we should
    disallow this whole thing and restrict DROP OWNED. Two further
    possibilities here:

    a) only a superuser can run it, period.
    b) any user can run it on itself, and this revokes all privileges
    granted to this role, regardless of where they come from.

    (In any case I think we should disallow alice from doing "DROP OWNED by
    arts".)


    As a final comment, I am inclined to go the simplest route possible,
    first because I can't spend infinite time on this, and also because I
    think messing too much with this might lead to strange security issues.

    --
    Álvaro Herrera http://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Training & Services

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedFeb 8, '13 at 5:09p
activeMar 25, '13 at 4:52p
posts6
users3
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2021 Grokbase