Re: [PoC] Federated Authn/z with OAUTHBEARER

From: Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, Antonin Houska <ah(at)cybertec(dot)at>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER
Date: 2024-11-26 13:51:20
Message-ID: CAOYmi+=ceXepXOrQXsxca-u57GxM+x_q34fpyNJ8PLmbUWPwEQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Nov 14, 2024 at 9:45 AM Jacob Champion
<jacob(dot)champion(at)enterprisedb(dot)com> wrote:
> 1. libpq connection strings must specify exactly one issuer

This is done in v37. The oauth_issuer connection parameter is now
required if the server requests OAUTHBEARER.

> 2. the discovery document coming from the server must belong to that
> libpq issuer

This is also complete. If, for example, the client uses
"oauth_issuer=https://example.com/issuer", then the server is only
allowed to send it to one of four places -- either the three standard
URIs

- https://example.com/issuer/.well-known/openid-configuration
- https://example.com/.well-known/oauth-authorization-server/issuer
- https://example.com/.well-known/openid-configuration/issuer

or the decidedly nonstandard

- https://example.com/issuer/.well-known/oauth-authorization-server

The query for server parameters may be skipped, whether for
performance or paranoia reasons, by providing one of the above
well-known URIs directly. For example,
"oauth_issuer=https://example.com/issuer/.well-known/openid-configuration".
You need to have worked out your oauth_scope setting in advance,
though, if you're not willing to ask the server for it.

> 3. the HBA should allow a choice of discovery document and validator

oauth_validator_library is now oauth_validator_libraries, and the HBA
supports a `validator` option.

The `issuer` option now works the same way as `oauth_issuer` on the
client: you either provide an issuer identifier or a well-known
discovery URI. If you only provide the issuer, the server will choose
OIDC-style discovery, which seems to be far and away the most popular
choice. You can override that with whatever well-known URI you like,
but libpq still only supports the four options above for its own
safety.

I'm not sure I like the way this option works in the HBA. The client
can immediately complain if you provide a useless URI, because it has
to perform all those checks anyway, but the server has no such
guardrails. I'm wondering if I should maybe split the HBA options into
two -- "issuer" and an optional "discovery" option, perhaps? -- and
try to help out the DBAs a little more.

> I don't like the idea of pg_hba being used to load
> arbitrary libraries, though; I think the superuser should have to
> designate a pool of "blessed" validator libraries to load through a
> GUC. As a UX improvement for the common case, maybe we don't require
> the HBA to have an explicit validator parameter if the conf contains
> exactly one blessed library.

I've implemented both of these. For example, if you have

oauth_validator_libraries = 'provider1'

then your oauth HBA lines can omit `validator`, and provider1 will be
used for all connections. Once you add a second, though:

oauth_validator_libraries = 'provider1, provider2'

then HBA parsing will complain about an ambiguous configuration until
you provide `validator` settings for each:

LOG: authentication method "oauth" requires argument "validator"
to be set when oauth_validator_libraries contains multiple options

> In case someone does want to develop a multi-issuer validator (say, to
> deal with the providers that have multiple issuers underneath their
> umbrella), we need to make sure that the configured issuer in use is
> available to the validator, so that they aren't susceptible to a
> mix-up attack of their own.

This is already provided via MyProcPort, which the library is free to
examine (and our tests currently make use of it, which I'd forgotten).

--

v37 also chips away at some of the upthread feedback:
- tests that expect no issuer connections have been changed to use an
invalid IP address
- the curl_multi_socket_all deprecation history has been recorded
- oauth_validator_libraries has been added to the config sample
- pgperltidy has been run on the new TAP tests

I also added an envvar (PGOAUTHCAFILE), which is itself buried
underneath PGOAUTHDEBUG=UNSAFE, to change the CA file in use by Curl.
The Python tests use that to test HTTPS issuers. I'm not convinced yet
whether that should be a fully fleshed out/documented feature for v1;
the whole idea of the OAuth system is that your browser should be able
to connect, and if you need to tweak the X.509 tree for your provider
then you're probably going to be doing it at the system level, not at
the application level. But it's a possibility.

Thanks,
--Jacob

Attachment Content-Type Size
since-v36.diff.txt text/plain 104.6 KB
v37-0001-Add-OAUTHBEARER-SASL-mechanism.patch application/octet-stream 248.7 KB
v37-0002-DO-NOT-MERGE-Add-pytest-suite-for-OAuth.patch application/octet-stream 204.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2024-11-26 13:56:12 Re: Don't overwrite scan key in systable_beginscan()
Previous Message David E. Wheeler 2024-11-26 13:48:05 Re: Potential ABI breakage in upcoming minor releases