Horton

Re: [PoC] Federated Authn/z with OAUTHBEARER

Jacob Champion <jacob.champion@enterprisedb.com>

View thread
From: Jacob Champion <jacob.champion@enterprisedb.com>
To: Antonin Houska <ah@cybertec.at>
Cc: Daniel Gustafsson <daniel@yesql.se>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2024-10-31T16:24:11Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. meson: Fix install-quiet after clean

  2. oauth: Run Autoconf tests with correct compiler flags

  3. Link libpq with libdl if the platform needs that.

  4. Doc: correct spelling of meson switch.

  5. oauth: Correct SSL dependency for libpq-oauth.a

  6. oauth: Fix Autoconf build on macOS

  7. oauth: Move the builtin flow into a separate module

  8. Remove a stray "pgrminclude" annotation

  9. oauth: Simplify copy of PGoauthBearerRequest

  10. oauth: Improve validator docs on interruptibility

  11. oauth: Disallow synchronous DNS in libcurl

  12. oauth: Fix postcondition for set_timer on macOS

  13. oauth: Use IPv4-only issuer in oauth_validator tests

  14. Work around OAuth/EVFILT_TIMER quirk on NetBSD.

  15. oauth: Fix incorrect const markers in struct

  16. Add missing entry to oauth_validator test .gitignore

  17. cirrus: Temporarily fix libcurl link error

  18. Add support for OAUTHBEARER SASL mechanism

  19. libpq: Handle asynchronous actions during SASL

  20. require_auth: prepare for multiple SASL mechanisms

  21. Move PG_MAX_AUTH_TOKEN_LENGTH to libpq/auth.h

  22. Make SASL max message length configurable

  23. jsonapi: fully initialize dummy lexer

  24. common/jsonapi: support libpq as a client

  25. Remove fe_memutils from libpgcommon_shlib

  26. Revert ECPG's use of pnstrdup()

  27. Explicitly require password for SCRAM exchange

  28. Refactor SASL exchange to return tri-state status

On Thu, Oct 31, 2024 at 4:05 AM Antonin Houska <ah@cybertec.at> wrote:
> > >   And regardless, the library appears to be loaded by every backend during
> > >   authentication. Why isn't it loaded by postmaster like libraries listed in
> > >   shared_preload_libraries? fork() would then ensure that the backends do have
> > >   the library in their address space.
> >
> > It _can_ be, if you want -- there's nothing that I know of preventing
> > the validator from also being preloaded with its own _PG_init(), is
> > there? But I don't think it's a good idea to force that, for the same
> > reason we want to allow SIGHUP.
>
> Loading the library by postmaster does not prevent the backends from reloading
> it on SIGHUP later. I was simply concerned about performance. (I proposed
> loading the library at another stage of backend initialization rather than
> adding _PG_init() to it.)

Okay. I think this is going to be one of the slower authentication
methods by necessity: the builtin flow in libpq requires a human in
the loop, and an online validator is going to be making several HTTP
calls from the backend. So if it turns out later that we need to
optimize the backend logic, I'd prefer to have a case study in hand;
otherwise I think we're likely to optimize the wrong things.

> Easiness of reading is the only "question" here :-) It's might not always be
> obvious why a variable should have some particular value. In general, the
> Assert() statements are almost always preceded with a comment in the PG
> source.

Oh, an assertion label! I can absolutely add one; I originally thought
you were proposing a label for the case itself.

> > >   Or, doesn't the FE_OAUTH_INIT branch of the switch statement actually fit
> > >   better into oauth_init()?
> >
> > oauth_init() is the mechanism initialization for the SASL framework
> > itself, which is shared with SCRAM. In the current architecture, the
> > init callback doesn't take the initial client response into
> > consideration at all.
>
> Sure. The FE_OAUTH_INIT branch in oauth_exchange() (FE) also does not generate
> the initial client response.

It might, if it ends up falling through to FE_OAUTH_REQUESTING_TOKEN.
There are two paths that can do that: the case where we have no
discovery URI, and the case where a custom user flow returns a token
synchronously (it was probably cached).

> Anyway, are you sure that pg_SASL_continue() can also receive the SASL_ASYNC
> value from oauth_exchange()? My understanding is that pg_SASL_init() receives
> it if there is no token, but after that, oauth_exchange() is not called util
> the token is available, and thus it should not return SASL_ASYNC anymore.

Correct -- the only way for the current implementation of the
OAUTHBEARER mechanism to return SASL_ASYNC is during the very first
call. That's not an assumption I want to put into the higher levels,
though; I think Michael will be unhappy with me if I introduce
additional SASL coupling after the decoupling work that's been done
over the last few releases. :D

Thanks again,
--Jacob