Re: Proposal: Support custom authentication methods using hooks

Greetings,

On Fri, Mar 18, 2022 at 00:24 Andres Freund <andres(at)anarazel(dot)de> wrote:

> Hi,
>
> On 2022-03-18 00:03:37 -0400, Stephen Frost wrote:
> > On Thu, Mar 17, 2022 at 23:25 Andres Freund <andres(at)anarazel(dot)de> wrote:
> > > It's imo a separate project to make the client side extensible. There's
> > > plenty
> > > of authentication methods that don't need any further client side
> support
> > > than
> > > the existing SASL (or password if OK for some use case) messages, which
> > > most
> > > clients (including libpq) already know.
> > >
> > > Of course the fact that libpq "only" speaks SCRAM-SHA-256 is a limit
> > > (unless
> > > you have server side access to clear text passwords, but brrr). But
> there's
> > > plenty that can be done with that. Proxying auth via a central postgres
> > > server
> > > with the secrets, sharing secrets with other data stores also
> understanding
> > > SCRAM-SHA-256, ...
> > >
> > > There definitely *also* are important authentication methods that
> can't be
> > > implemented without further client side support. Some of those could
> > > plausibly
> > > be tackled on the protocol / libpq level in a way that they could be
> used
> > > by
> > > multiple authentication methods. Other authentication methods
> definitely
> > > need
> > > dedicated code in the client (although the protocol likely can be
> fairly
> > > generic).
> > >
> > > Given that there's benefit from the server side extensibility alone, I
> > > don't
> > > see much benefit in tying the server side extensibility to the client
> side
> > > extensibility.
> >
> >
> > How are we going to reasonably test this then?
>
> The test module in the patch is a good starting point.

Without a complete, independent, “this is how it will really be used” on
both the server and client side set of tests I’m not sure that it is.

> I also don’t think that I agree that it’s acceptable to only have the
> > ability to extend the authentication on the server side as that implies a
> > whole bunch of client-side work that goes above and beyond just
> > implementing an authentication system if it’s not possible to leverage
> > libpq (which nearly all languages out there use..). Not addressing that
> > side of it also makes me concerned that whatever we do here won’t be
> > right/enough/etc.
>
> You're skipping over my point of everything that can be made to use
> SASL-SCRAM-256 just working with the existing libpq? Again?

The plan is to make scram pluggable on the client side? That isn’t what’s
been proposed here that I’ve seen. Or is the idea that we are going to
have built-in support for some subset of “custom” things, but only on the
client side because it’s hard to do custom things there, but they’re custom
and have to be loaded through shared preload libraries on the server side?

If you want to argue that somehow that communicating via SASL-SCRAM-256
> from a
> plugin is not a sufficient use-case, or that the API should be shaped more
> specifically to just use SASL-SCRAM-256, fine. Argue it. Otherwise I don't
> know what to do except ignore you.

One thrust of my argument is that if we are going to support custom
authentication methods then we need to consider both sides of that, not
just the server side. Another is- it’s highly unlikely that the next
authentication method will actually be able to be implemented using these
custom hooks, based on the history we have seen of pluggable authentication
systems, and therefore I don’t agree that they make sense or will be what
we need in the future, which seems to be a large thrust of the argument
here. I’m also concerned about the risks that this presents to the project
in that there will be arguments about where the fault lies between the
hooks and the core code, as this is just inherently security-related bits,
yet that doesn’t seem to be addressed. Either way though, it strikes me as
likely to be leaving our users in a bad position either way.

>
I also wasn’t aware that we felt that it’s acceptable to just ignore other
committers.

Thanks,

Stephen

>