Re: Proposal: Support custom authentication methods using hooks

Greetings,

* Andres Freund (andres(at)anarazel(dot)de) wrote:
> On 2022-08-03 17:21:58 -0400, Stephen Frost wrote:
> > * Andres Freund (andres(at)anarazel(dot)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