Re: Add "password_protocol" connection parameter to libpq

On 8/13/19 12:25 PM, Jeff Davis wrote:
> On Tue, 2019-08-13 at 11:56 +0900, Michael Paquier wrote:
>> I tend to prefer #2 as well and that's the kind of approach we were
>> tending to agree on when we discussed this issue during the v11 beta
>> for the downgrade issues with libpq. And as you say extend it so as
>> we can apply filtering of more AUTH_REQ requests, inclusing GSS and
>> krb5.
>
> Can you please offer a concrete proposal? I know the proposals I've put
> out aren't perfect (otherwise there wouldn't be three of them), so if
> you have something better, please share.

I think all of them get at the same thing, i.e. specifying which
password protocol you want to use, and a lot of it is a matter of how
much onus we want to put on the user.

Back to the thee proposals[1], I've warmed up to #3 a bit. I do think it
puts more onus on the client to set the correct knobs to get the desired
outcome, but what I like is the specific `channel_binding=require`
attribute.

However, I don't think it's completely future proof to adding a new hash
digest. If we wanted to prevent someone from using scram-sha-256 in a
scram-sha-512 world, we'd likely need an option for that.

Alternatively, we could combine 2 & 3, e.g.:

channel_binding = {disable|prefer|require}

# comma-separated list of protocols that are ok to the user, remove
# ones you don't want. empty means all is ok
password_protocol = "plaintext,md5,scram-sha-256,scram-sha-256-plus"

If the client selects "channel_binding=require" but does not include a
protocol that supports it, we should error. Likewise, if the client does
something like "channel_binding=require" and
"password_protocol=scram-sha-256,scram-sha-256-plus" but the server
refuses to do channel binding, we should error.

I think this gives us both future-proofing against newer password digest
methods + the fix for the downgrade issue.

I would not be opposed to extending "password_protocol" to read
"auth_protocol" or the like and work for everything covered in AUTH_REQ,
but I would need to think about it some more.

Thanks,

Jonathan

[1]
https://www.postgresql.org/message-id/daf0017a1a5c2caabf88a4e00f66b4fcbdfeccad.camel%40j-davis.com