Re: Proposal: Support custom authentication methods using hooks

Greetings,

On Thu, Mar 17, 2022 at 23:25 Andres Freund <andres(at)anarazel(dot)de> wrote:

> On 2022-03-17 22:13:27 -0400, Stephen Frost wrote:
> > This isn’t the first time I asked about this on this thread, yet the
> > question about why this is only being discussed as backend changes, and
> > with the only goal seeming to be to have a backend loadable module,
> without
> > anything on the client side for something that’s clearly both a server
> and
> > client side concern, seems to just be ignored, which make me feel like my
> > comments and the concerns that I raise aren’t being appreciated.
>
> 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?

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.

This is not an area, in my view, where flexibility for the sake of it is
good. Misunderstandings between the client and the server or between how
the core code and the hooks interact seem very likely and could easily lead
to insecure setups and a good chance for CVEs.

Much like encryption, authentication isn’t easy to get right. We should be
working to implement standard that experts, through RFCs and similar
well-defined protocols, have defined in the core code with lots of eyes
looking at it.

So, very much a -1 from me for trying to extend the backend in this way. I
see a lot of risk and not much, if any, benefit. I’d much rather see us
add support directly in the core code, on the client and sever side, for
OAUTH and other well defined authentication methods, and we even have an
active patch for that could make progress on that with a bit of review.

Thanks,

Stephen

>