since-v3.nocfbot.diff
application/octet-stream
Filename: since-v3.nocfbot.diff
Type: application/octet-stream
Part: 2
Message:
Re: Custom oauth validator options
1: f36e6becc34 ! 1: a9a507604c5 oauth: Let validators provide failure DETAILs
@@ Commit message
Reported-by: Álvaro Herrera <alvherre@kurilemu.de>
Reported-by: Zsolt Parragi <zsolt.parragi@percona.com>
+ Reviewed-by: Chao Li <li.evan.chao@gmail.com>
+ Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
+ Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
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 <jacob.champion@enterprisedb.com>
## 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 <zsolt.parragi@percona.com>
Suggested-by: VASUKI M <vasukianand0119@gmail.com>
Investigated-by: Zsolt Parragi <zsolt.parragi@percona.com>
+ Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
+ 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));
+ }