aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/backend/catalog/pg_parameter_acl.c6
-rw-r--r--src/backend/utils/misc/guc.c218
-rw-r--r--src/include/utils/guc.h2
-rw-r--r--src/test/modules/unsafe_tests/expected/guc_privs.out26
-rw-r--r--src/test/modules/unsafe_tests/sql/guc_privs.sql14
5 files changed, 170 insertions, 96 deletions
diff --git a/src/backend/catalog/pg_parameter_acl.c b/src/backend/catalog/pg_parameter_acl.c
index 073392e2c4f..f4bc10bafed 100644
--- a/src/backend/catalog/pg_parameter_acl.c
+++ b/src/backend/catalog/pg_parameter_acl.c
@@ -82,11 +82,7 @@ ParameterAclCreate(const char *parameter)
* To prevent cluttering pg_parameter_acl with useless entries, insist
* that the name be valid.
*/
- if (!check_GUC_name_for_parameter_acl(parameter))
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_NAME),
- errmsg("invalid parameter name \"%s\"",
- parameter)));
+ check_GUC_name_for_parameter_acl(parameter);
/* Convert name to the form it should have in pg_parameter_acl. */
parname = convert_GUC_name_for_parameter_acl(parameter);
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c25c697a069..39d3775e801 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -250,6 +250,8 @@ static void write_auto_conf_file(int fd, const char *filename, ConfigVariable *h
static void replace_auto_config_value(ConfigVariable **head_p, ConfigVariable **tail_p,
const char *name, const char *value);
static bool valid_custom_variable_name(const char *name);
+static bool assignable_custom_variable_name(const char *name, bool skip_errors,
+ int elevel);
static void do_serialize(char **destptr, Size *maxbytes,
const char *fmt,...) pg_attribute_printf(3, 4);
static bool call_bool_check_hook(struct config_bool *conf, bool *newval,
@@ -1063,7 +1065,7 @@ add_guc_variable(struct config_generic *var, int elevel)
*
* It must be two or more identifiers separated by dots, where the rules
* for what is an identifier agree with scan.l. (If you change this rule,
- * adjust the errdetail in find_option().)
+ * adjust the errdetail in assignable_custom_variable_name().)
*/
static bool
valid_custom_variable_name(const char *name)
@@ -1099,6 +1101,71 @@ valid_custom_variable_name(const char *name)
}
/*
+ * Decide whether an unrecognized variable name is allowed to be SET.
+ *
+ * It must pass the syntactic rules of valid_custom_variable_name(),
+ * and it must not be in any namespace already reserved by an extension.
+ * (We make this separate from valid_custom_variable_name() because we don't
+ * apply the reserved-namespace test when reading configuration files.)
+ *
+ * If valid, return true. Otherwise, return false if skip_errors is true,
+ * else throw a suitable error at the specified elevel (and return false
+ * if that's less than ERROR).
+ */
+static bool
+assignable_custom_variable_name(const char *name, bool skip_errors, int elevel)
+{
+ /* If there's no separator, it can't be a custom variable */
+ const char *sep = strchr(name, GUC_QUALIFIER_SEPARATOR);
+
+ if (sep != NULL)
+ {
+ size_t classLen = sep - name;
+ ListCell *lc;
+
+ /* The name must be syntactically acceptable ... */
+ if (!valid_custom_variable_name(name))
+ {
+ if (!skip_errors)
+ ereport(elevel,
+ (errcode(ERRCODE_INVALID_NAME),
+ errmsg("invalid configuration parameter name \"%s\"",
+ name),
+ errdetail("Custom parameter names must be two or more simple identifiers separated by dots.")));
+ return false;
+ }
+ /* ... and it must not match any previously-reserved prefix */
+ foreach(lc, reserved_class_prefix)
+ {
+ const char *rcprefix = lfirst(lc);
+
+ if (strlen(rcprefix) == classLen &&
+ strncmp(name, rcprefix, classLen) == 0)
+ {
+ if (!skip_errors)
+ ereport(elevel,
+ (errcode(ERRCODE_INVALID_NAME),
+ errmsg("invalid configuration parameter name \"%s\"",
+ name),
+ errdetail("\"%s\" is a reserved prefix.",
+ rcprefix)));
+ return false;
+ }
+ }
+ /* OK to create it */
+ return true;
+ }
+
+ /* Unrecognized single-part name */
+ if (!skip_errors)
+ ereport(elevel,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("unrecognized configuration parameter \"%s\"",
+ name)));
+ return false;
+}
+
+/*
* Create and add a placeholder variable for a custom variable name.
*/
static struct config_generic *
@@ -1191,52 +1258,15 @@ find_option(const char *name, bool create_placeholders, bool skip_errors,
if (create_placeholders)
{
/*
- * Check if the name is valid, and if so, add a placeholder. If it
- * doesn't contain a separator, don't assume that it was meant to be a
- * placeholder.
+ * Check if the name is valid, and if so, add a placeholder.
*/
- const char *sep = strchr(name, GUC_QUALIFIER_SEPARATOR);
-
- if (sep != NULL)
- {
- size_t classLen = sep - name;
- ListCell *lc;
-
- /* The name must be syntactically acceptable ... */
- if (!valid_custom_variable_name(name))
- {
- if (!skip_errors)
- ereport(elevel,
- (errcode(ERRCODE_INVALID_NAME),
- errmsg("invalid configuration parameter name \"%s\"",
- name),
- errdetail("Custom parameter names must be two or more simple identifiers separated by dots.")));
- return NULL;
- }
- /* ... and it must not match any previously-reserved prefix */
- foreach(lc, reserved_class_prefix)
- {
- const char *rcprefix = lfirst(lc);
-
- if (strlen(rcprefix) == classLen &&
- strncmp(name, rcprefix, classLen) == 0)
- {
- if (!skip_errors)
- ereport(elevel,
- (errcode(ERRCODE_INVALID_NAME),
- errmsg("invalid configuration parameter name \"%s\"",
- name),
- errdetail("\"%s\" is a reserved prefix.",
- rcprefix)));
- return NULL;
- }
- }
- /* OK, create it */
+ if (assignable_custom_variable_name(name, skip_errors, elevel))
return add_placeholder_variable(name, elevel);
- }
+ else
+ return NULL; /* error message, if any, already emitted */
}
- /* Unknown name */
+ /* Unknown name and we're not supposed to make a placeholder */
if (!skip_errors)
ereport(elevel,
(errcode(ERRCODE_UNDEFINED_OBJECT),
@@ -1369,18 +1399,16 @@ convert_GUC_name_for_parameter_acl(const char *name)
/*
* Check whether we should allow creation of a pg_parameter_acl entry
* for the given name. (This can be applied either before or after
- * canonicalizing it.)
+ * canonicalizing it.) Throws error if not.
*/
-bool
+void
check_GUC_name_for_parameter_acl(const char *name)
{
/* OK if the GUC exists. */
- if (find_option(name, false, true, DEBUG1) != NULL)
- return true;
+ if (find_option(name, false, true, DEBUG5) != NULL)
+ return;
/* Otherwise, it'd better be a valid custom GUC name. */
- if (valid_custom_variable_name(name))
- return true;
- return false;
+ (void) assignable_custom_variable_name(name, false, ERROR);
}
/*
@@ -4515,52 +4543,64 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
{
struct config_generic *record;
- record = find_option(name, false, false, ERROR);
- Assert(record != NULL);
-
- /*
- * Don't allow parameters that can't be set in configuration files to
- * be set in PG_AUTOCONF_FILENAME file.
- */
- if ((record->context == PGC_INTERNAL) ||
- (record->flags & GUC_DISALLOW_IN_FILE) ||
- (record->flags & GUC_DISALLOW_IN_AUTO_FILE))
- ereport(ERROR,
- (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
- errmsg("parameter \"%s\" cannot be changed",
- name)));
-
- /*
- * If a value is specified, verify that it's sane.
- */
- if (value)
+ /* We don't want to create a placeholder if there's not one already */
+ record = find_option(name, false, true, DEBUG5);
+ if (record != NULL)
{
- union config_var_val newval;
- void *newextra = NULL;
-
- /* Check that it's acceptable for the indicated parameter */
- if (!parse_and_validate_value(record, name, value,
- PGC_S_FILE, ERROR,
- &newval, &newextra))
+ /*
+ * Don't allow parameters that can't be set in configuration files
+ * to be set in PG_AUTOCONF_FILENAME file.
+ */
+ if ((record->context == PGC_INTERNAL) ||
+ (record->flags & GUC_DISALLOW_IN_FILE) ||
+ (record->flags & GUC_DISALLOW_IN_AUTO_FILE))
ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("invalid value for parameter \"%s\": \"%s\"",
- name, value)));
+ (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+ errmsg("parameter \"%s\" cannot be changed",
+ name)));
+
+ /*
+ * If a value is specified, verify that it's sane.
+ */
+ if (value)
+ {
+ union config_var_val newval;
+ void *newextra = NULL;
- if (record->vartype == PGC_STRING && newval.stringval != NULL)
- guc_free(newval.stringval);
- guc_free(newextra);
+ if (!parse_and_validate_value(record, name, value,
+ PGC_S_FILE, ERROR,
+ &newval, &newextra))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for parameter \"%s\": \"%s\"",
+ name, value)));
+ if (record->vartype == PGC_STRING && newval.stringval != NULL)
+ guc_free(newval.stringval);
+ guc_free(newextra);
+ }
+ }
+ else
+ {
/*
- * We must also reject values containing newlines, because the
- * grammar for config files doesn't support embedded newlines in
- * string literals.
+ * Variable not known; check we'd be allowed to create it. (We
+ * cannot validate the value, but that's fine. A non-core GUC in
+ * the config file cannot cause postmaster start to fail, so we
+ * don't have to be too tense about possibly installing a bad
+ * value.)
*/
- if (strchr(value, '\n'))
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("parameter value for ALTER SYSTEM must not contain a newline")));
+ (void) assignable_custom_variable_name(name, false, ERROR);
}
+
+ /*
+ * We must also reject values containing newlines, because the grammar
+ * for config files doesn't support embedded newlines in string
+ * literals.
+ */
+ if (value && strchr(value, '\n'))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter value for ALTER SYSTEM must not contain a newline")));
}
/*
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index e89083ee0e6..902e57c0ca3 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -363,7 +363,7 @@ extern const char *GetConfigOptionResetString(const char *name);
extern int GetConfigOptionFlags(const char *name, bool missing_ok);
extern void ProcessConfigFile(GucContext context);
extern char *convert_GUC_name_for_parameter_acl(const char *name);
-extern bool check_GUC_name_for_parameter_acl(const char *name);
+extern void check_GUC_name_for_parameter_acl(const char *name);
extern void InitializeGUCOptions(void);
extern bool SelectConfigFiles(const char *userDoption, const char *progname);
extern void ResetAllOptions(void);
diff --git a/src/test/modules/unsafe_tests/expected/guc_privs.out b/src/test/modules/unsafe_tests/expected/guc_privs.out
index f43a1da214e..6c0ad898341 100644
--- a/src/test/modules/unsafe_tests/expected/guc_privs.out
+++ b/src/test/modules/unsafe_tests/expected/guc_privs.out
@@ -220,9 +220,31 @@ SELECT 1 FROM pg_parameter_acl WHERE parname = 'none.such';
----------
(0 rows)
+-- Superuser should be able to ALTER SYSTEM SET a non-existent custom GUC.
+ALTER SYSTEM SET none.such = 'whiz bang';
+-- None of the above should have created a placeholder GUC for none.such.
+SHOW none.such; -- error
+ERROR: unrecognized configuration parameter "none.such"
+-- However, if we reload ...
+SELECT pg_reload_conf();
+ pg_reload_conf
+----------------
+ t
+(1 row)
+
+-- and start a new session to avoid race condition ...
+\c -
+SET SESSION AUTHORIZATION regress_admin;
+-- then it should be there.
+SHOW none.such;
+ none.such
+-----------
+ whiz bang
+(1 row)
+
-- Can't grant on a non-existent core GUC.
GRANT ALL ON PARAMETER no_such_guc TO regress_host_resource_admin; -- fail
-ERROR: invalid parameter name "no_such_guc"
+ERROR: unrecognized configuration parameter "no_such_guc"
-- Initially there are no privileges and no catalog entry for this GUC.
SELECT has_parameter_privilege('regress_host_resource_admin', 'enable_material', 'SET');
has_parameter_privilege
@@ -459,6 +481,8 @@ SELECT set_config ('temp_buffers', '8192', false); -- ok
ALTER SYSTEM RESET autovacuum_work_mem; -- ok, privileges have been granted
ALTER SYSTEM RESET ALL; -- fail, insufficient privileges
ERROR: permission denied to perform ALTER SYSTEM RESET ALL
+ALTER SYSTEM SET none.such2 = 'whiz bang'; -- fail, not superuser
+ERROR: permission denied to set parameter "none.such2"
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
diff --git a/src/test/modules/unsafe_tests/sql/guc_privs.sql b/src/test/modules/unsafe_tests/sql/guc_privs.sql
index 7a4fb24b9d1..9bcbbfa9040 100644
--- a/src/test/modules/unsafe_tests/sql/guc_privs.sql
+++ b/src/test/modules/unsafe_tests/sql/guc_privs.sql
@@ -98,6 +98,19 @@ GRANT ALL ON PARAMETER none.such TO regress_host_resource_admin;
SELECT 1 FROM pg_parameter_acl WHERE parname = 'none.such';
REVOKE ALL ON PARAMETER "None.Such" FROM regress_host_resource_admin;
SELECT 1 FROM pg_parameter_acl WHERE parname = 'none.such';
+
+-- Superuser should be able to ALTER SYSTEM SET a non-existent custom GUC.
+ALTER SYSTEM SET none.such = 'whiz bang';
+-- None of the above should have created a placeholder GUC for none.such.
+SHOW none.such; -- error
+-- However, if we reload ...
+SELECT pg_reload_conf();
+-- and start a new session to avoid race condition ...
+\c -
+SET SESSION AUTHORIZATION regress_admin;
+-- then it should be there.
+SHOW none.such;
+
-- Can't grant on a non-existent core GUC.
GRANT ALL ON PARAMETER no_such_guc TO regress_host_resource_admin; -- fail
@@ -190,6 +203,7 @@ ALTER SYSTEM RESET lc_messages; -- fail, insufficient privileges
SELECT set_config ('temp_buffers', '8192', false); -- ok
ALTER SYSTEM RESET autovacuum_work_mem; -- ok, privileges have been granted
ALTER SYSTEM RESET ALL; -- fail, insufficient privileges
+ALTER SYSTEM SET none.such2 = 'whiz bang'; -- fail, not superuser
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