From 1632eb23a16954a53b325e3336cdddba19db5439 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Thu, 11 Dec 2025 23:56:08 +0000 Subject: [PATCH] Split PGOAUTHDEBUG=UNSAFE into multiple options --- src/interfaces/libpq-oauth/Makefile | 12 +- src/interfaces/libpq-oauth/meson.build | 6 +- src/interfaces/libpq-oauth/oauth-curl.c | 18 +-- src/interfaces/libpq-oauth/oauth-utils.c | 11 -- src/interfaces/libpq-oauth/oauth-utils.h | 2 +- src/interfaces/libpq/Makefile | 3 +- src/interfaces/libpq/fe-auth-oauth-debug.c | 142 +++++++++++++++++++++ src/interfaces/libpq/fe-auth-oauth.c | 16 +-- src/interfaces/libpq/fe-auth-oauth.h | 19 ++- src/interfaces/libpq/meson.build | 1 + 10 files changed, 192 insertions(+), 38 deletions(-) create mode 100644 src/interfaces/libpq/fe-auth-oauth-debug.c diff --git a/src/interfaces/libpq-oauth/Makefile b/src/interfaces/libpq-oauth/Makefile index 51145f085a8..1ae7b00e816 100644 --- a/src/interfaces/libpq-oauth/Makefile +++ b/src/interfaces/libpq-oauth/Makefile @@ -30,15 +30,25 @@ override CFLAGS += $(PTHREAD_CFLAGS) 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 += -DUSE_DYNAMIC_OAUTH oauth-curl_shlib.o: override CPPFLAGS_SHLIB += -DUSE_DYNAMIC_OAUTH +fe-auth-oauth-debug_shlib.o: override CPPFLAGS_SHLIB += -DUSE_DYNAMIC_OAUTH + +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 + $(CC) $(CFLAGS) $(CFLAGS_SL) $(CPPFLAGS) $(CPPFLAGS_SHLIB) -c $< -o $@ # Add shlib-/stlib-specific objects. $(shlib): override OBJS += $(OBJS_SHLIB) diff --git a/src/interfaces/libpq-oauth/meson.build b/src/interfaces/libpq-oauth/meson.build index 505e1671b86..5c916b25e81 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. @@ -50,7 +51,10 @@ libpq_oauth_so = shared_module(libpq_oauth_name, 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-oauth/oauth-curl.c b/src/interfaces/libpq-oauth/oauth-curl.c index bd0a656a166..185dbb64bab 100644 --- a/src/interfaces/libpq-oauth/oauth-curl.c +++ b/src/interfaces/libpq-oauth/oauth-curl.c @@ -277,7 +277,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? */ }; @@ -978,7 +978,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; @@ -1753,7 +1753,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 @@ -1785,7 +1785,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); @@ -1799,7 +1799,7 @@ setup_curl_handles(struct async_ctx *actx) * the flow to work at all, so any changes to the roots are likely to be * done system-wide. */ - if (actx->debugging) + if (actx->debug_flags.custom_ca) { const char *env; @@ -2265,7 +2265,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) @@ -2787,8 +2787,8 @@ pg_fe_run_oauth_flow_impl(PGconn *conn) actx->mux = PGINVALID_SOCKET; actx->timerfd = -1; - /* Should we enable unsafe features? */ - actx->debugging = oauth_unsafe_debugging_enabled(); + /* Parse debug flags from environment */ + actx->debug_flags = oauth_get_debug_flags(); state->async_ctx = actx; @@ -3068,7 +3068,7 @@ pg_fe_run_oauth_flow(PGconn *conn) actx = state->async_ctx; Assert(actx || result == PGRES_POLLING_FAILED); - if (actx && actx->debugging) + if (actx && actx->debug_flags.poll_counts) { actx->dbg_num_calls++; if (result == PGRES_POLLING_OK || result == PGRES_POLLING_FAILED) diff --git a/src/interfaces/libpq-oauth/oauth-utils.c b/src/interfaces/libpq-oauth/oauth-utils.c index 45fdc7579f2..719208fead3 100644 --- a/src/interfaces/libpq-oauth/oauth-utils.c +++ b/src/interfaces/libpq-oauth/oauth-utils.c @@ -142,17 +142,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 f4ffefef208..1762db7d3be 100644 --- a/src/interfaces/libpq-oauth/oauth-utils.h +++ b/src/interfaces/libpq-oauth/oauth-utils.h @@ -76,7 +76,7 @@ typedef enum } PGTernaryBool; extern void libpq_append_conn_error(PGconn *conn, const char *fmt,...) pg_attribute_printf(2, 3); -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/Makefile b/src/interfaces/libpq/Makefile index 9fe321147fc..157f80cfb5b 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/fe-auth-oauth-debug.c b/src/interfaces/libpq/fe-auth-oauth-debug.c new file mode 100644 index 00000000000..56bc05e6820 --- /dev/null +++ b/src/interfaces/libpq/fe-auth-oauth-debug.c @@ -0,0 +1,142 @@ +/*------------------------------------------------------------------------- + * + * 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. + * Updates the flag if the option is recognized and allowed. + * Prints warnings for unrecognized options or unsafe options without permission. + */ +static void +parse_debug_option(const char *option, oauth_debug_flags *flags, bool allow_unsafe) +{ + bool *flag_ptr = NULL; + bool is_unsafe = false; + + if (strcmp(option, "http") == 0) + { + flag_ptr = &flags->http; + is_unsafe = true; + } + else if (strcmp(option, "trace") == 0) + { + flag_ptr = &flags->trace; + is_unsafe = true; + } + else if (strcmp(option, "custom-ca") == 0) + { + flag_ptr = &flags->custom_ca; + is_unsafe = true; + } + else if (strcmp(option, "fast-retry") == 0) + { + flag_ptr = &flags->fast_retry; + } + else if (strcmp(option, "poll-counts") == 0) + { + flag_ptr = &flags->poll_counts; + } + else if (strcmp(option, "print-plugin-errors") == 0) + { + flag_ptr = &flags->print_plugin_errors; + } + + if (!flag_ptr) + { + fprintf(stderr, + "WARNING: PGOAUTHDEBUG: unrecognized debug option \"%s\" (ignored)\n", + option); + return; + } + + if (is_unsafe && !allow_unsafe) + { + fprintf(stderr, + "WARNING: PGOAUTHDEBUG: unsafe option \"%s\" requires UNSAFE: prefix (ignored)\n" + "Use: PGOAUTHDEBUG=UNSAFE:%s\n", + option, option); + return; + } + + *flag_ptr = true; +} + +/* + * 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.custom_ca = 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) + { + parse_debug_option(option, &flags, unsafe_prefix); + 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 d146c5f567c..96878c59369 100644 --- a/src/interfaces/libpq/fe-auth-oauth.c +++ b/src/interfaces/libpq/fe-auth-oauth.c @@ -376,7 +376,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. */ @@ -870,7 +870,7 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state) * * 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 false; @@ -884,7 +884,7 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state) * This is more of an error condition than the one above, but due to * the dlerror() threadsafety issue, lock it behind PGOAUTHDEBUG too. */ - 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); @@ -1385,13 +1385,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); -} diff --git a/src/interfaces/libpq/fe-auth-oauth.h b/src/interfaces/libpq/fe-auth-oauth.h index 0d59e91605b..1f0cbea7eeb 100644 --- a/src/interfaces/libpq/fe-auth-oauth.h +++ b/src/interfaces/libpq/fe-auth-oauth.h @@ -42,8 +42,25 @@ 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 +{ + /* potentially UNSAFE features */ + bool http; + bool trace; + bool custom_ca; + + /* SAFE features */ + bool fast_retry; + bool poll_counts; + bool print_plugin_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); extern bool use_builtin_flow(PGconn *conn, fe_oauth_state *state); /* Mechanisms in fe-auth-oauth.c */ diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build index b259c998fa2..19508b39a67 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', -- 2.43.0