mirrors
/
postgres
mirror of https://github.com/postgres/postgres
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix missed corner cases for grantable permissions on GUCs. 

We allow users to set the values of not-yet-loaded extension GUCs,
remembering those values in "placeholder" GUC entries.  When/if
the extension is loaded later in the session, we need to verify that
the user had permissions to set the GUC.  That was done correctly
before commit a0ffa885e, but as of that commit, we'd check the
permissions of the active role when the LOAD happens, not the role
that had set the value.  (This'd be a security bug if it had made it
into a released version.)

In principle this is simple enough to fix: we just need to remember
the exact role OID that set each GUC value, and use that not
GetUserID() when verifying permissions.  Maintaining that data in
the guc.c data structures is slightly tedious, but fortunately it's
all basically just copy-n-paste of the logic for tracking the
GucSource of each setting, as we were already doing.

Another oversight is that validate_option_array_item() hadn't
been taught to check for granted GUC privileges.  This appears
to manifest only in that ALTER ROLE/DATABASE RESET ALL will
fail to reset settings that the user should be allowed to reset.

Patch by myself and Nathan Bossart, per report from Nathan Bossart.
Back-patch to v15 where the faulty code came in.

Discussion: https://postgr.es/m/20220706224727.GA2158260@nathanxps13
This commit is contained in:
Tom Lane 2022-07-19 17:21:55 -04:00
parent d6a3aeb9a3
commit 13d8388151
8 changed files with 280 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/backend/commands/extension.c
View File

