public inbox for [email protected]help / color / mirror / Atom feed
Re: Custom oauth validator options 7+ messages / 2 participants [nested] [flat]
* Re: Custom oauth validator options @ 2026-01-05 18:53 Jacob Champion <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Jacob Champion @ 2026-01-05 18:53 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Thu, Dec 18, 2025 at 12:29 PM Zsolt Parragi <[email protected]> wrote: > > > I think I need to do more staring at the intersection of GUC > > registration and session_preload_libraries, because my memory of the > > order of operations was faulty. I won't be able to do that before the > > holidays, most likely. > > Maybe I'm missing something, but why do we need > session_preload_libraries? Well, how do you want "global" GUCs registered by the validator to behave when OAuth isn't used for the connection? > The question is if non-validator libraries should be able to define > PGC_HBA variables. I think we should try for that, yeah. Otherwise I suspect considerable pushback on the idea of modifying the GucContext enum for something that can only be used by OAuth. > * require session_preload_libraries. We proceed with authentication > even with unresolved HBA variables, but abort the connection if there > are still unknown parameters after loading session preload. Of those choices, this _seems_ nicest. It'd be good to get a feel for how it behaves in practice though. Thanks, --Jacob ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Custom oauth validator options @ 2026-01-14 18:20 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Zsolt Parragi @ 2026-01-14 18:20 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > Well, how do you want "global" GUCs registered by the validator to > behave when OAuth isn't used for the connection? My assumption was that with session_preload we only validate the line used for the current login, not all the lines. This way we don't have to always include all validator/hba plugins in session_preload, for every login. This is what I implemented for now, but if you think it would be better to validate every line, I can adjust that. > Of those choices, this _seems_ nicest. It'd be good to get a feel for > how it behaves in practice though. See the attached v2, with the above comment. Other than the above question (validate everything vs the current line), I'm also not entirely sure if we do need PGC_HBA. It could also work with PGC_SIGHUP + the new PGC_S_HBA value in GucSources. Attachments: [application/octet-stream] v2-0001-Introduce-PGC_HBA-GUC-variables-settable-in-pg_hba.c.patch (42.2K, 2-v2-0001-Introduce-PGC_HBA-GUC-variables-settable-in-pg_hba.c.patch) download | inline diff: From f2d7410b2a70389a400e03dd2039f9778f02fda6 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi <[email protected]> Date: Wed, 7 Jan 2026 19:25:11 +0000 Subject: [PATCH] Introduce PGC_HBA: GUC variables settable in pg_hba.conf Add a new GUC context level PGC_HBA that allows custom variables to be set based on which pg_hba.conf line matches during authentication. This enables authentication plugins and extensions to receive configuration parameters that vary based on HBA matching rules (client address, database, user, authentication method, etc.). Motivation: OAuth support introduced several OAuth-specific configuration parameters to pg_hba.conf, and added support for third-party validator plugins. These plugins often require additional configuration, which is only possible with GUC variables. As these plugins execute before authentication completes, they can only depend on GUC variables defined in postgresql.conf or postgresql.auto.conf. This limitation creates a configuration problem: pg_hba.conf allows administrators to define several different OAuth configurations for a single PostgreSQL instance, but validator plugin-specific variables can only be configured once per server. With the changes in this commit, validator plugins can now define their GUC variables with PGC_HBA context, allowing different settings per HBA line. Another use case is that extensions loaded via shared_preload_libraries or session_preload_libraries can define these variables, allowing administrators to configure authentication/authorization-related settings per HBA line, for example to help with row level security policies. User interface: The new parameters reuse the existing syntax of pg_hba.conf; GUC variables can be defined the same way as existing hardcoded parameters: host all all 192.168.1.0/24 oauth myext.setting=value1 host all all 0.0.0.0/0 oauth myext.setting=value2 This also provides an easy migration path in the future to convert existing pg_hba parameters to GUC variables. Extension interface: We define a new GUC context, PGC_HBA, and a new source, PGC_S_HBA. PGC_HBA variables must be defined during shared_preload_libraries or session_preload_libraries. This ensures all custom variables are registered before connections are accepted. PGC_HBA variables can be set from: - postgresql.conf - postgresql.auto.conf (ALTER SYSTEM) - pg_hba.conf (new) PGC_HBA variables cannot be set via: - ALTER USER SET / ALTER DATABASE SET - SET command - Connection parameters (PGOPTIONS) Only PGC_HBA variables can be set with the new PGC_S_HBA source; existing GUC variable definitions do not need to handle the new source type. Error handling change: This feature requires a behavior change in pg_hba.conf error handling which may affect users not using the new feature. Since we allow both shared_preload_libraries and session_preload_libraries to define PGC_HBA variables (enabling updates of potentially security-related libraries without a server restart), we can no longer reject unknown parameters in pg_hba.conf at postmaster startup. Instead, unknown parameters are treated as placeholders (similar to custom GUC variables) and error handling is delayed until after session_preload_libraries completes. If any placeholder HBA variables remain undefined at that point, the connection is aborted with a FATAL error, even if authentication previously succeeded. --- src/backend/libpq/hba.c | 59 +++++- src/backend/utils/init/miscinit.c | 7 + src/backend/utils/init/postinit.c | 8 + src/backend/utils/misc/guc.c | 82 ++++++++ src/backend/utils/misc/guc_tables.c | 2 + src/include/libpq/hba.h | 7 + src/include/miscadmin.h | 2 + src/include/utils/guc.h | 3 + src/test/modules/meson.build | 2 + src/test/modules/test_hba_guc/Makefile | 21 ++ src/test/modules/test_hba_guc/meson.build | 35 ++++ .../test_hba_guc/t/001_hba_guc_variables.pl | 56 ++++++ .../test_hba_guc/t/002_hba_guc_sources.pl | 187 ++++++++++++++++++ .../test_hba_guc/t/003_hba_guc_precedence.pl | 105 ++++++++++ .../test_hba_guc/test_hba_guc--1.0.sql | 22 +++ src/test/modules/test_hba_guc/test_hba_guc.c | 93 +++++++++ .../modules/test_hba_guc/test_hba_guc.conf | 3 + .../modules/test_hba_guc/test_hba_guc.control | 5 + .../modules/test_hba_guc_contexts/Makefile | 18 ++ .../modules/test_hba_guc_contexts/meson.build | 28 +++ .../t/001_context_validation.pl | 55 ++++++ .../test_hba_guc_contexts.c | 79 ++++++++ 22 files changed, 870 insertions(+), 9 deletions(-) create mode 100644 src/test/modules/test_hba_guc/Makefile create mode 100644 src/test/modules/test_hba_guc/meson.build create mode 100644 src/test/modules/test_hba_guc/t/001_hba_guc_variables.pl create mode 100644 src/test/modules/test_hba_guc/t/002_hba_guc_sources.pl create mode 100644 src/test/modules/test_hba_guc/t/003_hba_guc_precedence.pl create mode 100644 src/test/modules/test_hba_guc/test_hba_guc--1.0.sql create mode 100644 src/test/modules/test_hba_guc/test_hba_guc.c create mode 100644 src/test/modules/test_hba_guc/test_hba_guc.conf create mode 100644 src/test/modules/test_hba_guc/test_hba_guc.control create mode 100644 src/test/modules/test_hba_guc_contexts/Makefile create mode 100644 src/test/modules/test_hba_guc_contexts/meson.build create mode 100644 src/test/modules/test_hba_guc_contexts/t/001_context_validation.pl create mode 100644 src/test/modules/test_hba_guc_contexts/test_hba_guc_contexts.c diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 87ee541e880..6fc4debf567 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -2507,15 +2507,21 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, } else { - ereport(elevel, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("unrecognized authentication option name: \"%s\"", - name), - errcontext("line %d of configuration file \"%s\"", - line_num, file_name))); - *err_msg = psprintf("unrecognized authentication option name: \"%s\"", - name); - return false; + /* + * Unrecognized option name - treat it as a potential GUC variable. + * Store the name=value pair in the HbaLine's guc_options list. + * Actual validation (checking if the GUC is defined) happens at + * connection time after session_preload_libraries completes. + */ + HbaOption *opt; + + opt = palloc(sizeof(HbaOption)); + opt->name = pstrdup(name); + opt->value = pstrdup(val); + hbaline->guc_options = lappend(hbaline->guc_options, opt); + + elog(DEBUG2, "pg_hba.conf line %d: storing GUC option %s = %s", + line_num, name, val); } return true; } @@ -3113,11 +3119,44 @@ load_ident(void) +/* + * apply_hba_guc_options + * Apply GUC variable settings from the matched HBA line. + * + * This function processes the guc_options list from the matched pg_hba.conf + * line and applies each GUC setting. Variables will either be set (if already + * defined) or create placeholders (if not yet defined). Validation of + * undefined variables and context checking happens later, after + * session_preload_libraries completes in postinit.c. + */ +static void +apply_hba_guc_options(Port *port) +{ + ListCell *lc; + + if (port->hba->guc_options == NIL) + return; + + foreach(lc, port->hba->guc_options) + { + HbaOption *opt = (HbaOption *) lfirst(lc); + + elog(DEBUG2, "Applying HBA GUC option: %s = %s", opt->name, opt->value); + + (void) set_config_option(opt->name, opt->value, + PGC_HBA, PGC_S_HBA, + GUC_ACTION_SET, true, ERROR, false); + } +} + /* * Determine what authentication method should be used when accessing database * "database" from frontend "raddr", user "user". Return the method and * an optional argument (stored in fields of *port), and STATUS_OK. * + * Also applies all GUC variables from the matched HBA line, as these variables + * might immediately required by authentication plugins. + * * If the file does not contain any entry matching the request, we return * method = uaImplicitReject. */ @@ -3125,6 +3164,8 @@ void hba_getauthmethod(Port *port) { check_hba(port); + + apply_hba_guc_options(port); } diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index 563f20374ff..964a77d02aa 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -1786,6 +1786,10 @@ char *local_preload_libraries_string = NULL; bool process_shared_preload_libraries_in_progress = false; bool process_shared_preload_libraries_done = false; +/* Flag telling that we are loading session_preload_libraries */ +bool process_session_preload_libraries_in_progress = false; +bool process_session_preload_libraries_done = false; + shmem_request_hook_type shmem_request_hook = NULL; bool process_shmem_requests_in_progress = false; @@ -1864,9 +1868,12 @@ process_shared_preload_libraries(void) void process_session_preload_libraries(void) { + process_session_preload_libraries_in_progress = true; load_libraries(session_preload_libraries_string, "session_preload_libraries", false); + process_session_preload_libraries_in_progress = false; + process_session_preload_libraries_done = true; load_libraries(local_preload_libraries_string, "local_preload_libraries", true); diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 52c05a9d1d5..d3bb88243c6 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -33,6 +33,7 @@ #include "catalog/pg_database.h" #include "catalog/pg_db_role_setting.h" #include "catalog/pg_tablespace.h" +#include "lib/stringinfo.h" #include "libpq/auth.h" #include "libpq/libpq-be.h" #include "mb/pg_wchar.h" @@ -1225,6 +1226,13 @@ InitPostgres(const char *in_dbname, Oid dboid, if ((flags & INIT_PG_LOAD_SESSION_LIBS) != 0) process_session_preload_libraries(); + /* + * Now that session_preload_libraries has completed, validate that all + * GUC variables set from pg_hba.conf are properly defined with the + * correct context. + */ + check_hba_guc_variables(); + /* fill in the remainder of this entry in the PgBackendStatus array */ if (!bootstrap) pgstat_bestart_final(); diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index ae9d5f3fb70..4afa412e385 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -3394,6 +3394,17 @@ set_config_with_handle(const char *name, config_handle *handle, * signals to individual backends only. */ break; + case PGC_HBA: + if (context != PGC_SIGHUP && context != PGC_POSTMASTER && + source != PGC_S_HBA) + { + ereport(elevel, + (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), + errmsg("parameter \"%s\" cannot be changed now", + record->name))); + return 0; + } + break; case PGC_SU_BACKEND: if (context == PGC_BACKEND) { @@ -4155,6 +4166,55 @@ get_config_handle(const char *name) return NULL; } +/* + * check_hba_guc_variables + * + * Check if any GUC variables set from pg_hba.conf (source = PGC_S_HBA) + * are still placeholders (undefined) or have the wrong context. + * + * For each invalid variable, we emit a FATAL error with an appropriate + * message explaining the problem. + * + * This should be called after session_preload_libraries completes to + * ensure extensions have had a chance to define their PGC_HBA variables. + */ +void +check_hba_guc_variables(void) +{ + HASH_SEQ_STATUS status; + GUCHashEntry *hentry; + + hash_seq_init(&status, guc_hashtab); + while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL) + { + struct config_generic *gconf = hentry->gucvar; + + if (gconf->source != PGC_S_HBA) + continue; + + if (gconf->flags & GUC_CUSTOM_PLACEHOLDER) + { + ereport(FATAL, + (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), + errmsg("authentication configuration error"), + errdetail("pg_hba.conf references undefined GUC variable \"%s\"", + gconf->name), + errhint("Ensure the extension defining this variable is loaded in session_preload_libraries or shared_preload_libraries."))); + } + + if (gconf->context != PGC_HBA) + { + ereport(FATAL, + (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), + errmsg("parameter \"%s\" cannot be set in pg_hba.conf", + gconf->name), + errdetail("Only variables with context PGC_HBA can be set from pg_hba.conf."), + errhint("This variable has context \"%s\".", + GucContext_Names[gconf->context]))); + } + } +} + /* * Set the fields for source file and line number the setting came from. @@ -4756,6 +4816,19 @@ init_custom_variable(const char *name, !process_shared_preload_libraries_in_progress) elog(FATAL, "cannot create PGC_POSTMASTER variables after startup"); + /* + * Only allow custom PGC_HBA variables to be created before + * session_preload_libraries completes. After that point, authentication + * has already occurred and check_hba_guc_variables has validated all + * PGC_HBA variables, so defining new ones would bypass validation. + */ + if (context == PGC_HBA && process_session_preload_libraries_done) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("PGC_HBA variables must be defined before session_preload_libraries completes"), + errdetail("Attempted to define \"%s\" after session preload", name), + errhint("Move the extension defining this variable to shared_preload_libraries or session_preload_libraries"))); + /* * We can't support custom GUC_LIST_QUOTE variables, because the wrong * things would happen if such a variable were set or pg_dump'd when the @@ -6603,6 +6676,15 @@ validate_option_array_item(const char *name, const char *value, (superuser() || pg_parameter_aclcheck(name, GetUserId(), ACL_SET) == ACLCHECK_OK)) /* ok */ ; + else if (gconf->context == PGC_HBA) + { + if (skipIfNoPermissions) + return false; + ereport(ERROR, + (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), + errmsg("parameter \"%s\" cannot be set by ALTER USER or ALTER DATABASE", name), + errhint("Use postgresql.conf, ALTER SYSTEM, or pg_hba.conf to set this parameter."))); + } else if (skipIfNoPermissions) return false; /* if a permissions error should be thrown, let set_config_option do it */ diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 73ff6ad0a32..b4302d07467 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -658,6 +658,7 @@ const char *const GucContext_Names[] = [PGC_INTERNAL] = "internal", [PGC_POSTMASTER] = "postmaster", [PGC_SIGHUP] = "sighup", + [PGC_HBA] = "hba", [PGC_SU_BACKEND] = "superuser-backend", [PGC_BACKEND] = "backend", [PGC_SUSET] = "superuser", @@ -684,6 +685,7 @@ const char *const GucSource_Names[] = [PGC_S_USER] = "user", [PGC_S_DATABASE_USER] = "database user", [PGC_S_CLIENT] = "client", + [PGC_S_HBA] = "pg_hba.conf", [PGC_S_OVERRIDE] = "override", [PGC_S_INTERACTIVE] = "interactive", [PGC_S_TEST] = "test", diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index 7b93ba4a709..e1848ad03d0 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -92,6 +92,12 @@ typedef struct AuthToken regex_t *regex; } AuthToken; +typedef struct HbaOption +{ + char *name; + char *value; +} HbaOption; + typedef struct HbaLine { char *sourcefile; @@ -140,6 +146,7 @@ typedef struct HbaLine char *oauth_scope; char *oauth_validator; bool oauth_skip_usermap; + List *guc_options; } HbaLine; typedef struct IdentLine diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index db559b39c4d..0a5df62f99a 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -512,6 +512,8 @@ extern void BaseInit(void); extern PGDLLIMPORT bool IgnoreSystemIndexes; extern PGDLLIMPORT bool process_shared_preload_libraries_in_progress; extern PGDLLIMPORT bool process_shared_preload_libraries_done; +extern PGDLLIMPORT bool process_session_preload_libraries_in_progress; +extern PGDLLIMPORT bool process_session_preload_libraries_done; extern PGDLLIMPORT bool process_shmem_requests_in_progress; extern PGDLLIMPORT char *session_preload_libraries_string; extern PGDLLIMPORT char *shared_preload_libraries_string; diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index bf39878c43e..461606ff62b 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -73,6 +73,7 @@ typedef enum PGC_INTERNAL, PGC_POSTMASTER, PGC_SIGHUP, + PGC_HBA, PGC_SU_BACKEND, PGC_BACKEND, PGC_SUSET, @@ -120,6 +121,7 @@ typedef enum PGC_S_USER, /* per-user setting */ PGC_S_DATABASE_USER, /* per-user-and-database setting */ PGC_S_CLIENT, /* from client connection request */ + PGC_S_HBA, /* from pg_hba.conf */ PGC_S_OVERRIDE, /* special case to forcibly set default */ PGC_S_INTERACTIVE, /* dividing line for error reporting */ PGC_S_TEST, /* test per-database or per-user setting */ @@ -456,6 +458,7 @@ extern int set_config_with_handle(const char *name, config_handle *handle, GucAction action, bool changeVal, int elevel, bool is_reload); extern config_handle *get_config_handle(const char *name); +extern void check_hba_guc_variables(void); extern void AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt); extern char *GetConfigOptionByName(const char *name, const char **varname, bool missing_ok); diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build index 1b31c5b98d6..210674b2774 100644 --- a/src/test/modules/meson.build +++ b/src/test/modules/meson.build @@ -29,6 +29,8 @@ subdir('test_dsm_registry') subdir('test_escape') subdir('test_extensions') subdir('test_ginpostinglist') +subdir('test_hba_guc') +subdir('test_hba_guc_contexts') subdir('test_int128') subdir('test_integerset') subdir('test_json_parser') diff --git a/src/test/modules/test_hba_guc/Makefile b/src/test/modules/test_hba_guc/Makefile new file mode 100644 index 00000000000..2b89088e338 --- /dev/null +++ b/src/test/modules/test_hba_guc/Makefile @@ -0,0 +1,21 @@ +# src/test/modules/test_hba_guc/Makefile + +MODULE_big = test_hba_guc +OBJS = \ + $(WIN32RES) \ + test_hba_guc.o +PGFILEDESC = "test_hba_guc - test module for PGC_HBA GUC variables" + +EXTENSION = test_hba_guc +DATA = test_hba_guc--1.0.sql + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = src/test/modules/test_hba_guc +top_builddir = ../../../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/src/test/modules/test_hba_guc/meson.build b/src/test/modules/test_hba_guc/meson.build new file mode 100644 index 00000000000..d8bf0233056 --- /dev/null +++ b/src/test/modules/test_hba_guc/meson.build @@ -0,0 +1,35 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +test_hba_guc_sources = files( + 'test_hba_guc.c', +) + +if host_system == 'windows' + test_hba_guc_sources += rc_lib_gen.process(win32ver_rc, extra_args: [ + '--NAME', 'test_hba_guc', + '--FILEDESC', 'test_hba_guc - test module for PGC_HBA GUC variables',]) +endif + +test_hba_guc = shared_module('test_hba_guc', + test_hba_guc_sources, + kwargs: pg_test_mod_args, +) +test_install_libs += test_hba_guc + +test_install_data += files( + 'test_hba_guc.control', + 'test_hba_guc--1.0.sql', +) + +tests += { + 'name': 'test_hba_guc', + 'sd': meson.current_source_dir(), + 'bd': meson.current_build_dir(), + 'tap': { + 'tests': [ + 't/001_hba_guc_variables.pl', + 't/002_hba_guc_sources.pl', + 't/003_hba_guc_precedence.pl', + ], + }, +} diff --git a/src/test/modules/test_hba_guc/t/001_hba_guc_variables.pl b/src/test/modules/test_hba_guc/t/001_hba_guc_variables.pl new file mode 100644 index 00000000000..625f3be1e51 --- /dev/null +++ b/src/test/modules/test_hba_guc/t/001_hba_guc_variables.pl @@ -0,0 +1,56 @@ +# Test PGC_HBA GUC variables in pg_hba.conf + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('main'); +$node->init; + +$node->append_conf('postgresql.conf', + "session_preload_libraries = 'test_hba_guc'"); + +my $hba_conf = $node->data_dir . '/pg_hba.conf'; +open my $hba_fh, '>', $hba_conf or die "Could not open $hba_conf: $!"; +print $hba_fh "# Test HBA configuration with GUC variables\n"; +print $hba_fh "local all all trust test_hba_guc.string_var=from_hba test_hba_guc.int_var=999\n"; +close $hba_fh; + +$node->start; +$node->safe_psql('postgres', 'CREATE EXTENSION test_hba_guc;'); +my $result = $node->safe_psql('postgres', + "SELECT current_setting('test_hba_guc.string_var'), current_setting('test_hba_guc.int_var');" +); +is($result, 'from_hba|999', + 'GUC variables set from pg_hba.conf are accessible'); + +$result = $node->safe_psql('postgres', + "SELECT source FROM pg_settings WHERE name = 'test_hba_guc.string_var';" +); +is($result, 'pg_hba.conf', + 'GUC variable source shows pg_hba.conf'); + +my $node_no_ext = PostgreSQL::Test::Cluster->new('no_extension'); +$node_no_ext->init; + +my $hba_conf_no_ext = $node_no_ext->data_dir . '/pg_hba.conf'; +open my $hba_no_ext_fh, '>', $hba_conf_no_ext or die "Could not open $hba_conf_no_ext: $!"; +print $hba_no_ext_fh "# Test HBA configuration with undefined GUC variables\n"; +print $hba_no_ext_fh "local all all trust test_hba_guc.undefined_var=value\n"; +close $hba_no_ext_fh; + +$node_no_ext->start; + +my ($ret, $stdout, $stderr) = $node_no_ext->psql('postgres', 'SELECT 1;'); +isnt($ret, 0, 'Connection rejected when HBA GUC variable is undefined'); +like($stderr, qr/authentication configuration error/, + 'Error message indicates authentication configuration problem'); +like($stderr, qr/undefined GUC variable/, + 'Error message mentions undefined GUC variable'); + +$node_no_ext->stop; +$node->stop; + +done_testing(); diff --git a/src/test/modules/test_hba_guc/t/002_hba_guc_sources.pl b/src/test/modules/test_hba_guc/t/002_hba_guc_sources.pl new file mode 100644 index 00000000000..e762b8a96d8 --- /dev/null +++ b/src/test/modules/test_hba_guc/t/002_hba_guc_sources.pl @@ -0,0 +1,187 @@ +# Test that PGC_HBA variables can only be set from appropriate sources +# +# PGC_HBA variables should be settable from: +# - postgresql.conf +# - postgresql.auto.conf (ALTER SYSTEM) +# - pg_hba.conf +# +# But NOT from: +# - ALTER USER SET +# - ALTER DATABASE SET +# - Connection parameters (PGOPTIONS) + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# Test 1: PGC_HBA variable CAN be set in postgresql.conf +{ + my $node = PostgreSQL::Test::Cluster->new('conf_allowed'); + $node->init; + + $node->append_conf('postgresql.conf', + "session_preload_libraries = 'test_hba_guc'"); + + $node->append_conf('postgresql.conf', + "test_hba_guc.string_var = 'from_postgresql_conf'"); + + my $hba_conf = $node->data_dir . '/pg_hba.conf'; + open my $hba_fh, '>', $hba_conf or die "Could not open $hba_conf: $!"; + print $hba_fh "local all all trust\n"; + close $hba_fh; + + $node->start; + + my $result = $node->safe_psql('postgres', + "SELECT current_setting('test_hba_guc.string_var');"); + is($result, 'from_postgresql_conf', + 'PGC_HBA variable can be set in postgresql.conf'); + + $result = $node->safe_psql('postgres', + "SELECT source FROM pg_settings WHERE name = 'test_hba_guc.string_var';"); + is($result, 'configuration file', + 'Source shows configuration file for postgresql.conf setting'); + + $node->stop; +} + +# Test 2: PGC_HBA variable CAN be set via ALTER SYSTEM (postgresql.auto.conf) +{ + my $node = PostgreSQL::Test::Cluster->new('alter_system_allowed'); + $node->init; + + $node->append_conf('postgresql.conf', + "session_preload_libraries = 'test_hba_guc'"); + + my $hba_conf = $node->data_dir . '/pg_hba.conf'; + open my $hba_fh, '>', $hba_conf or die "Could not open $hba_conf: $!"; + print $hba_fh "local all all trust\n"; + close $hba_fh; + + $node->start; + + $node->safe_psql('postgres', + "ALTER SYSTEM SET test_hba_guc.string_var = 'from_alter_system';"); + + $node->reload; + + my $result = $node->safe_psql('postgres', + "SELECT current_setting('test_hba_guc.string_var');"); + is($result, 'from_alter_system', + 'PGC_HBA variable can be set via ALTER SYSTEM'); + + $result = $node->safe_psql('postgres', + "SELECT source FROM pg_settings WHERE name = 'test_hba_guc.string_var';"); + is($result, 'configuration file', + 'Source shows configuration file for ALTER SYSTEM setting'); + + $node->stop; +} + +# Test 3: PGC_HBA variable CANNOT be set via ALTER USER SET +{ + my $node = PostgreSQL::Test::Cluster->new('alter_user_rejected'); + $node->init; + + $node->append_conf('postgresql.conf', + "session_preload_libraries = 'test_hba_guc'"); + + my $hba_conf = $node->data_dir . '/pg_hba.conf'; + open my $hba_fh, '>', $hba_conf or die "Could not open $hba_conf: $!"; + print $hba_fh "local all all trust\n"; + close $hba_fh; + + $node->start; + + $node->safe_psql('postgres', "CREATE USER testuser;"); + + my ($ret, $stdout, $stderr) = $node->psql('postgres', + "ALTER USER testuser SET test_hba_guc.string_var = 'from_alter_user';"); + isnt($ret, 0, 'ALTER USER SET rejected for PGC_HBA variable'); + like($stderr, qr/cannot be set by ALTER USER or ALTER DATABASE/, + 'Error message indicates ALTER USER is not allowed for PGC_HBA'); + + $node->stop; +} + +# Test 4: PGC_HBA variable CANNOT be set via ALTER DATABASE SET +{ + my $node = PostgreSQL::Test::Cluster->new('alter_database_rejected'); + $node->init; + + $node->append_conf('postgresql.conf', + "session_preload_libraries = 'test_hba_guc'"); + + my $hba_conf = $node->data_dir . '/pg_hba.conf'; + open my $hba_fh, '>', $hba_conf or die "Could not open $hba_conf: $!"; + print $hba_fh "local all all trust\n"; + close $hba_fh; + + $node->start; + + my ($ret, $stdout, $stderr) = $node->psql('postgres', + "ALTER DATABASE postgres SET test_hba_guc.string_var = 'from_alter_database';"); + isnt($ret, 0, 'ALTER DATABASE SET rejected for PGC_HBA variable'); + like($stderr, qr/cannot be set by ALTER USER or ALTER DATABASE/, + 'Error message indicates ALTER DATABASE is not allowed for PGC_HBA'); + + $node->stop; +} + +# Test 5: PGC_HBA variable CANNOT be set via connection parameter (PGOPTIONS) +{ + my $node = PostgreSQL::Test::Cluster->new('pgoptions_rejected'); + $node->init; + + $node->append_conf('postgresql.conf', + "session_preload_libraries = 'test_hba_guc'"); + + my $hba_conf = $node->data_dir . '/pg_hba.conf'; + open my $hba_fh, '>', $hba_conf or die "Could not open $hba_conf: $!"; + print $hba_fh "local all all trust\n"; + close $hba_fh; + + $node->start; + + # Connection succeeds but parameter is not set (gets default value) + local $ENV{PGOPTIONS} = '-c test_hba_guc.string_var=from_pgoptions'; + my $result = $node->safe_psql('postgres', + "SELECT current_setting('test_hba_guc.string_var');"); + is($result, 'default_value', + 'PGC_HBA variable from PGOPTIONS is not set, uses default value'); + + my $logfile = $node->logfile; + my $log_content = slurp_file($logfile); + like($log_content, qr/parameter "test_hba_guc\.string_var" cannot be changed now/, + 'Warning logged when trying to set PGC_HBA via PGOPTIONS'); + + $node->stop; +} + +# Test 6: PGC_HBA variable CANNOT be set via SET command +{ + my $node = PostgreSQL::Test::Cluster->new('set_rejected'); + $node->init; + + $node->append_conf('postgresql.conf', + "session_preload_libraries = 'test_hba_guc'"); + + my $hba_conf = $node->data_dir . '/pg_hba.conf'; + open my $hba_fh, '>', $hba_conf or die "Could not open $hba_conf: $!"; + print $hba_fh "local all all trust\n"; + close $hba_fh; + + $node->start; + + my ($ret, $stdout, $stderr) = $node->psql('postgres', + "SET test_hba_guc.string_var = 'from_set';"); + isnt($ret, 0, 'SET command rejected for PGC_HBA variable'); + like($stderr, qr/cannot be changed|cannot be set/, + 'Error message indicates SET is not allowed for PGC_HBA'); + + $node->stop; +} + +done_testing(); diff --git a/src/test/modules/test_hba_guc/t/003_hba_guc_precedence.pl b/src/test/modules/test_hba_guc/t/003_hba_guc_precedence.pl new file mode 100644 index 00000000000..4afaad95c73 --- /dev/null +++ b/src/test/modules/test_hba_guc/t/003_hba_guc_precedence.pl @@ -0,0 +1,105 @@ +# Test that PGC_HBA variables from pg_hba.conf respect line precedence +# +# pg_hba.conf is evaluated top-to-bottom, and the first matching line wins. +# This test verifies that GUC values are taken from the correct matching line. + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# Test 1: First matching line wins (same user, same database, same auth methods) +{ + my $node = PostgreSQL::Test::Cluster->new('first_match_wins'); + $node->init; + + $node->append_conf('postgresql.conf', + "session_preload_libraries = 'test_hba_guc'"); + + my $hba_conf = $node->data_dir . '/pg_hba.conf'; + open my $hba_fh, '>', $hba_conf or die "Could not open $hba_conf: $!"; + print $hba_fh "local all all trust test_hba_guc.string_var=first_line\n"; + print $hba_fh "local all all trust test_hba_guc.string_var=second_line\n"; + print $hba_fh "local all all trust test_hba_guc.string_var=third_line\n"; + close $hba_fh; + + $node->start; + + my $result = $node->safe_psql('postgres', + "SELECT current_setting('test_hba_guc.string_var');"); + is($result, 'first_line', + 'First matching HBA line wins when multiple lines match'); + + $node->stop; +} + +# Test 2: Database-specific GUC values +{ + my $node = PostgreSQL::Test::Cluster->new('database_specific'); + $node->init; + + $node->append_conf('postgresql.conf', + "session_preload_libraries = 'test_hba_guc'"); + + # Create test databases + $node->start; + $node->safe_psql('postgres', 'CREATE DATABASE testdb1;'); + $node->safe_psql('postgres', 'CREATE DATABASE testdb2;'); + $node->stop; + + # Create pg_hba.conf with database-specific values + my $hba_conf = $node->data_dir . '/pg_hba.conf'; + open my $hba_fh, '>', $hba_conf or die "Could not open $hba_conf: $!"; + print $hba_fh "local testdb1 all trust test_hba_guc.string_var=from_testdb1\n"; + print $hba_fh "local testdb2 all trust test_hba_guc.string_var=from_testdb2\n"; + print $hba_fh "local all all trust test_hba_guc.string_var=from_wildcard\n"; + close $hba_fh; + + $node->start; + + my $result = $node->safe_psql('testdb1', + "SELECT current_setting('test_hba_guc.string_var');"); + is($result, 'from_testdb1', + 'Database-specific GUC value applied for testdb1'); + + $result = $node->safe_psql('testdb2', + "SELECT current_setting('test_hba_guc.string_var');"); + is($result, 'from_testdb2', + 'Database-specific GUC value applied for testdb2'); + + $result = $node->safe_psql('postgres', + "SELECT current_setting('test_hba_guc.string_var');"); + is($result, 'from_wildcard', + 'Wildcard GUC value applied for postgres database'); + + $node->stop; +} + +# Test 3: Empty/no GUC options on first match, GUC options on later line +# The first matching line wins even if it has no GUC options +{ + my $node = PostgreSQL::Test::Cluster->new('no_gucs_first'); + $node->init; + + $node->append_conf('postgresql.conf', + "session_preload_libraries = 'test_hba_guc'"); + + my $hba_conf = $node->data_dir . '/pg_hba.conf'; + open my $hba_fh, '>', $hba_conf or die "Could not open $hba_conf: $!"; + print $hba_fh "local all all trust\n"; + print $hba_fh "local all all trust test_hba_guc.string_var=not_used\n"; + close $hba_fh; + + $node->start; + + # Should get default value since first matching line has no GUC options + my $result = $node->safe_psql('postgres', + "SELECT current_setting('test_hba_guc.string_var');"); + is($result, 'default_value', + 'Default GUC value when first matching line has no GUC options'); + + $node->stop; +} + +done_testing(); diff --git a/src/test/modules/test_hba_guc/test_hba_guc--1.0.sql b/src/test/modules/test_hba_guc/test_hba_guc--1.0.sql new file mode 100644 index 00000000000..eb29d174f52 --- /dev/null +++ b/src/test/modules/test_hba_guc/test_hba_guc--1.0.sql @@ -0,0 +1,22 @@ +/* src/test/modules/test_hba_guc/test_hba_guc--1.0.sql */ + +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use "CREATE EXTENSION test_hba_guc" to load this file. \quit + +-- Function to get current value of test_hba_guc.string_var +CREATE FUNCTION get_test_hba_string() +RETURNS text +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT; + +-- Function to get current value of test_hba_guc.int_var +CREATE FUNCTION get_test_hba_int() +RETURNS integer +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT; + +-- Function to get current value of test_hba_guc.bool_var +CREATE FUNCTION get_test_hba_bool() +RETURNS boolean +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT; diff --git a/src/test/modules/test_hba_guc/test_hba_guc.c b/src/test/modules/test_hba_guc/test_hba_guc.c new file mode 100644 index 00000000000..74b4a559ac5 --- /dev/null +++ b/src/test/modules/test_hba_guc/test_hba_guc.c @@ -0,0 +1,93 @@ +/*------------------------------------------------------------------------- + * + * test_hba_guc.c + * Test module for PGC_HBA GUC variables + * + * This module tests the PGC_HBA context level for GUC variables, which + * allows variables to be set in pg_hba.conf or postgresql.conf but not + * by client connections. + * + * Copyright (c) 2026, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/test/modules/test_hba_guc/test_hba_guc.c + * + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "fmgr.h" +#include "utils/builtins.h" +#include "utils/guc.h" + +PG_MODULE_MAGIC; + +static char *test_hba_string = NULL; +static int test_hba_int = 0; +static bool test_hba_bool = false; + +PG_FUNCTION_INFO_V1(get_test_hba_string); +PG_FUNCTION_INFO_V1(get_test_hba_int); +PG_FUNCTION_INFO_V1(get_test_hba_bool); + +void +_PG_init(void) +{ + DefineCustomStringVariable("test_hba_guc.string_var", + "Test PGC_HBA string variable", + "This variable can only be set in pg_hba.conf or postgresql.conf", + &test_hba_string, + "default_value", + PGC_HBA, + 0, + NULL, + NULL, + NULL); + + DefineCustomIntVariable("test_hba_guc.int_var", + "Test PGC_HBA integer variable", + "This variable can only be set in pg_hba.conf or postgresql.conf", + &test_hba_int, + 42, + 0, + 10000, + PGC_HBA, + 0, + NULL, + NULL, + NULL); + + DefineCustomBoolVariable("test_hba_guc.bool_var", + "Test PGC_HBA boolean variable", + "This variable can only be set in pg_hba.conf or postgresql.conf", + &test_hba_bool, + false, + PGC_HBA, + 0, + NULL, + NULL, + NULL); + + MarkGUCPrefixReserved("test_hba_guc"); +} + +Datum +get_test_hba_string(PG_FUNCTION_ARGS) +{ + if (test_hba_string == NULL) + PG_RETURN_NULL(); + PG_RETURN_TEXT_P(cstring_to_text(test_hba_string)); +} + +Datum +get_test_hba_int(PG_FUNCTION_ARGS) +{ + PG_RETURN_INT32(test_hba_int); +} + +Datum +get_test_hba_bool(PG_FUNCTION_ARGS) +{ + PG_RETURN_BOOL(test_hba_bool); +} diff --git a/src/test/modules/test_hba_guc/test_hba_guc.conf b/src/test/modules/test_hba_guc/test_hba_guc.conf new file mode 100644 index 00000000000..29ee434a784 --- /dev/null +++ b/src/test/modules/test_hba_guc/test_hba_guc.conf @@ -0,0 +1,3 @@ +# Configuration for test_hba_guc regression tests +# Load in session_preload_libraries to test the proper validation flow +session_preload_libraries = 'test_hba_guc' diff --git a/src/test/modules/test_hba_guc/test_hba_guc.control b/src/test/modules/test_hba_guc/test_hba_guc.control new file mode 100644 index 00000000000..1f4a99069ba --- /dev/null +++ b/src/test/modules/test_hba_guc/test_hba_guc.control @@ -0,0 +1,5 @@ +# test_hba_guc extension +comment = 'Test module for PGC_HBA GUC variables' +default_version = '1.0' +module_pathname = '$libdir/test_hba_guc' +relocatable = true diff --git a/src/test/modules/test_hba_guc_contexts/Makefile b/src/test/modules/test_hba_guc_contexts/Makefile new file mode 100644 index 00000000000..7f805b0abd8 --- /dev/null +++ b/src/test/modules/test_hba_guc_contexts/Makefile @@ -0,0 +1,18 @@ +# src/test/modules/test_hba_guc_contexts/Makefile + +MODULE_big = test_hba_guc_contexts +OBJS = \ + $(WIN32RES) \ + test_hba_guc_contexts.o +PGFILEDESC = "test_hba_guc_contexts - test module for PGC_HBA context validation" + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = src/test/modules/test_hba_guc_contexts +top_builddir = ../../../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/src/test/modules/test_hba_guc_contexts/meson.build b/src/test/modules/test_hba_guc_contexts/meson.build new file mode 100644 index 00000000000..e2c0eeed229 --- /dev/null +++ b/src/test/modules/test_hba_guc_contexts/meson.build @@ -0,0 +1,28 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +test_hba_guc_contexts_sources = files( + 'test_hba_guc_contexts.c', +) + +if host_system == 'windows' + test_hba_guc_contexts_sources += rc_lib_gen.process(win32ver_rc, extra_args: [ + '--NAME', 'test_hba_guc_contexts', + '--FILEDESC', 'test_hba_guc_contexts - test module for PGC_HBA context validation',]) +endif + +test_hba_guc_contexts = shared_module('test_hba_guc_contexts', + test_hba_guc_contexts_sources, + kwargs: pg_test_mod_args, +) +test_install_libs += test_hba_guc_contexts + +tests += { + 'name': 'test_hba_guc_contexts', + 'sd': meson.current_source_dir(), + 'bd': meson.current_build_dir(), + 'tap': { + 'tests': [ + 't/001_context_validation.pl', + ], + }, +} diff --git a/src/test/modules/test_hba_guc_contexts/t/001_context_validation.pl b/src/test/modules/test_hba_guc_contexts/t/001_context_validation.pl new file mode 100644 index 00000000000..29f9f27844a --- /dev/null +++ b/src/test/modules/test_hba_guc_contexts/t/001_context_validation.pl @@ -0,0 +1,55 @@ +# Test that only PGC_HBA variables can be set from pg_hba.conf + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +sub test_hba_rejected +{ + my ($node_name, $var_name, $test_desc) = @_; + + my $node = PostgreSQL::Test::Cluster->new($node_name); + $node->init; + + $node->append_conf('postgresql.conf', + "shared_preload_libraries = 'test_hba_guc_contexts'"); + + my $hba_conf = $node->data_dir . '/pg_hba.conf'; + open my $hba_fh, '>', $hba_conf or die "Could not open $hba_conf: $!"; + print $hba_fh "local all all trust test_hba_guc_contexts.$var_name=from_hba\n"; + close $hba_fh; + + $node->start; + + my ($ret, $stdout, $stderr) = $node->psql('postgres', 'SELECT 1;'); + isnt($ret, 0, $test_desc); + like($stderr, qr/cannot be changed|cannot be set/, + 'Error message indicates variable cannot be set from pg_hba.conf'); + + $node->stop; + return; +} + +# Test 1: PGC_POSTMASTER variable should NOT be settable from pg_hba.conf +test_hba_rejected('postmaster_rejected', 'postmaster_var', + 'Connection rejected when trying to set PGC_POSTMASTER from pg_hba.conf'); + +# Test 2: PGC_SIGHUP variable should NOT be settable from pg_hba.conf +test_hba_rejected('sighup_rejected', 'sighup_var', + 'Connection rejected when trying to set PGC_SIGHUP from pg_hba.conf'); + +# Test 3: PGC_BACKEND variable should NOT be settable from pg_hba.conf +test_hba_rejected('backend_rejected', 'backend_var', + 'Connection rejected when trying to set PGC_BACKEND from pg_hba.conf'); + +# Test 4: PGC_SUSET variable should NOT be settable from pg_hba.conf +test_hba_rejected('suset_rejected', 'suset_var', + 'Connection rejected when trying to set PGC_SUSET from pg_hba.conf'); + +# Test 5: PGC_USERSET variable should NOT be settable from pg_hba.conf +test_hba_rejected('userset_rejected', 'userset_var', + 'Connection rejected when trying to set PGC_USERSET from pg_hba.conf'); + +done_testing(); diff --git a/src/test/modules/test_hba_guc_contexts/test_hba_guc_contexts.c b/src/test/modules/test_hba_guc_contexts/test_hba_guc_contexts.c new file mode 100644 index 00000000000..3da44d99ba0 --- /dev/null +++ b/src/test/modules/test_hba_guc_contexts/test_hba_guc_contexts.c @@ -0,0 +1,79 @@ +/*------------------------------------------------------------------------- + * + * test_hba_guc_contexts.c + * Test module for validating PGC_HBA context restrictions + * + * This module defines GUC variables with different context levels to test + * that only PGC_HBA variables can be set from pg_hba.conf. + * + * Copyright (c) 2026, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/test/modules/test_hba_guc_contexts/test_hba_guc_contexts.c + * + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "fmgr.h" +#include "utils/guc.h" + +PG_MODULE_MAGIC; + +static char *test_postmaster_var = NULL; +static char *test_sighup_var = NULL; +static char *test_backend_var = NULL; +static char *test_suset_var = NULL; +static char *test_userset_var = NULL; + +void +_PG_init(void) +{ + DefineCustomStringVariable("test_hba_guc_contexts.postmaster_var", + "Test PGC_POSTMASTER variable", + "This should NOT be settable from pg_hba.conf", + &test_postmaster_var, + "postmaster_default", + PGC_POSTMASTER, + 0, + NULL, NULL, NULL); + + DefineCustomStringVariable("test_hba_guc_contexts.sighup_var", + "Test PGC_SIGHUP variable", + "This should NOT be settable from pg_hba.conf", + &test_sighup_var, + "sighup_default", + PGC_SIGHUP, + 0, + NULL, NULL, NULL); + + DefineCustomStringVariable("test_hba_guc_contexts.backend_var", + "Test PGC_BACKEND variable", + "This should NOT be settable from pg_hba.conf", + &test_backend_var, + "backend_default", + PGC_BACKEND, + 0, + NULL, NULL, NULL); + + DefineCustomStringVariable("test_hba_guc_contexts.suset_var", + "Test PGC_SUSET variable", + "This should NOT be settable from pg_hba.conf", + &test_suset_var, + "suset_default", + PGC_SUSET, + 0, + NULL, NULL, NULL); + + DefineCustomStringVariable("test_hba_guc_contexts.userset_var", + "Test PGC_USERSET variable", + "This should NOT be settable from pg_hba.conf", + &test_userset_var, + "userset_default", + PGC_USERSET, + 0, + NULL, NULL, NULL); + + MarkGUCPrefixReserved("test_hba_guc_contexts"); +} -- 2.43.0 ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Custom oauth validator options @ 2026-01-16 01:54 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Jacob Champion @ 2026-01-16 01:54 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Wed, Jan 14, 2026 at 10:20 AM Zsolt Parragi <[email protected]> wrote: > > > Well, how do you want "global" GUCs registered by the validator to > > behave when OAuth isn't used for the connection? > > My assumption was that with session_preload we only validate the line > used for the current login, not all the lines. This way we don't have > to always include all validator/hba plugins in session_preload, for > every login. > > This is what I implemented for now, but if you think it would be > better to validate every line, I can adjust that. Let me think about that a bit, and look over your v2 approach; my kneejerk reaction was that this is a dangerous situation to be in. I want to know that my HBA is invalid when I reload, not later on down the line. But my understanding of GUCs from session_preload_libraries also had some wishful thinking behind it. I believed that the situation today was stricter than it actually is: $ tail -2 data/postgresql.conf session_preload_libraries = auto_explain auto_explain.blahblahblah = yes $ psql postgres WARNING: invalid configuration parameter name "auto_explain.blahblahblah", removing it DETAIL: "auto_explain" is now a reserved prefix. psql (18.0) Type "help" for help. And it makes sense that the postmaster is not going to somehow unload and reload those libraries during SIGHUP, just to check GUC settings. Hrmmm... (If we did go in this direction, I think we might want to be punishingly strict for the first iteration of the feature. PGC_HBA providers should prefix their settings to avoid confusion and/or future collisions anyway, so if we don't know what the GUC is, and its prefix doesn't match either a validator name -- which is DBA-blessed -- or a valid session_preload_libraries item, I'm not sure we should even wait to complain.) > > Of those choices, this _seems_ nicest. It'd be good to get a feel for > > how it behaves in practice though. > > See the attached v2, with the above comment. Thank you! > Other than the above question (validate everything vs the current > line), I'm also not entirely sure if we do need PGC_HBA. It could also > work with PGC_SIGHUP + the new PGC_S_HBA value in GucSources. I might be misunderstanding, but wouldn't that imply that DBAs could now put every existing SIGHUP setting into HBA? That doesn't seem good. My hope was that some existing SIGHUP variables could be downgraded to HBA variables (say, gss_accept_delegation or authentication_timeout -- though there might be a chicken-and-egg issue for the latter?), but that's going to be a short list. --Jacob ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Custom oauth validator options @ 2026-01-16 08:30 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Zsolt Parragi @ 2026-01-16 08:30 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > Let me think about that a bit, and look over your v2 approach; my > kneejerk reaction was that this is a dangerous situation to be in. I > want to know that my HBA is invalid when I reload, not later on down > the line. Yes, I see that concern, but that's a bit trickier. To do that properly we have to validate the variables, including their values, not just their names. If we only validate the names, that doesn't guarantee anything. > And it makes sense that the postmaster is not going to somehow unload > and reload those libraries during SIGHUP, just to check GUC settings. > Hrmmm... Would it be a good idea for it to dlopen/dlclose libraries? The requirements of dlclose are not that strict, I'm not sure if it could cause any issues. Opening a quick background process to verify it seems safer, but even then, it could only verify the libraries mentioned directly in the configuration. I could write the code that does this for pg_hba on startup/reload, but the tricky part is that we have to document that properly, to make sure that the extensions also expects and handles the situation correctly (that we try to validate gucs for all hba lines). Or start one background process per hba line... > I might be misunderstanding, but wouldn't that imply that DBAs could > now put every existing SIGHUP setting into HBA? That doesn't seem > good. Yes, that would mean that. I'm not saying that would be better/semantically correct, but technically it could also work, that's why I mentioned it. The main use of PGC_HBA in this patch is to add additional error reporting / separate what can be placed into pg_hba. I could argue both for this approach and the opposite where we allow other variables in pg_hba. ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Custom oauth validator options @ 2026-01-16 16:39 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Jacob Champion @ 2026-01-16 16:39 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Fri, Jan 16, 2026 at 12:31 AM Zsolt Parragi <[email protected]> wrote: > Yes, I see that concern, but that's a bit trickier. To do that > properly we have to validate the variables, including their values, > not just their names. If we only validate the names, that doesn't > guarantee anything. Right. This goes back to your question upthread as to why I brought session_preload_libraries into all this -- I thought session_preload_libraries already had handling for this, but it doesn't. > Would it be a good idea for it to dlopen/dlclose libraries? No, unfortunately. > The > requirements of dlclose are not that strict, I'm not sure if it could > cause any issues. Last I knew (which was a while back), unloading a shared library tree is fraught with peril, especially when you add dependency diamonds and static initializers. I seem to remember that Windows, C++, and OpenSSL all have particular areas of pain. My guess is that people are going to make mistakes, leave dangling references around, and then either crash the postmaster or (worse) copy a crashable address space into every new backend. And that's before you get into hooks and GUCs and etc. We used to have a _PG_fini() callback for modules. It was disabled a long time ago [1], then recently entirely removed from the codebase. > > I might be misunderstanding, but wouldn't that imply that DBAs could > > now put every existing SIGHUP setting into HBA? That doesn't seem > > good. > > Yes, that would mean that. I'm not saying that would be > better/semantically correct, but technically it could also work, > that's why I mentioned it. Okay, but that wouldn't be a committable change. > The main use of PGC_HBA in this patch is to > add additional error reporting / separate what can be placed into > pg_hba. I could argue both for this approach and the opposite where we > allow other variables in pg_hba. Not sure what you mean by this (maybe once I can really test the patch, I'll see?), but the reason I suggested placing PGC_HBA right after PGC_SIGHUP is this: any SU_BACKEND or below variable seems logically appropriate to set per HBA line, if the DBA wants (they're all per-session), and anything SIGHUP or above is inappropriate/unsafe (they're per-cluster). Does your patch disable the former? [checks] Ah, it does prohibit those. Why? --Jacob [1] https://postgr.es/c/602a9ef5a7c60151e10293ae3c4bb3fbb0132d03 ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Custom oauth validator options @ 2026-01-16 17:13 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Zsolt Parragi @ 2026-01-16 17:13 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > Last I knew (which was a while back), Yes, I didn't want to say anything for sure, but I have similar memories on Windows a while ago. I don't know anything for sure about today, and especially on Linux, but delegating things to another process seems to be a safer approach to me. > [checks] Ah, it does prohibit those. Why? Mainly because I couldn't decide where it should fit if the variable is set at multiple places (or if we need multiple sources like PGC_S_DATABASE_USER). * A hba line can be completely generic, which should be above DATABASE (ALTER DATABASE setting should override HBA setting, as it is more specific) * Or very specific about one user in one database using a specific authentication method, which should be below DATABASE_USER as it is more specific. (hba setting should override ALTER USER ... IN DATABASE setting) The first choice seems more logical to me, as that's how pg_hba is usually used, but I thought this could still be confusing. ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Custom oauth validator options @ 2026-01-16 17:52 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Jacob Champion @ 2026-01-16 17:52 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Fri, Jan 16, 2026 at 9:14 AM Zsolt Parragi <[email protected]> wrote: > * A hba line can be completely generic, which should be above DATABASE > (ALTER DATABASE setting should override HBA setting, as it is more > specific) I think settings in the database should override the ones from the HBA, yes. So that would put PGC_S_HBA right between, what, _ARGV and _GLOBAL? > The first choice seems more logical to me, as that's how pg_hba is > usually used, but I thought this could still be confusing. I agree it could be, but is it any more confusing than if you were to set work_mem in postgresql.conf today, and then `ALTER ROLE ALL SET work_mem` to something completely different? Usability improvements for that should be made GUC-wide, I think, and not influence the chosen order of operations for this feature (as long as there are no new security concerns). I don't want any project veterans, whether DBAs or maintainers, to be surprised by how a new GUC context behaves. --Jacob ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-01-16 17:52 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-01-05 18:53 Re: Custom oauth validator options Jacob Champion <[email protected]> 2026-01-14 18:20 ` Zsolt Parragi <[email protected]> 2026-01-16 01:54 ` Jacob Champion <[email protected]> 2026-01-16 08:30 ` Zsolt Parragi <[email protected]> 2026-01-16 16:39 ` Jacob Champion <[email protected]> 2026-01-16 17:13 ` Zsolt Parragi <[email protected]> 2026-01-16 17:52 ` Jacob Champion <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox