From 2aed080f177bf68ed81aef2528a2e19ae72718b0 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Fri, 5 Jan 2024 14:50:53 +0100 Subject: [PATCH v6 08/10] Add _pq_.protocol_managed_params protocol extension MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new protocol extension that allows changing certain GUCs to be protocol extensions, i.e. changing their context to PGC_PROTOCOL or PGC_SU_PROTOCOL. One benefit this provides is the ability to sandbox SQL scripts by downgrading the connection to a low privileged user, without having the ability to change back by using SET ROLE: ``` psql "user=postgres _pq_.protocol_managed_params=role options='-c role=pg_read_all_data'" > SELECT CURRENT_USER; current_user ────────────────── pg_read_all_data (1 row) > SET ROLE postgres; ERROR: parameter can only be set at the protocol level "role" ``` --- doc/src/sgml/config.sgml | 49 ++++++++++++++ doc/src/sgml/libpq.sgml | 10 +++ src/backend/utils/misc/guc.c | 99 +++++++++++++++++++++++++++++ src/backend/utils/misc/guc_tables.c | 12 ++++ src/include/utils/guc.h | 2 + src/include/utils/guc_hooks.h | 2 + src/interfaces/libpq/fe-connect.c | 4 ++ src/interfaces/libpq/fe-protocol3.c | 3 + src/interfaces/libpq/libpq-int.h | 1 + 9 files changed, 182 insertions(+) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 61038472c5a..6add5c3966e 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -10978,6 +10978,55 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' + + Protocol extension parameters + + Some parameters control the behaviour of the protocol, these are called + protocol extension parameters. These types of parameters are different + from all other runtime parameters in a few important ways. First, they all + start with a _pq_. prefix. Secondly, they can only be + set at the protocol level using the StartupMessage and ParameterSet + messages. + It's not possible to set them in any other way (not through command line + arguments, configuration files, SET, etc). Finally, if you configure one + of them in the StartupMessage, but the server doesn't support the + parameter the connection attempt does not fail like with other options. + Instead the connection attempt continues as normal as these protocol + parameters are considered optional to implement by the server. To check if + the parameter was set you need to check the return value of + . If one of the + requested parameters was not supported by the server it will be listed + there. + + + + + + _pq_.protocol_managed_params (string) + + _pq_.protocol_managed_params configuration parameter + + + + + This parameter can be used to "upgrade" regular runtime parameters to a + protocol extension parameter. Thus disallowing it to be set in any + other way than through the StartupMessage and ParameterSet protocol + messages. + This can be useful to limit the power of an attacker with arbitrary SQL + execution. For example, if you set + _pq_.protocol_managed_params to + role then you can connect as a + highly privileged user to PostgreSQL but + change role to a user with fewer privileges. And then the attacker with + only SQL access is unable to change back the session authorization. + + + + + + + Customized Options diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 9963af5ace5..32abd220cef 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -2212,6 +2212,16 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + + + _pq_.protocol_managed_params + + + Specifies a value for the + configuration parameter. + + + diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 27f84a4d05f..b10f2295d13 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -46,9 +46,11 @@ #include "utils/builtins.h" #include "utils/conffiles.h" #include "utils/float.h" +#include "utils/guc_hooks.h" #include "utils/guc_tables.h" #include "utils/memutils.h" #include "utils/timestamp.h" +#include "utils/varlena.h" #define CONFIG_FILENAME "postgresql.conf" @@ -6951,3 +6953,100 @@ call_enum_check_hook(struct config_enum *conf, int *newval, void **extra, return true; } + +/* + * GUC check_hook for protocol_managed_params + */ +bool +check_protocol_managed_params(char **newval, void **extra, GucSource source) +{ + List *namelist; + char *protocol_params_str = pstrdup(*newval); + + if (!SplitIdentifierString(protocol_params_str, ',', &namelist)) + { + /* syntax error in name list */ + GUC_check_errdetail("List syntax is invalid."); + pfree(protocol_params_str); + list_free(namelist); + return false; + } + + foreach_ptr(char, pname, namelist) + { + /* + * We explicitly allow unknown parameters here (but we still warn for + * them). So that it is possible to add version specific parameters to + * the protocol_managed_parameters list in the StartupMessage without + * knowing the current server version yet. + */ + struct config_generic *config = find_option(pname, false, false, WARNING); + + if (strncmp(pname, "_pq_.", 5) == 0) + { + GUC_check_errdetail("Parameter \"%s\" is a protocol extension.", pname); + pfree(protocol_params_str); + list_free(namelist); + return false; + } + + if (!config) + continue; + + if (config->context != PGC_PROTOCOL && config->context != PGC_USERSET && config->context != PGC_SUSET) + { + GUC_check_errdetail("Parameter \"%s\" is not a user-settable parameter.", pname); + pfree(protocol_params_str); + list_free(namelist); + return false; + } + } + + + pfree(protocol_params_str); + list_free(namelist); + return true; +} + +/* + * GUC check_hook for protocol_managed_params + */ +void +assign_protocol_managed_params(const char *newval, void *extra) +{ + List *namelist; + char *old_protocol_params_str = pstrdup(protocol_managed_params); + char *protocol_params_str = pstrdup(newval); + + if (!SplitIdentifierString(old_protocol_params_str, ',', &namelist)) + { + elog(ERROR, "List syntax is invalid and check hook should have checked."); + } + + foreach_ptr(char, pname, namelist) + { + struct config_generic *config = find_option(pname, false, false, ERROR); + + if (config) + config->context = config->context == PGC_PROTOCOL ? PGC_USERSET : PGC_SUSET; + } + + list_free(namelist); + + if (!SplitIdentifierString(protocol_params_str, ',', &namelist)) + { + elog(ERROR, "List syntax is invalid and check hook should have checked."); + } + + foreach_ptr(char, pname, namelist) + { + struct config_generic *config = find_option(pname, false, true, ERROR); + + if (config) + config->context = config->context == PGC_USERSET ? PGC_PROTOCOL : PGC_SU_PROTOCOL; + } + + pfree(old_protocol_params_str); + pfree(protocol_params_str); + list_free(namelist); +} diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 33cbce2edd3..a4b3ed0c909 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -492,6 +492,8 @@ extern const struct config_enum_entry dynamic_shared_memory_options[]; /* * GUC option variables that are exported from this module */ +char *protocol_managed_params = ""; + bool log_duration = false; bool Debug_print_plan = false; bool Debug_print_parse = false; @@ -3856,6 +3858,16 @@ struct config_real ConfigureNamesReal[] = struct config_string ConfigureNamesString[] = { + { + {"_pq_.protocol_managed_params", PGC_PROTOCOL, PROTOCOL_EXTENSION, + gettext_noop("List of additional parameters to be only managed at the protocol level."), + NULL, + GUC_LIST_INPUT | GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE + }, + &protocol_managed_params, + "", + check_protocol_managed_params, assign_protocol_managed_params, NULL + }, { {"archive_command", PGC_SIGHUP, WAL_ARCHIVING, gettext_noop("Sets the shell command that will be called to archive a WAL file."), diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index a39b4426ac9..45479550d10 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -242,6 +242,8 @@ typedef enum /* GUC vars that are actually defined in guc_tables.c, rather than elsewhere */ +extern PGDLLIMPORT char *protocol_managed_params; + extern PGDLLIMPORT bool Debug_print_plan; extern PGDLLIMPORT bool Debug_print_parse; extern PGDLLIMPORT bool Debug_print_rewritten; diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h index 5300c44f3b0..04938b4d777 100644 --- a/src/include/utils/guc_hooks.h +++ b/src/include/utils/guc_hooks.h @@ -25,6 +25,8 @@ * Please keep the declarations in order by GUC variable name. */ +extern bool check_protocol_managed_params(char **newval, void **extra, GucSource source); +extern void assign_protocol_managed_params(const char *newval, void *extra); extern bool check_application_name(char **newval, void **extra, GucSource source); extern void assign_application_name(const char *newval, void *extra); diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 5f45bee567d..e28121a34a7 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -359,6 +359,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "Load-Balance-Hosts", "", 8, /* sizeof("disable") = 8 */ offsetof(struct pg_conn, load_balance_hosts)}, + {"_pq_.protocol_managed_params", NULL, NULL, NULL, + "Pq-Protocol-Managed-Params", "", 40, + offsetof(struct pg_conn, pq_protocol_managed_params)}, + /* Terminating entry --- MUST BE LAST */ {NULL, NULL, NULL, NULL, NULL, NULL, 0} diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index d8c7b541b97..13e88be0d8f 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -2319,6 +2319,9 @@ build_startup_packet(const PGconn *conn, char *packet, } } + if (conn->pq_protocol_managed_params && conn->pq_protocol_managed_params[0]) + ADD_STARTUP_OPTION("_pq_.protocol_managed_params", conn->pq_protocol_managed_params); + /* Add trailing terminator */ if (packet) packet[packet_len] = '\0'; diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 8f152c51252..58c34fe2c21 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -409,6 +409,7 @@ struct pg_conn char *target_session_attrs; /* desired session properties */ char *require_auth; /* name of the expected auth method */ char *load_balance_hosts; /* load balance over hosts */ + char *pq_protocol_managed_params; /* _pq_.protocol_managed_params */ /* Optional file to write trace info to */ FILE *Pfdebug; -- 2.34.1