Re: [PoC] Federated Authn/z with OAUTHBEARER

From: Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Bruce Momjian <bruce(at)momjian(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Antonin Houska <ah(at)cybertec(dot)at>
Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER
Date: 2025-03-26 19:09:14
Message-ID: CAOYmi+=PkcF8DRw49Jp-9AZDobahOHwnH2p0snYPsv94x==3oA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Mar 21, 2025 at 11:02 AM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> >> How about we provide the current libpq.so without linking to curl and also a
> >> libpq-oauth.so that has curl support? If we do it right libpq-oauth.so would
> >> itself link to libpq.so, making libpq-oauth.so a fairly small library.
> >>
> >> That way packagers can split libpq-oauth.so into a separate package, while
> >> still just building once.
> >>
> >> That'd be a bit of work on the buildsystem side, but it seems doable.
> >
> > That certainly seems worth exploring.
>
> This is being worked on.

Attached is a proof of concept, with code from Daniel and myself,
which passes the CI as a starting point.

Roughly speaking, some things to debate are
- the module API itself
- how much to duplicate from libpq vs how much to export
- is this even what you had in mind

libpq-oauth.so is dlopen'd when needed. If it's not found or it
doesn't have the right symbols, builtin OAuth will not happen. Right
now we have an SO version of 1; maybe we want to remove the SO version
entirely to better indicate that it shouldn't be linked?

Two symbols are exported for the async authentication callbacks. Since
the module understands PGconn internals, maybe we could simplify this
to a single callback that manipulates the connection directly.

To keep the diff small to start, the current patch probably exports
too much. I think appendPQExpBufferVA makes sense, considering we
export much of the PQExpBuffer API already, but I imagine we won't
want to expose the pg_g_threadlock symbol. (libpq could maybe push
that pointer into the libpq-oauth module at load time, instead of
having the module pull it.) And we could probably go either way with
the PQauthDataHook; I prefer having a getter and setter for future
flexibility, but it would be simpler to just export the hook directly.

The following functions are duplicated from libpq:
- libpq_block_sigpipe
- libpq_reset_sigpipe
- libpq_binddomain
- libpq_[n]gettext
- libpq_append_conn_error
- oauth_unsafe_debugging_enabled

Those don't seem too bad to me, though maybe there's a good way to
deduplicate. But i18n needs further work. It builds right now, but I
don't think it works yet.

WDYT?

Thanks,
--Jacob

Attachment Content-Type Size
0001-WIP-split-Device-Authorization-flow-into-dlopen-d-mo.patch application/octet-stream 20.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Álvaro Herrera 2025-03-26 19:21:52 Re: vacuum_truncate configuration parameter and isset_offset
Previous Message Nikolay Shaplov 2025-03-26 19:05:29 Re: vacuum_truncate configuration parameter and isset_offset