public inbox for [email protected]help / color / mirror / Atom feed
Re: Custom oauth validator options 11+ messages / 2 participants [nested] [flat]
* Re: Custom oauth validator options @ 2026-02-04 11:42 Zsolt Parragi <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Zsolt Parragi @ 2026-02-04 11:42 UTC (permalink / raw) To: Nikolay Shaplov <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; Jacob Champion <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] I've looked into the patch in more detail. (I will post a review later to that thread, I have some notes I have to format properly) As for this use case, we could use this (for my original B or C options), but I see a potential problem: First, we either only use this code to feed the unknown parameters to the options parser, and keep the existing hba options parser as is for the hardcoded parameters. Or also include the fixed options in it, so that everything works exactly the same. Then we either make it limited to oauth validators, or try to keep it generic for any session_preload_libraries. If we only use this for unknown options, and limit it to oauth validators, then options.h/c could work as is. If we want to implement anything more generic, we'll face issues, as the current API only supports validating the input once. In the most generic case: * Parsing the core hba options in postmaster * Validating core options, ignoring unknowns * Loading the oauth validator * Validating options again, as the validator needs its custom options - having unknown remaining options is still valid * Running the validator * Loading session preload libraries * Validating options again - unknown options are an error now So up to 3 times, and it also needs a way for extensions to edit spec sets. (In the simple case, only the validator has to know about that) I think this makes this impractical for the more complex applications, but if we want to go back to the minimal original concept, limited to oauth validators, it could work. I'm also not sure how useful this non-GUC API would be for extensions other than validators, which also tells me that this version should be limited to the validators. Also, this isn't an approach with an easy way to convert it into PGC_HBA, as it requires a clearly different api in the validator - I don't see a nice way to do that without changing the API used by the validators. ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Custom oauth validator options @ 2026-03-20 17:46 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Jacob Champion @ 2026-03-20 17:46 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] Hi all, First, an apology for the state of this thread -- I thought I had already responded to the prior message last month. Turns out, I had not... Instead, I'm dropping a surprise alternative patchset on top of a thread that I let go cold, which is rude, and I'm sorry. Here are my overall thoughts on the approaches shared so far. And thank you, Zsolt, for doing so much legwork here; that effort was not wasted at all. 1) A GUC-centric solution -- option (d) -- is the Ideal Solution here, IMO. We shouldn't have to reinvent the wheel. 2) A GUC-centric solution is not going to land for 19, and I'd be surprised if it landed for 20, given the coalition that I'll need to build to say either "yes" or "no". There are too many questions about session_preload_libraries and prefixing and etc., and I'm honestly not a fan of the guc_prefix_enforcement approach. 3) I'm not convinced yet that GUCs and relation options are similar enough to share a framework. (This is not a rejection of the reloption refactoring work, just a statement that I don't want to rely on it to solve this problem.) 4) I really don't like the hba_parse_option_hook. I prefer APIs over hooks as a general architectural point, and more practically, I don't want to hand control to extensions during HBA parsing. I don't think they're going to coordinate with each other well, and I think they're likely to couple against internals in ways we don't want to support (which is my general problem with hooks). = Option (b) = I don't want this problem to go unfixed for 2+ years, so I think it would be best to reinvent a very small wheel that doesn't take up a lot of maintenance effort once it's in, and then simply replace it with the Ideal Solution eventually. This is my take on option (b), which is what Vasuki M advocated for upthread. It's just a setter/getter API for string keys and values: static const char* opts[] = { "my_parameter" }; void startup_cb(ValidatorModuleState *state) { RegisterOAuthHBAOptions(state, lengthof(opts), opts); } bool validate_cb(const ValidatorModuleState *state, ...) { const char *param; if ((param = GetOAuthHBAOption(state, "my_parameter")) != NULL) do_something_with(param); ... } And then in your HBA: host all all ::1/128 oauth validator.my_parameter=foo ... The core implementation is conceptually simple. Most of the lines in the patch are guardrails, to reduce the probability of regret over this temporary solution: - Parameter names aren't freeform; they're restricted to almost-alphanumeric identifiers. (We've wanted to steal symbols for HBA features in the past.) - Name syntax is checked on reload, but cross-referencing the registered list must be deferred to connection time. That's unusual for users, so the WARNING that gets printed in this case is extremely verbose; that way hopefully no one will be confused about what is happening. - The compiler won't let you register names during validate_cb, and we won't let you retrieve options during startup_cb, so we retain control over the internal order of operations. (This is discussed more deeply in the patch.) I was worried that we'd need a third API call to report option parsing failures from the validator. Instead of doing that, I've made it easier to link an authentication failure to a validator internal error that caused it, in v3-0001, which serendipitously fixes a separate sharp edge [1]. I feel good about that patch even if -0002 doesn't make it. v3-0002 still lacks user documentation, which I need to write a lot of -- but if everyone dislikes this approach then I'd rather not spend the time there. = WDYT? = I realize this puts you all in a difficult position; the effect is kind of "take it or leave it", which again was not my intent. I considered letting this lapse for 19 instead. But since I believe the ideal solution is one we can't have for a while, and there's good research and discussion of alternatives in this thread, waiting may not produce a more committable short-term solution in the end. Let me know if any of you disagree, though -- especially if you think the status quo is preferable! -- and I can shelve -0002 for now. (I'll continue with -0001 at [1] either way.) Thanks, --Jacob [1] https://postgr.es/m/202601241015.y5uvxd7oxnfs%40alvherre.pgsql Attachments: [application/octet-stream] v3-0001-oauth-Let-validators-provide-failure-DETAILs.patch (12.6K, 2-v3-0001-oauth-Let-validators-provide-failure-DETAILs.patch) download | inline diff: From f36e6becc34d05834c3d808575b04d39badf7569 Mon Sep 17 00:00:00 2001 From: Jacob Champion <[email protected]> Date: Thu, 19 Mar 2026 09:37:20 -0700 Subject: [PATCH v3 1/2] oauth: Let validators provide failure DETAILs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At the moment, the only way for a validator module to report error details on failure is to log them separately before returning from validate_cb. Independently of that problem, the ereport() calls that we make during validation failure partially duplicate some of the work of auth_failed(). The end result is overly verbose and confusing for readers of the logs: [768233] LOG: [my_validator] bad signature in bearer token [768233] LOG: OAuth bearer authentication failed for user "jacob" [768233] DETAIL: Validator failed to authorize the provided token. [768233] FATAL: OAuth bearer authentication failed for user "jacob" [768233] DETAIL: Connection matched file ".../pg_hba.conf" line ... Solve both problems by making use of the existing logdetail pointer that's provided by ClientAuthentication. Validator modules may set ValidatorModuleResult->error_detail to override our default generic message. The end result looks something like [242284] FATAL: OAuth bearer authentication failed for user "jacob" [242284] DETAIL: [my_validator] bad signature in bearer token Connection matched file ".../pg_hba.conf" line ... Reported-by: Álvaro Herrera <[email protected]> Reported-by: Zsolt Parragi <[email protected]> Discussion: https://postgr.es/m/202601241015.y5uvxd7oxnfs%40alvherre.pgsql Discussion: TODO --- doc/src/sgml/oauth-validators.sgml | 21 +++++++++- src/include/libpq/oauth.h | 14 +++++++ src/backend/libpq/auth-oauth.c | 24 +++++------ src/backend/libpq/auth.c | 2 +- .../modules/oauth_validator/t/001_server.pl | 40 ++++++++++++++++++- src/test/modules/oauth_validator/validator.c | 29 ++++++++++++++ 6 files changed, 114 insertions(+), 16 deletions(-) diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml index 704089dd7b3..a7140eae84e 100644 --- a/doc/src/sgml/oauth-validators.sgml +++ b/doc/src/sgml/oauth-validators.sgml @@ -192,11 +192,18 @@ <term>Logging</term> <listitem> <para> - Modules may use the same <link linkend="error-message-reporting">logging + To simply log the reason for a validation failure, validators may set + the freeform <structfield>error_detail</structfield> field during the + <xref linkend="oauth-validator-callback-validate"/>. This is printed only + to the server log, as part of the final authentication failure message, + and it is not shared with the client. + </para> + <para> + Modules may also use the same <link linkend="error-message-reporting">logging facilities</link> as standard extensions; however, the rules for emitting log entries to the client are subtly different during the authentication phase of the connection. Generally speaking, modules should log - verification problems at the <symbol>COMMERROR</symbol> level and return + problems at the <symbol>COMMERROR</symbol> level and return normally, instead of using <symbol>ERROR</symbol>/<symbol>FATAL</symbol> to unwind the stack, to avoid leaking information to unauthenticated clients. @@ -370,6 +377,7 @@ typedef struct ValidatorModuleResult { bool authorized; char *authn_id; + char *error_detail; } ValidatorModuleResult; </programlisting> @@ -387,6 +395,15 @@ typedef struct ValidatorModuleResult Otherwise the validator should return <literal>true</literal> to indicate that it has processed the token and made an authorization decision. </para> + <para> + In either failure case (validation error or internal error) the module may + store a user-readable reason for the failure in <structfield>result->error_detail</structfield>. + This will be printed to the server logs (not sent to the client) as a + <literal>DETAIL</literal> entry for the authentication failure. The memory + pointed to by <structfield>error_detail</structfield> may be either palloc'd + or of static duration. <structfield>error_detail</structfield> is ignored + on success. + </para> <para> The behavior after <function>validate_cb</function> returns depends on the specific HBA setup. Normally, the <structfield>result->authn_id</structfield> user diff --git a/src/include/libpq/oauth.h b/src/include/libpq/oauth.h index 4a822e9a1f2..60f493acddd 100644 --- a/src/include/libpq/oauth.h +++ b/src/include/libpq/oauth.h @@ -49,6 +49,20 @@ typedef struct ValidatorModuleResult * delegation. See the validator module documentation for details. */ char *authn_id; + + /* + * When validation fails, this may optionally be set to a string + * containing an explanation for the failure. It will be sent to the + * server log only; it is not provided to the client, and it's ignored if + * validation succeeds. + * + * This description will be attached to the final authentication failure + * message in the logs, as a DETAIL, which may be preferable to separate + * ereport() calls that have to be correlated by the reader. + * + * This string may be either of static duration or palloc'd. + */ + char *error_detail; } ValidatorModuleResult; /* diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c index 11365048951..6d4be70aada 100644 --- a/src/backend/libpq/auth-oauth.c +++ b/src/backend/libpq/auth-oauth.c @@ -73,7 +73,7 @@ struct oauth_ctx static char *sanitize_char(char c); static char *parse_kvpairs_for_auth(char **input); static void generate_error_response(struct oauth_ctx *ctx, char **output, int *outputlen); -static bool validate(Port *port, const char *auth); +static bool validate(Port *port, const char *auth, const char **logdetail); /* Constants seen in an OAUTHBEARER client initial response. */ #define KVSEP 0x01 /* separator byte for key/value pairs */ @@ -279,7 +279,7 @@ oauth_exchange(void *opaq, const char *input, int inputlen, errmsg("malformed OAUTHBEARER message"), errdetail("Message contains additional data after the final terminator.")); - if (!validate(ctx->port, auth)) + if (!validate(ctx->port, auth, logdetail)) { generate_error_response(ctx, output, outputlen); @@ -635,7 +635,7 @@ validate_token_format(const char *header) * authorization. Returns true if validation succeeds. */ static bool -validate(Port *port, const char *auth) +validate(Port *port, const char *auth, const char **logdetail) { int map_status; ValidatorModuleResult *ret; @@ -662,7 +662,10 @@ validate(Port *port, const char *auth) { ereport(WARNING, errcode(ERRCODE_INTERNAL_ERROR), - errmsg("internal error in OAuth validator module")); + errmsg("internal error in OAuth validator module"), + ret->error_detail ? errdetail_log("%s", ret->error_detail) : 0); + + *logdetail = ret->error_detail; return false; } @@ -675,10 +678,10 @@ validate(Port *port, const char *auth) if (!ret->authorized) { - ereport(LOG, - errmsg("OAuth bearer authentication failed for user \"%s\"", - port->user_name), - errdetail_log("Validator failed to authorize the provided token.")); + if (ret->error_detail) + *logdetail = ret->error_detail; + else + *logdetail = _("Validator failed to authorize the provided token."); status = false; goto cleanup; @@ -699,10 +702,7 @@ validate(Port *port, const char *auth) /* Make sure the validator authenticated the user. */ if (ret->authn_id == NULL || ret->authn_id[0] == '\0') { - ereport(LOG, - errmsg("OAuth bearer authentication failed for user \"%s\"", - port->user_name), - errdetail_log("Validator provided no identity.")); + *logdetail = _("Validator provided no identity."); status = false; goto cleanup; diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index e04aa2e68ed..6801ea9c566 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -625,7 +625,7 @@ ClientAuthentication(Port *port) status = STATUS_OK; break; case uaOAuth: - status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, NULL); + status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, &logdetail); break; } diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index cdad2ae8011..28d123b833e 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -521,8 +521,8 @@ $node->connect_fails( expected_stderr => qr/OAuth bearer authentication failed/, log_like => [ qr/connection authenticated: identity=""/, - qr/DETAIL:\s+Validator provided no identity/, qr/FATAL:\s+OAuth bearer authentication failed/, + qr/DETAIL:\s+Validator provided no identity/, ]); # Even if a validator authenticates the user, if the token isn't considered @@ -541,10 +541,48 @@ $node->connect_fails( expected_stderr => qr/OAuth bearer authentication failed/, log_like => [ qr/connection authenticated: identity="test\@example\.org"/, + qr/FATAL:\s+OAuth bearer authentication failed/, qr/DETAIL:\s+Validator failed to authorize the provided token/, + ]); + +# Validators can provide their own explanations. +$bgconn->query_safe( + "ALTER SYSTEM SET oauth_validator.error_detail TO 'something failed'"); +$node->reload; +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + +$node->connect_fails( + "$common_connstr user=test", + "validator must authorize token explicitly (custom logdetail)", + expected_stderr => qr/OAuth bearer authentication failed/, + log_like => [ + qr/connection authenticated: identity="test\@example\.org"/, qr/FATAL:\s+OAuth bearer authentication failed/, + qr/DETAIL:\s+something failed/, ]); +$bgconn->query_safe( + "ALTER SYSTEM SET oauth_validator.internal_error TO true"); +$node->reload; +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + +$node->connect_fails( + "$common_connstr user=test", + "validator internal error (custom logdetail)", + expected_stderr => qr/OAuth bearer authentication failed/, + log_like => [ + qr/WARNING:\s+internal error in OAuth validator module/, + qr/DETAIL:\s+something failed/, + ]); + +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.error_detail"); +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.internal_error"); +$node->reload; +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + # # Test user mapping. # diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c index 0b983a9dc8f..353e0e0d32a 100644 --- a/src/test/modules/oauth_validator/validator.c +++ b/src/test/modules/oauth_validator/validator.c @@ -40,6 +40,8 @@ static const OAuthValidatorCallbacks validator_callbacks = { /* GUCs */ static char *authn_id = NULL; static bool authorize_tokens = true; +static char *error_detail = NULL; +static bool internal_error = false; /*--- * Extension entry point. Sets up GUCs for use by tests: @@ -51,6 +53,13 @@ static bool authorize_tokens = true; * - oauth_validator.authorize_tokens * Sets whether to successfully validate incoming * tokens. Defaults to true. + * + * - oauth_validator.error_detail + * Sets an error message to be included as a + * DETAIL on failure. + * + * - oauth_validator.internal_error + * Reports an internal error to the server. */ void _PG_init(void) @@ -71,6 +80,22 @@ _PG_init(void) PGC_SIGHUP, 0, NULL, NULL, NULL); + DefineCustomStringVariable("oauth_validator.error_detail", + "Error message to print during failures", + NULL, + &error_detail, + NULL, + PGC_SIGHUP, + 0, + NULL, NULL, NULL); + DefineCustomBoolVariable("oauth_validator.internal_error", + "Should the validator report an internal error?", + NULL, + &internal_error, + false, + PGC_SIGHUP, + 0, + NULL, NULL, NULL); MarkGUCPrefixReserved("oauth_validator"); } @@ -133,6 +158,10 @@ validate_token(const ValidatorModuleState *state, MyProcPort->hba->oauth_issuer, MyProcPort->hba->oauth_scope); + res->error_detail = error_detail; /* only relevant for failures */ + if (internal_error) + return false; + res->authorized = authorize_tokens; if (authn_id) res->authn_id = pstrdup(authn_id); -- 2.34.1 [application/octet-stream] v3-0002-WIP-oauth-Allow-validators-to-register-custom-HBA.patch (20.0K, 3-v3-0002-WIP-oauth-Allow-validators-to-register-custom-HBA.patch) download | inline diff: From 9726ac39442dd3af479903bfab62894765a6ab2f Mon Sep 17 00:00:00 2001 From: Jacob Champion <[email protected]> Date: Tue, 17 Mar 2026 12:01:28 -0700 Subject: [PATCH v3 2/2] WIP: oauth: Allow validators to register custom HBA options (lacks user documentation) Two new API entry points for validator callbacks: - RegisterOAuthHBAOptions - GetOAuthHBAOption Registering options "foo" and "bar" allows a user to set validator.foo and validator.bar on an `oauth` HBA line. The bulk of the patch is not the conceptually simple API implementation, but guardrails on the simple API to make sure it doesn't bind our hands in the future, either for callback architecture or HBA syntax. Suggested-by: Zsolt Parragi <[email protected]> Suggested-by: VASUKI M <[email protected]> Investigated-by: Zsolt Parragi <[email protected]> --- src/include/libpq/hba.h | 2 + src/include/libpq/oauth.h | 15 +- src/backend/libpq/auth-oauth.c | 225 ++++++++++++++++++ src/backend/libpq/hba.c | 25 ++ .../modules/oauth_validator/t/001_server.pl | 97 ++++++++ src/test/modules/oauth_validator/validator.c | 48 +++- 6 files changed, 407 insertions(+), 5 deletions(-) diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index c4570ce9b3f..e8898561c8c 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -140,6 +140,8 @@ typedef struct HbaLine char *oauth_scope; char *oauth_validator; bool oauth_skip_usermap; + List *oauth_opt_keys; + List *oauth_opt_vals; } HbaLine; typedef struct IdentLine diff --git a/src/include/libpq/oauth.h b/src/include/libpq/oauth.h index 60f493acddd..86f463a284e 100644 --- a/src/include/libpq/oauth.h +++ b/src/include/libpq/oauth.h @@ -96,6 +96,17 @@ typedef struct OAuthValidatorCallbacks ValidatorValidateCB validate_cb; } OAuthValidatorCallbacks; +/* + * A validator can register a list of custom option names during its startup_cb, + * then later retrieve the user settings for each during validation. This + * enables per-HBA-line configuration. For more information, refer to the OAuth + * validator modules documentation. + */ +extern void RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]); +extern const char *GetOAuthHBAOption(const ValidatorModuleState *state, + const char *optname); + /* * Type of the shared library symbol _PG_oauth_validator_module_init which is * required for all validator modules. This function will be invoked during @@ -107,9 +118,7 @@ extern PGDLLEXPORT const OAuthValidatorCallbacks *_PG_oauth_validator_module_ini /* Implementation */ extern PGDLLIMPORT const pg_be_sasl_mech pg_be_oauth_mech; -/* - * Ensure a validator named in the HBA is permitted by the configuration. - */ extern bool check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg); +extern bool valid_oauth_hba_option_name(const char *name); #endif /* PG_OAUTH_H */ diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c index 6d4be70aada..032b49e1743 100644 --- a/src/backend/libpq/auth-oauth.c +++ b/src/backend/libpq/auth-oauth.c @@ -25,6 +25,7 @@ #include "libpq/hba.h" #include "libpq/oauth.h" #include "libpq/sasl.h" +#include "miscadmin.h" #include "storage/fd.h" #include "storage/ipc.h" #include "utils/json.h" @@ -40,10 +41,15 @@ static int oauth_exchange(void *opaq, const char *input, int inputlen, static void load_validator_library(const char *libname); static void shutdown_validator_library(void *arg); +static bool check_validator_hba_options(Port *port, const char **logdetail); static ValidatorModuleState *validator_module_state; static const OAuthValidatorCallbacks *ValidatorCallbacks; +static MemoryContext ValidatorMemoryContext; +static List *ValidatorOptions; +static bool ValidatorOptionsChecked; + /* Mechanism declaration */ const pg_be_sasl_mech pg_be_oauth_mech = { .get_mechanisms = oauth_get_mechanisms, @@ -108,6 +114,9 @@ oauth_init(Port *port, const char *selected_mech, const char *shadow_pass) errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("client selected an invalid SASL authentication mechanism")); + /* Save our memory context for later use by client API calls. */ + ValidatorMemoryContext = CurrentMemoryContext; + ctx = palloc0_object(struct oauth_ctx); ctx->state = OAUTH_STATE_INIT; @@ -279,6 +288,16 @@ oauth_exchange(void *opaq, const char *input, int inputlen, errmsg("malformed OAUTHBEARER message"), errdetail("Message contains additional data after the final terminator.")); + /* + * Make sure all custom HBA options are understood by the validator before + * continuing, since we couldn't check them during server start/reload. + */ + if (!check_validator_hba_options(ctx->port, logdetail)) + { + ctx->state = OAUTH_STATE_FINISHED; + return PG_SASL_EXCHANGE_FAILURE; + } + if (!validate(ctx->port, auth, logdetail)) { generate_error_response(ctx, output, outputlen); @@ -807,6 +826,9 @@ shutdown_validator_library(void *arg) { if (ValidatorCallbacks->shutdown_cb != NULL) ValidatorCallbacks->shutdown_cb(validator_module_state); + + /* The backing memory for this is about to disappear. */ + ValidatorOptions = NIL; } /* @@ -892,3 +914,206 @@ done: return (*err_msg == NULL); } + +/* + * Client APIs for validator implementations + * + * Since we're currently not threaded, we only allow one validator in the + * process at a time. So we can make use of globals for now instead of looking + * up information using the state pointer. We probably shouldn't assume that the + * module hasn't temporarily changed memory contexts on us, though; functions + * here should defensively use an appropriate context when making global + * allocations. + */ + +/* + * Adds to the list of allowed validator.* HBA options. Used during the + * startup_cb. + */ +void +RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]) +{ + MemoryContext oldcontext; + + if (!state) + { + Assert(false); + return; + } + + oldcontext = MemoryContextSwitchTo(ValidatorMemoryContext); + + for (int i = 0; i < num; i++) + { + if (!valid_oauth_hba_option_name(opts[i])) + { + /* + * The user can't set this option in the HBA, so GetOAuthHBAOption + * would always return NULL. + */ + ereport(WARNING, + errmsg("HBA option name \"%s\" is invalid and will be ignored", + opts[i]), + /* translator: the second %s is a function name */ + errcontext("validator module \"%s\", in call to %s", + MyProcPort->hba->oauth_validator, + "RegisterOAuthHBAOptions")); + continue; + } + + ValidatorOptions = lappend(ValidatorOptions, pstrdup(opts[i])); + } + + MemoryContextSwitchTo(oldcontext); + + /* + * Wait to validate the HBA against the registered options until later + * (see check_validator_hba_options()). + * + * Delaying allows the validator to make multiple registration calls, to + * append to the list; it lets us make the check in a place where we can + * report the error without leaking details to the client; and it avoids + * exporting the order of operations between HBA matching and the + * startup_cb call as an API guarantee. (The last issue may become + * relevant with a threaded model.) + */ +} + +/* + * Restrict the names available to custom HBA options, so that we don't + * accidentally prevent future syntax extensions to HBA files. + */ +bool +valid_oauth_hba_option_name(const char *name) +{ + /* + * This list is not incredibly principled, since the goal is just to bound + * compatibility guarantees for our HBA parser. Alphanumerics seem + * obviously fine, and it's difficult to argue against the punctuation + * that's already included in some HBA option names and identifiers. + */ + static const char *name_allowed_set = + "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789_-"; + + size_t span; + + if (!name[0]) + return false; + + span = strspn(name, name_allowed_set); + return name[span] == '\0'; +} + +/* + * Verifies that all validator.* HBA options specified by the user were actually + * registered by the validator library in use. + */ +static bool +check_validator_hba_options(Port *port, const char **logdetail) +{ + HbaLine *hba = port->hba; + + foreach_ptr(char, key, hba->oauth_opt_keys) + { + bool found = false; + + /* O(n^2) shouldn't be a problem here in practice. */ + foreach_ptr(char, optname, ValidatorOptions) + { + if (strcmp(key, optname) == 0) + { + found = true; + break; + } + } + + if (!found) + { + /* + * Bad option name. Mirror the error messages in hba.c here, + * keeping in mind that the original "validator." prefix was + * stripped from the key during parsing. + * + * Since this is affecting live connections, which is unusual for + * HBA, be noisy with a WARNING. (Warnings aren't sent to clients + * prior to successful authentication, so this won't disclose the + * server config.) It'll duplicate some of the information in the + * logdetail, but that should make it hard to miss the connection + * between the two. + */ + char *name = psprintf("validator.%s", key); + + *logdetail = psprintf(_("unrecognized authentication option name: \"%s\""), + name); + ereport(WARNING, + errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("unrecognized authentication option name: \"%s\"", + name), + /* translator: the first %s is the name of the module */ + errdetail("The installed validator module (\"%s\") did not define an option named \"%s\".", + hba->oauth_validator, key), + errhint("All OAuth connections matching this line will fail. Correct the option and reload the server configuration."), + errcontext("line %d of configuration file \"%s\"", + hba->linenumber, hba->sourcefile)); + + return false; + } + } + + ValidatorOptionsChecked = true; /* unfetter GetOAuthHBAOption() */ + return true; +} + +/* + * Retrieves the setting for a validator.* HBA option, or NULL if not found. + * This may only be used during the validate_cb and shutdown_cb. + */ +const char * +GetOAuthHBAOption(const ValidatorModuleState *state, const char *optname) +{ + HbaLine *hba = MyProcPort->hba; + ListCell *lc_k; + ListCell *lc_v; + const char *ret = NULL; + + if (!ValidatorOptionsChecked) + { + /* + * Prevent the startup_cb from retrieving HBA options that it has just + * registered. This probably seems strange -- why refuse to hand out + * information we already know? -- but this lets us reserve the + * ability to perform the startup_cb call earlier, before we know + * which HBA line is matched by a connection, without breaking this + * API. + */ + return NULL; + } + + if (!state || !hba) + { + Assert(false); + return NULL; + } + + Assert(list_length(hba->oauth_opt_keys) == list_length(hba->oauth_opt_vals)); + + forboth(lc_k, hba->oauth_opt_keys, lc_v, hba->oauth_opt_vals) + { + const char *key = lfirst(lc_k); + const char *val = lfirst(lc_v); + + if (strcmp(key, optname) == 0) + { + /* + * Don't return yet -- when regular HBA options are specified more + * than once, the last one wins. Do the same for these options. + */ + ret = val; + } + } + + return ret; +} diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 87ee541e880..0569e8fced8 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -2497,6 +2497,31 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, REQUIRE_AUTH_OPTION(uaOAuth, "validator", "oauth"); hbaline->oauth_validator = pstrdup(val); } + else if (strncmp(name, "validator.", strlen("validator.")) == 0) + { + const char *key = name + strlen("validator."); + + /* + * Validator modules may register their own per-HBA-line options. + * Unfortunately, since we don't want to require these modules to be + * loaded into the postmaster, we don't know if the options are valid + * yet and must store them for later. Perform only a basic syntax + * check here. + */ + if (!valid_oauth_hba_option_name(key)) + { + ereport(elevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("invalid OAuth validator option name: \"%s\"", name), + errcontext("line %d of configuration file \"%s\"", + line_num, file_name))); + return false; + } + + REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); + hbaline->oauth_opt_keys = lappend(hbaline->oauth_opt_keys, pstrdup(key)); + hbaline->oauth_opt_vals = lappend(hbaline->oauth_opt_vals, pstrdup(val)); + } else if (strcmp(name, "delegate_ident_mapping") == 0) { REQUIRE_AUTH_OPTION(uaOAuth, "delegate_ident_mapping", "oauth"); diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index 28d123b833e..0756e6038c4 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -579,10 +579,29 @@ $node->connect_fails( $bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.error_detail"); $bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.internal_error"); + +# We complain when bad option names are registered, but connections may proceed +# (since users can't set those options in the HBA anyway). +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.authn_id"); +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.authorize_tokens"); +$bgconn->query_safe("ALTER SYSTEM SET oauth_validator.invalid_hba TO true"); + $node->reload; $log_start = $node->wait_for_log(qr/reloading configuration files/, $log_start); +$node->connect_ok( + "$common_connstr user=test", + "bad registered HBA option", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [ + qr/WARNING:\s+HBA option name "bad option name" is invalid and will be ignored/, + qr/CONTEXT:\s+validator module "validator", in call to RegisterOAuthHBAOptions/, + ]); + +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.invalid_hba"); + # # Test user mapping. # @@ -651,6 +670,84 @@ $node->reload; $log_start = $node->wait_for_log(qr/reloading configuration files/, $log_start); +$bgconn->quit; # the tests below restart the server + +# +# Test validator-specific HBA options. +# + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all test oauth issuer="$issuer" scope="openid postgres" delegate_ident_mapping=1 \\ + validator.authn_id="ignored" validator.authn_id="other-identity" +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.log="testalt message" +}); + +$node->reload; +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + +$node->connect_ok( + "$common_connstr user=test", + "custom HBA setting (test)", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [qr/connection authenticated: identity="other-identity"/]); +$node->connect_ok( + "$common_connstr user=testalt", + "custom HBA setting (testalt)", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [ + qr/LOG:\s+testalt message/, + qr/connection authenticated: identity="testalt"/, + ]); + +# bad syntax +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.=1 +}); + +$log_start = -s $node->logfile; +$node->restart(fail_ok => 1); +$node->log_check("empty HBA option name", + $log_start, + log_like => [qr/invalid OAuth validator option name: "validator\."/]); + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.@@=1 +}); + +$log_start = -s $node->logfile; +$node->restart(fail_ok => 1); +$node->log_check("invalid HBA option name", + $log_start, + log_like => [qr/invalid OAuth validator option name: "validator\.@@"/]); + +# unknown settings (validation is deferred to connect time) +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" \\ + validator.log=ignored validator.bad=1 +}); +$node->restart; + +$node->connect_fails( + "$common_connstr user=testalt", + "bad HBA setting", + expected_stderr => qr/OAuth bearer authentication failed/, + log_like => [ + qr/WARNING:\s+unrecognized authentication option name: "validator\.bad"/, + qr/FATAL:\s+OAuth bearer authentication failed/, + qr/DETAIL:\s+unrecognized authentication option name: "validator\.bad"/, + ]); + # # Test multiple validators. # diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c index 353e0e0d32a..85fb4c08bf2 100644 --- a/src/test/modules/oauth_validator/validator.c +++ b/src/test/modules/oauth_validator/validator.c @@ -42,13 +42,21 @@ static char *authn_id = NULL; static bool authorize_tokens = true; static char *error_detail = NULL; static bool internal_error = false; +static bool invalid_hba = false; + +/* HBA options */ +static const char *hba_opts[] = { + "authn_id", /* overrides the default authn_id */ + "log", /* logs an arbitrary string */ +}; /*--- * Extension entry point. Sets up GUCs for use by tests: * * - oauth_validator.authn_id Sets the user identifier to return during token * validation. Defaults to the username in the - * startup packet. + * startup packet, or the validator.authn_id HBA + * option if it is set. * * - oauth_validator.authorize_tokens * Sets whether to successfully validate incoming @@ -96,6 +104,14 @@ _PG_init(void) PGC_SIGHUP, 0, NULL, NULL, NULL); + DefineCustomBoolVariable("oauth_validator.invalid_hba", + "Should the validator register an invalid option?", + NULL, + &invalid_hba, + false, + PGC_SIGHUP, + 0, + NULL, NULL, NULL); MarkGUCPrefixReserved("oauth_validator"); } @@ -124,6 +140,29 @@ validator_startup(ValidatorModuleState *state) if (state->sversion != PG_VERSION_NUM) elog(ERROR, "oauth_validator: sversion set to %d", state->sversion); + /* + * Test the behavior of custom HBA options. Registered options should not + * be retrievable during startup (we want to discourage modules from + * relying on the relative order of client connections and the + * startup_cb). + */ + RegisterOAuthHBAOptions(state, lengthof(hba_opts), hba_opts); + for (int i = 0; i < lengthof(hba_opts); i++) + { + if (GetOAuthHBAOption(state, hba_opts[i])) + elog(ERROR, + "oauth_validator: GetOAuthValidatorOption(\"%s\") was non-NULL during startup_cb", + hba_opts[i]); + } + + if (invalid_hba) + { + /* Register a bad option, which should print a WARNING to the logs. */ + const char *invalid = "bad option name"; + + RegisterOAuthHBAOptions(state, 1, &invalid); + } + state->private_data = PRIVATE_COOKIE; } @@ -141,7 +180,7 @@ validator_shutdown(ValidatorModuleState *state) /* * Validator implementation. Logs the incoming data and authorizes the token by - * default; the behavior can be modified via the module's GUC settings. + * default; the behavior can be modified via the module's GUC and HBA settings. */ static bool validate_token(const ValidatorModuleState *state, @@ -153,6 +192,9 @@ validate_token(const ValidatorModuleState *state, elog(ERROR, "oauth_validator: private state cookie changed to %p in validate", state->private_data); + if (GetOAuthHBAOption(state, "log")) + elog(LOG, "%s", GetOAuthHBAOption(state, "log")); + elog(LOG, "oauth_validator: token=\"%s\", role=\"%s\"", token, role); elog(LOG, "oauth_validator: issuer=\"%s\", scope=\"%s\"", MyProcPort->hba->oauth_issuer, @@ -165,6 +207,8 @@ validate_token(const ValidatorModuleState *state, res->authorized = authorize_tokens; if (authn_id) res->authn_id = pstrdup(authn_id); + else if (GetOAuthHBAOption(state, "authn_id")) + res->authn_id = pstrdup(GetOAuthHBAOption(state, "authn_id")); else res->authn_id = pstrdup(role); -- 2.34.1 ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Custom oauth validator options @ 2026-03-23 21:45 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 2 replies; 11+ messages in thread From: Zsolt Parragi @ 2026-03-23 21:45 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > I considered letting this lapse for 19 instead That was also my conclusion. After the discussion in the SNI thread I started working on a PoC for a more modern syntax for hba/ident/hosts, hoping that a generic extensibility/guc patch could be based on that. I also didn't want to start a thread about this before the feature freeze, so I'm still waiting/prototyping for a few weeks. I'm also not against adding an oauth-only feature for 19, that was my original intention before getting completely distracted by the guc direction :) + else if (strncmp(name, "validator.", strlen("validator.")) == 0) + { + const char *key = name + strlen("validator."); This is my only concern with this patch: since we have a list separated validatr names as a GUC already, couldn't we require a <validator_name>. prefix instead of the fixed "validator.", to keep the hba configuration consistent with gucs? Validators would still have to handle these options differently, but at least it would look consistent from the user perspective - global setting in postgresql.conf, same hba-line specific override in pg_hba.conf. (also, validators already added global GUCs in pg18, and this would also keep it consistent with that) + REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); Shouldn't this check go before the name validation? ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Custom oauth validator options @ 2026-03-27 23:03 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 1 sibling, 1 reply; 11+ messages in thread From: Jacob Champion @ 2026-03-27 23:03 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Mon, Mar 23, 2026 at 2:45 PM Zsolt Parragi <[email protected]> wrote: > This is my only concern with this patch: since we have a list > separated validatr names as a GUC already, couldn't we require a > <validator_name>. prefix instead of the fixed "validator.", to keep > the hba configuration consistent with gucs? Well, the `validator.` prefix lets us end-run the namespace issue [1]. It's one thing if I claim that single prefix in parse_hba_auth_opt(); it's another thing if I camp out on literally every identifier containing a dot. I'm also not convinced that it's worth spending additional code here to decide _which_ of the blessed validators is in force for the current line. (Deferring the check of the option names is bad enough, but there appears to be no way around that.) > Validators would still have to handle these options differently, but > at least it would look consistent from the user perspective - global > setting in postgresql.conf, same hba-line specific override in > pg_hba.conf. (also, validators already added global GUCs in pg18, and > this would also keep it consistent with that) After the wild goose chase I sent you on, I think consistency-in-form-but-not-function is more likely to be a liability than a benefit. Sure, validator authors will be able to pretend that users can override particular GUCs per-line, but that's not what's actually happening, so that could increase user confusion and support burden for very little practical upside. (As one example, `SHOW my_validator.setting` isn't going to behave intuitively.) Since my pitch here is "this is an architectural dead end, but it'll get us moving while we pursue the better route," I prefer something that's very obviously bespoke. Especially since validators will have to migrate from the old way to the new way, if we get our wish. I don't really want anyone to spend time resolving the collision of the two behaviors; I'd rather just let the old ugly configuration solution wither (or die), and encourage everyone to switch as rapidly as possible. > + REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); > > Shouldn't this check go before the name validation? Yeah, I agree. (My original code had a more generic error message when the name check failed, but now that the message is OAuth-specific, I don't think it makes sense to pretend that it could belong to any other auth method.) Thanks! --Jacob [1] https://postgr.es/m/CAOYmi%2Bn9%2BVDNayxsZuG30YLxOXrVB2Wu%3DjBR4WrEdJvxjTATKw%40mail.gmail.com ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Custom oauth validator options @ 2026-03-27 23:03 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 1 sibling, 0 replies; 11+ messages in thread From: Jacob Champion @ 2026-03-27 23:03 UTC (permalink / raw) To: Daniel Gustafsson <[email protected]>; +Cc: Zsolt Parragi <[email protected]>; Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Tue, Mar 24, 2026 at 5:29 AM Daniel Gustafsson <[email protected]> wrote: > FWIW I support this line of investigation and look forward to seeing what it > could look like. +many --Jacob ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Custom oauth validator options @ 2026-03-30 21:46 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Zsolt Parragi @ 2026-03-30 21:46 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > (As one example, `SHOW > my_validator.setting` isn't going to behave intuitively.) Right, I didn't think about that scenario, it might be better to keep this intentionally different. That won't be the most user friendly option, but it is still definitely better than not having the ability to configure this. ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Custom oauth validator options @ 2026-03-30 23:54 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Jacob Champion @ 2026-03-30 23:54 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Mon, Mar 30, 2026 at 2:46 PM Zsolt Parragi <[email protected]> wrote: > That won't be the most user friendly option, but it is still > definitely better than not having the ability to configure this. Sounds good. Barring other objections, then, I'll plan to move forward with this approach for PG19. Thanks! --Jacob ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Custom oauth validator options @ 2026-04-02 21:26 Jacob Champion <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Jacob Champion @ 2026-04-02 21:26 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Mon, Mar 30, 2026 at 4:54 PM Jacob Champion <[email protected]> wrote: > Sounds good. Barring other objections, then, I'll plan to move forward > with this approach for PG19. v4-0002 (based on top of [1]) addresses Zsolt's feedback and adds the missing user documentation. I believe this is now a complete patch proposal; tear it apart. :D Thanks, --Jacob [1] https://postgr.es/m/CAOYmi%2BmvFS7Ukmacb1z%3DxWO7M%2BDPF41GZsJiJ6sh4U1Qm_yWOA%40mail.gmail.com Attachments: [application/octet-stream] v4-0001-oauth-Let-validators-provide-failure-DETAILs.patch (12.9K, 2-v4-0001-oauth-Let-validators-provide-failure-DETAILs.patch) download | inline diff: From a9a507604c579ac1bf3c07cd8353ada2fbac1fb8 Mon Sep 17 00:00:00 2001 From: Jacob Champion <[email protected]> Date: Thu, 19 Mar 2026 09:37:20 -0700 Subject: [PATCH v4 1/2] oauth: Let validators provide failure DETAILs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At the moment, the only way for a validator module to report error details on failure is to log them separately before returning from validate_cb. Independently of that problem, the ereport() calls that we make during validation failure partially duplicate some of the work of auth_failed(). The end result is overly verbose and confusing for readers of the logs: [768233] LOG: [my_validator] bad signature in bearer token [768233] LOG: OAuth bearer authentication failed for user "jacob" [768233] DETAIL: Validator failed to authorize the provided token. [768233] FATAL: OAuth bearer authentication failed for user "jacob" [768233] DETAIL: Connection matched file ".../pg_hba.conf" line ... Solve both problems by making use of the existing logdetail pointer that's provided by ClientAuthentication. Validator modules may set ValidatorModuleResult->error_detail to override our default generic message. The end result looks something like [242284] FATAL: OAuth bearer authentication failed for user "jacob" [242284] DETAIL: [my_validator] bad signature in bearer token Connection matched file ".../pg_hba.conf" line ... Reported-by: Álvaro Herrera <[email protected]> Reported-by: Zsolt Parragi <[email protected]> Reviewed-by: Chao Li <[email protected]> Reviewed-by: Daniel Gustafsson <[email protected]> Reviewed-by: Zsolt Parragi <[email protected]> Discussion: https://postgr.es/m/202601241015.y5uvxd7oxnfs%40alvherre.pgsql --- doc/src/sgml/oauth-validators.sgml | 23 ++++++++++- src/include/libpq/oauth.h | 14 +++++++ src/backend/libpq/auth-oauth.c | 24 +++++------ src/backend/libpq/auth.c | 2 +- .../modules/oauth_validator/t/001_server.pl | 40 ++++++++++++++++++- src/test/modules/oauth_validator/validator.c | 29 ++++++++++++++ 6 files changed, 116 insertions(+), 16 deletions(-) diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml index 704089dd7b3..5f29f2be186 100644 --- a/doc/src/sgml/oauth-validators.sgml +++ b/doc/src/sgml/oauth-validators.sgml @@ -192,11 +192,20 @@ <term>Logging</term> <listitem> <para> - Modules may use the same <link linkend="error-message-reporting">logging + To simply log the reason for a validation failure, modules may set the + freeform <structfield>error_detail</structfield> field during the + <link linkend="oauth-validator-callback-validate">validate callback</link>. + (<xref linkend="error-style-guide"/> has guidelines for writing good + <literal>DETAIL</literal> messages.) <structfield>error_detail</structfield> + is printed only to the server log, as part of the final authentication + failure message, and it is not shared with the client. + </para> + <para> + Modules may also use the same <link linkend="error-message-reporting">logging facilities</link> as standard extensions; however, the rules for emitting log entries to the client are subtly different during the authentication phase of the connection. Generally speaking, modules should log - verification problems at the <symbol>COMMERROR</symbol> level and return + problems at the <symbol>COMMERROR</symbol> level and return normally, instead of using <symbol>ERROR</symbol>/<symbol>FATAL</symbol> to unwind the stack, to avoid leaking information to unauthenticated clients. @@ -370,6 +379,7 @@ typedef struct ValidatorModuleResult { bool authorized; char *authn_id; + char *error_detail; } ValidatorModuleResult; </programlisting> @@ -387,6 +397,15 @@ typedef struct ValidatorModuleResult Otherwise the validator should return <literal>true</literal> to indicate that it has processed the token and made an authorization decision. </para> + <para> + In either failure case (validation error or internal error) the module may + store a user-readable reason for the failure in <structfield>result->error_detail</structfield>. + This will be printed to the server logs (not sent to the client) as a + <literal>DETAIL</literal> entry for the authentication failure. The memory + pointed to by <structfield>error_detail</structfield> may be either palloc'd + or of static duration. <structfield>error_detail</structfield> is ignored + on success. + </para> <para> The behavior after <function>validate_cb</function> returns depends on the specific HBA setup. Normally, the <structfield>result->authn_id</structfield> user diff --git a/src/include/libpq/oauth.h b/src/include/libpq/oauth.h index 4a822e9a1f2..60f493acddd 100644 --- a/src/include/libpq/oauth.h +++ b/src/include/libpq/oauth.h @@ -49,6 +49,20 @@ typedef struct ValidatorModuleResult * delegation. See the validator module documentation for details. */ char *authn_id; + + /* + * When validation fails, this may optionally be set to a string + * containing an explanation for the failure. It will be sent to the + * server log only; it is not provided to the client, and it's ignored if + * validation succeeds. + * + * This description will be attached to the final authentication failure + * message in the logs, as a DETAIL, which may be preferable to separate + * ereport() calls that have to be correlated by the reader. + * + * This string may be either of static duration or palloc'd. + */ + char *error_detail; } ValidatorModuleResult; /* diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c index 894efe3c904..6a75b79efbf 100644 --- a/src/backend/libpq/auth-oauth.c +++ b/src/backend/libpq/auth-oauth.c @@ -74,7 +74,7 @@ struct oauth_ctx static char *sanitize_char(char c); static char *parse_kvpairs_for_auth(char **input); static void generate_error_response(struct oauth_ctx *ctx, char **output, int *outputlen); -static bool validate(Port *port, const char *auth); +static bool validate(Port *port, const char *auth, const char **logdetail); /* Constants seen in an OAUTHBEARER client initial response. */ #define KVSEP 0x01 /* separator byte for key/value pairs */ @@ -305,7 +305,7 @@ oauth_exchange(void *opaq, const char *input, int inputlen, ctx->state = OAUTH_STATE_ERROR_DISCOVERY; status = PG_SASL_EXCHANGE_CONTINUE; } - else if (!validate(ctx->port, auth)) + else if (!validate(ctx->port, auth, logdetail)) { generate_error_response(ctx, output, outputlen); @@ -650,7 +650,7 @@ validate_token_format(const char *header) * authorization. Returns true if validation succeeds. */ static bool -validate(Port *port, const char *auth) +validate(Port *port, const char *auth, const char **logdetail) { int map_status; ValidatorModuleResult *ret; @@ -677,7 +677,10 @@ validate(Port *port, const char *auth) { ereport(WARNING, errcode(ERRCODE_INTERNAL_ERROR), - errmsg("internal error in OAuth validator module")); + errmsg("internal error in OAuth validator module"), + ret->error_detail ? errdetail_log("%s", ret->error_detail) : 0); + + *logdetail = ret->error_detail; return false; } @@ -690,10 +693,10 @@ validate(Port *port, const char *auth) if (!ret->authorized) { - ereport(LOG, - errmsg("OAuth bearer authentication failed for user \"%s\"", - port->user_name), - errdetail_log("Validator failed to authorize the provided token.")); + if (ret->error_detail) + *logdetail = ret->error_detail; + else + *logdetail = _("Validator failed to authorize the provided token."); status = false; goto cleanup; @@ -714,10 +717,7 @@ validate(Port *port, const char *auth) /* Make sure the validator authenticated the user. */ if (ret->authn_id == NULL || ret->authn_id[0] == '\0') { - ereport(LOG, - errmsg("OAuth bearer authentication failed for user \"%s\"", - port->user_name), - errdetail_log("Validator provided no identity.")); + *logdetail = _("Validator provided no identity."); status = false; goto cleanup; diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index fdacc060381..47b5eeb8f22 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -639,7 +639,7 @@ ClientAuthentication(Port *port) status = STATUS_OK; break; case uaOAuth: - status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, NULL, + status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, &logdetail, &abandoned); break; } diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index c9c46e63539..ac62555675a 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -544,8 +544,8 @@ $node->connect_fails( expected_stderr => qr/OAuth bearer authentication failed/, log_like => [ qr/connection authenticated: identity=""/, - qr/DETAIL:\s+Validator provided no identity/, qr/FATAL:\s+OAuth bearer authentication failed/, + qr/DETAIL:\s+Validator provided no identity/, ]); # Even if a validator authenticates the user, if the token isn't considered @@ -564,10 +564,48 @@ $node->connect_fails( expected_stderr => qr/OAuth bearer authentication failed/, log_like => [ qr/connection authenticated: identity="test\@example\.org"/, + qr/FATAL:\s+OAuth bearer authentication failed/, qr/DETAIL:\s+Validator failed to authorize the provided token/, + ]); + +# Validators can provide their own explanations. +$bgconn->query_safe( + "ALTER SYSTEM SET oauth_validator.error_detail TO 'something failed'"); +$node->reload; +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + +$node->connect_fails( + "$common_connstr user=test", + "validator must authorize token explicitly (custom logdetail)", + expected_stderr => qr/OAuth bearer authentication failed/, + log_like => [ + qr/connection authenticated: identity="test\@example\.org"/, qr/FATAL:\s+OAuth bearer authentication failed/, + qr/DETAIL:\s+something failed/, ]); +$bgconn->query_safe( + "ALTER SYSTEM SET oauth_validator.internal_error TO true"); +$node->reload; +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + +$node->connect_fails( + "$common_connstr user=test", + "validator internal error (custom logdetail)", + expected_stderr => qr/OAuth bearer authentication failed/, + log_like => [ + qr/WARNING:\s+internal error in OAuth validator module/, + qr/DETAIL:\s+something failed/, + ]); + +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.error_detail"); +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.internal_error"); +$node->reload; +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + # # Test user mapping. # diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c index 0b983a9dc8f..353e0e0d32a 100644 --- a/src/test/modules/oauth_validator/validator.c +++ b/src/test/modules/oauth_validator/validator.c @@ -40,6 +40,8 @@ static const OAuthValidatorCallbacks validator_callbacks = { /* GUCs */ static char *authn_id = NULL; static bool authorize_tokens = true; +static char *error_detail = NULL; +static bool internal_error = false; /*--- * Extension entry point. Sets up GUCs for use by tests: @@ -51,6 +53,13 @@ static bool authorize_tokens = true; * - oauth_validator.authorize_tokens * Sets whether to successfully validate incoming * tokens. Defaults to true. + * + * - oauth_validator.error_detail + * Sets an error message to be included as a + * DETAIL on failure. + * + * - oauth_validator.internal_error + * Reports an internal error to the server. */ void _PG_init(void) @@ -71,6 +80,22 @@ _PG_init(void) PGC_SIGHUP, 0, NULL, NULL, NULL); + DefineCustomStringVariable("oauth_validator.error_detail", + "Error message to print during failures", + NULL, + &error_detail, + NULL, + PGC_SIGHUP, + 0, + NULL, NULL, NULL); + DefineCustomBoolVariable("oauth_validator.internal_error", + "Should the validator report an internal error?", + NULL, + &internal_error, + false, + PGC_SIGHUP, + 0, + NULL, NULL, NULL); MarkGUCPrefixReserved("oauth_validator"); } @@ -133,6 +158,10 @@ validate_token(const ValidatorModuleState *state, MyProcPort->hba->oauth_issuer, MyProcPort->hba->oauth_scope); + res->error_detail = error_detail; /* only relevant for failures */ + if (internal_error) + return false; + res->authorized = authorize_tokens; if (authn_id) res->authn_id = pstrdup(authn_id); -- 2.34.1 [application/octet-stream] v4-0002-oauth-Allow-validators-to-register-custom-HBA-opt.patch (33.9K, 3-v4-0002-oauth-Allow-validators-to-register-custom-HBA-opt.patch) download | inline diff: From 5039ce74370bca19f133bac29fcc220e24cb9b20 Mon Sep 17 00:00:00 2001 From: Jacob Champion <[email protected]> Date: Tue, 17 Mar 2026 12:01:28 -0700 Subject: [PATCH v4 2/2] oauth: Allow validators to register custom HBA options OAuth validators can already use custom GUCs to configure behavior globally, but we currently provide no ability to adjust settings for individual HBA entries, because the original design focused on a world where a provider covered a "single audience" of users for one database cluster. This assumption does not apply to multitenant use cases, where a single validator may be controlling access for wildly different user groups. To improve this use case, add two new API calls for use by validator callbacks: RegisterOAuthHBAOptions() and GetOAuthHBAOption(). Registering options "foo" and "bar" allows a user to set "validator.foo" and "validator.bar" in an oauth HBA entry. These options are stringly typed (syntax validation is solely the responsibility of the defining module), and names are restricted to a subset of ASCII to avoid tying our hands with future HBA syntax improvements. Unfortunately, we can't check the custom option names during a reload of the configuration, like we do with standard HBA options, unless we were to require all validators to be loaded via shared_preload_libraries. (I consider this to be a nonstarter: most validators should probably use session_preload_libraries at most, since requiring a full restart of a production server just to update authentication behavior will be unacceptable to many users.) Instead, the new validator.* options are checked against the registered list at connection time. Multiple alternatives were proposed and/or prototyped, including extending the GUC system to allow per-HBA overrides, joining forces with recent refactoring work on the reloptions subsystem, and giving the ability to customize HBA options to all PostgreSQL extensions. I personally believe per-HBA GUC overrides are the best option, because several existing GUCs like authentication_timeout and pre_auth_delay would fit there usefully. But the recent addition of SNI per-host settings in 4f433025f indicates that a more general solution is needed, and I expect that to take multiple releases' worth of discussion. This compromise patch, then, is intentionally designed to be an architectural dead end: simple to describe, cheap to maintain, and providing just enough functionality to let validators move forward for PG19. The hope is that it will be replaced in the future by a solution that can handle per-host, per-HBA, and other per-context configuration with the same functionality that GUCs provide today. In the meantime, the bulk of the code in this patch consists of strict guardrails on the simple API, to try to ensure that we don't have any reason to regret its existence during its unknown lifespan. Suggested-by: Zsolt Parragi <[email protected]> Suggested-by: VASUKI M <[email protected]> Investigated-by: Zsolt Parragi <[email protected]> Reviewed-by: Zsolt Parragi <[email protected]> Discussion: https://postgr.es/m/CAN4CZFM3b8u5uNNNsY6XCya257u%2BDofms3su9f11iMCxvCacag%40mail.gmail.com --- doc/src/sgml/client-auth.sgml | 39 +++ doc/src/sgml/oauth-validators.sgml | 221 ++++++++++++++++- src/include/libpq/hba.h | 2 + src/include/libpq/oauth.h | 15 +- src/backend/libpq/auth-oauth.c | 225 ++++++++++++++++++ src/backend/libpq/hba.c | 26 ++ .../modules/oauth_validator/t/001_server.pl | 97 ++++++++ src/test/modules/oauth_validator/validator.c | 48 +++- 8 files changed, 667 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index a347ee18980..bca09a80416 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -2532,6 +2532,45 @@ host ... radius radiusservers="server1,server2" radiussecrets="""secret one"","" </listitem> </varlistentry> + <varlistentry> + <term id="auth-oauth-validator-option"> + <literal>validator.<replaceable>option</replaceable></literal> + </term> + <listitem> + <para> + Validator modules may <link linkend="oauth-validator-hba">define</link> + additional configuration options for <literal>oauth</literal> + HBA entries. These validator-specific options are accessible via the + <literal>validator.*</literal> "namespace". For example, a module may + register the <literal>validator.foo</literal> and + <literal>validator.bar</literal> options and define their effects on + authentication. + </para> + <para> + The name, syntax, and behavior of each <replaceable>option</replaceable> + are not determined by <productname>PostgreSQL</productname>; consult the + documentation for the validator module in use. + </para> + <warning> + <para> + A limitation of the current implementation is that unrecognized + <replaceable>option</replaceable> names will not be caught until + connection time. A <literal>pg_ctl reload</literal> will succeed, but + matching connections will fail: +<programlisting> +LOG: connection received: host=[local] +WARNING: unrecognized authentication option name: "validator.bad" +DETAIL: The installed validator module ("my_validator") did not define an option named "bad". +HINT: All OAuth connections matching this line will fail. Correct the option and reload the server configuration. +CONTEXT: line 2 of configuration file "data/pg_hba.conf" +</programlisting> + Use caution when making changes to validator-specific HBA options in + production systems. + </para> + </warning> + </listitem> + </varlistentry> + <varlistentry> <term><literal>map</literal></term> <listitem> diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml index 5f29f2be186..7c9e3dd931a 100644 --- a/doc/src/sgml/oauth-validators.sgml +++ b/doc/src/sgml/oauth-validators.sgml @@ -251,6 +251,11 @@ <symbol>delegate_ident_mapping=1</symbol> mode, and what additional configuration is required in order to do so. </para> + <para> + If an implementation provides <link linkend="oauth-validator-hba">custom + HBA options</link>, the names and syntax of those options should be + documented as well. + </para> </listitem> </varlistentry> </variablelist> @@ -343,7 +348,8 @@ typedef const OAuthValidatorCallbacks *(*OAuthValidatorModuleInit) (void); <title>Startup Callback</title> <para> The <function>startup_cb</function> callback is executed directly after - loading the module. This callback can be used to set up local state and + loading the module. This callback can be used to set up local state, + define <link linkend="oauth-validator-hba">custom HBA options</link>, and perform additional initialization if required. If the validator module has state it can use <structfield>state->private_data</structfield> to store it. @@ -432,4 +438,217 @@ typedef void (*ValidatorShutdownCB) (ValidatorModuleState *state); </sect2> </sect1> + + <sect1 id="oauth-validator-hba"> + <title>Custom HBA Options</title> + + <para> + Like other preloaded libraries, validator modules may define + <link linkend="runtime-config-custom">custom GUC parameters</link> for user + configuration in <filename>postgresql.conf</filename>. However, it may be + desirable to configure behavior at a more granular level (say, for a + particular issuer or a group of users) instead of globally. + </para> + + <para> + Beginning in <productname>PostgreSQL</productname> 19, validator + implementations may define custom options for use inside + <filename>pg_hba.conf</filename>. These options are then + <link linkend="auth-oauth-validator-option">made available</link> to the user + as <literal>validator.<replaceable>option</replaceable></literal>. The API + for registering and retrieving custom options is described below. + </para> + + <sect2 id="oauth-validator-hba-api"> + <title>Options API</title> + <para> + Modules register custom HBA option names during the <function>startup_cb</function> + callback, using <function>RegisterOAuthHBAOptions()</function>: + +<programlisting> +/* + * Register a list of custom option names for use in pg_hba.conf. For each name + * "foo" registered here, that option will be provided as "validator.foo" in + * the HBA. + * + * Valid option names consist of alphanumeric ASCII, underscore (_), and hyphen + * (-). Invalid option names will be ignored with a WARNING logged at + * connection time. + * + * This function may only be called during the startup_cb callback. Multiple + * calls are permitted, which will append to the existing list of registered + * options; options cannot be unregistered. + * + * Parameters: + * + * - state: the state pointer passed to the startup_cb callback + * - num: the number of options in the opts array + * - opts: an array of null-terminated option names to register + * + * The list of option names is copied internally, and the opts array is not + * required to remain valid after the call. + */ +void RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]); +</programlisting> + </para> + + <para> + Each option's value, if set, may be later retrieved using + <function>GetOAuthHBAOption()</function>: + +<programlisting> +/* + * Retrieve the string value of an HBA option which was registered via + * RegisterOAuthHBAOptions(). Usable only during validate_cb or shutdown_cb. + * + * If the user has set the corresponding option in pg_hba.conf, this function + * returns that value as a null-terminated string, which must not be modified + * or freed. NULL is returned instead if the user has not set this option, if + * the option name was not registered, or if this function is incorrectly called + * during the startup_cb. + * + * Parameters: + * + * - state: the state pointer passed to the validate_cb/shutdown_cb callback + * - optname: the name of the option to retrieve + */ +const char *GetOAuthHBAOption(const ValidatorModuleState *state, + const char *optname); +</programlisting> + </para> + + <para> + See <xref linkend="oauth-validator-hba-example-usage"/> for sample usage. + </para> + </sect2> + + <sect2 id="oauth-validator-hba-limitations"> + <title>Limitations</title> + <para> + <itemizedlist> + <listitem> + <para> + Option names are limited to ASCII alphanumeric characters, + underscores (<literal>_</literal>), and hyphens (<literal>-</literal>). + </para> + </listitem> + <listitem> + <para> + Option values are always freeform strings (in contrast to custom GUCs, + which support numerics, booleans, and enums). + </para> + </listitem> + <listitem> + <para> + Option names and values cannot be checked by the server during a reload of + the configuration. Any unregistered options in <filename>pg_hba.conf</filename> + will instead result in connection failures. It is the responsibility of + each module to document and verify the syntax of option values as needed. + <footnote> + <para> + If a module finds an invalid option value during <function>validate_cb</function>, + it's recommended to <link linkend="oauth-validator-callback-validate">signal + an internal error</link> by setting <structfield>result->error_detail</structfield> + to a description of the problem and returning <literal>false</literal>. + </para> + </footnote> + </para> + </listitem> + </itemizedlist> + </para> + </sect2> + + <sect2 id="oauth-validator-hba-example-usage"> + <title>Example Usage</title> + + <para> + For a hypothetical module, the options <literal>foo</literal> and + <literal>bar</literal> could be registered as follows: + +<programlisting> +static void +validator_startup(ValidatorModuleState *state) +{ + static const char *opts[] = { + "foo", /* description of access privileges */ + "bar", /* magic URL for additional administrator powers */ + }; + + RegisterOAuthHBAOptions(state, lengthof(opts), opts); + + /* ...other setup... */ +} +</programlisting> + </para> + + <para> + The following sample entries in <filename>pg_hba.conf</filename> can then + make use of these options: + +<programlisting> +# TYPE DATABASE USER ADDRESS METHOD +hostssl postgres admin 0.0.0.0/0 oauth issuer=https://admin.example.com \ + scope="pg-admin openid email" \ + map=oauth-email \ + validator.foo="admin access" \ + validator.bar=https://magic.example.com + +hostssl postgres all 0.0.0.0/0 oauth issuer=https://www.example.com \ + scope="pg-user openid email" \ + map=oauth-email \ + validator.foo="user access" +</programlisting> + </para> + + <para> + The module can retrieve the option settings from the HBA during validation: + +<programlisting> +static bool +validate_token(const ValidatorModuleState *state, + const char *token, const char *role, + ValidatorModuleResult *res) +{ + const char *foo = GetOAuthHBAOption(state, "foo"); /* "admin access" or "user access" */ + const char *bar = GetOAuthHBAOption(state, "bar"); /* "https://magic.example.com" or NULL */ + + if (bar && !is_valid_url(bar)) + { + res->error_detail = psprintf("validator.bar (\"%s\") is not a valid URL.", bar); + return false; + } + + /* proceed to validate token */ +} +</programlisting> + </para> + + <para> + When multiple validators are in use, their registered option lists remain + independent: + +<programlisting> +<lineannotation>in postgresql.conf:</lineannotation> +oauth_validator_libraries = 'example_org, my_validator' + +<lineannotation>in pg_hba.conf:</lineannotation> +# TYPE DATABASE USER ADDRESS METHOD +hostssl postgres admin 0.0.0.0/0 oauth issuer=https://admin.example.com \ + scope="pg-admin openid email" \ + map=oauth-email \ + validator=my_validator \ + validator.foo="admin access" \ + validator.bar=https://magic.example.com + +hostssl postgres all 0.0.0.0/0 oauth issuer=https://www.example.org \ + scope="pg-user openid profile" \ + validator=example_org \ + delegate_ident_mapping=1 \ + validator.magic=on \ + validator.more_magic=off +</programlisting> + </para> + </sect2> + </sect1> </chapter> diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index c4570ce9b3f..e8898561c8c 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -140,6 +140,8 @@ typedef struct HbaLine char *oauth_scope; char *oauth_validator; bool oauth_skip_usermap; + List *oauth_opt_keys; + List *oauth_opt_vals; } HbaLine; typedef struct IdentLine diff --git a/src/include/libpq/oauth.h b/src/include/libpq/oauth.h index 60f493acddd..86f463a284e 100644 --- a/src/include/libpq/oauth.h +++ b/src/include/libpq/oauth.h @@ -96,6 +96,17 @@ typedef struct OAuthValidatorCallbacks ValidatorValidateCB validate_cb; } OAuthValidatorCallbacks; +/* + * A validator can register a list of custom option names during its startup_cb, + * then later retrieve the user settings for each during validation. This + * enables per-HBA-line configuration. For more information, refer to the OAuth + * validator modules documentation. + */ +extern void RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]); +extern const char *GetOAuthHBAOption(const ValidatorModuleState *state, + const char *optname); + /* * Type of the shared library symbol _PG_oauth_validator_module_init which is * required for all validator modules. This function will be invoked during @@ -107,9 +118,7 @@ extern PGDLLEXPORT const OAuthValidatorCallbacks *_PG_oauth_validator_module_ini /* Implementation */ extern PGDLLIMPORT const pg_be_sasl_mech pg_be_oauth_mech; -/* - * Ensure a validator named in the HBA is permitted by the configuration. - */ extern bool check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg); +extern bool valid_oauth_hba_option_name(const char *name); #endif /* PG_OAUTH_H */ diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c index 6a75b79efbf..ea34ebdb733 100644 --- a/src/backend/libpq/auth-oauth.c +++ b/src/backend/libpq/auth-oauth.c @@ -25,6 +25,7 @@ #include "libpq/hba.h" #include "libpq/oauth.h" #include "libpq/sasl.h" +#include "miscadmin.h" #include "storage/fd.h" #include "storage/ipc.h" #include "utils/json.h" @@ -40,10 +41,15 @@ static int oauth_exchange(void *opaq, const char *input, int inputlen, static void load_validator_library(const char *libname); static void shutdown_validator_library(void *arg); +static bool check_validator_hba_options(Port *port, const char **logdetail); static ValidatorModuleState *validator_module_state; static const OAuthValidatorCallbacks *ValidatorCallbacks; +static MemoryContext ValidatorMemoryContext; +static List *ValidatorOptions; +static bool ValidatorOptionsChecked; + /* Mechanism declaration */ const pg_be_sasl_mech pg_be_oauth_mech = { .get_mechanisms = oauth_get_mechanisms, @@ -109,6 +115,9 @@ oauth_init(Port *port, const char *selected_mech, const char *shadow_pass) errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("client selected an invalid SASL authentication mechanism")); + /* Save our memory context for later use by client API calls. */ + ValidatorMemoryContext = CurrentMemoryContext; + ctx = palloc0_object(struct oauth_ctx); ctx->state = OAUTH_STATE_INIT; @@ -293,6 +302,16 @@ oauth_exchange(void *opaq, const char *input, int inputlen, errmsg("malformed OAUTHBEARER message"), errdetail("Message contains additional data after the final terminator.")); + /* + * Make sure all custom HBA options are understood by the validator before + * continuing, since we couldn't check them during server start/reload. + */ + if (!check_validator_hba_options(ctx->port, logdetail)) + { + ctx->state = OAUTH_STATE_FINISHED; + return PG_SASL_EXCHANGE_FAILURE; + } + if (auth[0] == '\0') { /* @@ -822,6 +841,9 @@ shutdown_validator_library(void *arg) { if (ValidatorCallbacks->shutdown_cb != NULL) ValidatorCallbacks->shutdown_cb(validator_module_state); + + /* The backing memory for this is about to disappear. */ + ValidatorOptions = NIL; } /* @@ -907,3 +929,206 @@ done: return (*err_msg == NULL); } + +/* + * Client APIs for validator implementations + * + * Since we're currently not threaded, we only allow one validator in the + * process at a time. So we can make use of globals for now instead of looking + * up information using the state pointer. We probably shouldn't assume that the + * module hasn't temporarily changed memory contexts on us, though; functions + * here should defensively use an appropriate context when making global + * allocations. + */ + +/* + * Adds to the list of allowed validator.* HBA options. Used during the + * startup_cb. + */ +void +RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]) +{ + MemoryContext oldcontext; + + if (!state) + { + Assert(false); + return; + } + + oldcontext = MemoryContextSwitchTo(ValidatorMemoryContext); + + for (int i = 0; i < num; i++) + { + if (!valid_oauth_hba_option_name(opts[i])) + { + /* + * The user can't set this option in the HBA, so GetOAuthHBAOption + * would always return NULL. + */ + ereport(WARNING, + errmsg("HBA option name \"%s\" is invalid and will be ignored", + opts[i]), + /* translator: the second %s is a function name */ + errcontext("validator module \"%s\", in call to %s", + MyProcPort->hba->oauth_validator, + "RegisterOAuthHBAOptions")); + continue; + } + + ValidatorOptions = lappend(ValidatorOptions, pstrdup(opts[i])); + } + + MemoryContextSwitchTo(oldcontext); + + /* + * Wait to validate the HBA against the registered options until later + * (see check_validator_hba_options()). + * + * Delaying allows the validator to make multiple registration calls, to + * append to the list; it lets us make the check in a place where we can + * report the error without leaking details to the client; and it avoids + * exporting the order of operations between HBA matching and the + * startup_cb call as an API guarantee. (The last issue may become + * relevant with a threaded model.) + */ +} + +/* + * Restrict the names available to custom HBA options, so that we don't + * accidentally prevent future syntax extensions to HBA files. + */ +bool +valid_oauth_hba_option_name(const char *name) +{ + /* + * This list is not incredibly principled, since the goal is just to bound + * compatibility guarantees for our HBA parser. Alphanumerics seem + * obviously fine, and it's difficult to argue against the punctuation + * that's already included in some HBA option names and identifiers. + */ + static const char *name_allowed_set = + "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789_-"; + + size_t span; + + if (!name[0]) + return false; + + span = strspn(name, name_allowed_set); + return name[span] == '\0'; +} + +/* + * Verifies that all validator.* HBA options specified by the user were actually + * registered by the validator library in use. + */ +static bool +check_validator_hba_options(Port *port, const char **logdetail) +{ + HbaLine *hba = port->hba; + + foreach_ptr(char, key, hba->oauth_opt_keys) + { + bool found = false; + + /* O(n^2) shouldn't be a problem here in practice. */ + foreach_ptr(char, optname, ValidatorOptions) + { + if (strcmp(key, optname) == 0) + { + found = true; + break; + } + } + + if (!found) + { + /* + * Unknown option name. Mirror the error messages in hba.c here, + * keeping in mind that the original "validator." prefix was + * stripped from the key during parsing. + * + * Since this is affecting live connections, which is unusual for + * HBA, be noisy with a WARNING. (Warnings aren't sent to clients + * prior to successful authentication, so this won't disclose the + * server config.) It'll duplicate some of the information in the + * logdetail, but that should make it hard to miss the connection + * between the two. + */ + char *name = psprintf("validator.%s", key); + + *logdetail = psprintf(_("unrecognized authentication option name: \"%s\""), + name); + ereport(WARNING, + errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("unrecognized authentication option name: \"%s\"", + name), + /* translator: the first %s is the name of the module */ + errdetail("The installed validator module (\"%s\") did not define an option named \"%s\".", + hba->oauth_validator, key), + errhint("All OAuth connections matching this line will fail. Correct the option and reload the server configuration."), + errcontext("line %d of configuration file \"%s\"", + hba->linenumber, hba->sourcefile)); + + return false; + } + } + + ValidatorOptionsChecked = true; /* unfetter GetOAuthHBAOption() */ + return true; +} + +/* + * Retrieves the setting for a validator.* HBA option, or NULL if not found. + * This may only be used during the validate_cb and shutdown_cb. + */ +const char * +GetOAuthHBAOption(const ValidatorModuleState *state, const char *optname) +{ + HbaLine *hba = MyProcPort->hba; + ListCell *lc_k; + ListCell *lc_v; + const char *ret = NULL; + + if (!ValidatorOptionsChecked) + { + /* + * Prevent the startup_cb from retrieving HBA options that it has just + * registered. This probably seems strange -- why refuse to hand out + * information we already know? -- but this lets us reserve the + * ability to perform the startup_cb call earlier, before we know + * which HBA line is matched by a connection, without breaking this + * API. + */ + return NULL; + } + + if (!state || !hba) + { + Assert(false); + return NULL; + } + + Assert(list_length(hba->oauth_opt_keys) == list_length(hba->oauth_opt_vals)); + + forboth(lc_k, hba->oauth_opt_keys, lc_v, hba->oauth_opt_vals) + { + const char *key = lfirst(lc_k); + const char *val = lfirst(lc_v); + + if (strcmp(key, optname) == 0) + { + /* + * Don't return yet -- when regular HBA options are specified more + * than once, the last one wins. Do the same for these options. + */ + ret = val; + } + } + + return ret; +} diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 87ee541e880..7694506aaf7 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -2497,6 +2497,32 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, REQUIRE_AUTH_OPTION(uaOAuth, "validator", "oauth"); hbaline->oauth_validator = pstrdup(val); } + else if (strncmp(name, "validator.", strlen("validator.")) == 0) + { + const char *key = name + strlen("validator."); + + REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); + + /* + * Validator modules may register their own per-HBA-line options. + * Unfortunately, since we don't want to require these modules to be + * loaded into the postmaster, we don't know if the options are valid + * yet and must store them for later. Perform only a basic syntax + * check here. + */ + if (!valid_oauth_hba_option_name(key)) + { + ereport(elevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("invalid OAuth validator option name: \"%s\"", name), + errcontext("line %d of configuration file \"%s\"", + line_num, file_name))); + return false; + } + + hbaline->oauth_opt_keys = lappend(hbaline->oauth_opt_keys, pstrdup(key)); + hbaline->oauth_opt_vals = lappend(hbaline->oauth_opt_vals, pstrdup(val)); + } else if (strcmp(name, "delegate_ident_mapping") == 0) { REQUIRE_AUTH_OPTION(uaOAuth, "delegate_ident_mapping", "oauth"); diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index ac62555675a..cb326b51597 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -602,10 +602,29 @@ $node->connect_fails( $bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.error_detail"); $bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.internal_error"); + +# We complain when bad option names are registered, but connections may proceed +# (since users can't set those options in the HBA anyway). +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.authn_id"); +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.authorize_tokens"); +$bgconn->query_safe("ALTER SYSTEM SET oauth_validator.invalid_hba TO true"); + $node->reload; $log_start = $node->wait_for_log(qr/reloading configuration files/, $log_start); +$node->connect_ok( + "$common_connstr user=test", + "bad registered HBA option", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [ + qr/WARNING:\s+HBA option name "bad option name" is invalid and will be ignored/, + qr/CONTEXT:\s+validator module "validator", in call to RegisterOAuthHBAOptions/, + ]); + +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.invalid_hba"); + # # Test user mapping. # @@ -674,6 +693,84 @@ $node->reload; $log_start = $node->wait_for_log(qr/reloading configuration files/, $log_start); +$bgconn->quit; # the tests below restart the server + +# +# Test validator-specific HBA options. +# + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all test oauth issuer="$issuer" scope="openid postgres" delegate_ident_mapping=1 \\ + validator.authn_id="ignored" validator.authn_id="other-identity" +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.log="testalt message" +}); + +$node->reload; +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + +$node->connect_ok( + "$common_connstr user=test", + "custom HBA setting (test)", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [qr/connection authenticated: identity="other-identity"/]); +$node->connect_ok( + "$common_connstr user=testalt", + "custom HBA setting (testalt)", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [ + qr/LOG:\s+testalt message/, + qr/connection authenticated: identity="testalt"/, + ]); + +# bad syntax +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.=1 +}); + +$log_start = -s $node->logfile; +$node->restart(fail_ok => 1); +$node->log_check("empty HBA option name", + $log_start, + log_like => [qr/invalid OAuth validator option name: "validator\."/]); + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.@@=1 +}); + +$log_start = -s $node->logfile; +$node->restart(fail_ok => 1); +$node->log_check("invalid HBA option name", + $log_start, + log_like => [qr/invalid OAuth validator option name: "validator\.@@"/]); + +# unknown settings (validation is deferred to connect time) +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" \\ + validator.log=ignored validator.bad=1 +}); +$node->restart; + +$node->connect_fails( + "$common_connstr user=testalt", + "bad HBA setting", + expected_stderr => qr/OAuth bearer authentication failed/, + log_like => [ + qr/WARNING:\s+unrecognized authentication option name: "validator\.bad"/, + qr/FATAL:\s+OAuth bearer authentication failed/, + qr/DETAIL:\s+unrecognized authentication option name: "validator\.bad"/, + ]); + # # Test multiple validators. # diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c index 353e0e0d32a..85fb4c08bf2 100644 --- a/src/test/modules/oauth_validator/validator.c +++ b/src/test/modules/oauth_validator/validator.c @@ -42,13 +42,21 @@ static char *authn_id = NULL; static bool authorize_tokens = true; static char *error_detail = NULL; static bool internal_error = false; +static bool invalid_hba = false; + +/* HBA options */ +static const char *hba_opts[] = { + "authn_id", /* overrides the default authn_id */ + "log", /* logs an arbitrary string */ +}; /*--- * Extension entry point. Sets up GUCs for use by tests: * * - oauth_validator.authn_id Sets the user identifier to return during token * validation. Defaults to the username in the - * startup packet. + * startup packet, or the validator.authn_id HBA + * option if it is set. * * - oauth_validator.authorize_tokens * Sets whether to successfully validate incoming @@ -96,6 +104,14 @@ _PG_init(void) PGC_SIGHUP, 0, NULL, NULL, NULL); + DefineCustomBoolVariable("oauth_validator.invalid_hba", + "Should the validator register an invalid option?", + NULL, + &invalid_hba, + false, + PGC_SIGHUP, + 0, + NULL, NULL, NULL); MarkGUCPrefixReserved("oauth_validator"); } @@ -124,6 +140,29 @@ validator_startup(ValidatorModuleState *state) if (state->sversion != PG_VERSION_NUM) elog(ERROR, "oauth_validator: sversion set to %d", state->sversion); + /* + * Test the behavior of custom HBA options. Registered options should not + * be retrievable during startup (we want to discourage modules from + * relying on the relative order of client connections and the + * startup_cb). + */ + RegisterOAuthHBAOptions(state, lengthof(hba_opts), hba_opts); + for (int i = 0; i < lengthof(hba_opts); i++) + { + if (GetOAuthHBAOption(state, hba_opts[i])) + elog(ERROR, + "oauth_validator: GetOAuthValidatorOption(\"%s\") was non-NULL during startup_cb", + hba_opts[i]); + } + + if (invalid_hba) + { + /* Register a bad option, which should print a WARNING to the logs. */ + const char *invalid = "bad option name"; + + RegisterOAuthHBAOptions(state, 1, &invalid); + } + state->private_data = PRIVATE_COOKIE; } @@ -141,7 +180,7 @@ validator_shutdown(ValidatorModuleState *state) /* * Validator implementation. Logs the incoming data and authorizes the token by - * default; the behavior can be modified via the module's GUC settings. + * default; the behavior can be modified via the module's GUC and HBA settings. */ static bool validate_token(const ValidatorModuleState *state, @@ -153,6 +192,9 @@ validate_token(const ValidatorModuleState *state, elog(ERROR, "oauth_validator: private state cookie changed to %p in validate", state->private_data); + if (GetOAuthHBAOption(state, "log")) + elog(LOG, "%s", GetOAuthHBAOption(state, "log")); + elog(LOG, "oauth_validator: token=\"%s\", role=\"%s\"", token, role); elog(LOG, "oauth_validator: issuer=\"%s\", scope=\"%s\"", MyProcPort->hba->oauth_issuer, @@ -165,6 +207,8 @@ validate_token(const ValidatorModuleState *state, res->authorized = authorize_tokens; if (authn_id) res->authn_id = pstrdup(authn_id); + else if (GetOAuthHBAOption(state, "authn_id")) + res->authn_id = pstrdup(GetOAuthHBAOption(state, "authn_id")); else res->authn_id = pstrdup(role); -- 2.34.1 [application/octet-stream] since-v3.nocfbot.diff (21.7K, 4-since-v3.nocfbot.diff) download | inline diff: 1: f36e6becc34 ! 1: a9a507604c5 oauth: Let validators provide failure DETAILs @@ Commit message Reported-by: Álvaro Herrera <[email protected]> Reported-by: Zsolt Parragi <[email protected]> + Reviewed-by: Chao Li <[email protected]> + Reviewed-by: Daniel Gustafsson <[email protected]> + Reviewed-by: Zsolt Parragi <[email protected]> Discussion: https://postgr.es/m/202601241015.y5uvxd7oxnfs%40alvherre.pgsql - Discussion: TODO ## doc/src/sgml/oauth-validators.sgml ## @@ @@ doc/src/sgml/oauth-validators.sgml <listitem> <para> - Modules may use the same <link linkend="error-message-reporting">logging -+ To simply log the reason for a validation failure, validators may set -+ the freeform <structfield>error_detail</structfield> field during the -+ <xref linkend="oauth-validator-callback-validate"/>. This is printed only -+ to the server log, as part of the final authentication failure message, -+ and it is not shared with the client. ++ To simply log the reason for a validation failure, modules may set the ++ freeform <structfield>error_detail</structfield> field during the ++ <link linkend="oauth-validator-callback-validate">validate callback</link>. ++ (<xref linkend="error-style-guide"/> has guidelines for writing good ++ <literal>DETAIL</literal> messages.) <structfield>error_detail</structfield> ++ is printed only to the server log, as part of the final authentication ++ failure message, and it is not shared with the client. + </para> + <para> + Modules may also use the same <link linkend="error-message-reporting">logging @@ src/backend/libpq/auth-oauth.c: struct oauth_ctx /* Constants seen in an OAUTHBEARER client initial response. */ #define KVSEP 0x01 /* separator byte for key/value pairs */ @@ src/backend/libpq/auth-oauth.c: oauth_exchange(void *opaq, const char *input, int inputlen, - errmsg("malformed OAUTHBEARER message"), - errdetail("Message contains additional data after the final terminator.")); - -- if (!validate(ctx->port, auth)) -+ if (!validate(ctx->port, auth, logdetail)) + ctx->state = OAUTH_STATE_ERROR_DISCOVERY; + status = PG_SASL_EXCHANGE_CONTINUE; + } +- else if (!validate(ctx->port, auth)) ++ else if (!validate(ctx->port, auth, logdetail)) { generate_error_response(ctx, output, outputlen); @@ src/backend/libpq/auth.c: ClientAuthentication(Port *port) status = STATUS_OK; break; case uaOAuth: -- status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, NULL); -+ status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, &logdetail); +- status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, NULL, ++ status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, &logdetail, + &abandoned); break; } - ## src/test/modules/oauth_validator/t/001_server.pl ## @@ src/test/modules/oauth_validator/t/001_server.pl: $node->connect_fails( 2: 9726ac39442 ! 2: 5039ce74370 WIP: oauth: Allow validators to register custom HBA options @@ Metadata Author: Jacob Champion <[email protected]> ## Commit message ## - WIP: oauth: Allow validators to register custom HBA options + oauth: Allow validators to register custom HBA options - (lacks user documentation) + OAuth validators can already use custom GUCs to configure behavior + globally, but we currently provide no ability to adjust settings for + individual HBA entries, because the original design focused on a world + where a provider covered a "single audience" of users for one database + cluster. This assumption does not apply to multitenant use cases, where + a single validator may be controlling access for wildly different user + groups. - Two new API entry points for validator callbacks: - - RegisterOAuthHBAOptions - - GetOAuthHBAOption + To improve this use case, add two new API calls for use by validator + callbacks: RegisterOAuthHBAOptions() and GetOAuthHBAOption(). + Registering options "foo" and "bar" allows a user to set "validator.foo" + and "validator.bar" in an oauth HBA entry. These options are stringly + typed (syntax validation is solely the responsibility of the defining + module), and names are restricted to a subset of ASCII to avoid tying + our hands with future HBA syntax improvements. - Registering options "foo" and "bar" allows a user to set validator.foo - and validator.bar on an `oauth` HBA line. + Unfortunately, we can't check the custom option names during a reload of + the configuration, like we do with standard HBA options, unless we were + to require all validators to be loaded via shared_preload_libraries. + (I consider this to be a nonstarter: most validators should probably use + session_preload_libraries at most, since requiring a full restart of a + production server just to update authentication behavior will be + unacceptable to many users.) Instead, the new validator.* options are + checked against the registered list at connection time. - The bulk of the patch is not the conceptually simple API implementation, - but guardrails on the simple API to make sure it doesn't bind our hands - in the future, either for callback architecture or HBA syntax. + Multiple alternatives were proposed and/or prototyped, including + extending the GUC system to allow per-HBA overrides, joining forces with + recent refactoring work on the reloptions subsystem, and giving the + ability to customize HBA options to all PostgreSQL extensions. I + personally believe per-HBA GUC overrides are the best option, because + several existing GUCs like authentication_timeout and pre_auth_delay + would fit there usefully. But the recent addition of SNI per-host + settings in 4f433025f indicates that a more general solution is needed, + and I expect that to take multiple releases' worth of discussion. + + This compromise patch, then, is intentionally designed to be an + architectural dead end: simple to describe, cheap to maintain, and + providing just enough functionality to let validators move forward for + PG19. The hope is that it will be replaced in the future by a solution + that can handle per-host, per-HBA, and other per-context configuration + with the same functionality that GUCs provide today. In the meantime, + the bulk of the code in this patch consists of strict guardrails on the + simple API, to try to ensure that we don't have any reason to regret its + existence during its unknown lifespan. Suggested-by: Zsolt Parragi <[email protected]> Suggested-by: VASUKI M <[email protected]> Investigated-by: Zsolt Parragi <[email protected]> + Reviewed-by: Zsolt Parragi <[email protected]> + Discussion: https://postgr.es/m/CAN4CZFM3b8u5uNNNsY6XCya257u%2BDofms3su9f11iMCxvCacag%40mail.gmail.com + + ## doc/src/sgml/client-auth.sgml ## +@@ doc/src/sgml/client-auth.sgml: host ... radius radiusservers="server1,server2" radiussecrets="""secret one"","" + </listitem> + </varlistentry> + ++ <varlistentry> ++ <term id="auth-oauth-validator-option"> ++ <literal>validator.<replaceable>option</replaceable></literal> ++ </term> ++ <listitem> ++ <para> ++ Validator modules may <link linkend="oauth-validator-hba">define</link> ++ additional configuration options for <literal>oauth</literal> ++ HBA entries. These validator-specific options are accessible via the ++ <literal>validator.*</literal> "namespace". For example, a module may ++ register the <literal>validator.foo</literal> and ++ <literal>validator.bar</literal> options and define their effects on ++ authentication. ++ </para> ++ <para> ++ The name, syntax, and behavior of each <replaceable>option</replaceable> ++ are not determined by <productname>PostgreSQL</productname>; consult the ++ documentation for the validator module in use. ++ </para> ++ <warning> ++ <para> ++ A limitation of the current implementation is that unrecognized ++ <replaceable>option</replaceable> names will not be caught until ++ connection time. A <literal>pg_ctl reload</literal> will succeed, but ++ matching connections will fail: ++<programlisting> ++LOG: connection received: host=[local] ++WARNING: unrecognized authentication option name: "validator.bad" ++DETAIL: The installed validator module ("my_validator") did not define an option named "bad". ++HINT: All OAuth connections matching this line will fail. Correct the option and reload the server configuration. ++CONTEXT: line 2 of configuration file "data/pg_hba.conf" ++</programlisting> ++ Use caution when making changes to validator-specific HBA options in ++ production systems. ++ </para> ++ </warning> ++ </listitem> ++ </varlistentry> ++ + <varlistentry> + <term><literal>map</literal></term> + <listitem> + + ## doc/src/sgml/oauth-validators.sgml ## +@@ + <symbol>delegate_ident_mapping=1</symbol> mode, and what additional + configuration is required in order to do so. + </para> ++ <para> ++ If an implementation provides <link linkend="oauth-validator-hba">custom ++ HBA options</link>, the names and syntax of those options should be ++ documented as well. ++ </para> + </listitem> + </varlistentry> + </variablelist> +@@ doc/src/sgml/oauth-validators.sgml: typedef const OAuthValidatorCallbacks *(*OAuthValidatorModuleInit) (void); + <title>Startup Callback</title> + <para> + The <function>startup_cb</function> callback is executed directly after +- loading the module. This callback can be used to set up local state and ++ loading the module. This callback can be used to set up local state, ++ define <link linkend="oauth-validator-hba">custom HBA options</link>, and + perform additional initialization if required. If the validator module + has state it can use <structfield>state->private_data</structfield> to + store it. +@@ doc/src/sgml/oauth-validators.sgml: typedef void (*ValidatorShutdownCB) (ValidatorModuleState *state); + </sect2> + + </sect1> ++ ++ <sect1 id="oauth-validator-hba"> ++ <title>Custom HBA Options</title> ++ ++ <para> ++ Like other preloaded libraries, validator modules may define ++ <link linkend="runtime-config-custom">custom GUC parameters</link> for user ++ configuration in <filename>postgresql.conf</filename>. However, it may be ++ desirable to configure behavior at a more granular level (say, for a ++ particular issuer or a group of users) instead of globally. ++ </para> ++ ++ <para> ++ Beginning in <productname>PostgreSQL</productname> 19, validator ++ implementations may define custom options for use inside ++ <filename>pg_hba.conf</filename>. These options are then ++ <link linkend="auth-oauth-validator-option">made available</link> to the user ++ as <literal>validator.<replaceable>option</replaceable></literal>. The API ++ for registering and retrieving custom options is described below. ++ </para> ++ ++ <sect2 id="oauth-validator-hba-api"> ++ <title>Options API</title> ++ <para> ++ Modules register custom HBA option names during the <function>startup_cb</function> ++ callback, using <function>RegisterOAuthHBAOptions()</function>: ++ ++<programlisting> ++/* ++ * Register a list of custom option names for use in pg_hba.conf. For each name ++ * "foo" registered here, that option will be provided as "validator.foo" in ++ * the HBA. ++ * ++ * Valid option names consist of alphanumeric ASCII, underscore (_), and hyphen ++ * (-). Invalid option names will be ignored with a WARNING logged at ++ * connection time. ++ * ++ * This function may only be called during the startup_cb callback. Multiple ++ * calls are permitted, which will append to the existing list of registered ++ * options; options cannot be unregistered. ++ * ++ * Parameters: ++ * ++ * - state: the state pointer passed to the startup_cb callback ++ * - num: the number of options in the opts array ++ * - opts: an array of null-terminated option names to register ++ * ++ * The list of option names is copied internally, and the opts array is not ++ * required to remain valid after the call. ++ */ ++void RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, ++ const char *opts[]); ++</programlisting> ++ </para> ++ ++ <para> ++ Each option's value, if set, may be later retrieved using ++ <function>GetOAuthHBAOption()</function>: ++ ++<programlisting> ++/* ++ * Retrieve the string value of an HBA option which was registered via ++ * RegisterOAuthHBAOptions(). Usable only during validate_cb or shutdown_cb. ++ * ++ * If the user has set the corresponding option in pg_hba.conf, this function ++ * returns that value as a null-terminated string, which must not be modified ++ * or freed. NULL is returned instead if the user has not set this option, if ++ * the option name was not registered, or if this function is incorrectly called ++ * during the startup_cb. ++ * ++ * Parameters: ++ * ++ * - state: the state pointer passed to the validate_cb/shutdown_cb callback ++ * - optname: the name of the option to retrieve ++ */ ++const char *GetOAuthHBAOption(const ValidatorModuleState *state, ++ const char *optname); ++</programlisting> ++ </para> ++ ++ <para> ++ See <xref linkend="oauth-validator-hba-example-usage"/> for sample usage. ++ </para> ++ </sect2> ++ ++ <sect2 id="oauth-validator-hba-limitations"> ++ <title>Limitations</title> ++ <para> ++ <itemizedlist> ++ <listitem> ++ <para> ++ Option names are limited to ASCII alphanumeric characters, ++ underscores (<literal>_</literal>), and hyphens (<literal>-</literal>). ++ </para> ++ </listitem> ++ <listitem> ++ <para> ++ Option values are always freeform strings (in contrast to custom GUCs, ++ which support numerics, booleans, and enums). ++ </para> ++ </listitem> ++ <listitem> ++ <para> ++ Option names and values cannot be checked by the server during a reload of ++ the configuration. Any unregistered options in <filename>pg_hba.conf</filename> ++ will instead result in connection failures. It is the responsibility of ++ each module to document and verify the syntax of option values as needed. ++ <footnote> ++ <para> ++ If a module finds an invalid option value during <function>validate_cb</function>, ++ it's recommended to <link linkend="oauth-validator-callback-validate">signal ++ an internal error</link> by setting <structfield>result->error_detail</structfield> ++ to a description of the problem and returning <literal>false</literal>. ++ </para> ++ </footnote> ++ </para> ++ </listitem> ++ </itemizedlist> ++ </para> ++ </sect2> ++ ++ <sect2 id="oauth-validator-hba-example-usage"> ++ <title>Example Usage</title> ++ ++ <para> ++ For a hypothetical module, the options <literal>foo</literal> and ++ <literal>bar</literal> could be registered as follows: ++ ++<programlisting> ++static void ++validator_startup(ValidatorModuleState *state) ++{ ++ static const char *opts[] = { ++ "foo", /* description of access privileges */ ++ "bar", /* magic URL for additional administrator powers */ ++ }; ++ ++ RegisterOAuthHBAOptions(state, lengthof(opts), opts); ++ ++ /* ...other setup... */ ++} ++</programlisting> ++ </para> ++ ++ <para> ++ The following sample entries in <filename>pg_hba.conf</filename> can then ++ make use of these options: ++ ++<programlisting> ++# TYPE DATABASE USER ADDRESS METHOD ++hostssl postgres admin 0.0.0.0/0 oauth issuer=https://admin.example.com \ ++ scope="pg-admin openid email" \ ++ map=oauth-email \ ++ validator.foo="admin access" \ ++ validator.bar=https://magic.example.com ++ ++hostssl postgres all 0.0.0.0/0 oauth issuer=https://www.example.com \ ++ scope="pg-user openid email" \ ++ map=oauth-email \ ++ validator.foo="user access" ++</programlisting> ++ </para> ++ ++ <para> ++ The module can retrieve the option settings from the HBA during validation: ++ ++<programlisting> ++static bool ++validate_token(const ValidatorModuleState *state, ++ const char *token, const char *role, ++ ValidatorModuleResult *res) ++{ ++ const char *foo = GetOAuthHBAOption(state, "foo"); /* "admin access" or "user access" */ ++ const char *bar = GetOAuthHBAOption(state, "bar"); /* "https://magic.example.com" or NULL */ ++ ++ if (bar && !is_valid_url(bar)) ++ { ++ res->error_detail = psprintf("validator.bar (\"%s\") is not a valid URL.", bar); ++ return false; ++ } ++ ++ /* proceed to validate token */ ++} ++</programlisting> ++ </para> ++ ++ <para> ++ When multiple validators are in use, their registered option lists remain ++ independent: ++ ++<programlisting> ++<lineannotation>in postgresql.conf:</lineannotation> ++oauth_validator_libraries = 'example_org, my_validator' ++ ++<lineannotation>in pg_hba.conf:</lineannotation> ++# TYPE DATABASE USER ADDRESS METHOD ++hostssl postgres admin 0.0.0.0/0 oauth issuer=https://admin.example.com \ ++ scope="pg-admin openid email" \ ++ map=oauth-email \ ++ validator=my_validator \ ++ validator.foo="admin access" \ ++ validator.bar=https://magic.example.com ++ ++hostssl postgres all 0.0.0.0/0 oauth issuer=https://www.example.org \ ++ scope="pg-user openid profile" \ ++ validator=example_org \ ++ delegate_ident_mapping=1 \ ++ validator.magic=on \ ++ validator.more_magic=off ++</programlisting> ++ </para> ++ </sect2> ++ </sect1> + </chapter> ## src/include/libpq/hba.h ## @@ src/include/libpq/hba.h: typedef struct HbaLine @@ src/backend/libpq/auth-oauth.c: oauth_exchange(void *opaq, const char *input, in + return PG_SASL_EXCHANGE_FAILURE; + } + - if (!validate(ctx->port, auth, logdetail)) + if (auth[0] == '\0') { - generate_error_response(ctx, output, outputlen); + /* @@ src/backend/libpq/auth-oauth.c: shutdown_validator_library(void *arg) { if (ValidatorCallbacks->shutdown_cb != NULL) @@ src/backend/libpq/auth-oauth.c: done: + if (!found) + { + /* -+ * Bad option name. Mirror the error messages in hba.c here, ++ * Unknown option name. Mirror the error messages in hba.c here, + * keeping in mind that the original "validator." prefix was + * stripped from the key during parsing. + * @@ src/backend/libpq/hba.c: parse_hba_auth_opt(char *name, char *val, HbaLine *hbal + { + const char *key = name + strlen("validator."); + ++ REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); ++ + /* + * Validator modules may register their own per-HBA-line options. + * Unfortunately, since we don't want to require these modules to be @@ src/backend/libpq/hba.c: parse_hba_auth_opt(char *name, char *val, HbaLine *hbal + return false; + } + -+ REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); + hbaline->oauth_opt_keys = lappend(hbaline->oauth_opt_keys, pstrdup(key)); + hbaline->oauth_opt_vals = lappend(hbaline->oauth_opt_vals, pstrdup(val)); + } ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Custom oauth validator options @ 2026-04-03 23:33 Jacob Champion <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Jacob Champion @ 2026-04-03 23:33 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Thu, Apr 2, 2026 at 2:26 PM Jacob Champion <[email protected]> wrote: > v4-0002 (based on top of [1]) addresses Zsolt's feedback and adds the > missing user documentation. I believe this is now a complete patch > proposal; tear it apart. :D v5 is a quick rebase to get rid of v4-0001; no other changes. --Jacob Attachments: [application/octet-stream] v5-0001-oauth-Allow-validators-to-register-custom-HBA-opt.patch (33.9K, 2-v5-0001-oauth-Allow-validators-to-register-custom-HBA-opt.patch) download | inline diff: From 03ac60629e1346195018dc2112b778b805931013 Mon Sep 17 00:00:00 2001 From: Jacob Champion <[email protected]> Date: Tue, 17 Mar 2026 12:01:28 -0700 Subject: [PATCH v5] oauth: Allow validators to register custom HBA options OAuth validators can already use custom GUCs to configure behavior globally, but we currently provide no ability to adjust settings for individual HBA entries, because the original design focused on a world where a provider covered a "single audience" of users for one database cluster. This assumption does not apply to multitenant use cases, where a single validator may be controlling access for wildly different user groups. To improve this use case, add two new API calls for use by validator callbacks: RegisterOAuthHBAOptions() and GetOAuthHBAOption(). Registering options "foo" and "bar" allows a user to set "validator.foo" and "validator.bar" in an oauth HBA entry. These options are stringly typed (syntax validation is solely the responsibility of the defining module), and names are restricted to a subset of ASCII to avoid tying our hands with future HBA syntax improvements. Unfortunately, we can't check the custom option names during a reload of the configuration, like we do with standard HBA options, unless we were to require all validators to be loaded via shared_preload_libraries. (I consider this to be a nonstarter: most validators should probably use session_preload_libraries at most, since requiring a full restart of a production server just to update authentication behavior will be unacceptable to many users.) Instead, the new validator.* options are checked against the registered list at connection time. Multiple alternatives were proposed and/or prototyped, including extending the GUC system to allow per-HBA overrides, joining forces with recent refactoring work on the reloptions subsystem, and giving the ability to customize HBA options to all PostgreSQL extensions. I personally believe per-HBA GUC overrides are the best option, because several existing GUCs like authentication_timeout and pre_auth_delay would fit there usefully. But the recent addition of SNI per-host settings in 4f433025f indicates that a more general solution is needed, and I expect that to take multiple releases' worth of discussion. This compromise patch, then, is intentionally designed to be an architectural dead end: simple to describe, cheap to maintain, and providing just enough functionality to let validators move forward for PG19. The hope is that it will be replaced in the future by a solution that can handle per-host, per-HBA, and other per-context configuration with the same functionality that GUCs provide today. In the meantime, the bulk of the code in this patch consists of strict guardrails on the simple API, to try to ensure that we don't have any reason to regret its existence during its unknown lifespan. Suggested-by: Zsolt Parragi <[email protected]> Suggested-by: VASUKI M <[email protected]> Investigated-by: Zsolt Parragi <[email protected]> Reviewed-by: Zsolt Parragi <[email protected]> Discussion: https://postgr.es/m/CAN4CZFM3b8u5uNNNsY6XCya257u%2BDofms3su9f11iMCxvCacag%40mail.gmail.com --- doc/src/sgml/client-auth.sgml | 39 +++ doc/src/sgml/oauth-validators.sgml | 221 ++++++++++++++++- src/include/libpq/hba.h | 2 + src/include/libpq/oauth.h | 15 +- src/backend/libpq/auth-oauth.c | 225 ++++++++++++++++++ src/backend/libpq/hba.c | 26 ++ .../modules/oauth_validator/t/001_server.pl | 97 ++++++++ src/test/modules/oauth_validator/validator.c | 48 +++- 8 files changed, 667 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index a347ee18980..bca09a80416 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -2532,6 +2532,45 @@ host ... radius radiusservers="server1,server2" radiussecrets="""secret one"","" </listitem> </varlistentry> + <varlistentry> + <term id="auth-oauth-validator-option"> + <literal>validator.<replaceable>option</replaceable></literal> + </term> + <listitem> + <para> + Validator modules may <link linkend="oauth-validator-hba">define</link> + additional configuration options for <literal>oauth</literal> + HBA entries. These validator-specific options are accessible via the + <literal>validator.*</literal> "namespace". For example, a module may + register the <literal>validator.foo</literal> and + <literal>validator.bar</literal> options and define their effects on + authentication. + </para> + <para> + The name, syntax, and behavior of each <replaceable>option</replaceable> + are not determined by <productname>PostgreSQL</productname>; consult the + documentation for the validator module in use. + </para> + <warning> + <para> + A limitation of the current implementation is that unrecognized + <replaceable>option</replaceable> names will not be caught until + connection time. A <literal>pg_ctl reload</literal> will succeed, but + matching connections will fail: +<programlisting> +LOG: connection received: host=[local] +WARNING: unrecognized authentication option name: "validator.bad" +DETAIL: The installed validator module ("my_validator") did not define an option named "bad". +HINT: All OAuth connections matching this line will fail. Correct the option and reload the server configuration. +CONTEXT: line 2 of configuration file "data/pg_hba.conf" +</programlisting> + Use caution when making changes to validator-specific HBA options in + production systems. + </para> + </warning> + </listitem> + </varlistentry> + <varlistentry> <term><literal>map</literal></term> <listitem> diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml index 5f29f2be186..7c9e3dd931a 100644 --- a/doc/src/sgml/oauth-validators.sgml +++ b/doc/src/sgml/oauth-validators.sgml @@ -251,6 +251,11 @@ <symbol>delegate_ident_mapping=1</symbol> mode, and what additional configuration is required in order to do so. </para> + <para> + If an implementation provides <link linkend="oauth-validator-hba">custom + HBA options</link>, the names and syntax of those options should be + documented as well. + </para> </listitem> </varlistentry> </variablelist> @@ -343,7 +348,8 @@ typedef const OAuthValidatorCallbacks *(*OAuthValidatorModuleInit) (void); <title>Startup Callback</title> <para> The <function>startup_cb</function> callback is executed directly after - loading the module. This callback can be used to set up local state and + loading the module. This callback can be used to set up local state, + define <link linkend="oauth-validator-hba">custom HBA options</link>, and perform additional initialization if required. If the validator module has state it can use <structfield>state->private_data</structfield> to store it. @@ -432,4 +438,217 @@ typedef void (*ValidatorShutdownCB) (ValidatorModuleState *state); </sect2> </sect1> + + <sect1 id="oauth-validator-hba"> + <title>Custom HBA Options</title> + + <para> + Like other preloaded libraries, validator modules may define + <link linkend="runtime-config-custom">custom GUC parameters</link> for user + configuration in <filename>postgresql.conf</filename>. However, it may be + desirable to configure behavior at a more granular level (say, for a + particular issuer or a group of users) instead of globally. + </para> + + <para> + Beginning in <productname>PostgreSQL</productname> 19, validator + implementations may define custom options for use inside + <filename>pg_hba.conf</filename>. These options are then + <link linkend="auth-oauth-validator-option">made available</link> to the user + as <literal>validator.<replaceable>option</replaceable></literal>. The API + for registering and retrieving custom options is described below. + </para> + + <sect2 id="oauth-validator-hba-api"> + <title>Options API</title> + <para> + Modules register custom HBA option names during the <function>startup_cb</function> + callback, using <function>RegisterOAuthHBAOptions()</function>: + +<programlisting> +/* + * Register a list of custom option names for use in pg_hba.conf. For each name + * "foo" registered here, that option will be provided as "validator.foo" in + * the HBA. + * + * Valid option names consist of alphanumeric ASCII, underscore (_), and hyphen + * (-). Invalid option names will be ignored with a WARNING logged at + * connection time. + * + * This function may only be called during the startup_cb callback. Multiple + * calls are permitted, which will append to the existing list of registered + * options; options cannot be unregistered. + * + * Parameters: + * + * - state: the state pointer passed to the startup_cb callback + * - num: the number of options in the opts array + * - opts: an array of null-terminated option names to register + * + * The list of option names is copied internally, and the opts array is not + * required to remain valid after the call. + */ +void RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]); +</programlisting> + </para> + + <para> + Each option's value, if set, may be later retrieved using + <function>GetOAuthHBAOption()</function>: + +<programlisting> +/* + * Retrieve the string value of an HBA option which was registered via + * RegisterOAuthHBAOptions(). Usable only during validate_cb or shutdown_cb. + * + * If the user has set the corresponding option in pg_hba.conf, this function + * returns that value as a null-terminated string, which must not be modified + * or freed. NULL is returned instead if the user has not set this option, if + * the option name was not registered, or if this function is incorrectly called + * during the startup_cb. + * + * Parameters: + * + * - state: the state pointer passed to the validate_cb/shutdown_cb callback + * - optname: the name of the option to retrieve + */ +const char *GetOAuthHBAOption(const ValidatorModuleState *state, + const char *optname); +</programlisting> + </para> + + <para> + See <xref linkend="oauth-validator-hba-example-usage"/> for sample usage. + </para> + </sect2> + + <sect2 id="oauth-validator-hba-limitations"> + <title>Limitations</title> + <para> + <itemizedlist> + <listitem> + <para> + Option names are limited to ASCII alphanumeric characters, + underscores (<literal>_</literal>), and hyphens (<literal>-</literal>). + </para> + </listitem> + <listitem> + <para> + Option values are always freeform strings (in contrast to custom GUCs, + which support numerics, booleans, and enums). + </para> + </listitem> + <listitem> + <para> + Option names and values cannot be checked by the server during a reload of + the configuration. Any unregistered options in <filename>pg_hba.conf</filename> + will instead result in connection failures. It is the responsibility of + each module to document and verify the syntax of option values as needed. + <footnote> + <para> + If a module finds an invalid option value during <function>validate_cb</function>, + it's recommended to <link linkend="oauth-validator-callback-validate">signal + an internal error</link> by setting <structfield>result->error_detail</structfield> + to a description of the problem and returning <literal>false</literal>. + </para> + </footnote> + </para> + </listitem> + </itemizedlist> + </para> + </sect2> + + <sect2 id="oauth-validator-hba-example-usage"> + <title>Example Usage</title> + + <para> + For a hypothetical module, the options <literal>foo</literal> and + <literal>bar</literal> could be registered as follows: + +<programlisting> +static void +validator_startup(ValidatorModuleState *state) +{ + static const char *opts[] = { + "foo", /* description of access privileges */ + "bar", /* magic URL for additional administrator powers */ + }; + + RegisterOAuthHBAOptions(state, lengthof(opts), opts); + + /* ...other setup... */ +} +</programlisting> + </para> + + <para> + The following sample entries in <filename>pg_hba.conf</filename> can then + make use of these options: + +<programlisting> +# TYPE DATABASE USER ADDRESS METHOD +hostssl postgres admin 0.0.0.0/0 oauth issuer=https://admin.example.com \ + scope="pg-admin openid email" \ + map=oauth-email \ + validator.foo="admin access" \ + validator.bar=https://magic.example.com + +hostssl postgres all 0.0.0.0/0 oauth issuer=https://www.example.com \ + scope="pg-user openid email" \ + map=oauth-email \ + validator.foo="user access" +</programlisting> + </para> + + <para> + The module can retrieve the option settings from the HBA during validation: + +<programlisting> +static bool +validate_token(const ValidatorModuleState *state, + const char *token, const char *role, + ValidatorModuleResult *res) +{ + const char *foo = GetOAuthHBAOption(state, "foo"); /* "admin access" or "user access" */ + const char *bar = GetOAuthHBAOption(state, "bar"); /* "https://magic.example.com" or NULL */ + + if (bar && !is_valid_url(bar)) + { + res->error_detail = psprintf("validator.bar (\"%s\") is not a valid URL.", bar); + return false; + } + + /* proceed to validate token */ +} +</programlisting> + </para> + + <para> + When multiple validators are in use, their registered option lists remain + independent: + +<programlisting> +<lineannotation>in postgresql.conf:</lineannotation> +oauth_validator_libraries = 'example_org, my_validator' + +<lineannotation>in pg_hba.conf:</lineannotation> +# TYPE DATABASE USER ADDRESS METHOD +hostssl postgres admin 0.0.0.0/0 oauth issuer=https://admin.example.com \ + scope="pg-admin openid email" \ + map=oauth-email \ + validator=my_validator \ + validator.foo="admin access" \ + validator.bar=https://magic.example.com + +hostssl postgres all 0.0.0.0/0 oauth issuer=https://www.example.org \ + scope="pg-user openid profile" \ + validator=example_org \ + delegate_ident_mapping=1 \ + validator.magic=on \ + validator.more_magic=off +</programlisting> + </para> + </sect2> + </sect1> </chapter> diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index c4570ce9b3f..e8898561c8c 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -140,6 +140,8 @@ typedef struct HbaLine char *oauth_scope; char *oauth_validator; bool oauth_skip_usermap; + List *oauth_opt_keys; + List *oauth_opt_vals; } HbaLine; typedef struct IdentLine diff --git a/src/include/libpq/oauth.h b/src/include/libpq/oauth.h index 60f493acddd..86f463a284e 100644 --- a/src/include/libpq/oauth.h +++ b/src/include/libpq/oauth.h @@ -96,6 +96,17 @@ typedef struct OAuthValidatorCallbacks ValidatorValidateCB validate_cb; } OAuthValidatorCallbacks; +/* + * A validator can register a list of custom option names during its startup_cb, + * then later retrieve the user settings for each during validation. This + * enables per-HBA-line configuration. For more information, refer to the OAuth + * validator modules documentation. + */ +extern void RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]); +extern const char *GetOAuthHBAOption(const ValidatorModuleState *state, + const char *optname); + /* * Type of the shared library symbol _PG_oauth_validator_module_init which is * required for all validator modules. This function will be invoked during @@ -107,9 +118,7 @@ extern PGDLLEXPORT const OAuthValidatorCallbacks *_PG_oauth_validator_module_ini /* Implementation */ extern PGDLLIMPORT const pg_be_sasl_mech pg_be_oauth_mech; -/* - * Ensure a validator named in the HBA is permitted by the configuration. - */ extern bool check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg); +extern bool valid_oauth_hba_option_name(const char *name); #endif /* PG_OAUTH_H */ diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c index 6a75b79efbf..ea34ebdb733 100644 --- a/src/backend/libpq/auth-oauth.c +++ b/src/backend/libpq/auth-oauth.c @@ -25,6 +25,7 @@ #include "libpq/hba.h" #include "libpq/oauth.h" #include "libpq/sasl.h" +#include "miscadmin.h" #include "storage/fd.h" #include "storage/ipc.h" #include "utils/json.h" @@ -40,10 +41,15 @@ static int oauth_exchange(void *opaq, const char *input, int inputlen, static void load_validator_library(const char *libname); static void shutdown_validator_library(void *arg); +static bool check_validator_hba_options(Port *port, const char **logdetail); static ValidatorModuleState *validator_module_state; static const OAuthValidatorCallbacks *ValidatorCallbacks; +static MemoryContext ValidatorMemoryContext; +static List *ValidatorOptions; +static bool ValidatorOptionsChecked; + /* Mechanism declaration */ const pg_be_sasl_mech pg_be_oauth_mech = { .get_mechanisms = oauth_get_mechanisms, @@ -109,6 +115,9 @@ oauth_init(Port *port, const char *selected_mech, const char *shadow_pass) errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("client selected an invalid SASL authentication mechanism")); + /* Save our memory context for later use by client API calls. */ + ValidatorMemoryContext = CurrentMemoryContext; + ctx = palloc0_object(struct oauth_ctx); ctx->state = OAUTH_STATE_INIT; @@ -293,6 +302,16 @@ oauth_exchange(void *opaq, const char *input, int inputlen, errmsg("malformed OAUTHBEARER message"), errdetail("Message contains additional data after the final terminator.")); + /* + * Make sure all custom HBA options are understood by the validator before + * continuing, since we couldn't check them during server start/reload. + */ + if (!check_validator_hba_options(ctx->port, logdetail)) + { + ctx->state = OAUTH_STATE_FINISHED; + return PG_SASL_EXCHANGE_FAILURE; + } + if (auth[0] == '\0') { /* @@ -822,6 +841,9 @@ shutdown_validator_library(void *arg) { if (ValidatorCallbacks->shutdown_cb != NULL) ValidatorCallbacks->shutdown_cb(validator_module_state); + + /* The backing memory for this is about to disappear. */ + ValidatorOptions = NIL; } /* @@ -907,3 +929,206 @@ done: return (*err_msg == NULL); } + +/* + * Client APIs for validator implementations + * + * Since we're currently not threaded, we only allow one validator in the + * process at a time. So we can make use of globals for now instead of looking + * up information using the state pointer. We probably shouldn't assume that the + * module hasn't temporarily changed memory contexts on us, though; functions + * here should defensively use an appropriate context when making global + * allocations. + */ + +/* + * Adds to the list of allowed validator.* HBA options. Used during the + * startup_cb. + */ +void +RegisterOAuthHBAOptions(ValidatorModuleState *state, int num, + const char *opts[]) +{ + MemoryContext oldcontext; + + if (!state) + { + Assert(false); + return; + } + + oldcontext = MemoryContextSwitchTo(ValidatorMemoryContext); + + for (int i = 0; i < num; i++) + { + if (!valid_oauth_hba_option_name(opts[i])) + { + /* + * The user can't set this option in the HBA, so GetOAuthHBAOption + * would always return NULL. + */ + ereport(WARNING, + errmsg("HBA option name \"%s\" is invalid and will be ignored", + opts[i]), + /* translator: the second %s is a function name */ + errcontext("validator module \"%s\", in call to %s", + MyProcPort->hba->oauth_validator, + "RegisterOAuthHBAOptions")); + continue; + } + + ValidatorOptions = lappend(ValidatorOptions, pstrdup(opts[i])); + } + + MemoryContextSwitchTo(oldcontext); + + /* + * Wait to validate the HBA against the registered options until later + * (see check_validator_hba_options()). + * + * Delaying allows the validator to make multiple registration calls, to + * append to the list; it lets us make the check in a place where we can + * report the error without leaking details to the client; and it avoids + * exporting the order of operations between HBA matching and the + * startup_cb call as an API guarantee. (The last issue may become + * relevant with a threaded model.) + */ +} + +/* + * Restrict the names available to custom HBA options, so that we don't + * accidentally prevent future syntax extensions to HBA files. + */ +bool +valid_oauth_hba_option_name(const char *name) +{ + /* + * This list is not incredibly principled, since the goal is just to bound + * compatibility guarantees for our HBA parser. Alphanumerics seem + * obviously fine, and it's difficult to argue against the punctuation + * that's already included in some HBA option names and identifiers. + */ + static const char *name_allowed_set = + "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789_-"; + + size_t span; + + if (!name[0]) + return false; + + span = strspn(name, name_allowed_set); + return name[span] == '\0'; +} + +/* + * Verifies that all validator.* HBA options specified by the user were actually + * registered by the validator library in use. + */ +static bool +check_validator_hba_options(Port *port, const char **logdetail) +{ + HbaLine *hba = port->hba; + + foreach_ptr(char, key, hba->oauth_opt_keys) + { + bool found = false; + + /* O(n^2) shouldn't be a problem here in practice. */ + foreach_ptr(char, optname, ValidatorOptions) + { + if (strcmp(key, optname) == 0) + { + found = true; + break; + } + } + + if (!found) + { + /* + * Unknown option name. Mirror the error messages in hba.c here, + * keeping in mind that the original "validator." prefix was + * stripped from the key during parsing. + * + * Since this is affecting live connections, which is unusual for + * HBA, be noisy with a WARNING. (Warnings aren't sent to clients + * prior to successful authentication, so this won't disclose the + * server config.) It'll duplicate some of the information in the + * logdetail, but that should make it hard to miss the connection + * between the two. + */ + char *name = psprintf("validator.%s", key); + + *logdetail = psprintf(_("unrecognized authentication option name: \"%s\""), + name); + ereport(WARNING, + errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("unrecognized authentication option name: \"%s\"", + name), + /* translator: the first %s is the name of the module */ + errdetail("The installed validator module (\"%s\") did not define an option named \"%s\".", + hba->oauth_validator, key), + errhint("All OAuth connections matching this line will fail. Correct the option and reload the server configuration."), + errcontext("line %d of configuration file \"%s\"", + hba->linenumber, hba->sourcefile)); + + return false; + } + } + + ValidatorOptionsChecked = true; /* unfetter GetOAuthHBAOption() */ + return true; +} + +/* + * Retrieves the setting for a validator.* HBA option, or NULL if not found. + * This may only be used during the validate_cb and shutdown_cb. + */ +const char * +GetOAuthHBAOption(const ValidatorModuleState *state, const char *optname) +{ + HbaLine *hba = MyProcPort->hba; + ListCell *lc_k; + ListCell *lc_v; + const char *ret = NULL; + + if (!ValidatorOptionsChecked) + { + /* + * Prevent the startup_cb from retrieving HBA options that it has just + * registered. This probably seems strange -- why refuse to hand out + * information we already know? -- but this lets us reserve the + * ability to perform the startup_cb call earlier, before we know + * which HBA line is matched by a connection, without breaking this + * API. + */ + return NULL; + } + + if (!state || !hba) + { + Assert(false); + return NULL; + } + + Assert(list_length(hba->oauth_opt_keys) == list_length(hba->oauth_opt_vals)); + + forboth(lc_k, hba->oauth_opt_keys, lc_v, hba->oauth_opt_vals) + { + const char *key = lfirst(lc_k); + const char *val = lfirst(lc_v); + + if (strcmp(key, optname) == 0) + { + /* + * Don't return yet -- when regular HBA options are specified more + * than once, the last one wins. Do the same for these options. + */ + ret = val; + } + } + + return ret; +} diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 87ee541e880..7694506aaf7 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -2497,6 +2497,32 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, REQUIRE_AUTH_OPTION(uaOAuth, "validator", "oauth"); hbaline->oauth_validator = pstrdup(val); } + else if (strncmp(name, "validator.", strlen("validator.")) == 0) + { + const char *key = name + strlen("validator."); + + REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth"); + + /* + * Validator modules may register their own per-HBA-line options. + * Unfortunately, since we don't want to require these modules to be + * loaded into the postmaster, we don't know if the options are valid + * yet and must store them for later. Perform only a basic syntax + * check here. + */ + if (!valid_oauth_hba_option_name(key)) + { + ereport(elevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("invalid OAuth validator option name: \"%s\"", name), + errcontext("line %d of configuration file \"%s\"", + line_num, file_name))); + return false; + } + + hbaline->oauth_opt_keys = lappend(hbaline->oauth_opt_keys, pstrdup(key)); + hbaline->oauth_opt_vals = lappend(hbaline->oauth_opt_vals, pstrdup(val)); + } else if (strcmp(name, "delegate_ident_mapping") == 0) { REQUIRE_AUTH_OPTION(uaOAuth, "delegate_ident_mapping", "oauth"); diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index ac62555675a..cb326b51597 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -602,10 +602,29 @@ $node->connect_fails( $bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.error_detail"); $bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.internal_error"); + +# We complain when bad option names are registered, but connections may proceed +# (since users can't set those options in the HBA anyway). +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.authn_id"); +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.authorize_tokens"); +$bgconn->query_safe("ALTER SYSTEM SET oauth_validator.invalid_hba TO true"); + $node->reload; $log_start = $node->wait_for_log(qr/reloading configuration files/, $log_start); +$node->connect_ok( + "$common_connstr user=test", + "bad registered HBA option", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [ + qr/WARNING:\s+HBA option name "bad option name" is invalid and will be ignored/, + qr/CONTEXT:\s+validator module "validator", in call to RegisterOAuthHBAOptions/, + ]); + +$bgconn->query_safe("ALTER SYSTEM RESET oauth_validator.invalid_hba"); + # # Test user mapping. # @@ -674,6 +693,84 @@ $node->reload; $log_start = $node->wait_for_log(qr/reloading configuration files/, $log_start); +$bgconn->quit; # the tests below restart the server + +# +# Test validator-specific HBA options. +# + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all test oauth issuer="$issuer" scope="openid postgres" delegate_ident_mapping=1 \\ + validator.authn_id="ignored" validator.authn_id="other-identity" +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.log="testalt message" +}); + +$node->reload; +$log_start = + $node->wait_for_log(qr/reloading configuration files/, $log_start); + +$node->connect_ok( + "$common_connstr user=test", + "custom HBA setting (test)", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [qr/connection authenticated: identity="other-identity"/]); +$node->connect_ok( + "$common_connstr user=testalt", + "custom HBA setting (testalt)", + expected_stderr => + qr@Visit https://example\.com/ and enter the code: postgresuser@, + log_like => [ + qr/LOG:\s+testalt message/, + qr/connection authenticated: identity="testalt"/, + ]); + +# bad syntax +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.=1 +}); + +$log_start = -s $node->logfile; +$node->restart(fail_ok => 1); +$node->log_check("empty HBA option name", + $log_start, + log_like => [qr/invalid OAuth validator option name: "validator\."/]); + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" validator.@@=1 +}); + +$log_start = -s $node->logfile; +$node->restart(fail_ok => 1); +$node->log_check("invalid HBA option name", + $log_start, + log_like => [qr/invalid OAuth validator option name: "validator\.@@"/]); + +# unknown settings (validation is deferred to connect time) +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all testalt oauth issuer="$issuer" scope="openid postgres" \\ + validator.log=ignored validator.bad=1 +}); +$node->restart; + +$node->connect_fails( + "$common_connstr user=testalt", + "bad HBA setting", + expected_stderr => qr/OAuth bearer authentication failed/, + log_like => [ + qr/WARNING:\s+unrecognized authentication option name: "validator\.bad"/, + qr/FATAL:\s+OAuth bearer authentication failed/, + qr/DETAIL:\s+unrecognized authentication option name: "validator\.bad"/, + ]); + # # Test multiple validators. # diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c index 353e0e0d32a..85fb4c08bf2 100644 --- a/src/test/modules/oauth_validator/validator.c +++ b/src/test/modules/oauth_validator/validator.c @@ -42,13 +42,21 @@ static char *authn_id = NULL; static bool authorize_tokens = true; static char *error_detail = NULL; static bool internal_error = false; +static bool invalid_hba = false; + +/* HBA options */ +static const char *hba_opts[] = { + "authn_id", /* overrides the default authn_id */ + "log", /* logs an arbitrary string */ +}; /*--- * Extension entry point. Sets up GUCs for use by tests: * * - oauth_validator.authn_id Sets the user identifier to return during token * validation. Defaults to the username in the - * startup packet. + * startup packet, or the validator.authn_id HBA + * option if it is set. * * - oauth_validator.authorize_tokens * Sets whether to successfully validate incoming @@ -96,6 +104,14 @@ _PG_init(void) PGC_SIGHUP, 0, NULL, NULL, NULL); + DefineCustomBoolVariable("oauth_validator.invalid_hba", + "Should the validator register an invalid option?", + NULL, + &invalid_hba, + false, + PGC_SIGHUP, + 0, + NULL, NULL, NULL); MarkGUCPrefixReserved("oauth_validator"); } @@ -124,6 +140,29 @@ validator_startup(ValidatorModuleState *state) if (state->sversion != PG_VERSION_NUM) elog(ERROR, "oauth_validator: sversion set to %d", state->sversion); + /* + * Test the behavior of custom HBA options. Registered options should not + * be retrievable during startup (we want to discourage modules from + * relying on the relative order of client connections and the + * startup_cb). + */ + RegisterOAuthHBAOptions(state, lengthof(hba_opts), hba_opts); + for (int i = 0; i < lengthof(hba_opts); i++) + { + if (GetOAuthHBAOption(state, hba_opts[i])) + elog(ERROR, + "oauth_validator: GetOAuthValidatorOption(\"%s\") was non-NULL during startup_cb", + hba_opts[i]); + } + + if (invalid_hba) + { + /* Register a bad option, which should print a WARNING to the logs. */ + const char *invalid = "bad option name"; + + RegisterOAuthHBAOptions(state, 1, &invalid); + } + state->private_data = PRIVATE_COOKIE; } @@ -141,7 +180,7 @@ validator_shutdown(ValidatorModuleState *state) /* * Validator implementation. Logs the incoming data and authorizes the token by - * default; the behavior can be modified via the module's GUC settings. + * default; the behavior can be modified via the module's GUC and HBA settings. */ static bool validate_token(const ValidatorModuleState *state, @@ -153,6 +192,9 @@ validate_token(const ValidatorModuleState *state, elog(ERROR, "oauth_validator: private state cookie changed to %p in validate", state->private_data); + if (GetOAuthHBAOption(state, "log")) + elog(LOG, "%s", GetOAuthHBAOption(state, "log")); + elog(LOG, "oauth_validator: token=\"%s\", role=\"%s\"", token, role); elog(LOG, "oauth_validator: issuer=\"%s\", scope=\"%s\"", MyProcPort->hba->oauth_issuer, @@ -165,6 +207,8 @@ validate_token(const ValidatorModuleState *state, res->authorized = authorize_tokens; if (authn_id) res->authn_id = pstrdup(authn_id); + else if (GetOAuthHBAOption(state, "authn_id")) + res->authn_id = pstrdup(GetOAuthHBAOption(state, "authn_id")); else res->authn_id = pstrdup(role); -- 2.34.1 ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Custom oauth validator options @ 2026-04-06 22:09 Zsolt Parragi <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 11+ messages in thread From: Zsolt Parragi @ 2026-04-06 22:09 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] > I believe this is now a complete patch > proposal; tear it apart. :D The patch looks good to me as is, I don't see any real issues with it, maybe one cosmetic question in the test. + if (GetOAuthHBAOption(state, "log")) + elog(LOG, "%s", GetOAuthHBAOption(state, "log")); + If we treat the test code as an example for real implementations, this and the other use could cache the option in a local variable instead of making duplicate calls, following the pattern of the documentation. ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Custom oauth validator options @ 2026-04-06 22:16 Jacob Champion <[email protected]> parent: Zsolt Parragi <[email protected]> 0 siblings, 0 replies; 11+ messages in thread From: Jacob Champion @ 2026-04-06 22:16 UTC (permalink / raw) To: Zsolt Parragi <[email protected]>; +Cc: Nikolay Shaplov <[email protected]>; Álvaro Herrera <[email protected]>; VASUKI M <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]; Robert Haas <[email protected]>; [email protected] On Mon, Apr 6, 2026 at 3:09 PM Zsolt Parragi <[email protected]> wrote: > > I believe this is now a complete patch > > proposal; tear it apart. :D > > The patch looks good to me as is, I don't see any real issues with it, Great! I am prepping for a commit Sometime Very Soon. > maybe one cosmetic question in the test. > > + if (GetOAuthHBAOption(state, "log")) > + elog(LOG, "%s", GetOAuthHBAOption(state, "log")); > + > > If we treat the test code as an example for real implementations, (It's not, though -- my test code is frequently abusive on purpose and should not be used as best-practice. If we want compilable sample code, that looks different from tests IMHO.) > this > and the other use could cache the option in a local variable instead > of making duplicate calls, following the pattern of the documentation. In this case, I want the test to pin the behavior that multiple calls should work as you'd expect. Thanks, --Jacob ^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2026-04-06 22:16 UTC | newest] Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-02-04 11:42 Re: Custom oauth validator options Zsolt Parragi <[email protected]> 2026-03-20 17:46 ` Jacob Champion <[email protected]> 2026-03-23 21:45 ` Zsolt Parragi <[email protected]> 2026-03-27 23:03 ` Jacob Champion <[email protected]> 2026-03-30 21:46 ` Zsolt Parragi <[email protected]> 2026-03-30 23:54 ` Jacob Champion <[email protected]> 2026-04-02 21:26 ` Jacob Champion <[email protected]> 2026-04-03 23:33 ` Jacob Champion <[email protected]> 2026-04-06 22:09 ` Zsolt Parragi <[email protected]> 2026-04-06 22:16 ` Jacob Champion <[email protected]> 2026-03-27 23:03 ` 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