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: Fri, 3 Apr 2026 10:20:13 -0700
Message-ID: <CAOYmi+mUHbkYyKv=ZffpAAHySvnCdLSy9QL7+tvUm6C+1BWtbQ@mail.gmail.com> (raw)
In-Reply-To: <CAN4CZFOe5P9ZQbxKXJTnJYDF8bpnMGThyQw9QzCffTEZ=MqKvw@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>
<CAOYmi+kCYZ3YiOu+oSv1gVW6LXQaNg4BcEpskYizWgfV1z12kA@mail.gmail.com>
<CAN4CZFPNL4xuNAwmD1-TUR-z+C84axb+cdhip26k3YBFYo9r1g@mail.gmail.com>
<CAOYmi+mN2OeFotwLKwwSDAuXHJ6xmrn3AtkUoHRcfAEENSFbMA@mail.gmail.com>
<CAOYmi+=865C93VegSzD9z4_uvutZwEZEUsS4P6jm3_i0paAMmA@mail.gmail.com>
<CAN4CZFOe5P9ZQbxKXJTnJYDF8bpnMGThyQw9QzCffTEZ=MqKvw@mail.gmail.com>
On Wed, Apr 1, 2026 at 2:13 PM Zsolt Parragi <[email protected]> wrote:
>
> > OAUTHDEBUG_LEGACY_UNSAFE?
>
> That sounds better
Done. v4 also renames oauth_get_debug_flags() to
oauth_parse_debug_flags() (and adds a related doc comment) after Chao
Li's feedback, and tightens up some of the new documentation.
Thanks,
--Jacob
Attachments:
[application/octet-stream] since-v3.nocfbot.diff (6.9K, 2-since-v3.nocfbot.diff)
download | inline diff:
1: 743b4a5c3a5 ! 1: c65877b0b59 Split PGOAUTHDEBUG=UNSAFE into multiple options
@@ Metadata
## Commit message ##
Split PGOAUTHDEBUG=UNSAFE into multiple options
- WIP
+ PGOAUTHDEBUG is a blunt instrument: you get all the debugging features,
+ or nothing. 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 improves
+ 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 <[email protected]>
Co-authored-by: Jacob Champion <[email protected]>
+ Reviewed-by: Chao Li <[email protected]>
+ Reviewed-by: Zsolt Parragi <[email protected]>
+ 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
## doc/src/sgml/libpq.sgml ##
@@ doc/src/sgml/libpq.sgml: typedef struct
@@ doc/src/sgml/libpq.sgml: typedef struct
+ </listitem>
+ </varlistentry>
+ </variablelist>
-+ </para>
+ </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.
++ 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
++ (<literal>call-count</literal>, <literal>plugin-errors</literal>) can be
++ used without the prefix.
+ </para>
+
+ <para>
@@ src/interfaces/libpq/oauth-debug.h (new)
+#define OAUTHDEBUG_PLUGIN_ERRORS (1<<17)
+
+/* all safe and unsafe flags, for the legacy UNSAFE behavior */
-+#define OAUTHDEBUG_UNSAFE_ALL ((uint32) ~0)
++#define OAUTHDEBUG_LEGACY_UNSAFE ((uint32) ~0)
+
+/* Flags are divided into "safe" and "unsafe" based on bit position. */
+#define OAUTHDEBUG_UNSAFE_MASK ((uint32) 0x0000FFFF)
@@ src/interfaces/libpq/oauth-debug.h (new)
+ * 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_get_debug_flags(void)
++oauth_parse_debug_flags(void)
+{
+ uint32 flags = 0;
+ const char *env = getenv("PGOAUTHDEBUG");
@@ src/interfaces/libpq/oauth-debug.h (new)
+ return flags;
+
+ if (strcmp(env, "UNSAFE") == 0)
-+ return OAUTHDEBUG_UNSAFE_ALL;
++ return OAUTHDEBUG_LEGACY_UNSAFE;
+
+ if (strncmp(env, "UNSAFE:", 7) == 0)
+ {
@@ src/interfaces/libpq-oauth/oauth-curl.c: pg_start_oauthbearer(PGconn *conn, PGoa
- /* Should we enable unsafe features? */
- actx->debugging = oauth_unsafe_debugging_enabled();
+ /* Parse debug flags from the environment. */
-+ actx->debug_flags = oauth_get_debug_flags();
++ actx->debug_flags = oauth_parse_debug_flags();
initPQExpBuffer(&actx->work_data);
initPQExpBuffer(&actx->errbuf);
@@ 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 = OAUTHDEBUG_UNSAFE_ALL;
++ actx->debug_flags = OAUTHDEBUG_LEGACY_UNSAFE;
initPQExpBuffer(&actx->errbuf);
@@ src/interfaces/libpq/fe-auth-oauth.c: issuer_from_well_known_uri(PGconn *conn, c
if (!authority_start
- && oauth_unsafe_debugging_enabled()
-+ && (oauth_get_debug_flags() & OAUTHDEBUG_UNSAFE_HTTP)
++ && (oauth_parse_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() & OAUTHDEBUG_PLUGIN_ERRORS)
++ if (oauth_parse_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() & OAUTHDEBUG_PLUGIN_ERRORS)
++ if (oauth_parse_debug_flags() & OAUTHDEBUG_PLUGIN_ERRORS)
fprintf(stderr, "failed dlsym for libpq-oauth: %s\n", dlerror());
dlclose(state->flow_module);
[application/octet-stream] v4-0001-Split-PGOAUTHDEBUG-UNSAFE-into-multiple-options.patch (21.0K, 3-v4-0001-Split-PGOAUTHDEBUG-UNSAFE-into-multiple-options.patch)
download | inline diff:
From c65877b0b59746f7291cc9568ae7167d10bb086f Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Tue, 31 Mar 2026 14:08:59 -0700
Subject: [PATCH v4] Split PGOAUTHDEBUG=UNSAFE into multiple options
PGOAUTHDEBUG is a blunt instrument: you get all the debugging features,
or nothing. 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 improves
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 <[email protected]>
Co-authored-by: Jacob Champion <[email protected]>
Reviewed-by: Chao Li <[email protected]>
Reviewed-by: Zsolt Parragi <[email protected]>
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
---
doc/src/sgml/libpq.sgml | 119 ++++++++++++---
src/interfaces/libpq-oauth/oauth-utils.h | 1 -
src/interfaces/libpq/fe-auth-oauth.h | 1 -
src/interfaces/libpq/oauth-debug.h | 142 ++++++++++++++++++
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, 277 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..0a19c2b553b 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -10643,35 +10643,104 @@ 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, 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
+ (<literal>call-count</literal>, <literal>plugin-errors</literal>) can be
+ used without the prefix.
+ </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..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/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/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/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)
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+mUHbkYyKv=ZffpAAHySvnCdLSy9QL7+tvUm6C+1BWtbQ@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