public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jacob Champion <[email protected]>
To: Zsolt Parragi <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [oauth] Split and extend PGOAUTHDEBUG
Date: Tue, 31 Mar 2026 16:50:19 -0700
Message-ID: <CAOYmi+kCYZ3YiOu+oSv1gVW6LXQaNg4BcEpskYizWgfV1z12kA@mail.gmail.com> (raw)
In-Reply-To: <CAN4CZFMKCB2OXPGW0R_hCSu4Gg==B7dBSrv6Mf-YuFcrUncADg@mail.gmail.com>
References: <CAN4CZFMmDZMH56O9vb_g7vHqAk8ryWFxBMV19C39PFghENg8kA@mail.gmail.com>
<CAOYmi+k_et3yXpJ8op71-95j7OYg-kX5bWLgW9YTV_5G7f+O1A@mail.gmail.com>
<CAOYmi+kivcSnazEJA=KWknd3azGYnU3mMq9SUvht5Zq74qNcYQ@mail.gmail.com>
<CAN4CZFMKCB2OXPGW0R_hCSu4Gg==B7dBSrv6Mf-YuFcrUncADg@mail.gmail.com>
On Tue, Mar 31, 2026 at 10:45 AM Zsolt Parragi
<[email protected]> wrote:
> I didn't want to write "print-poll-counts" and "print-trace" as those
> are just longer, while simply writing "plugin-errors" without print
> also seemed wrong. Maybe it could be "plugin-debug" instead, that
> sounds good even withour print?
I like `plugin-errors`, actually -- "I want to debug these plugin
errors." Adding -debug to the name doesn't seem great to me, since
it's clear from the context that we're debugging.
`dos-retry` still doesn't quite convey what we're doing...
`dos-endpoint` maybe? `busy-loop`? The unsafe aspect is the resource
consumption...
To keep the brainstorming going, I chose the following for v3, but
they're by no means settled:
- http
- trace
- dos-endpoint
- call-count
- plugin-errors
> > I have a sample patch locally for these suggestions, if you'd like.
>
> I can create a patch with these updates tomorrow, but if you already
> have it, that might be easier/quicker.
In the interest of time I've attached it as a single patch, but the
range-diff is rough to read. If you'd like, I can split the code
motion apart from the logical changes tomorrow, to see if it helps
with review.
--Jacob
Attachments:
[application/octet-stream] since-v2.nocfbot.diff (25.6K, 2-since-v2.nocfbot.diff)
download | inline diff:
1: e51f717e07c ! 1: 743b4a5c3a5 Split PGOAUTHDEBUG=UNSAFE into multiple options
@@
## Metadata ##
-Author: Zsolt Parragi <[email protected]>
+Author: Jacob Champion <[email protected]>
## Commit message ##
Split PGOAUTHDEBUG=UNSAFE into multiple options
+ WIP
+
+ Author: Zsolt Parragi <[email protected]>
+ Co-authored-by: Jacob Champion <[email protected]>
+
## doc/src/sgml/libpq.sgml ##
@@ doc/src/sgml/libpq.sgml: typedef struct
</para>
@@ doc/src/sgml/libpq.sgml: typedef struct
+ debug options:
+
+ <programlisting>
-+PGOAUTHDEBUG=option1,option2,... <lineannotation>for safe options only</lineannotation>
-+PGOAUTHDEBUG=UNSAFE:option1,option2,... <lineannotation>when using unsafe options</lineannotation>
-+PGOAUTHDEBUG=UNSAFE <lineannotation>legacy format; enables all options</lineannotation>
++PGOAUTHDEBUG=option1,option2,... <lineannotation>for safe options only</lineannotation>
++PGOAUTHDEBUG=UNSAFE:option1,option2,... <lineannotation>when using unsafe options</lineannotation>
++PGOAUTHDEBUG=UNSAFE <lineannotation>legacy format; enables all options</lineannotation>
+ </programlisting>
+ </para>
@@ doc/src/sgml/libpq.sgml: typedef struct
+ </varlistentry>
+
+ <varlistentry>
-+ <term><literal>fast-retry</literal> (safe)</term>
++ <term><literal>dos-endpoint</literal> (unsafe)</term>
+ <listitem>
+ <para>
+ 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.
++ 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.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
-+ <term><literal>poll-counts</literal> (safe)</term>
++ <term><literal>call-count</literal> (safe)</term>
+ <listitem>
+ <para>
-+ Prints the total number of poll() calls to standard error when the
-+ OAuth flow completes. This helps developers debug the async multiplexer
-+ behavior.
++ 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.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
-+ <term><literal>print-plugin-errors</literal> (safe)</term>
++ <term><literal>plugin-errors</literal> (safe)</term>
+ <listitem>
+ <para>
+ Prints plugin loading errors to standard error. This helps developers
@@ doc/src/sgml/libpq.sgml: typedef struct
+ </para>
+
+ <para>
-+ Unsafe options (<literal>http</literal>, <literal>trace</literal>)
-+ require the <literal>UNSAFE:</literal> prefix.
++ Unsafe options (<literal>http</literal>, <literal>trace</literal>,
++ <literal>dos-endpoint</literal>) require the <literal>UNSAFE:</literal> 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 (<literal>fast-retry</literal>,
-+ <literal>poll-counts</literal>, <literal>print-plugin-errors</literal>) can
-+ be used without the prefix.
++ list continue to work. Safe options (<literal>call-count</literal>,
++ <literal>plugin-errors</literal>) can be used without the prefix.
</para>
+
+ <para>
@@ doc/src/sgml/libpq.sgml: typedef struct
+ <para>
+ Examples:
+ <programlisting>
-+PGOAUTHDEBUG=fast-retry,poll-counts <lineannotation>safe options only</lineannotation>
-+PGOAUTHDEBUG=UNSAFE:http,trace <lineannotation>enable HTTP and traffic logging</lineannotation>
-+PGOAUTHDEBUG=UNSAFE:http,poll-counts <lineannotation>mix of unsafe and safe</lineannotation>
-+PGOAUTHDEBUG=UNSAFE <lineannotation>legacy; enables all options</lineannotation>
++PGOAUTHDEBUG=call-count <lineannotation>safe options only</lineannotation>
++PGOAUTHDEBUG=UNSAFE:http,trace <lineannotation>enable HTTP and traffic logging</lineannotation>
++PGOAUTHDEBUG=UNSAFE:http,call-count <lineannotation>mix of unsafe and safe</lineannotation>
+ </programlisting>
+ </para>
+
@@ doc/src/sgml/libpq.sgml: typedef struct
<para>
- 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
-+ <literal>trace</literal> option in particular exposes secrets that can be
-+ used to attack your clients and servers. Do not share the output with third
-+ parties.
++ 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 <literal>trace</literal> output with third parties.
</para>
</warning>
</sect2>
- ## src/interfaces/libpq-oauth/meson.build ##
-@@ src/interfaces/libpq-oauth/meson.build: endif
-
- libpq_oauth_sources = files(
- 'oauth-curl.c',
-+ '../libpq/fe-auth-oauth-debug.c',
- )
-
- # The shared library needs additional glue symbols.
-@@ src/interfaces/libpq-oauth/meson.build: 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: [
-
- ## src/interfaces/libpq/meson.build ##
-@@
-
- libpq_sources = files(
- 'fe-auth-oauth.c',
-+ 'fe-auth-oauth-debug.c',
- 'fe-auth-scram.c',
- 'fe-auth.c',
- 'fe-cancel.c',
-
- ## src/interfaces/libpq-oauth/Makefile ##
-@@ src/interfaces/libpq-oauth/Makefile: 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)
-
- ## src/interfaces/libpq/Makefile ##
-@@ src/interfaces/libpq/Makefile: 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)
-
## src/interfaces/libpq-oauth/oauth-utils.h ##
-@@
- #ifndef OAUTH_UTILS_H
- #define OAUTH_UTILS_H
-
-+#include "fe-auth-oauth.h"
- #include "libpq-fe.h"
- #include "pqexpbuffer.h"
-
@@ src/interfaces/libpq-oauth/oauth-utils.h: 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);
## src/interfaces/libpq/fe-auth-oauth.h ##
@@ src/interfaces/libpq/fe-auth-oauth.h: 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;
+ ## src/interfaces/libpq/oauth-debug.h (new) ##
+@@
++/*-------------------------------------------------------------------------
++ *
++ * 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 */
+
## src/interfaces/libpq-oauth/oauth-curl.c ##
+@@
+
+ #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
@@ src/interfaces/libpq-oauth/oauth-curl.c: 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 */
++ uint32 debug_flags; /* can we give developer assistance? */
int dbg_num_calls; /* (debug mode) how many times were we called? */
};
@@ src/interfaces/libpq-oauth/oauth-curl.c: parse_interval(struct async_ctx *actx,
if (parsed < 1)
- return actx->debugging ? 0 : 1;
-+ return actx->debug_flags.fast_retry ? 0 : 1;
++ return (actx->debug_flags & OAUTHDEBUG_UNSAFE_DOS_ENDPOINT) ? 0 : 1;
else if (parsed >= INT_MAX)
return INT_MAX;
@@ src/interfaces/libpq-oauth/oauth-curl.c: setup_curl_handles(struct async_ctx *ac
CHECK_SETOPT(actx, CURLOPT_NOSIGNAL, 1L, return false);
- if (actx->debugging)
-+ if (actx->debug_flags.trace)
++ if (actx->debug_flags & OAUTHDEBUG_UNSAFE_TRACE)
{
/*
* Set a callback for retrieving error information from libcurl, the
@@ src/interfaces/libpq-oauth/oauth-curl.c: setup_curl_handles(struct async_ctx *ac
#endif
- if (actx->debugging)
-+ if (actx->debug_flags.http)
++ if (actx->debug_flags & OAUTHDEBUG_UNSAFE_HTTP)
protos = unsafe;
CHECK_SETOPT(actx, popt, protos, return false);
@@ src/interfaces/libpq-oauth/oauth-curl.c: check_for_device_flow(struct async_ctx
* we'll use for the flow.
*/
- if (!actx->debugging)
-+ if (!actx->debug_flags.http)
++ if ((actx->debug_flags & OAUTHDEBUG_UNSAFE_HTTP) == 0)
{
if (pg_strncasecmp(provider->device_authorization_endpoint,
HTTPS_SCHEME, strlen(HTTPS_SCHEME)) != 0)
@@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn, stru
* of calls to this function and print that at the end of the flow.
*/
- if (actx->debugging)
-+ if (actx && actx->debug_flags.poll_counts)
++ if (actx->debug_flags & OAUTHDEBUG_CALL_COUNT)
{
actx->dbg_num_calls++;
if (result == PGRES_POLLING_OK || result == PGRES_POLLING_FAILED)
@@ src/interfaces/libpq-oauth/test-oauth-curl.c: 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;
++ actx->debug_flags = OAUTHDEBUG_UNSAFE_ALL;
initPQExpBuffer(&actx->errbuf);
- ## src/interfaces/libpq/fe-auth-oauth-debug.c (new) ##
-@@
-+/*-------------------------------------------------------------------------
-+ *
-+ * 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 <stdio.h>
-+#include <stdlib.h>
-+#include <string.h>
-+
-+#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;
-+}
-
## src/interfaces/libpq/fe-auth-oauth.c ##
+@@
+ #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"
+
@@ src/interfaces/libpq/fe-auth-oauth.c: 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
++ && (oauth_get_debug_flags() & OAUTHDEBUG_UNSAFE_HTTP)
&& pg_strncasecmp(wkuri, HTTP_SCHEME, strlen(HTTP_SCHEME)) == 0)
{
/* Allow http:// for testing only. */
@@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st
* Note that POSIX dlerror() isn't guaranteed to be threadsafe.
*/
- if (oauth_unsafe_debugging_enabled())
-+ if (oauth_get_debug_flags().print_plugin_errors)
++ if (oauth_get_debug_flags() & OAUTHDEBUG_PLUGIN_ERRORS)
fprintf(stderr, "failed dlopen for libpq-oauth: %s\n", dlerror());
return 0;
@@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st
* threadsafety issue.
*/
- if (oauth_unsafe_debugging_enabled())
-+ if (oauth_get_debug_flags().print_plugin_errors)
++ if (oauth_get_debug_flags() & OAUTHDEBUG_PLUGIN_ERRORS)
fprintf(stderr, "failed dlsym for libpq-oauth: %s\n", dlerror());
- dlclose(state->builtin_flow);
+ dlclose(state->flow_module);
@@ src/interfaces/libpq/fe-auth-oauth.c: pqClearOAuthToken(PGconn *conn)
conn->oauth_token = NULL;
}
@@ src/interfaces/libpq/fe-auth-oauth.c: pqClearOAuthToken(PGconn *conn)
-
- return (env && strcmp(env, "UNSAFE") == 0);
-}
+-
+ /*
+ * Hook v1 Poisoning
+ *
+
+ ## src/test/modules/oauth_validator/t/001_server.pl ##
+@@ src/test/modules/oauth_validator/t/001_server.pl: $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";
+
+@@ src/test/modules/oauth_validator/t/001_server.pl: $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.
+
+ ## src/tools/pginclude/headerscheck ##
+@@ src/tools/pginclude/headerscheck: 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: 933f6432f87 < -: ----------- Add new PGOAUTHDEBUG option: issuer-mismatch
[application/octet-stream] v3-0001-Split-PGOAUTHDEBUG-UNSAFE-into-multiple-options.patch (19.4K, 3-v3-0001-Split-PGOAUTHDEBUG-UNSAFE-into-multiple-options.patch)
download | inline diff:
From 743b4a5c3a5781e9c7dd5345110ff7b19c1d6707 Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Tue, 31 Mar 2026 14:08:59 -0700
Subject: [PATCH v3] Split PGOAUTHDEBUG=UNSAFE into multiple options
WIP
Author: Zsolt Parragi <[email protected]>
Co-authored-by: Jacob Champion <[email protected]>
---
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
</para>
<para>
- A "dangerous debugging mode" may be enabled by setting the environment
- variable <envar>PGOAUTHDEBUG=UNSAFE</envar>. 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 <envar>PGOAUTHDEBUG</envar>
+ environment variable. This functionality is provided for ease of local
+ development and testing. The variable accepts a comma-separated list of
+ debug options:
+
+ <programlisting>
+PGOAUTHDEBUG=option1,option2,... <lineannotation>for safe options only</lineannotation>
+PGOAUTHDEBUG=UNSAFE:option1,option2,... <lineannotation>when using unsafe options</lineannotation>
+PGOAUTHDEBUG=UNSAFE <lineannotation>legacy format; enables all options</lineannotation>
+ </programlisting>
+ </para>
- <itemizedlist spacing="compact">
- <listitem>
- <para>
- permits the use of unencrypted HTTP during the OAuth provider exchange
- </para>
- </listitem>
- <listitem>
- <para>
- prints HTTP traffic (containing several critical secrets) to standard
- error during the OAuth flow
- </para>
- </listitem>
- <listitem>
- <para>
- permits the use of zero-second retry intervals, which can cause the
- client to busy-loop and pointlessly consume CPU
- </para>
- </listitem>
- </itemizedlist>
+ <para>
+ Available debug options:
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>http</literal> (unsafe)</term>
+ <listitem>
+ <para>
+ 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.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>trace</literal> (unsafe)</term>
+ <listitem>
+ <para>
+ 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.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>dos-endpoint</literal> (unsafe)</term>
+ <listitem>
+ <para>
+ 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.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>call-count</literal> (safe)</term>
+ <listitem>
+ <para>
+ 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.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>plugin-errors</literal> (safe)</term>
+ <listitem>
+ <para>
+ Prints plugin loading errors to standard error. This helps developers
+ and package maintainers debug issues when the OAuth plugin fails to load.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </para>
+
+ <para>
+ Unsafe options (<literal>http</literal>, <literal>trace</literal>,
+ <literal>dos-endpoint</literal>) require the <literal>UNSAFE:</literal> 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 (<literal>call-count</literal>,
+ <literal>plugin-errors</literal>) can be used without the prefix.
</para>
+
+ <para>
+ 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.
+ </para>
+
+ <para>
+ Examples:
+ <programlisting>
+PGOAUTHDEBUG=call-count <lineannotation>safe options only</lineannotation>
+PGOAUTHDEBUG=UNSAFE:http,trace <lineannotation>enable HTTP and traffic logging</lineannotation>
+PGOAUTHDEBUG=UNSAFE:http,call-count <lineannotation>mix of unsafe and safe</lineannotation>
+ </programlisting>
+ </para>
+
<warning>
<para>
- 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 <literal>trace</literal> output with third parties.
</para>
</warning>
</sect2>
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
view thread (13+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [oauth] Split and extend PGOAUTHDEBUG
In-Reply-To: <CAOYmi+kCYZ3YiOu+oSv1gVW6LXQaNg4BcEpskYizWgfV1z12kA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox