Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER

From: Stephen Frost <[email-masked]>
Date: Mon, 20 Feb 2023 17:35:36 -0500

Greetings,

* mahendrakar s ([email-masked]) wrote:
> The "issuer" field has been removed to align  with the RFC
> implementation - https://www.rfc-editor.org/rfc/rfc7628.
> This patch "v6" is a single patch to support the OAUTH BEARER token
> through psql connection string.
> Below flow is supported. Added the documentation in the commit messages.
> 
>  +----------------------+                                 +----------+
>   |             +-------+                                  | Postgres |
>   | PQconnect ->|       |                                  |          |
>   |             |       |                                  |   +-----------+
>   |             |       | ---------- Empty Token---------> | > |           |
>   |             | libpq | <-- Error(Discovery + Scope ) -- | < | Pre-Auth  |
>   |          +------+   |                                  |   |  Hook     |
>   |     +- < | Hook |   |                                  |   +-----------+
>   |     |    +------+   |                                  |          |
>   |     v       |       |                                  |          |
>   |  [get token]|       |                                  |          |
>   |     |       |       |                                  |          |
>   |     +       |       |                                  |   +-----------+
>   | PQconnect > |       | --------- Access Token --------> | > | Validator |
>   |             |       | <---------- Auth Result -------- | < |   Hook    |
>   |             |       |                                  |   +-----------+
>   |             +-------+                                  |          |
>   +----------------------+                                 +----------+
> 
> Please note that we are working on modifying/adding new tests (from
> Jacob's Patch) with the latest changes. Will add a patch with tests
> soon.

Having skimmed back through this thread again, I still feel that the
direction that was originally being taken (actually support something in
libpq and the backend, be it with libiddawc or something else or even
our own code, and not just throw hooks in various places) makes a lot
more sense and is a lot closer to how Kerberos and client-side certs and
even LDAP auth work today.  That also seems like a much better answer
for our users when it comes to new authentication methods than having
extensions and making libpq developers have to write their own custom
code, not to mention that we'd still need to implement something in psql
to provide such a hook if we are to have psql actually usefully exercise
this, no?

In the Kerberos test suite we have today, we actually bring up a proper
Kerberos server, set things up, and then test end-to-end installing a
keytab for the server, getting a TGT, getting a service ticket, testing
authentication and encryption, etc.  Looking around, it seems like the
equivilant would perhaps be to use Glewlwyd and libiddawc or libcurl and
our own code to really be able to test this and show that it works and
that we're doing it correctly, and to let us know if we break something.

Thanks,

Stephen

---

From: Jacob Champion <[email-masked]>
Date: Tue, 21 Feb 2023 14:24:12 -0800

On Mon, Feb 20, 2023 at 2:35 PM Stephen Frost <[email-masked]> wrote:
> even LDAP auth work today.

Cool, that helps focus the effort. Thanks!

> That also seems like a much better answer

I don't mind letting clients implement their own flows... as long as
it's optional. So even if we did use a hook in the end, I agree that
we've got to exercise it ourselves.

> that we're doing it correctly, and to let us know if we break something.

The original patchset includes a test server in Python -- a major
advantage being that you can test the client and server independently
of each other, since the implementation is so asymmetric. Additionally
testing against something like Glewlwyd would be a great way to stack
coverage. (If we *only* test against a packaged server, though, it'll
be harder to test our stuff in the presence of malfunctions and other
corner cases.)

Thanks,
--Jacob

---

From: Andrey Chudnovsky <[email-masked]>
Date: Tue, 21 Feb 2023 23:00:46 -0800

Thanks for the feedback,

> to provide such a hook if we are to have psql actually usefully exercise

libpq implementation is the long term plan. However, our intention is
to start with the protocol implementation which allows us to build on
top of.

While device code is the right solution for psql, having that as the
only one can result in incentive to use it in the cases it's not
intended to.
Reasonably good implementation should support all of the following:
(1.) authorization code with pkce (for GUI applications)
(2.) device code (for console user logins)
(3.) client secret
(4.) some support for client certificate flow

(1.) and (4.) require more work to get implemented, though necessary
for encouraging the most secure grant types.
As we didn't have those pieces, we're proposing starting with the
protocol, which can be used by the ecosystem to build token flow
implementations.
Then add the libpq support for individual grant types.

We originally looked at starting with bare bone protocol for PG16 and
adding libpq support in PG17.
That plan won't happen, though still splitting the work into separate
stages would make more sense in my opinion.

Several questions to follow up:
(a.) Would you support committing the protocol first? or you see libpq
implementation for grants as the prerequisite to consider the auth
type?
(b.) As of today, the server side core does not validate that the
token is actually a valid jwt token. Instead relies on the extensions
to do the validation.
Do you think server core should do the basic validation before passing
to extensions to prevent the auth type being used for anything other
than OAUTH flows?

Tests are the plan for the commit-ready implementation.

Thanks!
Andrey.

---

From: Stephen Frost <[email-masked]>
Date: Thu, 23 Feb 2023 13:47:55 -0500

Greetings,

> Cool, that helps focus the effort. Thanks!

Great, glad to hear that.

> we've got to exercise it ourselves.

This really doesn't feel like a great area to try and do hooks or
similar in, not the least because that approach has been tried and tried
again (PAM, GSSAPI, SASL would all be examples..) and frankly none of
them has turned out great (which is why we can't just tell people "well,
install the pam_oauth2 and watch everything work!") and this strikes me
as trying to do that yet again but worse as it's not even a dedicated
project trying to solve the problem but more like a side project.  SCRAM
was good, we've come a long way thanks to that, this feels like it
should be more in line with that rather than trying to invent yet
another new "generic" set of hooks/APIs that will just cause DBAs and
our users headaches trying to make work.

> be harder to test our stuff in the presence of malfunctions and other

Oh, that's even better- I agree entirely that having test code that can
be instructed to return specific errors so that we can test that our
code responds properly is great (and is why pgbackrest has things like
a stub'd out libpq, fake s3, GCS, and Azure servers, and more) and would
certainly want to keep that, even if we also build out a test that uses
a real server to provide integration testing with not-our-code too.

Thanks!

Stephen

---

From: Andrey Chudnovsky <[email-masked]>
Date: Thu, 23 Feb 2023 15:04:22 -0800

> project trying to solve the problem but more like a side project.

In this case it's not intended to be an open-ended hook, but rather an
implementation of a specific rfc (rfc-7628) which defines a
client-server communication for the authentication flow.
The rfc itself does leave a lot of flexibility on specific parts of
the implementation. Which do require hooks:
(1.) Server side hook to validate the token, which is specific to the
OAUTH provider.
(2.) Client side hook to request the client to obtain the token.

On (1.), we would need a hook for the OAUTH provider extension to do
validation. We can though do some basic check that the credential is
indeed a JWT token signed by the requested issuer.

Specifically (2.) is where we can provide a layer in libpq to simplify
the integration. i.e. implement some OAUTH flows.
Though we would need some flexibility for the clients to bring their own token:
For example there are cases where the credential to obtain the token
is stored in a separate secure location and the token is returned from
a separate service or pushed from a more secure environment.

> our users headaches trying to make work.
As I mentioned above, it's an rfc implementation, rather than our invention.
When it comes to DBAs and the users.
Builtin libpq implementations which allows psql and pgadmin to
seamlessly connect should suffice those needs.
While extensibility would allow the ecosystem to be open for OAUTH
providers, SAAS developers, PAAS providers and other institutional
players.

Thanks!
Andrey.

---

From: Stephen Frost <[email-masked]>
Date: Mon, 27 Feb 2023 15:31:04 -0500

Greetings,

> the implementation. Which do require hooks:

Color me skeptical on an RFC that requires hooks.

> (2.) Client side hook to request the client to obtain the token.

Perhaps I'm missing it... but weren't these handled with what the
original patch that Jacob had was doing?

> a separate service or pushed from a more secure environment.

In those cases... we could, if we wanted, simply implement the code to
actually pull the token, no?  We don't *have* to have a hook here for
this, we could just make it work.

> As I mentioned above, it's an rfc implementation, rather than our invention.

While I only took a quick look, I didn't see anything in that RFC that
explicitly says that hooks or a plugin or a library or such is required
to meet the RFC.  Sure, there are places which say that the
implementation is specific to a particular server or client but that's
not the same thing.

> providers, SAAS developers, PAAS providers and other institutional

Each to end up writing their own code to do largely the same thing
without the benefit of the larger community to be able to review and
ensure that it's done properly?

That doesn't sound like a great approach to me.

Thanks,

Stephen