From: Jacob Champion Date: Tue, 7 Apr 2026 15:15:14 +0000 (-0700) Subject: libpq: Split PGOAUTHDEBUG=UNSAFE into multiple options X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6d00fb9048fe61381c9f4d542cfd2bc767d95a3b;p=thirdparty%2Fpostgresql.git libpq: Split PGOAUTHDEBUG=UNSAFE into multiple options PGOAUTHDEBUG is a blunt instrument: you get all the debugging features, or none of them. The most annoying consequence during manual use is the Curl debug trace, which tends to obscure the device flow prompt entirely. The promotion of PGOAUTHCAFILE into its own feature in 993368113 improved the situation somewhat, but there's still the discomfort of knowing you have to opt into many dangerous behaviors just to get the single debug feature you wanted. Explode the PGOAUTHDEBUG syntax into a comma-separated list. The old "UNSAFE" value enables everything, like before. Any individual unsafe features still require the envvar to begin with an "UNSAFE:" prefix, to try to interrupt the flow of someone who is about to do something they should not. So now, rather than PGOAUTHDEBUG=UNSAFE # enable all the unsafe things a developer can say PGOAUTHDEBUG=call-count # only show me the call count. safe! PGOAUTHDEBUG=UNSAFE:trace # print secrets, but don't allow HTTP To avoid adding more build system scaffolding to libpq-oauth, implement this entirely in a small private header. This unfortunately can't be standalone, so it needs a headerscheck exception. Author: Zsolt Parragi Co-authored-by: Jacob Champion Reviewed-by: Chao Li Reviewed-by: Zsolt Parragi Discussion: https://postgr.es/m/CAOYmi%2B%3DfbZNJSkHVci%3DGpR8XPYObK%3DH%2B2ERRha0LDTS%2BifsWnw%40mail.gmail.com Discussion: https://postgr.es/m/CAN4CZFMmDZMH56O9vb_g7vHqAk8ryWFxBMV19C39PFghENg8kA%40mail.gmail.com --- diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index a48d3161495..0a19c2b553b 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -10643,35 +10643,104 @@ typedef struct - A "dangerous debugging mode" may be enabled by setting the environment - variable PGOAUTHDEBUG=UNSAFE. This functionality is provided - for ease of local development and testing only. It does several things that - you will not want a production system to do: + Debug features may be enabled by setting the PGOAUTHDEBUG + environment variable. This functionality is provided for ease of local + development and testing. The variable accepts a comma-separated list of + debug options: + + +PGOAUTHDEBUG=option1,option2,... for safe options only +PGOAUTHDEBUG=UNSAFE:option1,option2,... when using unsafe options +PGOAUTHDEBUG=UNSAFE legacy format; enables all options + + - - - - permits the use of unencrypted HTTP during the OAuth provider exchange - - - - - prints HTTP traffic (containing several critical secrets) to standard - error during the OAuth flow - - - - - permits the use of zero-second retry intervals, which can cause the - client to busy-loop and pointlessly consume CPU - - - + + Available debug options: + + + + http (unsafe) + + + Permits the use of unencrypted HTTP during the OAuth provider exchange. + This allows OAuth credentials to be transmitted over unencrypted + connections, which is extremely dangerous and should only be used for + local testing. + + + + + + trace (unsafe) + + + Prints HTTP traffic to standard error during the OAuth flow. This output + contains critical secrets including bearer tokens, client secrets, access + tokens, and authorization codes. Never share this output with third + parties. + + + + + + dos-endpoint (unsafe) + + + Permits the use of zero-second retry intervals instead of the normal + minimum of one second. This speeds up tests, but in normal operation it + will cause the client to busy-loop, consuming CPU and network resources. + + + + + + call-count (safe) + + + Prints the total number of calls to the flow plugin to standard error + when the OAuth flow completes. This helps developers debug the async + callback behavior. + + + + + + plugin-errors (safe) + + + Prints plugin loading errors to standard error. This helps developers + and package maintainers debug issues when the OAuth plugin fails to load. + + + + + + + Unsafe options (http, trace, + dos-endpoint) require the UNSAFE: prefix. + If unsafe options are specified without this prefix, or if an option name is + unrecognized, a warning is printed to standard error and that option is + ignored. Other valid options in the list continue to work. Safe options + (call-count, plugin-errors) can be + used without the prefix. + + + + Examples: + +PGOAUTHDEBUG=call-count safe options only +PGOAUTHDEBUG=UNSAFE:http,trace enable HTTP and traffic logging +PGOAUTHDEBUG=UNSAFE:http,call-count mix of unsafe and safe + + + - Do not share the output of the OAuth flow traffic with third parties. It - contains secrets that can be used to attack your clients and servers. + Never use unsafe debug options in production environments. They expose + secrets and behaviors that can be used to attack your clients and servers. + Do not share trace output with third parties. diff --git a/src/interfaces/libpq-oauth/oauth-curl.c b/src/interfaces/libpq-oauth/oauth-curl.c index 3baede1b2e7..abbef93f95f 100644 --- a/src/interfaces/libpq-oauth/oauth-curl.c +++ b/src/interfaces/libpq-oauth/oauth-curl.c @@ -49,6 +49,12 @@ #endif /* USE_DYNAMIC_OAUTH */ +/* + * oauth-debug.h needs the declaration of libpq_gettext(), from one of the above + * sources. + */ +#include "oauth-debug.h" + /* One final guardrail against accidental inclusion... */ #if defined(USE_DYNAMIC_OAUTH) && defined(LIBPQ_INT_H) #error do not rely on libpq-int.h in dynamic builds of libpq-oauth @@ -274,7 +280,7 @@ struct async_ctx int running; /* is asynchronous work in progress? */ bool user_prompted; /* have we already sent the authz prompt? */ bool used_basic_auth; /* did we send a client secret? */ - bool debugging; /* can we give unsafe developer assistance? */ + uint32 debug_flags; /* can we give developer assistance? */ int dbg_num_calls; /* (debug mode) how many times were we called? */ }; @@ -1023,7 +1029,7 @@ parse_interval(struct async_ctx *actx, const char *interval_str) parsed = ceil(parsed); if (parsed < 1) - return actx->debugging ? 0 : 1; + return (actx->debug_flags & OAUTHDEBUG_UNSAFE_DOS_ENDPOINT) ? 0 : 1; else if (parsed >= INT_MAX) return INT_MAX; @@ -1797,7 +1803,7 @@ setup_curl_handles(struct async_ctx *actx) */ CHECK_SETOPT(actx, CURLOPT_NOSIGNAL, 1L, return false); - if (actx->debugging) + if (actx->debug_flags & OAUTHDEBUG_UNSAFE_TRACE) { /* * Set a callback for retrieving error information from libcurl, the @@ -1829,7 +1835,7 @@ setup_curl_handles(struct async_ctx *actx) const long unsafe = CURLPROTO_HTTPS | CURLPROTO_HTTP; #endif - if (actx->debugging) + if (actx->debug_flags & OAUTHDEBUG_UNSAFE_HTTP) protos = unsafe; CHECK_SETOPT(actx, popt, protos, return false); @@ -2297,7 +2303,7 @@ check_for_device_flow(struct async_ctx *actx) * decent time to bail out if we're not using HTTPS for the endpoints * we'll use for the flow. */ - if (!actx->debugging) + if ((actx->debug_flags & OAUTHDEBUG_UNSAFE_HTTP) == 0) { if (pg_strncasecmp(provider->device_authorization_endpoint, HTTPS_SCHEME, strlen(HTTPS_SCHEME)) != 0) @@ -3027,7 +3033,7 @@ pg_fe_run_oauth_flow(PGconn *conn, struct PGoauthBearerRequest *request, * drain_timer_events(), when we're in debug mode, track the total number * of calls to this function and print that at the end of the flow. */ - if (actx->debugging) + if (actx->debug_flags & OAUTHDEBUG_CALL_COUNT) { actx->dbg_num_calls++; if (result == PGRES_POLLING_OK || result == PGRES_POLLING_FAILED) @@ -3087,8 +3093,8 @@ pg_start_oauthbearer(PGconn *conn, PGoauthBearerRequestV2 *request) * Now finish filling in the actx. */ - /* Should we enable unsafe features? */ - actx->debugging = oauth_unsafe_debugging_enabled(); + /* Parse debug flags from the environment. */ + actx->debug_flags = oauth_parse_debug_flags(); initPQExpBuffer(&actx->work_data); initPQExpBuffer(&actx->errbuf); diff --git a/src/interfaces/libpq-oauth/oauth-utils.c b/src/interfaces/libpq-oauth/oauth-utils.c index ccb0d9bf2c5..004d41f02aa 100644 --- a/src/interfaces/libpq-oauth/oauth-utils.c +++ b/src/interfaces/libpq-oauth/oauth-utils.c @@ -75,17 +75,6 @@ libpq_gettext(const char *msgid) #endif /* ENABLE_NLS */ -/* - * Returns true if the PGOAUTHDEBUG=UNSAFE flag is set in the environment. - */ -bool -oauth_unsafe_debugging_enabled(void) -{ - const char *env = getenv("PGOAUTHDEBUG"); - - return (env && strcmp(env, "UNSAFE") == 0); -} - /* * Duplicate SOCK_ERRNO* definitions from libpq-int.h, for use by * pq_block/reset_sigpipe(). diff --git a/src/interfaces/libpq-oauth/oauth-utils.h b/src/interfaces/libpq-oauth/oauth-utils.h index 293e9936989..dacd2dbacfe 100644 --- a/src/interfaces/libpq-oauth/oauth-utils.h +++ b/src/interfaces/libpq-oauth/oauth-utils.h @@ -35,7 +35,6 @@ typedef enum PG_BOOL_NO /* No (false) */ } PGTernaryBool; -extern bool oauth_unsafe_debugging_enabled(void); extern int pq_block_sigpipe(sigset_t *osigset, bool *sigpipe_pending); extern void pq_reset_sigpipe(sigset_t *osigset, bool sigpipe_pending, bool got_epipe); diff --git a/src/interfaces/libpq-oauth/test-oauth-curl.c b/src/interfaces/libpq-oauth/test-oauth-curl.c index 4328a332738..9db39053dd3 100644 --- a/src/interfaces/libpq-oauth/test-oauth-curl.c +++ b/src/interfaces/libpq-oauth/test-oauth-curl.c @@ -89,7 +89,7 @@ init_test_actx(void) actx->mux = PGINVALID_SOCKET; actx->timerfd = -1; - actx->debugging = true; + actx->debug_flags = OAUTHDEBUG_LEGACY_UNSAFE; initPQExpBuffer(&actx->errbuf); diff --git a/src/interfaces/libpq/fe-auth-oauth.c b/src/interfaces/libpq/fe-auth-oauth.c index ac03d1d4f9d..826f7461cb3 100644 --- a/src/interfaces/libpq/fe-auth-oauth.c +++ b/src/interfaces/libpq/fe-auth-oauth.c @@ -26,6 +26,7 @@ #include "fe-auth.h" #include "fe-auth-oauth.h" #include "mb/pg_wchar.h" +#include "oauth-debug.h" #include "pg_config_paths.h" #include "utils/memdebug.h" @@ -389,7 +390,7 @@ issuer_from_well_known_uri(PGconn *conn, const char *wkuri) authority_start = wkuri + strlen(HTTPS_SCHEME); if (!authority_start - && oauth_unsafe_debugging_enabled() + && (oauth_parse_debug_flags() & OAUTHDEBUG_UNSAFE_HTTP) && pg_strncasecmp(wkuri, HTTP_SCHEME, strlen(HTTP_SCHEME)) == 0) { /* Allow http:// for testing only. */ @@ -900,7 +901,7 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *re * * Note that POSIX dlerror() isn't guaranteed to be threadsafe. */ - if (oauth_unsafe_debugging_enabled()) + if (oauth_parse_debug_flags() & OAUTHDEBUG_PLUGIN_ERRORS) fprintf(stderr, "failed dlopen for libpq-oauth: %s\n", dlerror()); return 0; @@ -922,7 +923,7 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *re * cause is still locked behind PGOAUTHDEBUG due to the dlerror() * threadsafety issue. */ - if (oauth_unsafe_debugging_enabled()) + if (oauth_parse_debug_flags() & OAUTHDEBUG_PLUGIN_ERRORS) fprintf(stderr, "failed dlsym for libpq-oauth: %s\n", dlerror()); dlclose(state->flow_module); @@ -1437,17 +1438,6 @@ pqClearOAuthToken(PGconn *conn) conn->oauth_token = NULL; } -/* - * Returns true if the PGOAUTHDEBUG=UNSAFE flag is set in the environment. - */ -bool -oauth_unsafe_debugging_enabled(void) -{ - const char *env = getenv("PGOAUTHDEBUG"); - - return (env && strcmp(env, "UNSAFE") == 0); -} - /* * Hook v1 Poisoning * diff --git a/src/interfaces/libpq/fe-auth-oauth.h b/src/interfaces/libpq/fe-auth-oauth.h index 872f5df551a..a50d7b03408 100644 --- a/src/interfaces/libpq/fe-auth-oauth.h +++ b/src/interfaces/libpq/fe-auth-oauth.h @@ -40,7 +40,6 @@ typedef struct } fe_oauth_state; extern void pqClearOAuthToken(PGconn *conn); -extern bool oauth_unsafe_debugging_enabled(void); /* Mechanisms in fe-auth-oauth.c */ extern const pg_fe_sasl_mech pg_oauth_mech; diff --git a/src/interfaces/libpq/oauth-debug.h b/src/interfaces/libpq/oauth-debug.h new file mode 100644 index 00000000000..4f0c87117e1 --- /dev/null +++ b/src/interfaces/libpq/oauth-debug.h @@ -0,0 +1,142 @@ +/*------------------------------------------------------------------------- + * + * oauth-debug.h + * Parsing logic for PGOAUTHDEBUG environment variable + * + * Both libpq and libpq-oauth need this logic, so it's packaged in a small + * header for convenience. This is not quite a standalone header, due to the + * complication introduced by libpq_gettext(); see note below. + * + * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION + * src/interfaces/libpq/oauth-debug.h + * + *------------------------------------------------------------------------- + */ + +#ifndef OAUTH_DEBUG_H +#define OAUTH_DEBUG_H + +#include "postgres_fe.h" + +/* + * XXX libpq-oauth can't compile against libpq-int.h, so clients of this header + * need to provide the declaration of libpq_gettext() before #including it. + * Fortunately, there are only two such clients. + */ +/* #include "libpq-int.h" */ + +/* + * Debug flags for the PGOAUTHDEBUG environment variable. Each flag controls a + * specific debug feature. OAUTHDEBUG_UNSAFE_* flags require the envvar to have + * a literal "UNSAFE:" prefix. + */ + +/* allow HTTP (unencrypted) connections */ +#define OAUTHDEBUG_UNSAFE_HTTP (1<<0) +/* log HTTP traffic (exposes secrets) */ +#define OAUTHDEBUG_UNSAFE_TRACE (1<<1) +/* allow zero-second retry intervals */ +#define OAUTHDEBUG_UNSAFE_DOS_ENDPOINT (1<<2) + +/* mind the gap in values; see OAUTHDEBUG_UNSAFE_MASK below */ + +/* print PQconnectPoll statistics */ +#define OAUTHDEBUG_CALL_COUNT (1<<16) +/* print plugin loading errors */ +#define OAUTHDEBUG_PLUGIN_ERRORS (1<<17) + +/* all safe and unsafe flags, for the legacy UNSAFE behavior */ +#define OAUTHDEBUG_LEGACY_UNSAFE ((uint32) ~0) + +/* Flags are divided into "safe" and "unsafe" based on bit position. */ +#define OAUTHDEBUG_UNSAFE_MASK ((uint32) 0x0000FFFF) + +static_assert(OAUTHDEBUG_CALL_COUNT == OAUTHDEBUG_UNSAFE_MASK + 1, + "the first safe OAUTHDEBUG flag should be above OAUTHDEBUG_UNSAFE_MASK"); + +/* + * Parses the PGOAUTHDEBUG environment variable and returns debug flags. + * + * Supported formats: + * PGOAUTHDEBUG=UNSAFE - legacy format, enables all features + * PGOAUTHDEBUG=option1,option2 - enable safe features only + * PGOAUTHDEBUG=UNSAFE:opt1,opt2 - enable unsafe and/or safe features + * + * Prints a warning and skips the invalid option if: + * - An unrecognized option is specified + * - An unsafe option is specified without the UNSAFE: prefix + * + * XXX The parsing, and any warnings, will happen each time the function is + * called, so consider caching the result in cases where that might get + * annoying. But don't try to cache inside this function, unless you also have a + * plan for getting libpq and libpq-oauth to share that cache safely... probably + * not worth the effort for a debugging aid? + */ +static uint32 +oauth_parse_debug_flags(void) +{ + uint32 flags = 0; + const char *env = getenv("PGOAUTHDEBUG"); + char *options_str; + char *option; + char *saveptr = NULL; + bool unsafe_allowed = false; + + if (!env || env[0] == '\0') + return flags; + + if (strcmp(env, "UNSAFE") == 0) + return OAUTHDEBUG_LEGACY_UNSAFE; + + if (strncmp(env, "UNSAFE:", 7) == 0) + { + unsafe_allowed = true; + env += 7; + } + + options_str = strdup(env); + if (!options_str) + return flags; + + option = strtok_r(options_str, ",", &saveptr); + while (option != NULL) + { + uint32 flag = 0; + + if (strcmp(option, "http") == 0) + flag = OAUTHDEBUG_UNSAFE_HTTP; + else if (strcmp(option, "trace") == 0) + flag = OAUTHDEBUG_UNSAFE_TRACE; + else if (strcmp(option, "dos-endpoint") == 0) + flag = OAUTHDEBUG_UNSAFE_DOS_ENDPOINT; + else if (strcmp(option, "call-count") == 0) + flag = OAUTHDEBUG_CALL_COUNT; + else if (strcmp(option, "plugin-errors") == 0) + flag = OAUTHDEBUG_PLUGIN_ERRORS; + else + fprintf(stderr, + libpq_gettext("WARNING: unrecognized PGOAUTHDEBUG option \"%s\" (ignored)\n"), + option); + + if (!unsafe_allowed && ((flag & OAUTHDEBUG_UNSAFE_MASK) != 0)) + { + flag = 0; + + fprintf(stderr, + libpq_gettext("WARNING: PGOAUTHDEBUG option \"%s\" is unsafe (ignored)\n"), + option); + } + + flags |= flag; + option = strtok_r(NULL, ",", &saveptr); + } + + free(options_str); + + return flags; +} + +#endif /* OAUTH_DEBUG_H */ diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index ac62555675a..37c9d511b4b 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -93,6 +93,21 @@ $node->connect_fails( qr@OAuth discovery URI "\Q$issuer\E/.well-known/openid-configuration" must use HTTPS@ ); +{ + # PGOAUTHDEBUG=http should have no effect (it needs an UNSAFE: marker). + local $ENV{PGOAUTHDEBUG} = "http"; + + $node->connect_fails( + "user=test dbname=postgres oauth_issuer=$issuer oauth_client_id=f02c6361-0635", + "HTTPS is required without debug mode (bad PGOAUTHDEBUG value)", + expected_stderr => qr[ + ^WARNING: .* \Qoption "http" is unsafe\E + .* + \QOAuth discovery URI "$issuer/.well-known/openid-configuration" must use HTTPS\E + ]msx + ); +} + # Switch to HTTPS. $issuer = "https://127.0.0.1:$port"; @@ -172,8 +187,11 @@ $node->connect_ok( ], log_unlike => [qr/FATAL.*OAuth bearer authentication failed/]); -# Enable PGOAUTHDEBUG for all remaining tests. -$ENV{PGOAUTHDEBUG} = "UNSAFE"; +# Enable some debugging features for all remaining tests: +# - trace, for detailed Curl logs on failure +# - dos-endpoint, to speed up the three-way handshake +# - call-count, for our later sanity check +$ENV{PGOAUTHDEBUG} = "UNSAFE:trace,dos-endpoint,call-count"; # The /alternate issuer uses slightly different parameters, along with an # OAuth-style discovery document. diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck index 4834ded3719..785d6f867ad 100755 --- a/src/tools/pginclude/headerscheck +++ b/src/tools/pginclude/headerscheck @@ -156,6 +156,8 @@ do test "$f" = src/include/catalog/syscache_ids.h && continue test "$f" = src/include/catalog/syscache_info.h && continue + test "$f" = src/interfaces/libpq/oauth-debug.h && continue + # We can't make these Bison output files compilable standalone # without using "%code require", which old Bison versions lack. # parser/gram.h will be included by parser/gramparse.h anyway.