From e51f717e07c6374c99c198a3ac18b6df67fb1620 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Thu, 11 Dec 2025 23:56:08 +0000 Subject: [PATCH v2 1/2] Split PGOAUTHDEBUG=UNSAFE into multiple options --- doc/src/sgml/libpq.sgml | 127 +++++++++++++---- src/interfaces/libpq-oauth/meson.build | 6 +- src/interfaces/libpq/meson.build | 1 + src/interfaces/libpq-oauth/Makefile | 11 +- src/interfaces/libpq/Makefile | 3 +- src/interfaces/libpq-oauth/oauth-utils.h | 3 +- src/interfaces/libpq/fe-auth-oauth.h | 18 ++- src/interfaces/libpq-oauth/oauth-curl.c | 16 +-- src/interfaces/libpq-oauth/oauth-utils.c | 11 -- src/interfaces/libpq-oauth/test-oauth-curl.c | 6 +- src/interfaces/libpq/fe-auth-oauth-debug.c | 140 +++++++++++++++++++ src/interfaces/libpq/fe-auth-oauth.c | 16 +-- 12 files changed, 295 insertions(+), 63 deletions(-) create mode 100644 src/interfaces/libpq/fe-auth-oauth-debug.c diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index a48d3161495..2e5fb9011e9 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -10643,35 +10643,112 @@ 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. + + + + + + fast-retry (safe) + + + Permits the use of zero-second retry intervals instead of the normal + minimum of one second. This can speed up tests but may cause the client + to busy-loop and consume CPU unnecessarily. + + + + + + poll-counts (safe) + + + Prints the total number of poll() calls to standard error when the + OAuth flow completes. This helps developers debug the async multiplexer + behavior. + + + + + + print-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) + 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 (fast-retry, + poll-counts, print-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=fast-retry,poll-counts safe options only +PGOAUTHDEBUG=UNSAFE:http,trace enable HTTP and traffic logging +PGOAUTHDEBUG=UNSAFE:http,poll-counts mix of unsafe and safe +PGOAUTHDEBUG=UNSAFE legacy; enables all options + + + - 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. The + trace option in particular exposes secrets that can be + used to attack your clients and servers. Do not share the output with third + parties. diff --git a/src/interfaces/libpq-oauth/meson.build b/src/interfaces/libpq-oauth/meson.build index ea3a900f4f1..d8cc92e0c2c 100644 --- a/src/interfaces/libpq-oauth/meson.build +++ b/src/interfaces/libpq-oauth/meson.build @@ -6,6 +6,7 @@ endif libpq_oauth_sources = files( 'oauth-curl.c', + '../libpq/fe-auth-oauth-debug.c', ) # The shared library needs additional glue symbols. @@ -62,7 +63,10 @@ endif libpq_oauth_test_deps = [] -oauth_test_sources = files('test-oauth-curl.c') + libpq_oauth_so_sources +oauth_test_sources = files( + 'test-oauth-curl.c', + '../libpq/fe-auth-oauth-debug.c', +) + libpq_oauth_so_sources if host_system == 'windows' oauth_test_sources += rc_bin_gen.process(win32ver_rc, extra_args: [ diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build index b0ae72167a1..d031f4962e5 100644 --- a/src/interfaces/libpq/meson.build +++ b/src/interfaces/libpq/meson.build @@ -2,6 +2,7 @@ libpq_sources = files( 'fe-auth-oauth.c', + 'fe-auth-oauth-debug.c', 'fe-auth-scram.c', 'fe-auth.c', 'fe-cancel.c', diff --git a/src/interfaces/libpq-oauth/Makefile b/src/interfaces/libpq-oauth/Makefile index 11e1a3cf528..c6097dda531 100644 --- a/src/interfaces/libpq-oauth/Makefile +++ b/src/interfaces/libpq-oauth/Makefile @@ -36,15 +36,24 @@ override CPPFLAGS_SHLIB += -DUSE_PRIVATE_ENCODING_FUNCS OBJS = \ $(WIN32RES) -OBJS_STATIC = oauth-curl.o +OBJS_STATIC = \ + oauth-curl.o \ + fe-auth-oauth-debug.o # The shared library needs additional glue symbols. OBJS_SHLIB = \ oauth-curl_shlib.o \ oauth-utils.o \ + fe-auth-oauth-debug_shlib.o oauth-utils.o: override CPPFLAGS += $(CPPFLAGS_SHLIB) +fe-auth-oauth-debug.o: $(libpq_srcdir)/fe-auth-oauth-debug.c + $(CC) $(CFLAGS) $(CPPFLAGS) -c $< -o $@ + +fe-auth-oauth-debug_shlib.o: $(libpq_srcdir)/fe-auth-oauth-debug.c fe-auth-oauth-debug.o + $(CC) $(CFLAGS) $(CFLAGS_SL) $(CPPFLAGS) $(CPPFLAGS_SHLIB) -c $< -o $@ + # Add shlib-/stlib-specific objects. $(shlib): override OBJS += $(OBJS_SHLIB) $(shlib): $(OBJS_SHLIB) diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile index 0963995eed4..099c6557e77 100644 --- a/src/interfaces/libpq/Makefile +++ b/src/interfaces/libpq/Makefile @@ -44,7 +44,8 @@ OBJS = \ legacy-pqsignal.o \ libpq-events.o \ pqexpbuffer.o \ - fe-auth.o + fe-auth.o \ + fe-auth-oauth-debug.o # File shared across all SSL implementations supported. ifneq ($(with_ssl),no) diff --git a/src/interfaces/libpq-oauth/oauth-utils.h b/src/interfaces/libpq-oauth/oauth-utils.h index 293e9936989..dd4e38d525c 100644 --- a/src/interfaces/libpq-oauth/oauth-utils.h +++ b/src/interfaces/libpq-oauth/oauth-utils.h @@ -15,6 +15,7 @@ #ifndef OAUTH_UTILS_H #define OAUTH_UTILS_H +#include "fe-auth-oauth.h" #include "libpq-fe.h" #include "pqexpbuffer.h" @@ -35,7 +36,7 @@ typedef enum PG_BOOL_NO /* No (false) */ } PGTernaryBool; -extern bool oauth_unsafe_debugging_enabled(void); +extern oauth_debug_flags oauth_get_debug_flags(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 511284614f7..fde5c30c013 100644 --- a/src/interfaces/libpq/fe-auth-oauth.h +++ b/src/interfaces/libpq/fe-auth-oauth.h @@ -38,8 +38,24 @@ typedef struct void *builtin_flow; } fe_oauth_state; +/* + * Debug flags for PGOAUTHDEBUG environment variable. + * Each flag controls a specific debug feature. + */ +typedef struct oauth_debug_flags +{ + /* UNSAFE features - require UNSAFE: prefix */ + bool http; /* allow HTTP (unencrypted) connections */ + bool trace; /* log HTTP traffic (exposes secrets) */ + + /* SAFE features - allowed without UNSAFE: prefix */ + bool fast_retry; /* allow zero-second retry intervals */ + bool poll_counts; /* print poll() statistics */ + bool print_plugin_errors; /* print plugin loading errors */ +} oauth_debug_flags; + extern void pqClearOAuthToken(PGconn *conn); -extern bool oauth_unsafe_debugging_enabled(void); +extern oauth_debug_flags oauth_get_debug_flags(void); /* Mechanisms in fe-auth-oauth.c */ extern const pg_fe_sasl_mech pg_oauth_mech; diff --git a/src/interfaces/libpq-oauth/oauth-curl.c b/src/interfaces/libpq-oauth/oauth-curl.c index 3baede1b2e7..564d76cf063 100644 --- a/src/interfaces/libpq-oauth/oauth-curl.c +++ b/src/interfaces/libpq-oauth/oauth-curl.c @@ -274,7 +274,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? */ + oauth_debug_flags debug_flags; /* can we give developer assistance */ int dbg_num_calls; /* (debug mode) how many times were we called? */ }; @@ -1023,7 +1023,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.fast_retry ? 0 : 1; else if (parsed >= INT_MAX) return INT_MAX; @@ -1797,7 +1797,7 @@ setup_curl_handles(struct async_ctx *actx) */ CHECK_SETOPT(actx, CURLOPT_NOSIGNAL, 1L, return false); - if (actx->debugging) + if (actx->debug_flags.trace) { /* * Set a callback for retrieving error information from libcurl, the @@ -1829,7 +1829,7 @@ setup_curl_handles(struct async_ctx *actx) const long unsafe = CURLPROTO_HTTPS | CURLPROTO_HTTP; #endif - if (actx->debugging) + if (actx->debug_flags.http) protos = unsafe; CHECK_SETOPT(actx, popt, protos, return false); @@ -2297,7 +2297,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.http) { if (pg_strncasecmp(provider->device_authorization_endpoint, HTTPS_SCHEME, strlen(HTTPS_SCHEME)) != 0) @@ -3027,7 +3027,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 && actx->debug_flags.poll_counts) { actx->dbg_num_calls++; if (result == PGRES_POLLING_OK || result == PGRES_POLLING_FAILED) @@ -3087,8 +3087,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..06815be9a0a 100644 --- a/src/interfaces/libpq-oauth/test-oauth-curl.c +++ b/src/interfaces/libpq-oauth/test-oauth-curl.c @@ -89,7 +89,11 @@ init_test_actx(void) actx->mux = PGINVALID_SOCKET; actx->timerfd = -1; - actx->debugging = true; + actx->debug_flags.http = true; + actx->debug_flags.trace = true; + actx->debug_flags.fast_retry = true; + actx->debug_flags.poll_counts = true; + actx->debug_flags.print_plugin_errors = true; initPQExpBuffer(&actx->errbuf); diff --git a/src/interfaces/libpq/fe-auth-oauth-debug.c b/src/interfaces/libpq/fe-auth-oauth-debug.c new file mode 100644 index 00000000000..f9a1b1f195f --- /dev/null +++ b/src/interfaces/libpq/fe-auth-oauth-debug.c @@ -0,0 +1,140 @@ +/*------------------------------------------------------------------------- + * + * fe-auth-oauth-debug.c + * Parsing logic for PGOAUTHDEBUG environment variable + * + * This file contains pure string parsing logic with no dependencies on + * libpq or libpq-oauth implementation details. It's compiled into both + * libraries to avoid code duplication. + * + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION + * src/interfaces/libpq/fe-auth-oauth-debug.c + * + *------------------------------------------------------------------------- + */ + +#include "postgres_fe.h" + +#include +#include +#include + +#include "fe-auth-oauth.h" + +/* + * Parse a single debug option from PGOAUTHDEBUG. + * Returns true if the option is recognized, false otherwise. + * Sets *is_unsafe to indicate if this option requires the UNSAFE: prefix. + */ +static bool +parse_debug_option(const char *option, oauth_debug_flags *flags, bool *is_unsafe) +{ + *is_unsafe = false; + + /* Unsafe options */ + if (strcmp(option, "http") == 0) + { + flags->http = true; + *is_unsafe = true; + return true; + } + else if (strcmp(option, "trace") == 0) + { + flags->trace = true; + *is_unsafe = true; + return true; + } + /* Safe options */ + else if (strcmp(option, "fast-retry") == 0) + { + flags->fast_retry = true; + return true; + } + else if (strcmp(option, "poll-counts") == 0) + { + flags->poll_counts = true; + return true; + } + else if (strcmp(option, "print-plugin-errors") == 0) + { + flags->print_plugin_errors = true; + return true; + } + + return false; +} + +/* + * 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 + */ +oauth_debug_flags +oauth_get_debug_flags(void) +{ + oauth_debug_flags flags = {0}; + const char *env = getenv("PGOAUTHDEBUG"); + char *options_str; + char *option; + char *saveptr = NULL; + bool unsafe_prefix = false; + + if (!env || env[0] == '\0') + return flags; + + if (strcmp(env, "UNSAFE") == 0) + { + flags.http = true; + flags.trace = true; + flags.fast_retry = true; + flags.poll_counts = true; + flags.print_plugin_errors = true; + return flags; + } + + if (strncmp(env, "UNSAFE:", 7) == 0) + { + unsafe_prefix = true; + env += 7; + } + + options_str = strdup(env); + if (!options_str) + return flags; + + option = strtok_r(options_str, ",", &saveptr); + while (option != NULL) + { + bool is_unsafe; + + if (!parse_debug_option(option, &flags, &is_unsafe)) + { + fprintf(stderr, + "WARNING: PGOAUTHDEBUG: unrecognized debug option \"%s\" (ignored)\n", + option); + } + else if (is_unsafe && !unsafe_prefix) + { + fprintf(stderr, + "WARNING: PGOAUTHDEBUG: unsafe option \"%s\" requires UNSAFE: prefix (ignored)\n" + "Use: PGOAUTHDEBUG=UNSAFE:%s\n", + option, option); + } + + option = strtok_r(NULL, ",", &saveptr); + } + + free(options_str); + + return flags; +} diff --git a/src/interfaces/libpq/fe-auth-oauth.c b/src/interfaces/libpq/fe-auth-oauth.c index f93184f04db..4bfe31b03cb 100644 --- a/src/interfaces/libpq/fe-auth-oauth.c +++ b/src/interfaces/libpq/fe-auth-oauth.c @@ -383,7 +383,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().http && pg_strncasecmp(wkuri, HTTP_SCHEME, strlen(HTTP_SCHEME)) == 0) { /* Allow http:// for testing only. */ @@ -897,7 +897,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().print_plugin_errors) fprintf(stderr, "failed dlopen for libpq-oauth: %s\n", dlerror()); return 0; @@ -911,7 +911,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().print_plugin_errors) fprintf(stderr, "failed dlsym for libpq-oauth: %s\n", dlerror()); dlclose(state->builtin_flow); @@ -1418,13 +1418,3 @@ 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); -} -- 2.34.1