Re: Add "password_protocol" connection parameter to libpq

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Jeff Davis <pgsql(at)j-davis(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Add "password_protocol" connection parameter to libpq
Date: 2019-08-16 01:28:15
Message-ID: 20190816012815.GW16436@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Greetings,

* Jeff Davis (pgsql(at)j-davis(dot)com) wrote:
> On Wed, 2019-08-14 at 11:38 +0900, Michael Paquier wrote:
> > What I got in mind was a comma-separated list of authorized protocols
> > which can be specified as a connection parameter, which extends to
> > all
> > the types of AUTH_REQ requests that libpq can understand, plus an
> > extra for channel binding. I also liked the idea mentioned upthread
> > of "any" to be an alias to authorize everything, which should be the
> > default. So you basically get at that:
> > auth_protocol = {any,password,md5,scram-sha-256,scram-sha-256-
> > plus,krb5,gss,sspi}
>
> What about something corresponding to the auth methods "trust" and
> "cert", where no authentication request is sent? That's a funny case,
> because the server trusts the client; but that doesn't imply that the
> client trusts the server.

I agree that "trust" is odd. If you want my 2c, we should have to
explicitly *enable* that for psql, otherwise there's the potential that
a MITM could perform a downgrade attack to "trust" and while that might
not expose a user's password, it could expose other information that the
client ends up sending (INSERTs, UPDATEs, etc).

When it comes to "cert"- there is certainly an authentication that
happens and we already have options in psql/libpq to require validation
of the server. If users want that, they should enable it (I wish we
could make it the default too but that's a different discussion...).

> This is another reason I don't really like the list. It's impossible to
> make it cleanly map to the auth methods, and there are a few ways it
> could be confusing to the users.

I agree with these concerns, just to be clear.

> Given that we all pretty much agree on the need for the separate
> channel_binding param, the question is whether we want to (a) address
> additional use cases with specific parameters that also justify
> themselves; or (b) have a generic list that is supposed to solve many
> future use cases.
>
> I vote (a). With (b), the generic list is likely to cause more
> confusion, ugliness, and clients that break needlessly in the future.

Admittedly, one doesn't preclude the other, and so we could move forward
with the channel binding param, and that's fine- but I seriously hope
that someone finds time to work on further improving the ability for
clients to control what happens during authentication as this, imv
anyway, is an area that we are weak in and it'd be great to improve on
it.

Thanks,

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2019-08-16 03:19:11 Re: [PATCH] Implement INSERT SET syntax
Previous Message Gareth Palmer 2019-08-16 01:21:01 Re: [PATCH] Implement INSERT SET syntax