From 71c1134ca8ea9267be6b605fa140d7d97f54ac6d Mon Sep 17 00:00:00 2001 From: Michael Banck Date: Tue, 15 Dec 2020 20:53:59 +0100 Subject: [PATCH] Add new PGC_ADMINSET guc context and pg_change_role_settings default role. This commit introduces `administrator' as a middle ground between `superuser' and `user' for guc contexts. Those settings initially include all previously `superuser'-context settings from the 'Reporting and Logging' ('What/When to Log' but not 'Where to Log') and both 'Statistics' categories. Further settings could be added in follow-up commits. Also, a new default role pg_change_role_settings is introduced. This allows administrators (that are not superusers) that have been granted this role to change the above PGC_ADMINSET settings on a per-role/database basis like "ALTER ROLE [...] SET log_statement". The rationale is to allow certain settings that affect logging to be changed in setups where the DBA has access to the database log, but is not a full superuser. Catversion is bumped. --- src/backend/commands/dbcommands.c | 7 +++- src/backend/commands/user.c | 7 ++-- src/backend/utils/misc/guc.c | 56 +++++++++++++++++++------------ src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_authid.dat | 5 +++ src/include/utils/guc.h | 1 + 6 files changed, 53 insertions(+), 25 deletions(-) diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index f27c3fe8c1..03787eb5cf 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -1657,7 +1657,12 @@ AlterDatabaseSet(AlterDatabaseSetStmt *stmt) */ shdepLockAndCheckObject(DatabaseRelationId, datid); - if (!pg_database_ownercheck(datid, GetUserId())) + /* + * Only allow the database owner or a member of the + * pg_change_role_settings default role to change database settings. + */ + if (!pg_database_ownercheck(datid, GetUserId()) && + !is_member_of_role(GetUserId(), DEFAULT_ROLE_CHANGE_ROLE_SETTINGS)) aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_DATABASE, stmt->dbname); diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 0e6800bf3e..884f15d1c5 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -936,7 +936,8 @@ AlterRoleSet(AlterRoleSetStmt *stmt) /* * To mess with a superuser you gotta be superuser; else you need - * createrole, or just want to change your own settings + * createrole, the pg_change_role_settings default role, or just want + * to change your own settings. */ if (roleform->rolsuper) { @@ -947,7 +948,9 @@ AlterRoleSet(AlterRoleSetStmt *stmt) } else { - if (!have_createrole_privilege() && roleid != GetUserId()) + if (!have_createrole_privilege() && + roleid != GetUserId() && + !is_member_of_role(GetUserId(), DEFAULT_ROLE_CHANGE_ROLE_SETTINGS)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied"))); diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 878fcc2236..875c928c40 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -634,6 +634,7 @@ const char *const GucContext_Names[] = /* PGC_SU_BACKEND */ "superuser-backend", /* PGC_BACKEND */ "backend", /* PGC_SUSET */ "superuser", + /* PGC_ADMINSET */ "administrator", /* PGC_USERSET */ "user" }; @@ -1317,7 +1318,7 @@ static struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, { - {"log_replication_commands", PGC_SUSET, LOGGING_WHAT, + {"log_replication_commands", PGC_ADMINSET, LOGGING_WHAT, gettext_noop("Logs each replication command."), NULL }, @@ -1360,7 +1361,7 @@ static struct config_bool ConfigureNamesBool[] = }, { - {"log_duration", PGC_SUSET, LOGGING_WHAT, + {"log_duration", PGC_ADMINSET, LOGGING_WHAT, gettext_noop("Logs the duration of each completed SQL statement."), NULL }, @@ -1405,7 +1406,7 @@ static struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, { - {"log_parser_stats", PGC_SUSET, STATS_MONITORING, + {"log_parser_stats", PGC_ADMINSET, STATS_MONITORING, gettext_noop("Writes parser performance statistics to the server log."), NULL }, @@ -1414,7 +1415,7 @@ static struct config_bool ConfigureNamesBool[] = check_stage_log_stats, NULL, NULL }, { - {"log_planner_stats", PGC_SUSET, STATS_MONITORING, + {"log_planner_stats", PGC_ADMINSET, STATS_MONITORING, gettext_noop("Writes planner performance statistics to the server log."), NULL }, @@ -1423,7 +1424,7 @@ static struct config_bool ConfigureNamesBool[] = check_stage_log_stats, NULL, NULL }, { - {"log_executor_stats", PGC_SUSET, STATS_MONITORING, + {"log_executor_stats", PGC_ADMINSET, STATS_MONITORING, gettext_noop("Writes executor performance statistics to the server log."), NULL }, @@ -1432,7 +1433,7 @@ static struct config_bool ConfigureNamesBool[] = check_stage_log_stats, NULL, NULL }, { - {"log_statement_stats", PGC_SUSET, STATS_MONITORING, + {"log_statement_stats", PGC_ADMINSET, STATS_MONITORING, gettext_noop("Writes cumulative performance statistics to the server log."), NULL }, @@ -1454,7 +1455,7 @@ static struct config_bool ConfigureNamesBool[] = #endif { - {"track_activities", PGC_SUSET, STATS_COLLECTOR, + {"track_activities", PGC_ADMINSET, STATS_COLLECTOR, gettext_noop("Collects information about executing commands."), gettext_noop("Enables the collection of information on the currently " "executing command of each session, along with " @@ -1465,7 +1466,7 @@ static struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, { - {"track_counts", PGC_SUSET, STATS_COLLECTOR, + {"track_counts", PGC_ADMINSET, STATS_COLLECTOR, gettext_noop("Collects statistics on database activity."), NULL }, @@ -1474,7 +1475,7 @@ static struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, { - {"track_io_timing", PGC_SUSET, STATS_COLLECTOR, + {"track_io_timing", PGC_ADMINSET, STATS_COLLECTOR, gettext_noop("Collects timing statistics for database I/O activity."), NULL }, @@ -1562,7 +1563,7 @@ static struct config_bool ConfigureNamesBool[] = #endif { - {"log_lock_waits", PGC_SUSET, LOGGING_WHAT, + {"log_lock_waits", PGC_ADMINSET, LOGGING_WHAT, gettext_noop("Logs long lock waits."), NULL }, @@ -2823,7 +2824,7 @@ static struct config_int ConfigureNamesInt[] = }, { - {"log_min_duration_sample", PGC_SUSET, LOGGING_WHEN, + {"log_min_duration_sample", PGC_ADMINSET, LOGGING_WHEN, gettext_noop("Sets the minimum execution time above which " "a sample of statements will be logged." " Sampling is determined by log_statement_sample_rate."), @@ -2836,7 +2837,7 @@ static struct config_int ConfigureNamesInt[] = }, { - {"log_min_duration_statement", PGC_SUSET, LOGGING_WHEN, + {"log_min_duration_statement", PGC_ADMINSET, LOGGING_WHEN, gettext_noop("Sets the minimum execution time above which " "all statements will be logged."), gettext_noop("Zero prints all queries. -1 turns this feature off."), @@ -2860,7 +2861,7 @@ static struct config_int ConfigureNamesInt[] = }, { - {"log_parameter_max_length", PGC_SUSET, LOGGING_WHAT, + {"log_parameter_max_length", PGC_ADMINSET, LOGGING_WHAT, gettext_noop("When logging statements, limit logged parameter values to first N bytes."), gettext_noop("-1 to print values in full."), GUC_UNIT_BYTE @@ -3335,7 +3336,7 @@ static struct config_int ConfigureNamesInt[] = }, { - {"log_temp_files", PGC_SUSET, LOGGING_WHAT, + {"log_temp_files", PGC_ADMINSET, LOGGING_WHAT, gettext_noop("Log the use of temporary files larger than this number of kilobytes."), gettext_noop("Zero logs all files. The default is -1 (turning this feature off)."), GUC_UNIT_KB @@ -3648,7 +3649,7 @@ static struct config_real ConfigureNamesReal[] = }, { - {"log_statement_sample_rate", PGC_SUSET, LOGGING_WHEN, + {"log_statement_sample_rate", PGC_ADMINSET, LOGGING_WHEN, gettext_noop("Fraction of statements exceeding log_min_duration_sample to be logged."), gettext_noop("Use a value between 0.0 (never log) and 1.0 (always log).") }, @@ -3658,7 +3659,7 @@ static struct config_real ConfigureNamesReal[] = }, { - {"log_transaction_sample_rate", PGC_SUSET, LOGGING_WHEN, + {"log_transaction_sample_rate", PGC_ADMINSET, LOGGING_WHEN, gettext_noop("Set the fraction of transactions to log for new transactions."), gettext_noop("Logs all statements from a fraction of transactions. " "Use a value between 0.0 (never log) and 1.0 (log all " @@ -4523,7 +4524,7 @@ static struct config_enum ConfigureNamesEnum[] = }, { - {"log_error_verbosity", PGC_SUSET, LOGGING_WHAT, + {"log_error_verbosity", PGC_ADMINSET, LOGGING_WHAT, gettext_noop("Sets the verbosity of logged messages."), NULL }, @@ -4533,7 +4534,7 @@ static struct config_enum ConfigureNamesEnum[] = }, { - {"log_min_messages", PGC_SUSET, LOGGING_WHEN, + {"log_min_messages", PGC_ADMINSET, LOGGING_WHEN, gettext_noop("Sets the message levels that are logged."), gettext_noop("Each level includes all the levels that follow it. The later" " the level, the fewer messages are sent.") @@ -4544,7 +4545,7 @@ static struct config_enum ConfigureNamesEnum[] = }, { - {"log_min_error_statement", PGC_SUSET, LOGGING_WHEN, + {"log_min_error_statement", PGC_ADMINSET, LOGGING_WHEN, gettext_noop("Causes all statements generating error at or above this level to be logged."), gettext_noop("Each level includes all the levels that follow it. The later" " the level, the fewer messages are sent.") @@ -4555,7 +4556,7 @@ static struct config_enum ConfigureNamesEnum[] = }, { - {"log_statement", PGC_SUSET, LOGGING_WHAT, + {"log_statement", PGC_ADMINSET, LOGGING_WHAT, gettext_noop("Sets the type of statements logged."), NULL }, @@ -4636,7 +4637,7 @@ static struct config_enum ConfigureNamesEnum[] = }, { - {"track_functions", PGC_SUSET, STATS_COLLECTOR, + {"track_functions", PGC_ADMINSET, STATS_COLLECTOR, gettext_noop("Collects function-level statistics on database activity."), NULL }, @@ -7166,6 +7167,19 @@ set_config_option(const char *name, const char *value, return 0; } break; + case PGC_ADMINSET: + if ((context == PGC_USERSET && + !((source == PGC_S_DATABASE_USER || source == PGC_S_TEST) && + is_member_of_role(GetUserId(), DEFAULT_ROLE_CHANGE_ROLE_SETTINGS))) || + context == PGC_BACKEND) + { + ereport(elevel, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied to set parameter \"%s\"", + name))); + return 0; + } + break; case PGC_USERSET: /* always okay */ break; diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index db4814e57a..3cd7d7df1c 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202012293 +#define CATALOG_VERSION_NO 202012311 #endif diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat index 7c08851550..f5985fe0c0 100644 --- a/src/include/catalog/pg_authid.dat +++ b/src/include/catalog/pg_authid.dat @@ -60,5 +60,10 @@ rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', rolpassword => '_null_', rolvaliduntil => '_null_' }, +{ oid => '8801', oid_symbol => 'DEFAULT_ROLE_CHANGE_ROLE_SETTINGS', + rolname => 'pg_change_role_settings', rolsuper => 'f', rolinherit => 't', + rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', + rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', + rolpassword => '_null_', rolvaliduntil => '_null_' }, ] diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index 6a20a3bcec..f3619b0c25 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -73,6 +73,7 @@ typedef enum PGC_SU_BACKEND, PGC_BACKEND, PGC_SUSET, + PGC_ADMINSET, PGC_USERSET } GucContext; -- 2.20.1