From 743b4a5c3a5781e9c7dd5345110ff7b19c1d6707 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Tue, 31 Mar 2026 14:08:59 -0700 Subject: [PATCH v3] Split PGOAUTHDEBUG=UNSAFE into multiple options WIP Author: Zsolt Parragi Co-authored-by: Jacob Champion --- doc/src/sgml/libpq.sgml | 124 ++++++++++++---- src/interfaces/libpq-oauth/oauth-utils.h | 1 - src/interfaces/libpq/fe-auth-oauth.h | 1 - src/interfaces/libpq/oauth-debug.h | 136 ++++++++++++++++++ src/interfaces/libpq-oauth/oauth-curl.c | 22 +-- src/interfaces/libpq-oauth/oauth-utils.c | 11 -- src/interfaces/libpq-oauth/test-oauth-curl.c | 2 +- src/interfaces/libpq/fe-auth-oauth.c | 18 +-- .../modules/oauth_validator/t/001_server.pl | 22 ++- src/tools/pginclude/headerscheck | 2 + 10 files changed, 276 insertions(+), 63 deletions(-) create mode 100644 src/interfaces/libpq/oauth-debug.h diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index a48d3161495..01a65419f99 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -10643,35 +10643,109 @@ 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, 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. + + + Unrecognized option names will also trigger a warning and be ignored, while + valid options continue to work. This helps catch typos in the environment + variable configuration without breaking the debugging of valid options. + + + + 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-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/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..0bd8467a09c --- /dev/null +++ b/src/interfaces/libpq/oauth-debug.h @@ -0,0 +1,136 @@ +/*------------------------------------------------------------------------- + * + * 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_UNSAFE_ALL ((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 + */ +static uint32 +oauth_get_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_UNSAFE_ALL; + + 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/interfaces/libpq-oauth/oauth-curl.c b/src/interfaces/libpq-oauth/oauth-curl.c index 3baede1b2e7..eb2fe35d0cc 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_get_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/test-oauth-curl.c b/src/interfaces/libpq-oauth/test-oauth-curl.c index 4328a332738..185c17e5807 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_UNSAFE_ALL; initPQExpBuffer(&actx->errbuf); diff --git a/src/interfaces/libpq/fe-auth-oauth.c b/src/interfaces/libpq/fe-auth-oauth.c index ac03d1d4f9d..c150f27df00 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_get_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_get_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_get_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/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index c9c46e63539..3d190c2ba71 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 14c466cc237..de50b6937af 100755 --- a/src/tools/pginclude/headerscheck +++ b/src/tools/pginclude/headerscheck @@ -153,6 +153,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. -- 2.34.1