diff options
author | Noah Misch | 2014-02-17 14:33:31 +0000 |
---|---|---|
committer | Noah Misch | 2014-02-17 14:33:31 +0000 |
commit | fea164a72a7bfd50d77ba5fb418d357f8f2bb7d0 (patch) | |
tree | decfbae5e3c54de074a6cd47fa60344b892e5bf7 /src/test | |
parent | 0983315b1d37cc17b2174dad87449d8402e357ee (diff) |
Shore up ADMIN OPTION restrictions.
Granting a role without ADMIN OPTION is supposed to prevent the grantee
from adding or removing members from the granted role. Issuing SET ROLE
before the GRANT bypassed that, because the role itself had an implicit
right to add or remove members. Plug that hole by recognizing that
implicit right only when the session user matches the current role.
Additionally, do not recognize it during a security-restricted operation
or during execution of a SECURITY DEFINER function. The restriction on
SECURITY DEFINER is not security-critical. However, it seems best for a
user testing his own SECURITY DEFINER function to see the same behavior
others will see. Back-patch to 8.4 (all supported versions).
The SQL standards do not conflate roles and users as PostgreSQL does;
only SQL roles have members, and only SQL users initiate sessions. An
application using PostgreSQL users and roles as SQL users and roles will
never attempt to grant membership in the role that is the session user,
so the implicit right to add or remove members will never arise.
The security impact was mostly that a role member could revoke access
from others, contrary to the wishes of his own grantor. Unapproved role
member additions are less notable, because the member can still largely
achieve that by creating a view or a SECURITY DEFINER function.
Reviewed by Andres Freund and Tom Lane. Reported, independently, by
Jonas Sundman and Noah Misch.
Security: CVE-2014-0060
Diffstat (limited to 'src/test')
-rw-r--r-- | src/test/regress/expected/privileges.out | 36 | ||||
-rw-r--r-- | src/test/regress/sql/privileges.sql | 29 |
2 files changed, 63 insertions, 2 deletions
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index fa574d744ee..1675075f682 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -32,7 +32,7 @@ ALTER GROUP regressgroup1 ADD USER regressuser4; ALTER GROUP regressgroup2 ADD USER regressuser2; -- duplicate NOTICE: role "regressuser2" is already a member of role "regressgroup2" ALTER GROUP regressgroup2 DROP USER regressuser2; -ALTER GROUP regressgroup2 ADD USER regressuser4; +GRANT regressgroup2 TO regressuser4 WITH ADMIN OPTION; -- test owner privileges SET SESSION AUTHORIZATION regressuser1; SELECT session_user, current_user; @@ -948,6 +948,40 @@ SELECT has_table_privilege('regressuser1', 'atest4', 'SELECT WITH GRANT OPTION') t (1 row) +-- Admin options +SET SESSION AUTHORIZATION regressuser4; +CREATE FUNCTION dogrant_ok() RETURNS void LANGUAGE sql SECURITY DEFINER AS + 'GRANT regressgroup2 TO regressuser5'; +GRANT regressgroup2 TO regressuser5; -- ok: had ADMIN OPTION +SET ROLE regressgroup2; +GRANT regressgroup2 TO regressuser5; -- fails: SET ROLE suspended privilege +ERROR: must have admin option on role "regressgroup2" +SET SESSION AUTHORIZATION regressuser1; +GRANT regressgroup2 TO regressuser5; -- fails: no ADMIN OPTION +ERROR: must have admin option on role "regressgroup2" +SELECT dogrant_ok(); -- ok: SECURITY DEFINER conveys ADMIN +NOTICE: role "regressuser5" is already a member of role "regressgroup2" +CONTEXT: SQL function "dogrant_ok" statement 1 + dogrant_ok +------------ + +(1 row) + +SET ROLE regressgroup2; +GRANT regressgroup2 TO regressuser5; -- fails: SET ROLE did not help +ERROR: must have admin option on role "regressgroup2" +SET SESSION AUTHORIZATION regressgroup2; +GRANT regressgroup2 TO regressuser5; -- ok: a role can self-admin +NOTICE: role "regressuser5" is already a member of role "regressgroup2" +CREATE FUNCTION dogrant_fails() RETURNS void LANGUAGE sql SECURITY DEFINER AS + 'GRANT regressgroup2 TO regressuser5'; +SELECT dogrant_fails(); -- fails: no self-admin in SECURITY DEFINER +ERROR: must have admin option on role "regressgroup2" +CONTEXT: SQL function "dogrant_fails" statement 1 +DROP FUNCTION dogrant_fails(); +SET SESSION AUTHORIZATION regressuser4; +DROP FUNCTION dogrant_ok(); +REVOKE regressgroup2 FROM regressuser5; -- has_sequence_privilege tests \c - CREATE SEQUENCE x_seq; diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index 38f8695475c..a0ff953c904 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -37,7 +37,7 @@ ALTER GROUP regressgroup1 ADD USER regressuser4; ALTER GROUP regressgroup2 ADD USER regressuser2; -- duplicate ALTER GROUP regressgroup2 DROP USER regressuser2; -ALTER GROUP regressgroup2 ADD USER regressuser4; +GRANT regressgroup2 TO regressuser4 WITH ADMIN OPTION; -- test owner privileges @@ -599,6 +599,33 @@ SELECT has_table_privilege('regressuser3', 'atest4', 'SELECT'); -- false SELECT has_table_privilege('regressuser1', 'atest4', 'SELECT WITH GRANT OPTION'); -- true +-- Admin options + +SET SESSION AUTHORIZATION regressuser4; +CREATE FUNCTION dogrant_ok() RETURNS void LANGUAGE sql SECURITY DEFINER AS + 'GRANT regressgroup2 TO regressuser5'; +GRANT regressgroup2 TO regressuser5; -- ok: had ADMIN OPTION +SET ROLE regressgroup2; +GRANT regressgroup2 TO regressuser5; -- fails: SET ROLE suspended privilege + +SET SESSION AUTHORIZATION regressuser1; +GRANT regressgroup2 TO regressuser5; -- fails: no ADMIN OPTION +SELECT dogrant_ok(); -- ok: SECURITY DEFINER conveys ADMIN +SET ROLE regressgroup2; +GRANT regressgroup2 TO regressuser5; -- fails: SET ROLE did not help + +SET SESSION AUTHORIZATION regressgroup2; +GRANT regressgroup2 TO regressuser5; -- ok: a role can self-admin +CREATE FUNCTION dogrant_fails() RETURNS void LANGUAGE sql SECURITY DEFINER AS + 'GRANT regressgroup2 TO regressuser5'; +SELECT dogrant_fails(); -- fails: no self-admin in SECURITY DEFINER +DROP FUNCTION dogrant_fails(); + +SET SESSION AUTHORIZATION regressuser4; +DROP FUNCTION dogrant_ok(); +REVOKE regressgroup2 FROM regressuser5; + + -- has_sequence_privilege tests \c - |