Subject: Re: [oauth] Stabilize the libpq-oauth ABI (and allow alternative implementations?)

From: Jacob Champion <[email-masked]>
Date: Tue, 24 Feb 2026 14:51:16 -0800

On Tue, Dec 16, 2025 at 9:40 AM Jacob Champion
<[email-masked]> wrote:
> Open questions remain:
> 1) 0004: Any objections to putting PQExpBuffer into libpq-fe.h?

Tom sounded lukewarm to this on the Discord, so I looked into
replacing the new usage with a simple char*.

That actually exposed a bug: appending error data during cleanup
doesn't help us at all, because we've already stopped using the error
buffer. In fact my whole idea of adding things to conn->errorMessage
during a teardown operation has been nearly useless from the
beginning. Elsewhere in libpq, we handle cases like these (which
should ideally just not happen) by printing warnings to stderr, so
v3-0001 does that instead.

Besides exposing a bug, switching to char* means anyone who's not
programming in C and already has code that handles pointer ownership
across the boundary can continue to make use of that, instead of
adapting to a completely new API for this one particular use case. We
already have the cleanup-callback requirement for the token anyway;
it's fine.

> 2) 0004: Thoughts on the v2 inheritance struct style as opposed to
> relying on implementations to double-check the struct length?

Still feels half-dozen or the other to me, so I'm planning to move
ahead with the inheritance model. I'll look into
VALGRIND_MAKE_MEM_NOACCESS (and/or other ways to catch people
scribbling where they shouldn't) for v4.

> 3) 0005: Should I add the thread lock to an init() API, or expose a
> new PQgetThreadLock() that other code can use?

v3-0002 implements PQgetThreadLock(). The conversation with Nico
Williams at [1] cemented this for me; it's entirely possible that
implementations will need the lock at other times besides
initialization, say if they're mixing OAuth with Kerberos. Adding a
way to retrieve it doesn't actually expose new functionality --
applications could always get at our internal implementation by
calling PQregisterThreadLock(NULL) -- but libraries can't use that API
safely.

Also, clients can probably make use of some of the newer ways of doing
this kind of initialization (pthread_once, etc.) that weren't in wide
use back when the init-function design showed up. We may not ever
actually need a separate init function, and if I'm wrong we can always
add one.

I think I'll put this in its own top-level thread for comment.

>    (In other words, a plugin architecture causes the compile-time
>    and run-time dependency arrows to point in opposite directions, so
>    plugins won't be able to rely on the LIBPQ_HAS_* macros to determine
>    what APIs are available to them.)
>
>    (TODO: Are there implications for our use of RTLD_NOW at dlopen() time?

To answer my own question, yes: any future libpq-oauth plugin that
needs PG20+ APIs will have to lazy-load them (weak symbol
declarations, etc.) or else accept that long-running applications may
fail OAuth connections until they restart.

A more general plugin system would need to solve this. For example, we
could load RTLD_LAZY, check for a minimum libpq version declared by
the plugin, and then either upgrade the bindings with RTLD_NOW, or
dlclose() and bail. But I don't think I need to be solving a
nonexistent problem now.

--Jacob

[1] https://postgr.es/m/aSSp03wmNMngi/Oe%40ubby

---

From: Zsolt Parragi <[email-masked]>
Date: Wed, 25 Feb 2026 17:16:33 +0000

+ * TODO: have to think about _all_ the security ramifications of this. What
+ * existing protections in LD_LIBRARY_PATH (and/or SIP) are we potentially
+ * bypassing? Should we check the permissions of the file somehow...?
+ * TODO: maybe disallow anything not underneath LIBDIR? or PKGLIBDIR?
+ * Should it have a naming convention?
+ */
+ const char *env = getenv("PGOAUTHMODULE");

and

> That's exactly the use case. (Also, library clients of postgres that
> should not install application hooks, other monolithic utilities that
> link against libpq, etc.) Who wants to recompile their clients just to
> switch how they get a token?

How would this work with library clients exactly (and even multiple
library clients)?

* Language bindings: there's already ongoing work on adding support
for the hooks into these libraries, so that's not really a use case
(and: 1. there's no recompilation cost for many of them 2. even if
there is, you can't easily use the same language for these plugins,
which is again a downside). And some bindings don't use libpq at all.
* postgres CLI tools: That's one use case, but it could work without
generic user-facing plugin support, limited to these tools.
* postgres_fwd/dblink and similar: I'm not sure how that would work,
what if different extensions require different plugins? This already
seems like a tricky question with the hooks

And from a configuration/security risk point:

* Restricting to system paths could work, but it limits who and how
can add these plugins, and removes the possibility of modifying a
specific application without system permissions. I can't just download
a binary application, and a separate oauth plugin, and use them
together, I need admin permission - that seems strange to me. (Unless
I compile libpq myself in a different prefix?)
* On the other hand requiring applications to specify allowed paths or
plugins directly, without environment variables goes back to the
previous configuration question

> What existing protections in LD_LIBRARY_PATH (and/or SIP) are we potentially bypassing?

>From the ld.so.8 manpage:

> In  secure-execution  mode, preload pathnames containing slashes are ignored.  Furthermore, shared objects are preloaded only from the standard search directories and only if they have set-user-ID mode bit enabled (which  is  not typical).

An easy solution could be using secure_getenv instead of getenv, which
would at least improve the situation?

And a few specific comments about the patches:

+oom:
+ request->error = libpq_gettext("out of memory");
+ return -1;

Shouldn't this also free conninfo if it is allocated?

+/* Features added in PostgreSQL v19: */
+/* Indicates presence of PQgetThreadLock */
+#define LIBPQ_HAS_GET_THREAD_LOCK

Should this be defined to 1?

+ /*
+ * We need to inject necessary function pointers into the module. This
+ * only needs to be done once -- even if the pointers are constant,
+ * assigning them while another thread is executing the flows feels
+ * like tempting fate.
+ */
+ if ((lockerr = pthread_mutex_lock(&init_mutex)) != 0)
+ {
+ /* Should not happen... but don't continue if it does. */
+ Assert(false);

- libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr);
- return 0;
- }
+ libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr);
+ return 0;
+ }

Shouldn't this path return -1, and also dlclose the library?

+
+ dlclose(state->flow_module);
+

Shouldn't dlcloses also reset state->flow_module after?

-free_async_ctx(PGconn *conn, struct async_ctx *actx)
+free_async_ctx(PGoauthBearerRequestV2 *req, struct async_ctx *actx)

req (or conn) doesn't seem to be used in this function, does it need
that parameter?

+ env = strchr(env, '\x01');
+ *env++ = '\0';

Isn't mutating environment variables UB?