Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitcd82afd

Browse files
committed
Fix improper interactions between session_authorization and role.
The SQL spec mandates that SET SESSION AUTHORIZATION impliesSET ROLE NONE. We tried to implement that within the lowest-levelfunctions that manipulate these settings, but that was a bad idea.In particular, guc.c assumes that it doesn't matter in what orderit applies GUC variable updates, but that was not the case for thesetwo variables. This problem, compounded by some hackish attempts towork around it, led to some security-grade issues:* Rolling back a transaction that had done SET SESSION AUTHORIZATIONwould revert to SET ROLE NONE, even if that had not been the previousstate, so that the effective user ID might now be different from whatit 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 copewith the current role's pg_authid row having disappeared, itsimplementation of that was incomplete so it would still fail.Fix by fully separating the miscinit.c functions that assignsession_authorization from those that assign role. To implement thespec's requirement, teach set_config_option itself to perform "SETROLE NONE" when it sets session_authorization. (This is undoubtedlyugly, but the alternatives seem worse. In particular, there's no wayto do it within assign_session_authorization without incompatiblechanges in the API for GUC assign hooks.) Also, improveParallelWorkerMain to directly set all the relevant user-ID variablesinstead of relying on some of them to get set indirectly. Thatallows us to survive not finding the pg_authid row during workerstartup.In v16 and earlier, this includes back-patching9987a7b whichfixed a violation of GUC coding rules: SetSessionAuthorizationis not an appropriate place to be throwing errors from.Security:CVE-2024-10978
1 parentedcda9b commitcd82afd

File tree

7 files changed

+310
-113
lines changed

7 files changed

+310
-113
lines changed

‎src/backend/access/transam/parallel.c

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,14 @@ typedef struct FixedParallelState
8383
/* Fixed-size state that workers must restore. */
8484
Oiddatabase_id;
8585
Oidauthenticated_user_id;
86-
Oidcurrent_user_id;
86+
Oidsession_user_id;
8787
Oidouter_user_id;
88+
Oidcurrent_user_id;
8889
Oidtemp_namespace_id;
8990
Oidtemp_toast_namespace_id;
9091
intsec_context;
91-
boolis_superuser;
92+
boolsession_user_is_superuser;
93+
boolrole_is_superuser;
9294
PGPROC*parallel_leader_pgproc;
9395
pid_tparallel_leader_pid;
9496
ProcNumberparallel_leader_proc_number;
@@ -336,9 +338,11 @@ InitializeParallelDSM(ParallelContext *pcxt)
336338
shm_toc_allocate(pcxt->toc,sizeof(FixedParallelState));
337339
fps->database_id=MyDatabaseId;
338340
fps->authenticated_user_id=GetAuthenticatedUserId();
341+
fps->session_user_id=GetSessionUserId();
339342
fps->outer_user_id=GetCurrentRoleId();
340-
fps->is_superuser=current_role_is_superuser;
341343
GetUserIdAndSecContext(&fps->current_user_id,&fps->sec_context);
344+
fps->session_user_is_superuser=GetSessionUserIsSuperuser();
345+
fps->role_is_superuser=current_role_is_superuser;
342346
GetTempNamespaceState(&fps->temp_namespace_id,
343347
&fps->temp_toast_namespace_id);
344348
fps->parallel_leader_pgproc=MyProc;
@@ -1404,6 +1408,17 @@ ParallelWorkerMain(Datum main_arg)
14041408

14051409
entrypt=LookupParallelWorkerFunction(library_name,function_name);
14061410