@ -912,6 +912,9 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
* We use the equivalent of a function SET option to allow the setting to * We use the equivalent of a function SET option to allow the setting to
* persist for exactly the duration of the script execution. guc.c also * persist for exactly the duration of the script execution. guc.c also
* takes care of undoing the setting on error. * takes care of undoing the setting on error.
*
* log_min_messages can't be set by ordinary users, so for that one we
* pretend to be superuser.
*/ */
save_nestlevel = NewGUCNestLevel(); save_nestlevel = NewGUCNestLevel();
@ -920,9 +923,10 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
PGC_USERSET, PGC_S_SESSION, PGC_USERSET, PGC_S_SESSION,
GUC_ACTION_SAVE, true, 0, false); GUC_ACTION_SAVE, true, 0, false);
if (log_min_messages < WARNING) if (log_min_messages < WARNING)
(void) set_config_option("log_min_messages", "warning", (void) set_config_option_ext("log_min_messages", "warning",
PGC_SUSET, PGC_S_SESSION, PGC_SUSET, PGC_S_SESSION,
GUC_ACTION_SAVE, true, 0, false); BOOTSTRAP_SUPERUSERID,
GUC_ACTION_SAVE, true, 0, false);
/* /*
* Similarly disable check_function_bodies, to ensure that SQL functions * Similarly disable check_function_bodies, to ensure that SQL functions

203
src/backend/utils/misc/guc.c
View File

@ -5172,7 +5172,8 @@ static void reapply_stacked_values(struct config_generic *variable,
struct config_string *pHolder, struct config_string *pHolder,
GucStack *stack, GucStack *stack,
const char *curvalue, const char *curvalue,
GucContext curscontext, GucSource cursource); GucContext curscontext, GucSource cursource,
Oid cursrole);
static void ShowGUCConfigOption(const char *name, DestReceiver *dest); static void ShowGUCConfigOption(const char *name, DestReceiver *dest);
static void ShowAllGUCConfig(DestReceiver *dest); static void ShowAllGUCConfig(DestReceiver *dest);
static char *_ShowOption(struct config_generic *record, bool use_units); static char *_ShowOption(struct config_generic *record, bool use_units);
@ -5897,10 +5898,10 @@ InitializeWalConsistencyChecking(void)
check_wal_consistency_checking_deferred = false; check_wal_consistency_checking_deferred = false;
set_config_option("wal_consistency_checking", set_config_option_ext("wal_consistency_checking",
wal_consistency_checking_string, wal_consistency_checking_string,
PGC_POSTMASTER, guc->source, guc->scontext, guc->source, guc->srole,
GUC_ACTION_SET, true, ERROR, false); GUC_ACTION_SET, true, ERROR, false);
/* checking should not be deferred again */ /* checking should not be deferred again */
Assert(!check_wal_consistency_checking_deferred); Assert(!check_wal_consistency_checking_deferred);
@ -5979,6 +5980,8 @@ InitializeOneGUCOption(struct config_generic *gconf)
gconf->reset_source = PGC_S_DEFAULT; gconf->reset_source = PGC_S_DEFAULT;
gconf->scontext = PGC_INTERNAL; gconf->scontext = PGC_INTERNAL;
gconf->reset_scontext = PGC_INTERNAL; gconf->reset_scontext = PGC_INTERNAL;
gconf->srole = BOOTSTRAP_SUPERUSERID;
gconf->reset_srole = BOOTSTRAP_SUPERUSERID;
gconf->stack = NULL; gconf->stack = NULL;
gconf->extra = NULL; gconf->extra = NULL;
gconf->last_reported = NULL; gconf->last_reported = NULL;
@ -6356,6 +6359,7 @@ ResetAllOptions(void)
gconf->source = gconf->reset_source; gconf->source = gconf->reset_source;
gconf->scontext = gconf->reset_scontext; gconf->scontext = gconf->reset_scontext;
gconf->srole = gconf->reset_srole;
if (gconf->flags & GUC_REPORT) if (gconf->flags & GUC_REPORT)
{ {
@ -6401,6 +6405,7 @@ push_old_value(struct config_generic *gconf, GucAction action)
{ {
/* SET followed by SET LOCAL, remember SET's value */ /* SET followed by SET LOCAL, remember SET's value */
stack->masked_scontext = gconf->scontext; stack->masked_scontext = gconf->scontext;
stack->masked_srole = gconf->srole;
set_stack_value(gconf, &stack->masked); set_stack_value(gconf, &stack->masked);
stack->state = GUC_SET_LOCAL; stack->state = GUC_SET_LOCAL;
} }
@ -6439,6 +6444,7 @@ push_old_value(struct config_generic *gconf, GucAction action)
} }
stack->source = gconf->source; stack->source = gconf->source;
stack->scontext = gconf->scontext; stack->scontext = gconf->scontext;
stack->srole = gconf->srole;
set_stack_value(gconf, &stack->prior); set_stack_value(gconf, &stack->prior);
gconf->stack = stack; gconf->stack = stack;
@ -6583,6 +6589,7 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
{ {
/* LOCAL migrates down */ /* LOCAL migrates down */
prev->masked_scontext = stack->scontext; prev->masked_scontext = stack->scontext;
prev->masked_srole = stack->srole;
prev->masked = stack->prior; prev->masked = stack->prior;
prev->state = GUC_SET_LOCAL; prev->state = GUC_SET_LOCAL;
} }
@ -6598,6 +6605,7 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
discard_stack_value(gconf, &stack->prior); discard_stack_value(gconf, &stack->prior);
/* copy down the masked state */ /* copy down the masked state */
prev->masked_scontext = stack->masked_scontext; prev->masked_scontext = stack->masked_scontext;
prev->masked_srole = stack->masked_srole;
if (prev->state == GUC_SET_LOCAL) if (prev->state == GUC_SET_LOCAL)
discard_stack_value(gconf, &prev->masked); discard_stack_value(gconf, &prev->masked);
prev->masked = stack->masked; prev->masked = stack->masked;
@ -6614,18 +6622,21 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
config_var_value newvalue; config_var_value newvalue;
GucSource newsource; GucSource newsource;
GucContext newscontext; GucContext newscontext;
Oid newsrole;
if (restoreMasked) if (restoreMasked)
{ {
newvalue = stack->masked; newvalue = stack->masked;
newsource = PGC_S_SESSION; newsource = PGC_S_SESSION;
newscontext = stack->masked_scontext; newscontext = stack->masked_scontext;
newsrole = stack->masked_srole;
} }
else else
{ {
newvalue = stack->prior; newvalue = stack->prior;
newsource = stack->source; newsource = stack->source;
newscontext = stack->scontext; newscontext = stack->scontext;
newsrole = stack->srole;
} }
switch (gconf->vartype) switch (gconf->vartype)
@ -6740,6 +6751,7 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
/* And restore source information */ /* And restore source information */
gconf->source = newsource; gconf->source = newsource;
gconf->scontext = newscontext; gconf->scontext = newscontext;
gconf->srole = newsrole;
} }
/* Finish popping the state stack */ /* Finish popping the state stack */
@ -7520,7 +7532,7 @@ parse_and_validate_value(struct config_generic *record,
/* /*
* Sets option `name' to given value. * set_config_option: sets option `name' to given value.
* *
* The value should be a string, which will be parsed and converted to * The value should be a string, which will be parsed and converted to
* the appropriate data type. The context and source parameters indicate * the appropriate data type. The context and source parameters indicate
@ -7563,6 +7575,46 @@ set_config_option(const char *name, const char *value,
GucContext context, GucSource source, GucContext context, GucSource source,
GucAction action, bool changeVal, int elevel, GucAction action, bool changeVal, int elevel,
bool is_reload) bool is_reload)
{
Oid srole;
/*
* Non-interactive sources should be treated as having all privileges,
* except for PGC_S_CLIENT. Note in particular that this is true for
* pg_db_role_setting sources (PGC_S_GLOBAL etc): we assume a suitable
* privilege check was done when the pg_db_role_setting entry was made.
*/
if (source >= PGC_S_INTERACTIVE || source == PGC_S_CLIENT)
srole = GetUserId();
else
srole = BOOTSTRAP_SUPERUSERID;
return set_config_option_ext(name, value,
context, source, srole,
action, changeVal, elevel,
is_reload);
}
/*
* set_config_option_ext: sets option `name' to given value.
*
* This API adds the ability to explicitly specify which role OID
* is considered to be setting the value. Most external callers can use
* set_config_option() and let it determine that based on the GucSource,
* but there are a few that are supplying a value that was determined
* in some special way and need to override the decision. Also, when
* restoring a previously-assigned value, it's important to supply the
* same role OID that set the value originally; so all guc.c callers
* that are doing that type of thing need to call this directly.
*
* Generally, srole should be GetUserId() when the source is a SQL operation,
* or BOOTSTRAP_SUPERUSERID if the source is a config file or similar.
*/
int
set_config_option_ext(const char *name, const char *value,
GucContext context, GucSource source, Oid srole,
GucAction action, bool changeVal, int elevel,
bool is_reload)
{ {
struct config_generic *record; struct config_generic *record;
union config_var_val newval_union; union config_var_val newval_union;
@ -7667,12 +7719,12 @@ set_config_option(const char *name, const char *value,
if (context == PGC_BACKEND) if (context == PGC_BACKEND)
{ {
/* /*
* Check whether the current user has been granted privilege * Check whether the requesting user has been granted
* to set this GUC. * privilege to set this GUC.
*/ */
AclResult aclresult; AclResult aclresult;
aclresult = pg_parameter_aclcheck(name, GetUserId(), ACL_SET); aclresult = pg_parameter_aclcheck(name, srole, ACL_SET);
if (aclresult != ACLCHECK_OK) if (aclresult != ACLCHECK_OK)
{ {
/* No granted privilege */ /* No granted privilege */
@ -7725,12 +7777,12 @@ set_config_option(const char *name, const char *value,
if (context == PGC_USERSET || context == PGC_BACKEND) if (context == PGC_USERSET || context == PGC_BACKEND)
{ {
/* /*
* Check whether the current user has been granted privilege * Check whether the requesting user has been granted
* to set this GUC. * privilege to set this GUC.
*/ */
AclResult aclresult; AclResult aclresult;
aclresult = pg_parameter_aclcheck(name, GetUserId(), ACL_SET); aclresult = pg_parameter_aclcheck(name, srole, ACL_SET);
if (aclresult != ACLCHECK_OK) if (aclresult != ACLCHECK_OK)
{ {
/* No granted privilege */ /* No granted privilege */
@ -7847,6 +7899,7 @@ set_config_option(const char *name, const char *value,
newextra = conf->reset_extra; newextra = conf->reset_extra;
source = conf->gen.reset_source; source = conf->gen.reset_source;
context = conf->gen.reset_scontext; context = conf->gen.reset_scontext;
srole = conf->gen.reset_srole;
} }
if (prohibitValueChange) if (prohibitValueChange)
@ -7881,6 +7934,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
conf->gen.source = source; conf->gen.source = source;
conf->gen.scontext = context; conf->gen.scontext = context;
conf->gen.srole = srole;
} }
if (makeDefault) if (makeDefault)
{ {
@ -7893,6 +7947,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
conf->gen.reset_source = source; conf->gen.reset_source = source;
conf->gen.reset_scontext = context; conf->gen.reset_scontext = context;
conf->gen.reset_srole = srole;
} }
for (stack = conf->gen.stack; stack; stack = stack->prev) for (stack = conf->gen.stack; stack; stack = stack->prev)
{ {
@ -7903,6 +7958,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
stack->source = source; stack->source = source;
stack->scontext = context; stack->scontext = context;
stack->srole = srole;
} }
} }
} }
@ -7941,6 +7997,7 @@ set_config_option(const char *name, const char *value,
newextra = conf->reset_extra; newextra = conf->reset_extra;
source = conf->gen.reset_source; source = conf->gen.reset_source;
context = conf->gen.reset_scontext; context = conf->gen.reset_scontext;
srole = conf->gen.reset_srole;
} }
if (prohibitValueChange) if (prohibitValueChange)
@ -7975,6 +8032,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
conf->gen.source = source; conf->gen.source = source;
conf->gen.scontext = context; conf->gen.scontext = context;
conf->gen.srole = srole;
} }
if (makeDefault) if (makeDefault)
{ {
@ -7987,6 +8045,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
conf->gen.reset_source = source; conf->gen.reset_source = source;
conf->gen.reset_scontext = context; conf->gen.reset_scontext = context;
conf->gen.reset_srole = srole;
} }
for (stack = conf->gen.stack; stack; stack = stack->prev) for (stack = conf->gen.stack; stack; stack = stack->prev)
{ {
@ -7997,6 +8056,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
stack->source = source; stack->source = source;
stack->scontext = context; stack->scontext = context;
stack->srole = srole;
} }
} }
} }
@ -8035,6 +8095,7 @@ set_config_option(const char *name, const char *value,
newextra = conf->reset_extra; newextra = conf->reset_extra;
source = conf->gen.reset_source; source = conf->gen.reset_source;
context = conf->gen.reset_scontext; context = conf->gen.reset_scontext;
srole = conf->gen.reset_srole;
} }
if (prohibitValueChange) if (prohibitValueChange)
@ -8069,6 +8130,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
conf->gen.source = source; conf->gen.source = source;
conf->gen.scontext = context; conf->gen.scontext = context;
conf->gen.srole = srole;
} }
if (makeDefault) if (makeDefault)
{ {
@ -8081,6 +8143,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
conf->gen.reset_source = source; conf->gen.reset_source = source;
conf->gen.reset_scontext = context; conf->gen.reset_scontext = context;
conf->gen.reset_srole = srole;
} }
for (stack = conf->gen.stack; stack; stack = stack->prev) for (stack = conf->gen.stack; stack; stack = stack->prev)
{ {
@ -8091,6 +8154,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
stack->source = source; stack->source = source;
stack->scontext = context; stack->scontext = context;
stack->srole = srole;
} }
} }
} }
@ -8145,6 +8209,7 @@ set_config_option(const char *name, const char *value,
newextra = conf->reset_extra; newextra = conf->reset_extra;
source = conf->gen.reset_source; source = conf->gen.reset_source;
context = conf->gen.reset_scontext; context = conf->gen.reset_scontext;
srole = conf->gen.reset_srole;
} }
if (prohibitValueChange) if (prohibitValueChange)
@ -8189,6 +8254,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
conf->gen.source = source; conf->gen.source = source;
conf->gen.scontext = context; conf->gen.scontext = context;
conf->gen.srole = srole;
} }
if (makeDefault) if (makeDefault)
@ -8202,6 +8268,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
conf->gen.reset_source = source; conf->gen.reset_source = source;
conf->gen.reset_scontext = context; conf->gen.reset_scontext = context;
conf->gen.reset_srole = srole;
} }
for (stack = conf->gen.stack; stack; stack = stack->prev) for (stack = conf->gen.stack; stack; stack = stack->prev)
{ {
@ -8213,6 +8280,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
stack->source = source; stack->source = source;
stack->scontext = context; stack->scontext = context;
stack->srole = srole;
} }
} }
} }
@ -8254,6 +8322,7 @@ set_config_option(const char *name, const char *value,
newextra = conf->reset_extra; newextra = conf->reset_extra;
source = conf->gen.reset_source; source = conf->gen.reset_source;
context = conf->gen.reset_scontext; context = conf->gen.reset_scontext;
srole = conf->gen.reset_srole;
} }
if (prohibitValueChange) if (prohibitValueChange)
@ -8288,6 +8357,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
conf->gen.source = source; conf->gen.source = source;
conf->gen.scontext = context; conf->gen.scontext = context;
conf->gen.srole = srole;
} }
if (makeDefault) if (makeDefault)
{ {
@ -8300,6 +8370,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
conf->gen.reset_source = source; conf->gen.reset_source = source;
conf->gen.reset_scontext = context; conf->gen.reset_scontext = context;
conf->gen.reset_srole = srole;
} }
for (stack = conf->gen.stack; stack; stack = stack->prev) for (stack = conf->gen.stack; stack; stack = stack->prev)
{ {
@ -8310,6 +8381,7 @@ set_config_option(const char *name, const char *value,
newextra); newextra);
stack->source = source; stack->source = source;
stack->scontext = context; stack->scontext = context;
stack->srole = srole;
} }
} }
} }
@ -9354,17 +9426,19 @@ define_custom_variable(struct config_generic *variable)
/* First, apply the reset value if any */ /* First, apply the reset value if any */
if (pHolder->reset_val) if (pHolder->reset_val)
(void) set_config_option(name, pHolder->reset_val, (void) set_config_option_ext(name, pHolder->reset_val,
pHolder->gen.reset_scontext, pHolder->gen.reset_scontext,
pHolder->gen.reset_source, pHolder->gen.reset_source,
GUC_ACTION_SET, true, WARNING, false); pHolder->gen.reset_srole,
GUC_ACTION_SET, true, WARNING, false);
/* That should not have resulted in stacking anything */ /* That should not have resulted in stacking anything */
Assert(variable->stack == NULL); Assert(variable->stack == NULL);
/* Now, apply current and stacked values, in the order they were stacked */ /* Now, apply current and stacked values, in the order they were stacked */
reapply_stacked_values(variable, pHolder, pHolder->gen.stack, reapply_stacked_values(variable, pHolder, pHolder->gen.stack,
*(pHolder->variable), *(pHolder->variable),
pHolder->gen.scontext, pHolder->gen.source); pHolder->gen.scontext, pHolder->gen.source,
pHolder->gen.srole);
/* Also copy over any saved source-location information */ /* Also copy over any saved source-location information */
if (pHolder->gen.sourcefile) if (pHolder->gen.sourcefile)
@ -9395,7 +9469,8 @@ reapply_stacked_values(struct config_generic *variable,
struct config_string *pHolder, struct config_string *pHolder,
GucStack *stack, GucStack *stack,
const char *curvalue, const char *curvalue,
GucContext curscontext, GucSource cursource) GucContext curscontext, GucSource cursource,
Oid cursrole)
{ {
const char *name = variable->name; const char *name = variable->name;
GucStack *oldvarstack = variable->stack; GucStack *oldvarstack = variable->stack;
@ -9405,43 +9480,45 @@ reapply_stacked_values(struct config_generic *variable,
/* First, recurse, so that stack items are processed bottom to top */ /* First, recurse, so that stack items are processed bottom to top */
reapply_stacked_values(variable, pHolder, stack->prev, reapply_stacked_values(variable, pHolder, stack->prev,
stack->prior.val.stringval, stack->prior.val.stringval,
stack->scontext, stack->source); stack->scontext, stack->source, stack->srole);
/* See how to apply the passed-in value */ /* See how to apply the passed-in value */
switch (stack->state) switch (stack->state)
{ {
case GUC_SAVE: case GUC_SAVE:
(void) set_config_option(name, curvalue, (void) set_config_option_ext(name, curvalue,
curscontext, cursource, curscontext, cursource, cursrole,
GUC_ACTION_SAVE, true, GUC_ACTION_SAVE, true,
WARNING, false); WARNING, false);
break; break;
case GUC_SET: case GUC_SET:
(void) set_config_option(name, curvalue, (void) set_config_option_ext(name, curvalue,
curscontext, cursource, curscontext, cursource, cursrole,
GUC_ACTION_SET, true, GUC_ACTION_SET, true,
WARNING, false); WARNING, false);
break; break;
case GUC_LOCAL: case GUC_LOCAL:
(void) set_config_option(name, curvalue, (void) set_config_option_ext(name, curvalue,
curscontext, cursource, curscontext, cursource, cursrole,
GUC_ACTION_LOCAL, true, GUC_ACTION_LOCAL, true,
WARNING, false); WARNING, false);
break; break;
case GUC_SET_LOCAL: case GUC_SET_LOCAL:
/* first, apply the masked value as SET */ /* first, apply the masked value as SET */
(void) set_config_option(name, stack->masked.val.stringval, (void) set_config_option_ext(name, stack->masked.val.stringval,
stack->masked_scontext, PGC_S_SESSION, stack->masked_scontext,
GUC_ACTION_SET, true, PGC_S_SESSION,
WARNING, false); stack->masked_srole,
GUC_ACTION_SET, true,
WARNING, false);
/* then apply the current value as LOCAL */ /* then apply the current value as LOCAL */
(void) set_config_option(name, curvalue, (void) set_config_option_ext(name, curvalue,
curscontext, cursource, curscontext, cursource, cursrole,
GUC_ACTION_LOCAL, true, GUC_ACTION_LOCAL, true,
WARNING, false); WARNING, false);
break; break;
} }
@ -9461,11 +9538,12 @@ reapply_stacked_values(struct config_generic *variable,
*/ */
if (curvalue != pHolder->reset_val || if (curvalue != pHolder->reset_val ||
curscontext != pHolder->gen.reset_scontext || curscontext != pHolder->gen.reset_scontext ||
cursource != pHolder->gen.reset_source) cursource != pHolder->gen.reset_source ||
cursrole != pHolder->gen.reset_srole)
{ {
(void) set_config_option(name, curvalue, (void) set_config_option_ext(name, curvalue,
curscontext, cursource, curscontext, cursource, cursrole,
GUC_ACTION_SET, true, WARNING, false); GUC_ACTION_SET, true, WARNING, false);
variable->stack = NULL; variable->stack = NULL;
} }
} }
@ -10577,6 +10655,7 @@ _ShowOption(struct config_generic *record, bool use_units)
* variable sourceline, integer * variable sourceline, integer
* variable source, integer * variable source, integer
* variable scontext, integer * variable scontext, integer
* variable srole, OID
*/ */
static void static void
write_one_nondefault_variable(FILE *fp, struct config_generic *gconf) write_one_nondefault_variable(FILE *fp, struct config_generic *gconf)
@ -10643,6 +10722,7 @@ write_one_nondefault_variable(FILE *fp, struct config_generic *gconf)
fwrite(&gconf->sourceline, 1, sizeof(gconf->sourceline), fp); fwrite(&gconf->sourceline, 1, sizeof(gconf->sourceline), fp);
fwrite(&gconf->source, 1, sizeof(gconf->source), fp); fwrite(&gconf->source, 1, sizeof(gconf->source), fp);
fwrite(&gconf->scontext, 1, sizeof(gconf->scontext), fp); fwrite(&gconf->scontext, 1, sizeof(gconf->scontext), fp);
fwrite(&gconf->srole, 1, sizeof(gconf->srole), fp);
} }
void void
@ -10738,6 +10818,7 @@ read_nondefault_variables(void)
int varsourceline; int varsourceline;
GucSource varsource; GucSource varsource;
GucContext varscontext; GucContext varscontext;
Oid varsrole;
/* /*
* Open file * Open file
@ -10774,10 +10855,12 @@ read_nondefault_variables(void)
elog(FATAL, "invalid format of exec config params file"); elog(FATAL, "invalid format of exec config params file");
if (fread(&varscontext, 1, sizeof(varscontext), fp) != sizeof(varscontext)) if (fread(&varscontext, 1, sizeof(varscontext), fp) != sizeof(varscontext))
elog(FATAL, "invalid format of exec config params file"); elog(FATAL, "invalid format of exec config params file");
if (fread(&varsrole, 1, sizeof(varsrole), fp) != sizeof(varsrole))
elog(FATAL, "invalid format of exec config params file");
(void) set_config_option(varname, varvalue, (void) set_config_option_ext(varname, varvalue,
varscontext, varsource, varscontext, varsource, varsrole,
GUC_ACTION_SET, true, 0, true); GUC_ACTION_SET, true, 0, true);
if (varsourcefile[0]) if (varsourcefile[0])
set_config_sourcefile(varname, varsourcefile, varsourceline); set_config_sourcefile(varname, varsourcefile, varsourceline);
@ -10931,6 +11014,7 @@ estimate_variable_size(struct config_generic *gconf)
size = add_size(size, sizeof(gconf->source)); size = add_size(size, sizeof(gconf->source));
size = add_size(size, sizeof(gconf->scontext)); size = add_size(size, sizeof(gconf->scontext));
size = add_size(size, sizeof(gconf->srole));
return size; return size;
} }
@ -11076,6 +11160,8 @@ serialize_variable(char **destptr, Size *maxbytes,
sizeof(gconf->source)); sizeof(gconf->source));
do_serialize_binary(destptr, maxbytes, &gconf->scontext, do_serialize_binary(destptr, maxbytes, &gconf->scontext,
sizeof(gconf->scontext)); sizeof(gconf->scontext));
do_serialize_binary(destptr, maxbytes, &gconf->srole,
sizeof(gconf->srole));
} }
/* /*
@ -11177,6 +11263,7 @@ RestoreGUCState(void *gucstate)
int varsourceline; int varsourceline;
GucSource varsource; GucSource varsource;
GucContext varscontext; GucContext varscontext;
Oid varsrole;
char *srcptr = (char *) gucstate; char *srcptr = (char *) gucstate;
char *srcend; char *srcend;
Size len; Size len;
@ -11304,12 +11391,15 @@ RestoreGUCState(void *gucstate)
&varsource, sizeof(varsource)); &varsource, sizeof(varsource));
read_gucstate_binary(&srcptr, srcend, read_gucstate_binary(&srcptr, srcend,
&varscontext, sizeof(varscontext)); &varscontext, sizeof(varscontext));
read_gucstate_binary(&srcptr, srcend,
&varsrole, sizeof(varsrole));
error_context_name_and_value[0] = varname; error_context_name_and_value[0] = varname;
error_context_name_and_value[1] = varvalue; error_context_name_and_value[1] = varvalue;
error_context_callback.arg = &error_context_name_and_value[0]; error_context_callback.arg = &error_context_name_and_value[0];
result = set_config_option(varname, varvalue, varscontext, varsource, result = set_config_option_ext(varname, varvalue,
GUC_ACTION_SET, true, ERROR, true); varscontext, varsource, varsrole,
GUC_ACTION_SET, true, ERROR, true);
if (result <= 0) if (result <= 0)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_INTERNAL_ERROR), (errcode(ERRCODE_INTERNAL_ERROR),
@ -11578,7 +11668,7 @@ GUCArrayDelete(ArrayType *array, const char *name)
/* /*
* Given a GUC array, delete all settings from it that our permission * Given a GUC array, delete all settings from it that our permission
* level allows: if superuser, delete them all; if regular user, only * level allows: if superuser, delete them all; if regular user, only
* those that are PGC_USERSET * those that are PGC_USERSET or we have permission to set
*/ */
ArrayType * ArrayType *
GUCArrayReset(ArrayType *array) GUCArrayReset(ArrayType *array)
@ -11664,14 +11754,16 @@ validate_option_array_item(const char *name, const char *value,
* *
* name is a known GUC variable. Check the value normally, check * name is a known GUC variable. Check the value normally, check
* permissions normally (i.e., allow if variable is USERSET, or if it's * permissions normally (i.e., allow if variable is USERSET, or if it's
* SUSET and user is superuser). * SUSET and user is superuser or holds ACL_SET permissions).
* *
* name is not known, but exists or can be created as a placeholder (i.e., * name is not known, but exists or can be created as a placeholder (i.e.,
* it has a valid custom name). We allow this case if you're a superuser, * it has a valid custom name). We allow this case if you're a superuser,
* otherwise not. Superusers are assumed to know what they're doing. We * otherwise not. Superusers are assumed to know what they're doing. We
* can't allow it for other users, because when the placeholder is * can't allow it for other users, because when the placeholder is
* resolved it might turn out to be a SUSET variable; * resolved it might turn out to be a SUSET variable. (With currently
* define_custom_variable assumes we checked that. * available infrastructure, we can actually handle such cases within the
* current session --- but once an entry is made in pg_db_role_setting,
* it's assumed to be fully validated.)
* *
* name is not known and can't be created as a placeholder. Throw error, * name is not known and can't be created as a placeholder. Throw error,
* unless skipIfNoPermissions is true, in which case return false. * unless skipIfNoPermissions is true, in which case return false.
@ -11689,7 +11781,8 @@ validate_option_array_item(const char *name, const char *value,
* We cannot do any meaningful check on the value, so only permissions * We cannot do any meaningful check on the value, so only permissions
* are useful to check. * are useful to check.
*/ */
if (superuser()) if (superuser() ||
pg_parameter_aclcheck(name, GetUserId(), ACL_SET) == ACLCHECK_OK)
return true; return true;
if (skipIfNoPermissions) if (skipIfNoPermissions)
return false; return false;
@ -11701,7 +11794,9 @@ validate_option_array_item(const char *name, const char *value,
/* manual permissions check so we can avoid an error being thrown */ /* manual permissions check so we can avoid an error being thrown */
if (gconf->context == PGC_USERSET) if (gconf->context == PGC_USERSET)
/* ok */ ; /* ok */ ;
else if (gconf->context == PGC_SUSET && superuser()) else if (gconf->context == PGC_SUSET &&
(superuser() ||
pg_parameter_aclcheck(name, GetUserId(), ACL_SET) == ACLCHECK_OK))
/* ok */ ; /* ok */ ;
else if (skipIfNoPermissions) else if (skipIfNoPermissions)
return false; return false;

5
src/include/utils/guc.h
View File

@ -388,6 +388,11 @@ extern int set_config_option(const char *name, const char *value,
GucContext context, GucSource source, GucContext context, GucSource source,
GucAction action, bool changeVal, int elevel, GucAction action, bool changeVal, int elevel,
bool is_reload); bool is_reload);
extern int set_config_option_ext(const char *name, const char *value,
GucContext context, GucSource source,
Oid srole,
GucAction action, bool changeVal, int elevel,
bool is_reload);
extern void AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt); extern void AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt);
extern char *GetConfigOptionByName(const char *name, const char **varname, extern char *GetConfigOptionByName(const char *name, const char **varname,
bool missing_ok); bool missing_ok);

8
src/include/utils/guc_tables.h
View File

@ -120,6 +120,8 @@ typedef struct guc_stack
/* masked value's source must be PGC_S_SESSION, so no need to store it */ /* masked value's source must be PGC_S_SESSION, so no need to store it */
GucContext scontext; /* context that set the prior value */ GucContext scontext; /* context that set the prior value */
GucContext masked_scontext; /* context that set the masked value */ GucContext masked_scontext; /* context that set the masked value */
Oid srole; /* role that set the prior value */
Oid masked_srole; /* role that set the masked value */
config_var_value prior; /* previous value of variable */ config_var_value prior; /* previous value of variable */
config_var_value masked; /* SET value in a GUC_SET_LOCAL entry */ config_var_value masked; /* SET value in a GUC_SET_LOCAL entry */
} GucStack; } GucStack;
@ -131,6 +133,10 @@ typedef struct guc_stack
* applications may use the long description as well, and will append * applications may use the long description as well, and will append
* it to the short description. (separated by a newline or '. ') * it to the short description. (separated by a newline or '. ')
* *
* srole is the role that set the current value, or BOOTSTRAP_SUPERUSERID
* if the value came from an internal source or the config file. Similarly
* for reset_srole (which is usually BOOTSTRAP_SUPERUSERID, but not always).
*
* Note that sourcefile/sourceline are kept here, and not pushed into stacked * Note that sourcefile/sourceline are kept here, and not pushed into stacked
* values, although in principle they belong with some stacked value if the * values, although in principle they belong with some stacked value if the
* active value is session- or transaction-local. This is to avoid bloating * active value is session- or transaction-local. This is to avoid bloating
@ -152,6 +158,8 @@ struct config_generic
GucSource reset_source; /* source of the reset_value */ GucSource reset_source; /* source of the reset_value */
GucContext scontext; /* context that set the current value */ GucContext scontext; /* context that set the current value */
GucContext reset_scontext; /* context that set the reset value */ GucContext reset_scontext; /* context that set the reset value */
Oid srole; /* role that set the current value */
Oid reset_srole; /* role that set the reset value */
GucStack *stack; /* stacked prior values */ GucStack *stack; /* stacked prior values */
void *extra; /* "extra" pointer for current actual value */ void *extra; /* "extra" pointer for current actual value */
char *last_reported; /* if variable is GUC_REPORT, value last sent char *last_reported; /* if variable is GUC_REPORT, value last sent

28
src/pl/plperl/expected/plperl_init.out
View File

@ -1,4 +1,4 @@
-- test plperl.on_plperl_init errors are fatal -- test plperl.on_plperl_init
-- This test tests setting on_plperl_init after loading plperl -- This test tests setting on_plperl_init after loading plperl
LOAD 'plperl'; LOAD 'plperl';
SET SESSION plperl.on_plperl_init = ' system("/nonesuch"); '; SET SESSION plperl.on_plperl_init = ' system("/nonesuch"); ';
@ -12,3 +12,29 @@ DO $$ warn 42 $$ language plperl;
ERROR: 'system' trapped by operation mask at line 1. ERROR: 'system' trapped by operation mask at line 1.
CONTEXT: while executing plperl.on_plperl_init CONTEXT: while executing plperl.on_plperl_init
PL/Perl anonymous code block PL/Perl anonymous code block
--
-- Reconnect (to unload plperl), then test setting on_plperl_init
-- as an unprivileged user
--
\c -
CREATE ROLE regress_plperl_user;
SET ROLE regress_plperl_user;
-- this succeeds, since the GUC isn't known yet
SET SESSION plperl.on_plperl_init = 'test';
RESET ROLE;
LOAD 'plperl';
WARNING: permission denied to set parameter "plperl.on_plperl_init"
SHOW plperl.on_plperl_init;
plperl.on_plperl_init
-----------------------
(1 row)
DO $$ warn 42 $$ language plperl;
WARNING: 42 at line 1.
-- now we won't be allowed to set it in the first place
SET ROLE regress_plperl_user;
SET SESSION plperl.on_plperl_init = 'test';
ERROR: permission denied to set parameter "plperl.on_plperl_init"
RESET ROLE;
DROP ROLE regress_plperl_user;

33
src/pl/plperl/sql/plperl_init.sql
View File

@ -1,4 +1,4 @@
-- test plperl.on_plperl_init errors are fatal -- test plperl.on_plperl_init
-- This test tests setting on_plperl_init after loading plperl -- This test tests setting on_plperl_init after loading plperl
LOAD 'plperl'; LOAD 'plperl';
@ -8,3 +8,34 @@ SET SESSION plperl.on_plperl_init = ' system("/nonesuch"); ';
SHOW plperl.on_plperl_init; SHOW plperl.on_plperl_init;
DO $$ warn 42 $$ language plperl; DO $$ warn 42 $$ language plperl;
--
-- Reconnect (to unload plperl), then test setting on_plperl_init
-- as an unprivileged user
--
\c -
CREATE ROLE regress_plperl_user;
SET ROLE regress_plperl_user;
-- this succeeds, since the GUC isn't known yet
SET SESSION plperl.on_plperl_init = 'test';
RESET ROLE;
LOAD 'plperl';
SHOW plperl.on_plperl_init;
DO $$ warn 42 $$ language plperl;
-- now we won't be allowed to set it in the first place
SET ROLE regress_plperl_user;
SET SESSION plperl.on_plperl_init = 'test';
RESET ROLE;
DROP ROLE regress_plperl_user;

36
src/test/modules/unsafe_tests/expected/guc_privs.out
View File

@ -418,6 +418,8 @@ FROM pg_get_object_address('parameter ACL', '{work_mem}', '{}') goa;
pg_parameter_acl | work_mem | 0 pg_parameter_acl | work_mem | 0
(1 row) (1 row)
-- Make a per-role setting that regress_host_resource_admin can't change
ALTER ROLE regress_host_resource_admin SET lc_messages = 'C';
-- Perform some operations as user 'regress_host_resource_admin' -- Perform some operations as user 'regress_host_resource_admin'
SET SESSION AUTHORIZATION regress_host_resource_admin; SET SESSION AUTHORIZATION regress_host_resource_admin;
ALTER SYSTEM SET autovacuum_work_mem = 32; -- ok, privileges have been granted ALTER SYSTEM SET autovacuum_work_mem = 32; -- ok, privileges have been granted
@ -457,6 +459,40 @@ SELECT set_config ('temp_buffers', '8192', false); -- ok
ALTER SYSTEM RESET autovacuum_work_mem; -- ok, privileges have been granted ALTER SYSTEM RESET autovacuum_work_mem; -- ok, privileges have been granted
ALTER SYSTEM RESET ALL; -- fail, insufficient privileges ALTER SYSTEM RESET ALL; -- fail, insufficient privileges
ERROR: permission denied to perform ALTER SYSTEM RESET ALL ERROR: permission denied to perform ALTER SYSTEM RESET ALL
ALTER ROLE regress_host_resource_admin SET lc_messages = 'POSIX'; -- fail
ERROR: permission denied to set parameter "lc_messages"
ALTER ROLE regress_host_resource_admin SET max_stack_depth = '1MB'; -- ok
SELECT setconfig FROM pg_db_role_setting
WHERE setrole = 'regress_host_resource_admin'::regrole;
setconfig
-------------------------------------
{lc_messages=C,max_stack_depth=1MB}
(1 row)
ALTER ROLE regress_host_resource_admin RESET max_stack_depth; -- ok
SELECT setconfig FROM pg_db_role_setting
WHERE setrole = 'regress_host_resource_admin'::regrole;
setconfig
-----------------
{lc_messages=C}
(1 row)
ALTER ROLE regress_host_resource_admin SET max_stack_depth = '1MB'; -- ok
SELECT setconfig FROM pg_db_role_setting
WHERE setrole = 'regress_host_resource_admin'::regrole;
setconfig
-------------------------------------
{lc_messages=C,max_stack_depth=1MB}
(1 row)
ALTER ROLE regress_host_resource_admin RESET ALL; -- doesn't reset lc_messages
SELECT setconfig FROM pg_db_role_setting
WHERE setrole = 'regress_host_resource_admin'::regrole;
setconfig
-----------------
{lc_messages=C}
(1 row)
-- Check dropping/revoking behavior -- Check dropping/revoking behavior
SET SESSION AUTHORIZATION regress_admin; SET SESSION AUTHORIZATION regress_admin;
DROP ROLE regress_host_resource_admin; -- fail, privileges remain DROP ROLE regress_host_resource_admin; -- fail, privileges remain

16
src/test/modules/unsafe_tests/sql/guc_privs.sql
View File

@ -163,6 +163,9 @@ SELECT classid::regclass,
objsubid objsubid
FROM pg_get_object_address('parameter ACL', '{work_mem}', '{}') goa; FROM pg_get_object_address('parameter ACL', '{work_mem}', '{}') goa;
-- Make a per-role setting that regress_host_resource_admin can't change
ALTER ROLE regress_host_resource_admin SET lc_messages = 'C';
-- Perform some operations as user 'regress_host_resource_admin' -- Perform some operations as user 'regress_host_resource_admin'
SET SESSION AUTHORIZATION regress_host_resource_admin; SET SESSION AUTHORIZATION regress_host_resource_admin;
ALTER SYSTEM SET autovacuum_work_mem = 32; -- ok, privileges have been granted ALTER SYSTEM SET autovacuum_work_mem = 32; -- ok, privileges have been granted
@ -187,6 +190,19 @@ ALTER SYSTEM RESET lc_messages; -- fail, insufficient privileges
SELECT set_config ('temp_buffers', '8192', false); -- ok SELECT set_config ('temp_buffers', '8192', false); -- ok
ALTER SYSTEM RESET autovacuum_work_mem; -- ok, privileges have been granted ALTER SYSTEM RESET autovacuum_work_mem; -- ok, privileges have been granted
ALTER SYSTEM RESET ALL; -- fail, insufficient privileges ALTER SYSTEM RESET ALL; -- fail, insufficient privileges
ALTER ROLE regress_host_resource_admin SET lc_messages = 'POSIX'; -- fail
ALTER ROLE regress_host_resource_admin SET max_stack_depth = '1MB'; -- ok
SELECT setconfig FROM pg_db_role_setting
WHERE setrole = 'regress_host_resource_admin'::regrole;
ALTER ROLE regress_host_resource_admin RESET max_stack_depth; -- ok
SELECT setconfig FROM pg_db_role_setting
WHERE setrole = 'regress_host_resource_admin'::regrole;
ALTER ROLE regress_host_resource_admin SET max_stack_depth = '1MB'; -- ok
SELECT setconfig FROM pg_db_role_setting
WHERE setrole = 'regress_host_resource_admin'::regrole;
ALTER ROLE regress_host_resource_admin RESET ALL; -- doesn't reset lc_messages
SELECT setconfig FROM pg_db_role_setting
WHERE setrole = 'regress_host_resource_admin'::regrole;
-- Check dropping/revoking behavior -- Check dropping/revoking behavior
SET SESSION AUTHORIZATION regress_admin; SET SESSION AUTHORIZATION regress_admin;