From b6ddcc5816a8a6a219c82c2b423e09a076ca5fdb Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Fri, 5 Jan 2024 14:50:53 +0100 Subject: [PATCH v12 12/14] 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 624518e0b01..4e06b6ea446 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -11272,6 +11272,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 cc887746b74..29799ef86a9 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -2283,6 +2283,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 29a03c6c288..263dc7ab7ae 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -47,9 +47,11 @@ #include "utils/acl.h" #include "utils/builtins.h" #include "utils/conffiles.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" @@ -6975,3 +6977,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 9cf005607d6..8028b8d1ecb 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -495,6 +495,8 @@ extern const struct config_enum_entry dynamic_shared_memory_options[]; * GUC option variables that are exported from this module */ bool AllowAlterSystem = true; +char *protocol_managed_params = ""; + bool log_duration = false; bool Debug_print_plan = false; bool Debug_print_parse = false; @@ -3949,6 +3951,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 61f6b1cf752..3fc92d4cced 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 d64dc5fcdb0..ebe048b5561 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 7267c0dd687..b0bc1efe4dc 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -364,6 +364,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "Load-Balance-Hosts", "", 8, /* sizeof("30001") = 8 */ offsetof(struct pg_conn, max_protocol_version)}, + {"_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 129de550953..ee9205494d7 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 498139a507d..3fad5f09ac7 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -410,6 +410,7 @@ struct pg_conn char *require_auth; /* name of the expected auth method */ char *load_balance_hosts; /* load balance over hosts */ char *max_protocol_version; /* maximum used protocol version */ + char *pq_protocol_managed_params; /* _pq_.protocol_managed_params */ bool cancelRequest; /* true if this connection is used to send a * cancel request, instead of being a normal -- 2.34.1