Re: Add "password_protocol" connection parameter to libpq

On 8/15/19 9:28 PM, Stephen Frost wrote:
> 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.

+1.

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

To be pedantic, +1 on the channel_binding param.

I do agree with option (a), but we should narrow down what that means
for this iteration.

I do see "password_protocol" making sense as a comma-separated list of
options e.g. {plaintext, md5, scram-sha-256}. I would ask if
scram-sha-256-plus makes the list if we have the channel_binding param?

If channel_binding = require, it would essentially ignore any non-plus
options in password_protocol and require scram-sha-256-plus. In a future
scram-sha-512 world, then having scram-sha-256-plus or
scram-sha-512-plus options in "password_protocol" then could be
necessary based on what the user would prefer or require in their
application.

So if we do add a "password_protocol" parameter, then we likely need to
include the -plus's.

I think this is also fairly easy for a user to configure. Some scenarios
scenarios I can see for this are:

1. The user requiring channel binding, so only "channel_binding=require"
is set.

2. A PostgreSQL cluster transitioning between SCRAM + MD5, and the user
setting password_protocol="scram-sha-256" to guarantee md5 auth does not
take place.

3. A user wanting to ensure a stronger method is used, with some
combination of the scram methods or md5, i.e. ensuring plaintext is not
used.

We would need to provide documentation around the types of password
validation methods are used for the external validators (e.g. LDAP) so
the user's known what to expect if their server is using those methods.

Jonathan