From 034b24310aedf63418175612e87e69ce7a5262c1 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Fri, 5 Jan 2024 14:50:53 +0100 Subject: [PATCH v4 6/7] 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. setting the GUC_PROTOCOL_EXTENSION flag to true for them. 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 | 47 ++++++++++++++++ doc/src/sgml/libpq.sgml | 10 ++++ src/backend/utils/misc/guc.c | 85 +++++++++++++++++++++++++++++ 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, 166 insertions(+) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index f323bba018f..8308cfdba2c 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -10977,6 +10977,53 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' + + Protocol Behaviour + + 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. + 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. + 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 72b66295f69..46f97bd69a3 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 5998e9d656a..07fa42ab073 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" @@ -6914,3 +6916,86 @@ 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) + { + if (!find_option(pname, false, true, ERROR)) + { + GUC_check_errdetail("Parameter \"%s\" is not recognized.", pname); + pfree(protocol_params_str); + list_free(namelist); + return false; + } + + 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; + } + } + + 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); + + config->flags &= ~GUC_PROTOCOL_EXTENSION; + } + + 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, false, ERROR); + + config->flags |= GUC_PROTOCOL_EXTENSION; + } + + 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 e53ebc6dc2b..e17170d5943 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; @@ -3854,6 +3856,16 @@ struct config_real ConfigureNamesReal[] = struct config_string ConfigureNamesString[] = { + { + {"_pq_.protocol_managed_params", PGC_USERSET, PROTOCOL_EXTENSION, + gettext_noop("List of additional parameters to be only managed at the protocol level."), + NULL, + GUC_PROTOCOL_EXTENSION | GUC_LIST_INPUT | GUC_LIST_QUOTE | 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 9890d09b3c8..aae7334ccd5 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -243,6 +243,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 75a0ee96785..6076c99f053 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -2297,6 +2297,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 d302a95ceaa..95ed3f0b0c7 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -408,6 +408,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