Re: Add "password_protocol" connection parameter to libpq - Mailing list pgsql-hackers

On 09/08/2019 23:27, Jonathan S. Katz wrote:
> On 8/9/19 11:51 AM, Jeff Davis wrote:
>> On Fri, 2019-08-09 at 09:28 -0400, Stephen Frost wrote:
>>> Having an 'any' option, as mentioned before, could be an alternative
>>> though.
>>
>> ...
>>
>>> I agree with the point that there isn't any guarantee that it'll
>>> always
>>> be clear-cut as to which of two methods is "better".
>>>
>>>  From a user perspective, it seems like the main things are "don't
>>> send
>>> my password in the clear to the server", and "require channel binding
>>> to
>>> prove there isn't a MITM".  I have to admit that I like the idea of
>>> requiring scram to be used and not allowing md5 though.
>>
>> So it seems like we are leaning toward:
>>
>>     password_protocol = any | {plaintext,md5,scram-sha-256,scram-sha-
>> 256-plus}[,...]
> 
> First, thanks for proposing / working on this, I like the idea! :) Happy
> to test/review.
> 
> As long as this one can handle the current upgrade path that's in place
> for going from md5 to SCRAM (which AIUI it should) this makes sense to
> me. As stated above, there is a clear hierarchy.
> 
> I would almost argue that "plaintext" shouldn't even be an option...if
> you have "any" set (arguably default?) then plaintext is available. With
> our currently supported versions + driver ecosystem, I hope no one needs
> to support a forced plaintext setup.

Keep in mind that RADIUS, LDAP and PAM authentication methods are 
'plaintext' over the wire. It's not that bad, when used with 
sslmode=verify-ca/full.

>> Or maybe:
>>
>>     channel_binding = {disable|prefer|require}
>>     password_plaintext = {disable|enable}
>>     password_md5 = {disable|enable}
>>
>> That seems reasonable. It's three options, but no normal use case would
>> need to set more than two, because channel binding forces scram-sha-
>> 256-plus.
> 
> Seems to be a lot to configure. I'm more of a fan of the previous
> method; it'd work nicely with how we've presently defined things and
> should be easy to put into a DSN/URI/env variable.

This is a multi-dimensional problem. "channel_binding=require" is one 
way to prevent MITM attacks, but sslmode=verify-ca is another. (Does 
Kerberos also prevent MITM?) Or you might want to enable plaintext 
passwords over SSL, but not without SSL.

I think we'll need something like the 'ssl_ciphers' GUC, where you can 
choose from a few reasonable default rules, but also enable/disable 
specific methods:

# anything goes (the default)
auth_methods = 'ANY'

# Disable plaintext password authentication. Anything else is accepted.
auth_methods = '-password'

# Only authentication methods that protect from
# Man-in-the-Middle attacks. This allows anything if the server's SSL
# certificate can be verified, and otherwise only SCRAM with
# channel binding
auth_methods = 'MITM'

# The same, but disable plaintext passwords and md5 altogether
auth_methods = 'MITM, -password, -md5'


I'm tempted to also allow 'SSL' and 'SSL-verify-full' as part of the 
same string, so that you could configure everything related to 
connection security in the same option. Not sure if that would make 
things simpler for users, or create more confusion.

- Heikki