mirror of
https://github.com/postgres/postgres.git
synced 2025-10-25 13:17:41 +03:00
Revert "Avoid race condition between "GRANT role" and "DROP ROLE"".
This reverts commit98fc31d649. That change allowed DROP OWNED BY to drop grants of the target role to other roles, arguing that nobody would need those privileges anymore. But that's not so: if you're not superuser, you still need admin privilege on the target role so you can drop it. It's not clear whether or how the dependency-based approach to solving the original problem can be adapted to keep these grants. Since v18 release is fast approaching, the sanest thing to do seems to be to revert this patch for now. The race-condition problem is low severity and not worth taking risks for. I didn't force a catversion bump in98fc31d64, so I won't do so here either. Reported-by: Dipesh Dhameliya <dipeshdhameliya125@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CABgZEgczOFicCJoqtrH9gbYMe_BV3Hq8zzCBRcMgmU6LRsihUA@mail.gmail.com Backpatch-through: 18
This commit is contained in:
@@ -33,7 +33,7 @@ DROP OWNED BY { <replaceable class="parameter">name</replaceable> | CURRENT_ROLE
|
|||||||
database that are owned by one of the specified roles. Any
|
database that are owned by one of the specified roles. Any
|
||||||
privileges granted to the given roles on objects in the current
|
privileges granted to the given roles on objects in the current
|
||||||
database or on shared objects (databases, tablespaces, configuration
|
database or on shared objects (databases, tablespaces, configuration
|
||||||
parameters, or other roles) will also be revoked.
|
parameters) will also be revoked.
|
||||||
</para>
|
</para>
|
||||||
</refsect1>
|
</refsect1>
|
||||||
|
|
||||||
|
|||||||
@@ -30,7 +30,6 @@
|
|||||||
#include "commands/defrem.h"
|
#include "commands/defrem.h"
|
||||||
#include "commands/seclabel.h"
|
#include "commands/seclabel.h"
|
||||||
#include "commands/user.h"
|
#include "commands/user.h"
|
||||||
#include "lib/qunique.h"
|
|
||||||
#include "libpq/crypt.h"
|
#include "libpq/crypt.h"
|
||||||
#include "miscadmin.h"
|
#include "miscadmin.h"
|
||||||
#include "storage/lmgr.h"
|
#include "storage/lmgr.h"
|
||||||
@@ -490,6 +489,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
|
|||||||
* Advance command counter so we can see new record; else tests in
|
* Advance command counter so we can see new record; else tests in
|
||||||
* AddRoleMems may fail.
|
* AddRoleMems may fail.
|
||||||
*/
|
*/
|
||||||
|
if (addroleto || adminmembers || rolemembers)
|
||||||
CommandCounterIncrement();
|
CommandCounterIncrement();
|
||||||
|
|
||||||
/* Default grant. */
|
/* Default grant. */
|
||||||
@@ -1904,8 +1904,7 @@ AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
|
|||||||
else
|
else
|
||||||
{
|
{
|
||||||
Oid objectId;
|
Oid objectId;
|
||||||
Oid *newmembers = (Oid *) palloc(3 * sizeof(Oid));
|
Oid *newmembers = palloc(sizeof(Oid));
|
||||||
int nnewmembers;
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* The values for these options can be taken directly from 'popt'.
|
* The values for these options can be taken directly from 'popt'.
|
||||||
@@ -1947,22 +1946,12 @@ AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
|
|||||||
new_record, new_record_nulls);
|
new_record, new_record_nulls);
|
||||||
CatalogTupleInsert(pg_authmem_rel, tuple);
|
CatalogTupleInsert(pg_authmem_rel, tuple);
|
||||||
|
|
||||||
/*
|
/* updateAclDependencies wants to pfree array inputs */
|
||||||
* Record dependencies on the roleid, member, and grantor, as if a
|
newmembers[0] = grantorId;
|
||||||
* pg_auth_members entry were an object ACL.
|
|
||||||
* updateAclDependencies() requires an input array that is
|
|
||||||
* palloc'd (it will free it), sorted, and de-duped.
|
|
||||||
*/
|
|
||||||
newmembers[0] = roleid;
|
|
||||||
newmembers[1] = memberid;
|
|
||||||
newmembers[2] = grantorId;
|
|
||||||
qsort(newmembers, 3, sizeof(Oid), oid_cmp);
|
|
||||||
nnewmembers = qunique(newmembers, 3, sizeof(Oid), oid_cmp);
|
|
||||||
|
|
||||||
updateAclDependencies(AuthMemRelationId, objectId,
|
updateAclDependencies(AuthMemRelationId, objectId,
|
||||||
0, InvalidOid,
|
0, InvalidOid,
|
||||||
0, NULL,
|
0, NULL,
|
||||||
nnewmembers, newmembers);
|
1, newmembers);
|
||||||
}
|
}
|
||||||
|
|
||||||
/* CCI after each change, in case there are duplicates in list */
|
/* CCI after each change, in case there are duplicates in list */
|
||||||
|
|||||||
@@ -113,36 +113,6 @@ CREATE USER regress_priv_user2;
|
|||||||
CREATE USER regress_priv_user3;
|
CREATE USER regress_priv_user3;
|
||||||
CREATE USER regress_priv_user4;
|
CREATE USER regress_priv_user4;
|
||||||
CREATE USER regress_priv_user5;
|
CREATE USER regress_priv_user5;
|
||||||
-- DROP OWNED should also act on granted and granted-to roles
|
|
||||||
GRANT regress_priv_user1 TO regress_priv_user2;
|
|
||||||
GRANT regress_priv_user2 TO regress_priv_user3;
|
|
||||||
SELECT roleid::regrole, member::regrole FROM pg_auth_members
|
|
||||||
WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
|
|
||||||
ORDER BY roleid::regrole::text;
|
|
||||||
roleid | member
|
|
||||||
--------------------+--------------------
|
|
||||||
regress_priv_user1 | regress_priv_user2
|
|
||||||
regress_priv_user2 | regress_priv_user3
|
|
||||||
(2 rows)
|
|
||||||
|
|
||||||
REASSIGN OWNED BY regress_priv_user2 TO regress_priv_user4; -- no effect
|
|
||||||
SELECT roleid::regrole, member::regrole FROM pg_auth_members
|
|
||||||
WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
|
|
||||||
ORDER BY roleid::regrole::text;
|
|
||||||
roleid | member
|
|
||||||
--------------------+--------------------
|
|
||||||
regress_priv_user1 | regress_priv_user2
|
|
||||||
regress_priv_user2 | regress_priv_user3
|
|
||||||
(2 rows)
|
|
||||||
|
|
||||||
DROP OWNED BY regress_priv_user2; -- removes both grants
|
|
||||||
SELECT roleid::regrole, member::regrole FROM pg_auth_members
|
|
||||||
WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
|
|
||||||
ORDER BY roleid::regrole::text;
|
|
||||||
roleid | member
|
|
||||||
--------+--------
|
|
||||||
(0 rows)
|
|
||||||
|
|
||||||
GRANT pg_read_all_data TO regress_priv_user6;
|
GRANT pg_read_all_data TO regress_priv_user6;
|
||||||
GRANT pg_write_all_data TO regress_priv_user7;
|
GRANT pg_write_all_data TO regress_priv_user7;
|
||||||
GRANT pg_read_all_settings TO regress_priv_user8 WITH ADMIN OPTION;
|
GRANT pg_read_all_settings TO regress_priv_user8 WITH ADMIN OPTION;
|
||||||
|
|||||||
@@ -90,21 +90,6 @@ CREATE USER regress_priv_user3;
|
|||||||
CREATE USER regress_priv_user4;
|
CREATE USER regress_priv_user4;
|
||||||
CREATE USER regress_priv_user5;
|
CREATE USER regress_priv_user5;
|
||||||
|
|
||||||
-- DROP OWNED should also act on granted and granted-to roles
|
|
||||||
GRANT regress_priv_user1 TO regress_priv_user2;
|
|
||||||
GRANT regress_priv_user2 TO regress_priv_user3;
|
|
||||||
SELECT roleid::regrole, member::regrole FROM pg_auth_members
|
|
||||||
WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
|
|
||||||
ORDER BY roleid::regrole::text;
|
|
||||||
REASSIGN OWNED BY regress_priv_user2 TO regress_priv_user4; -- no effect
|
|
||||||
SELECT roleid::regrole, member::regrole FROM pg_auth_members
|
|
||||||
WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
|
|
||||||
ORDER BY roleid::regrole::text;
|
|
||||||
DROP OWNED BY regress_priv_user2; -- removes both grants
|
|
||||||
SELECT roleid::regrole, member::regrole FROM pg_auth_members
|
|
||||||
WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
|
|
||||||
ORDER BY roleid::regrole::text;
|
|
||||||
|
|
||||||
GRANT pg_read_all_data TO regress_priv_user6;
|
GRANT pg_read_all_data TO regress_priv_user6;
|
||||||
GRANT pg_write_all_data TO regress_priv_user7;
|
GRANT pg_write_all_data TO regress_priv_user7;
|
||||||
GRANT pg_read_all_settings TO regress_priv_user8 WITH ADMIN OPTION;
|
GRANT pg_read_all_settings TO regress_priv_user8 WITH ADMIN OPTION;
|
||||||
|
|||||||
Reference in New Issue
Block a user