Re: Proposal: Support custom authentication methods using hooks

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: samay sharma <smilingsamay(at)gmail(dot)com>, Jacob Champion <jchampion(at)timescale(dot)com>, Aleksander Alekseev <aleksander(at)timescale(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, "peter(dot)eisentraut(at)enterprisedb(dot)com" <peter(dot)eisentraut(at)enterprisedb(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Proposal: Support custom authentication methods using hooks
Date: 2022-08-03 21:21:58
Message-ID: 20220803212158.GF32653@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Greetings,

* 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.

> If you're ideologically opposed to allowing extensibility in this specific
> area: Say so, instead of repeating this bogus argument over and over.

Considering how much I pushed for and supported the effort to include
SCRAM, I hardly would say that I'm against making improvements in this
area.

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.

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. If we
got SASL added and it included support for SCRAM and Kerberos through
that and was a common enough library that we felt comfortable ripping
out our own implementations in favor of what that library provides,
sure, that'd be something to consider too (though I tend to doubt we'd
get so lucky as to have one that worked with the existing SASL/SCRAM
stuff we have today since we had to do some odd things there with the
username, as I recall).

> > 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 also don't agree that this makes sense as an extension as we don't
> > have any way for extensions to make changes in libpq or psql, again
> > leading to the issue that it either can't be exercised or we create some
> > dependency on an external SASL library for libpq but object to having
> > that same dependency on the server side, which doesn't seem sensible to
> > me. Requiring admins to jump through hoops to install an extension
> > where we have such a dependency on a library anyway doesn't make sense
> > either.
>
> This argument doesn't make a whole lot of sense to me - as explained above you
> can use the existing scram flow for plenty usecases. I'd be a bit more
> convinced if you'd argue that the extension API should just allow providing a
> different source of secrets for the existing scram flow (I'd argue that that's
> not the best point for extensibility, but that'd be more a question of taste).

As I think I said before, I don't particularly object to the idea of
having an alternative backing store for pg_authid (though note that I'm
actively working on extending that further to allow multiple
authenticators to be able to be configured for a role, so whatever that
backing store is would need to be adjusted if that ends up getting
committed into core). That's quite a different thing from my
perspective.

Thanks,

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2022-08-03 21:24:24 Re: A test for replay of regression tests
Previous Message Peter Geoghegan 2022-08-03 21:08:42 Re: pg15b2: large objects lost on upgrade