Horton

Re: [PoC] Federated Authn/z with OAUTHBEARER

Jacob Champion <pchampion@vmware.com>

View thread
From: Jacob Champion <pchampion@vmware.com>
To: "hlinnaka@iki.fi" <hlinnaka@iki.fi>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2021-06-22T23:22:31Z
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 Fri, 2021-06-18 at 11:31 +0300, Heikki Linnakangas wrote:
> On 08/06/2021 19:37, Jacob Champion wrote:
> > We've been working on ways to expand the list of third-party auth
> > methods that Postgres provides. Some example use cases might be "I want
> > to let anyone with a Google account read this table" or "let anyone who
> > belongs to this GitHub organization connect as a superuser".
> 
> Cool!

Glad you think so! :D

> > The iddawc dependency for client-side OAuth was extremely helpful to
> > develop this proof of concept quickly, but I don't think it would be an
> > appropriate component to build a real feature on. It's extremely
> > heavyweight -- it incorporates a huge stack of dependencies, including
> > a logging framework and a web server, to implement features we would
> > probably never use -- and it's fairly difficult to debug in practice.
> > If a device authorization flow were the only thing that libpq needed to
> > support natively, I think we should just depend on a widely used HTTP
> > client, like libcurl or neon, and implement the minimum spec directly
> > against the existing test suite.
> 
> You could punt and let the application implement that stuff. I'm 
> imagining that the application code would look something like this:
> 
> conn = PQconnectStartParams(...);
> for (;;)
> {
>      status = PQconnectPoll(conn)
>      switch (status)
>      {
>          case CONNECTION_SASL_TOKEN_REQUIRED:
>              /* open a browser for the user, get token */
>              token = open_browser()
>              PQauthResponse(token);
>              break;
>          ...
>      }
> }

I was toying with the idea of having a callback for libpq clients,
where they could take full control of the OAuth flow if they wanted to.
Doing it inline with PQconnectPoll seems like it would work too. It has
a couple of drawbacks that I can see:

- If a client isn't currently using a poll loop, they'd have to switch
to one to be able to use OAuth connections. Not a difficult change, but
considering all the other hurdles to making this work, I'm hoping to
minimize the hoop-jumping.

- A client would still have to receive a bunch of OAuth parameters from
some new libpq API in order to construct the correct URL to visit, so
the overall complexity for implementers might be higher than if we just
passed those params directly in a callback.

> It would be nice to have a simple default implementation, though, for 
> psql and all the other client applications that come with PostgreSQL itself.

I agree. I think having a bare-bones implementation in libpq itself
would make initial adoption *much* easier, and then if specific
applications wanted to have richer control over an authorization flow,
then they could implement that themselves with the aforementioned
callback.

The Device Authorization flow was the most minimal working
implementation I could find, since it doesn't require a web browser on
the system, just the ability to print a prompt to the console. But if
anyone knows of a better flow for this use case, I'm all ears.

> > If you've read this far, thank you for your interest, and I hope you
> > enjoy playing with it!
> 
> A few small things caught my eye in the backend oauth_exchange function:
> 
> > +       /* Handle the client's initial message. */
> > +       p = strdup(input);
> 
> this strdup() should be pstrdup().

Thanks, I'll fix that in the next re-roll.

> In the same function, there are a bunch of reports like this:
> 
> >                    ereport(ERROR,
> > +                          (errcode(ERRCODE_PROTOCOL_VIOLATION),
> > +                           errmsg("malformed OAUTHBEARER message"),
> > +                           errdetail("Comma expected, but found character \"%s\".",
> > +                                     sanitize_char(*p))));
> 
> I don't think the double quotes are needed here, because sanitize_char 
> will return quotes if it's a single character. So it would end up 
> looking like this: ... found character "'x'".

I'll fix this too. Thanks!

--Jacob