Re: Proposal: Support custom authentication methods using hooks - Mailing list pgsql-hackers

Greetings,

Want to resurface the OAUTH support topic in the context of the
concerns raised here.

> How about- if we just added OAUTH support directly into libpq and the
> backend, would that work with Azure's OIDC provider? If not, why not?
> If it does, then what's the justification for trying to allow custom
> backend-only authentication methods?

We've explored this option, and prepared a patch which has bare-bone
OAUTHBEARER mechanism implementation on both server- and libpq- sides.
Based on existing SASL exchange support used for SCRAM.

Most of the work has been done by @Jacob Champion and was referenced
in this thread before.
We validated the approach would work with Azure AD and other OIDC
providers with token validation logic delegated to provider-specific
extension.

The thread link is here:

https://www.postgresql.org/message-id/flat/CABkiuWo4fJQ7dhqgYLtJh41kpCkT6iXOO8Eym3Rdh5tx2RJCJw%40mail.gmail.com#f94c36969a68a07c087fa9af0f5401e1

With the client- and server- side support, the proposed patch would allow:
- Identity providers to publish PG extensions to validate their
specific token format.
- Client ecosystem to build generic OAUTH flows using OIDC provider
metadata defined in RFCs
- Cloud providers to support a combination of Identity providers they work with.

Looking forward to bring the discussion to that thread, and decide if
we can proceed with the OAUTH support.

Thanks,
Andrey.

On Wed, Jan 25, 2023 at 10:55 AM Stephen Frost <sfrost@snowman.net> wrote:
>
> Greetings,
>
> * Andres Freund (andres@anarazel.de) wrote:
> > On 2022-08-03 17:21:58 -0400, Stephen Frost wrote:
> > > * Andres Freund (andres@anarazel.de) wrote:
> > > > On 2022-08-03 16:28:08 -0400, Stephen Frost wrote:
> > > > > Again, server-side only is not interesting and not a direction that
> > > > > makes sense to go in because it doesn't provide any way to have
> > > > > trust established in both directions, which is what all modern
> > > > > authentication methods do (certificates, kerberos, scram) and is what we
> > > > > should expect from anything new in this space.
> > > >
> > > > As explained repeatedly before, that's plainly not true. The patch allows
> > > > e.g. to use the exact scram flow we already have, with the code we have for
> > > > that (e.g. using a different source of secret).  In fact the example extension
> > > > does so (starting in v3, from 2022-03-15):
> > >
> > > Sure, thanks to the bespoke code in libpq for supporting SCRAM.  I don't
> > > think it makes sense to move the server-side code for that into an
> > > extension as it just makes work for a bunch of people.  Having a way to
> > > have the authenticator token for scram exist elsewhere doesn't strike me
> > > as unreasonable but that's not the same thing and is certainly more
> > > constrained in terms of what it's doing.
> >
> > The question is: Why is providing that ability not a good step, even if
> > somewhat constrainted? One argument would be that a later "protocol level
> > extensibility" effort would require significant changes to the API - but I
> > don't think that'd be the case. And even if, this isn't a large extensibility
> > surface, so API evolution wouldn't be a problem.
>
> I'm quite confused by this as I feel like I've said multiple times that
> having a way to have the SCRAM authenticator token come from somewhere
> else seems reasonable.  If that's not what you're referring to above by
> 'that ability' then it'd really help if you could clarify what you mean
> there.
>
> > > Further, I outlined exactly how extensability in this area could be
> > > achieved: use some good third party library that provides multiple SASL
> > > methods then just add support for that library to *PG*, on both the
> > > libpq and the server sides.
> >
> > > What I don't think makes sense is adding extensibility to the server
> > > side, especially if it's through a 3rd party library, but then not
> > > adding support for that to libpq.  I don't follow what the rationale is
> > > for explicitly excluding libpq from this discussion.
> >
> > The rationale is trivial: Breaking down projects into manageable, separately
> > useful, steps is a very sensible way of doing development. Even if we get
> > protocol extensibility, we're still going to need an extension API like it's
> > provided by the patchset. After protocol extensibility the authentication
> > extensions would then have more functions it could call to control the auth
> > flow with the client.
> >
> > > To be clear- I'm not explicitly saying that we can only add
> > > extensibility with SCRAM, I'm just saying that whatever we're doing here
> > > to add other actual authentication methods we should be ensuring is done
> > > on both sides with a way for each side to authenticate the other side,
> > > as all of the modern authentication methods we have already do.
> >
> > But *why* do these need to be tied together?
>
> I'm also confused by this.  Surely you'd need a way to *test* such a new
> authentication library being added to the core code and that naturally
> requires having it be implemented on both sides, so I don't see why you
> wouldn't just do that from the get go.  I don't see how it makes sense
> to talk about such a project as if it could possibly be server-side
> only.  The example being discussed- adding SASL support using a third
> party library such as libsasl2, would certainly be something that you'd
> add on both sides and then test if you got it right and make sure that
> it worked.  Recent examples also bear this out- I can't imagine anyone
> would have gotten behind the idea of adding GSSAPI encryption to just
> the server side, even if you could somehow argue that it would have been
> a separately useful step because maybe someone out there happened to
> have implemented it in their own client.  The same is true of Kerberos
> credential delegation- yeah, we could implement that just on the server,
> but if libpq isn't doing it then we couldn't test it as part of the core
> project and it's certainly not some huge amount of additional work to
> implement it in libpq too, where it then immediately becomes available
> to a large number of downstream interfaces like psycopg2 and various
> others.  The few non-libpq-based interfaces typically follow what we do
> in core on the client side anyway, such as the JDBC project.
>
> If the idea is that we should just add hooks to allow someone to
> reimplement SCRAM but external to the core server and only on the server
> side then what's going to be on the client side to interface with that?
> How would one add support for whatever that new authentication method is
> to libpq, where we don't have the ability to plug in other
> authentication methods?  Further, how would we have any idea how to test
> that we don't break that?  Lastly, if we find some serious flaw in our
> existing implementation of SCRAM (no matter how unlikely ...) that
> requires us to change these hooks, we're in a world of trouble.  This is
> quite different from other parts of the system as it's inherent to how
> users are externally authenticated and that's not an area to take
> lightly.
>
> > > > > If anything, the other auth methods should be ripped out entirely (password,
> > > > > md5, ldap, etc), but certainly not used as a basis for new work or a place
> > > > > to try and add new features, as they're all well known to have serious
> > > > > vulnerabilities.
> > > >
> > > > I don't think we'd help users if we ripped out all those methods without a
> > > > replacement, but relegating them to contrib/ and thus requiring that they
> > > > explicitly get configured in the server seems like a decent step. But imo
> > > > that's a separate discussion.
> > >
> > > We have a replacement for password and md5 and it's SCRAM.  For my 2c, I
> > > don't see the value in continuing to have those in any form at this
> > > point.  I concede that I may not get consensus on that but I don't
> > > really see how moving them to contrib would actually be helpful.
> >
> > I'm much more on board with ripping out password and md5 than ldap, radius,
> > pam et al.
>
> That would at least be some progress in the right direction, though not
> as far as I'd like to go.  Combined with adding options to libpq which
> prevents it from ever sending a password in the clear across the wire,
> as required by all of those other methods, would definitely help since
> then we could strongly encourage users to enable that option, or better,
> set it by default and force users to have to set some option that
> explicitly allows such insecure password handling to be allowed.
>
> Thanks,
>
> Stephen