1411+
/*
1412+
* Restore current session authorization and role id. No verification
1413+
* happens here, we just blindly adopt the leader's state. Note that this
1414+
* has to happen before InitPostgres, since InitializeSessionUserId will
1415+
* not set these variables.
1416+
*/
1417+
SetAuthenticatedUserId(fps->authenticated_user_id);
1418+
SetSessionAuthorization(fps->session_user_id,
1419+
fps->session_user_is_superuser);
1420+
SetCurrentRoleId(fps->outer_user_id,fps->role_is_superuser);
1421+
14071422
/* Restore database connection. */
14081423
BackgroundWorkerInitializeConnectionByOid(fps->database_id,
14091424
fps->authenticated_user_id,
@@ -1469,13 +1484,13 @@ ParallelWorkerMain(Datum main_arg)
14691484
InvalidateSystemCaches();
14701485

14711486
/*
1472-
* Restore current role id. Skip verifying whether session user is
1473-
* allowed to become this role and blindly restore the leader's state for
1474-
* current role.
1487+
* Restore current user ID and security context. No verification happens
1488+
* here, we just blindly adopt the leader's state. We can't do this till
1489+
* after restoring GUCs, else we'll get complaints about restoring
1490+
* session_authorization and role. (In effect, we're assuming that all
1491+
* the restored values are okay to set, even if we are now inside a
1492+
* restricted context.)
14751493
*/
1476-
SetCurrentRoleId(fps->outer_user_id,fps->is_superuser);
1477-
1478-
/* Restore user ID and security context. */
14791494
SetUserIdAndSecContext(fps->current_user_id,fps->sec_context);
14801495

14811496
/* Restore temp-namespace state to ensure search path matches leader's. */

‎src/backend/commands/variable.c

Lines changed: 70 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -811,63 +811,77 @@ check_session_authorization(char **newval, void **extra, GucSource source)
811811
if (*newval==NULL)
812812
return true;
813813

814-
if (!IsTransactionState())
814+
if (InitializingParallelWorker)
815815
{
816816
/*
817-
*Can't do catalog lookups, so fail. The result of this is that
818-
*session_authorization cannot be set in postgresql.conf, which seems
819-
*like a good thing anyway, so we don't work hard to avoid it.
817+
*In parallel worker initialization, we want to copy the leader's
818+
*state even if it no longer matches the catalogs. ParallelWorkerMain
819+
*already installed the correct role OID and superuser state.
820820
*/
821-
return false;
821+
roleid=GetSessionUserId();
822+
is_superuser=GetSessionUserIsSuperuser();
822823
}
824+
else
825+
{
826+
if (!IsTransactionState())
827+
{
828+
/*
829+
* Can't do catalog lookups, so fail. The result of this is that
830+
* session_authorization cannot be set in postgresql.conf, which
831+
* seems like a good thing anyway, so we don't work hard to avoid
832+
* it.
833+
*/
834+
return false;
835+
}
823836

824-
/*
825-
* When source == PGC_S_TEST, we don't throw a hard error for a
826-
* nonexistent user name or insufficient privileges, only a NOTICE. See
827-
* comments in guc.h.
828-
*/
837+
/*
838+
* When source == PGC_S_TEST, we don't throw a hard error for a
839+
* nonexistent user name or insufficient privileges, only a NOTICE.
840+
* See comments in guc.h.
841+
*/
829842

830-
/* Look up the username */
831-
roleTup=SearchSysCache1(AUTHNAME,PointerGetDatum(*newval));
832-
if (!HeapTupleIsValid(roleTup))
833-
{
834-
if (source==PGC_S_TEST)
843+
/* Look up the username */
844+
roleTup=SearchSysCache1(AUTHNAME,PointerGetDatum(*newval));
845+
if (!HeapTupleIsValid(roleTup))
835846
{
836-
ereport(NOTICE,
837-
(errcode(ERRCODE_UNDEFINED_OBJECT),
838-
errmsg("role \"%s\" does not exist",*newval)));
839-
return true;
847+
if (source==PGC_S_TEST)
848+
{
849+
ereport(NOTICE,
850+
(errcode(ERRCODE_UNDEFINED_OBJECT),
851+
errmsg("role \"%s\" does not exist",*newval)));
852+
return true;
853+
}
854+
GUC_check_errmsg("role \"%s\" does not exist",*newval);
855+
return false;
840856
}
841-
GUC_check_errmsg("role \"%s\" does not exist",*newval);
842-
return false;
843-
}
844857

845-
roleform= (Form_pg_authid)GETSTRUCT(roleTup);
846-
roleid=roleform->oid;
847-
is_superuser=roleform->rolsuper;
858+
roleform= (Form_pg_authid)GETSTRUCT(roleTup);
859+
roleid=roleform->oid;
860+
is_superuser=roleform->rolsuper;
848861

849-
ReleaseSysCache(roleTup);
862+
ReleaseSysCache(roleTup);
850863

851-
/*
852-
* Only superusers may SET SESSION AUTHORIZATION a role other than itself.
853-
* Note that in case of multiple SETs in a single session, the original
854-
* authenticated user's superuserness is what matters.
855-
*/
856-
if (roleid!=GetAuthenticatedUserId()&&
857-
!superuser_arg(GetAuthenticatedUserId()))
858-
{
859-
if (source==PGC_S_TEST)
864+
/*
865+
* Only superusers may SET SESSION AUTHORIZATION a role other than
866+
* itself. Note that in case of multiple SETs in a single session, the
867+
* original authenticated user's superuserness is what matters.
868+
*/
869+
if (roleid!=GetAuthenticatedUserId()&&
870+
!superuser_arg(GetAuthenticatedUserId()))
860871
{
861-
ereport(NOTICE,
862-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
863-
errmsg("permission will be denied to set session authorization \"%s\"",
864-
*newval)));
865-
return true;
872+
if (source==PGC_S_TEST)
873+
{
874+
ereport(NOTICE,
875+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
876+
errmsg("permission will be denied to set session authorization \"%s\"",
877+
*newval)));
878+
return true;
879+
}
880+
GUC_check_errcode(ERRCODE_INSUFFICIENT_PRIVILEGE);
881+
GUC_check_errmsg("permission denied to set session authorization \"%s\"",
882+
*newval);
883+
return false;
866884
}
867-
GUC_check_errcode(ERRCODE_INSUFFICIENT_PRIVILEGE);
868-
GUC_check_errmsg("permission denied to set session authorization \"%s\"",
869-
*newval);
870-
return false;
871885
}
872886

873887
/* Set up "extra" struct for assign_session_authorization to use */
@@ -918,6 +932,16 @@ check_role(char **newval, void **extra, GucSource source)
918932
roleid=InvalidOid;
919933
is_superuser= false;
920934
}
935+
elseif (InitializingParallelWorker)
936+
{
937+
/*
938+
* In parallel worker initialization, we want to copy the leader's
939+
* state even if it no longer matches the catalogs. ParallelWorkerMain
940+
* already installed the correct role OID and superuser state.
941+
*/
942+
roleid=GetCurrentRoleId();
943+
is_superuser=current_role_is_superuser;
944+
}
921945
else
922946
{
923947
if (!IsTransactionState())
@@ -957,13 +981,8 @@ check_role(char **newval, void **extra, GucSource source)
957981

958982
ReleaseSysCache(roleTup);
959983

960-
/*
961-
* Verify that session user is allowed to become this role, but skip
962-
* this in parallel mode, where we must blindly recreate the parallel
963-
* leader's state.
964-
*/
965-
if (!InitializingParallelWorker&&
966-
!member_can_set_role(GetSessionUserId(),roleid))
984+
/* Verify that session user is allowed to become this role */
985+
if (!member_can_set_role(GetSessionUserId(),roleid))
967986
{
968987
if (source==PGC_S_TEST)
969988
{

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp