summaryrefslogtreecommitdiff
path: root/src/test
diff options
context:
space:
mode:
authorTom Lane2024-11-11 15:29:54 +0000
committerTom Lane2024-11-11 15:29:54 +0000
commitcd82afdda5e9d3269706a142e9093ba83f484185 (patch)
tree6b85878dd063d5b0ac800f7a803617eaa40ebdc3 /src/test
parentedcda9bb4c4500b75bb4a16c7c59834398ca2906 (diff)
Fix improper interactions between session_authorization and role.
The SQL spec mandates that SET SESSION AUTHORIZATION implies SET ROLE NONE. We tried to implement that within the lowest-level functions that manipulate these settings, but that was a bad idea. In particular, guc.c assumes that it doesn't matter in what order it applies GUC variable updates, but that was not the case for these two variables. This problem, compounded by some hackish attempts to work around it, led to some security-grade issues: * Rolling back a transaction that had done SET SESSION AUTHORIZATION would revert to SET ROLE NONE, even if that had not been the previous state, so that the effective user ID might now be different from what it had been. * The same for SET SESSION AUTHORIZATION in a function SET clause. * If a parallel worker inspected current_setting('role'), it saw "none" even when it should see something else. Also, although the parallel worker startup code intended to cope with the current role's pg_authid row having disappeared, its implementation of that was incomplete so it would still fail. Fix by fully separating the miscinit.c functions that assign session_authorization from those that assign role. To implement the spec's requirement, teach set_config_option itself to perform "SET ROLE NONE" when it sets session_authorization. (This is undoubtedly ugly, but the alternatives seem worse. In particular, there's no way to do it within assign_session_authorization without incompatible changes in the API for GUC assign hooks.) Also, improve ParallelWorkerMain to directly set all the relevant user-ID variables instead of relying on some of them to get set indirectly. That allows us to survive not finding the pg_authid row during worker startup. In v16 and earlier, this includes back-patching 9987a7bf3 which fixed a violation of GUC coding rules: SetSessionAuthorization is not an appropriate place to be throwing errors from. Security: CVE-2024-10978
Diffstat (limited to 'src/test')
-rw-r--r--src/test/regress/expected/privileges.out67
-rw-r--r--src/test/regress/sql/privileges.sql33
2 files changed, 100 insertions, 0 deletions
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index eb4b762ea10..1296da0d579 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -141,6 +141,73 @@ SET ROLE pg_read_all_stats; -- fail, granted without SET option
ERROR: permission denied to set role "pg_read_all_stats"
RESET ROLE;
RESET SESSION AUTHORIZATION;
+-- test interaction of SET SESSION AUTHORIZATION and SET ROLE,
+-- as well as propagation of these settings to parallel workers
+GRANT regress_priv_user9 TO regress_priv_user8;
+SET SESSION AUTHORIZATION regress_priv_user8;
+SET ROLE regress_priv_user9;
+SET debug_parallel_query = 0;
+SELECT session_user, current_role, current_user, current_setting('role') as role;
+ session_user | current_role | current_user | role
+--------------------+--------------------+--------------------+--------------------
+ regress_priv_user8 | regress_priv_user9 | regress_priv_user9 | regress_priv_user9
+(1 row)
+
+SET debug_parallel_query = 1;
+SELECT session_user, current_role, current_user, current_setting('role') as role;
+ session_user | current_role | current_user | role
+--------------------+--------------------+--------------------+--------------------
+ regress_priv_user8 | regress_priv_user9 | regress_priv_user9 | regress_priv_user9
+(1 row)
+
+BEGIN;
+SET SESSION AUTHORIZATION regress_priv_user10;
+SET debug_parallel_query = 0;
+SELECT session_user, current_role, current_user, current_setting('role') as role;
+ session_user | current_role | current_user | role
+---------------------+---------------------+---------------------+------
+ regress_priv_user10 | regress_priv_user10 | regress_priv_user10 | none
+(1 row)
+
+SET debug_parallel_query = 1;
+SELECT session_user, current_role, current_user, current_setting('role') as role;
+ session_user | current_role | current_user | role
+---------------------+---------------------+---------------------+------
+ regress_priv_user10 | regress_priv_user10 | regress_priv_user10 | none
+(1 row)
+
+ROLLBACK;
+SET debug_parallel_query = 0;
+SELECT session_user, current_role, current_user, current_setting('role') as role;
+ session_user | current_role | current_user | role
+--------------------+--------------------+--------------------+--------------------
+ regress_priv_user8 | regress_priv_user9 | regress_priv_user9 | regress_priv_user9
+(1 row)
+
+SET debug_parallel_query = 1;
+SELECT session_user, current_role, current_user, current_setting('role') as role;
+ session_user | current_role | current_user | role
+--------------------+--------------------+--------------------+--------------------
+ regress_priv_user8 | regress_priv_user9 | regress_priv_user9 | regress_priv_user9
+(1 row)
+
+RESET SESSION AUTHORIZATION;
+-- session_user at this point is installation-dependent
+SET debug_parallel_query = 0;
+SELECT session_user = current_role as c_r_ok, session_user = current_user as c_u_ok, current_setting('role') as role;
+ c_r_ok | c_u_ok | role
+--------+--------+------
+ t | t | none
+(1 row)
+
+SET debug_parallel_query = 1;
+SELECT session_user = current_role as c_r_ok, session_user = current_user as c_u_ok, current_setting('role') as role;
+ c_r_ok | c_u_ok | role
+--------+--------+------
+ t | t | none
+(1 row)
+
+RESET debug_parallel_query;
REVOKE pg_read_all_settings FROM regress_priv_user8;
DROP USER regress_priv_user10;
DROP USER regress_priv_user9;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index eeb4c002926..5880bc018de 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -124,6 +124,39 @@ SET ROLE pg_read_all_stats; -- fail, granted without SET option
RESET ROLE;
RESET SESSION AUTHORIZATION;
+
+-- test interaction of SET SESSION AUTHORIZATION and SET ROLE,
+-- as well as propagation of these settings to parallel workers
+GRANT regress_priv_user9 TO regress_priv_user8;
+
+SET SESSION AUTHORIZATION regress_priv_user8;
+SET ROLE regress_priv_user9;
+SET debug_parallel_query = 0;
+SELECT session_user, current_role, current_user, current_setting('role') as role;
+SET debug_parallel_query = 1;
+SELECT session_user, current_role, current_user, current_setting('role') as role;
+
+BEGIN;
+SET SESSION AUTHORIZATION regress_priv_user10;
+SET debug_parallel_query = 0;
+SELECT session_user, current_role, current_user, current_setting('role') as role;
+SET debug_parallel_query = 1;
+SELECT session_user, current_role, current_user, current_setting('role') as role;
+ROLLBACK;
+SET debug_parallel_query = 0;
+SELECT session_user, current_role, current_user, current_setting('role') as role;
+SET debug_parallel_query = 1;
+SELECT session_user, current_role, current_user, current_setting('role') as role;
+
+RESET SESSION AUTHORIZATION;
+-- session_user at this point is installation-dependent
+SET debug_parallel_query = 0;
+SELECT session_user = current_role as c_r_ok, session_user = current_user as c_u_ok, current_setting('role') as role;
+SET debug_parallel_query = 1;
+SELECT session_user = current_role as c_r_ok, session_user = current_user as c_u_ok, current_setting('role') as role;
+
+RESET debug_parallel_query;
+
REVOKE pg_read_all_settings FROM regress_priv_user8;
DROP USER regress_priv_user10;