Re: SCRAM with channel binding downgrade attack - Mailing list pgsql-hackers

From Magnus Hagander
Subject Re: SCRAM with channel binding downgrade attack
Date
Msg-id CABUevEw4CvkE+00CYZsYwdUM1a7CB+5TNM5uVzEuGS0nogZQug@mail.gmail.com
Whole thread Raw
In response to Re: SCRAM with channel binding downgrade attack  (Heikki Linnakangas <hlinnaka@iki.fi>)
Responses Re: SCRAM with channel binding downgrade attack  (Michael Paquier <michael@paquier.xyz>)
Re: SCRAM with channel binding downgrade attack  (Heikki Linnakangas <hlinnaka@iki.fi>)
List pgsql-hackers
On Wed, May 23, 2018 at 8:46 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 22/05/18 11:22, Michael Paquier wrote:
On Sat, May 19, 2018 at 09:35:57PM +0900, Michael Paquier wrote:
The previous patch has actually problems with servers using "trust",
"password" and any protocol which can send directly AUTH_REQ_OK as those
could still enforce a downgrade to not use channel binding, so we
actually need to make sure that AUTH_REQ_SASL_FIN has been received when
channel binding is required when looking at a AUTH_REQ_OK message.

Okay, so I have digested the previous comments with the attached.
scram_channel_binding is modified as follows (from commit message):
- "prefer", is the default and behaves so as channel binding is used if
available.  If the cluster does not support it then it is not used. This
does not protect from downgrade attacks.
- "disable", which is the equivalent of the empty value previously,
disables channel binding.
- "tls-unique" and "tls-server-end-point" make channel binding a
requirement and use the channel binding of the same name for
connection.  Note that in this case, if the connection is attempted
without SSL or if server does not support channel binding then libpq
refuses the connection as well.

"tls-unique" and "tls-server-end-point" are overly technical to users. They don't care which one is used, there's no difference in security. Furthermore, if we add another channel binding type in the future, perhaps because someone finds a vulnerability in those types and a new one is added to address it, users would surely like to be automatically switched over to the new binding type. If they have "tls-unique" hardcoded in connection strings, that won't happen.

So I think the options should be "prefer", "disable", and "require". That's also symmetrical with sslmode, which is nice.

As a general point, I still don't like being symmetrical with sslmode=prefer, because that's a silly setting for sslmode :) It basically says "I don't care about the security guarantees, I just want the overhead please". Now, granted, the over head for SCRAM channel-binding is certainly a lot less than it is for TLS. But if we just want to go with symmetry, it would perhaps make more sense to implement the "allow" mode which makes more sense on the TLS side as well.


We could provide "tls-unique" and "tls-server-end-point" in addition to those, but I'd consider those to be developer only settings, useful only for testing the protocol.

It would be nice to have a libpq setting like "authenticate_server=require", which would mean "I want man-in-the-middle protection".

That seems like a bad name for such a thing. Shouldn't it be "authenticate_server=no_man_in_the_middle" (not those words but you get the idea). Specifically, protecting against man in the middle attack does not equal authenticating server -- you can still be connected to the wrong server just with no second attacker between you and the first attacker.

 
With that, a connection would be allowed, if either the server's SSL certificate is verified as with "sslmode=verify-full", *or* SCRAM authentication with channel binding was used. Or perhaps cram it into sslmode, "sslmode=verify-full-or-scram-channel-binding", just with a nicer name. (We can do that after v11 though, I think.)

sslmode=verify-full is very different from SCRAM with channel binding, isn't it? As in, SCRAM with channel binding at no point proves which server you're talking to -- only that you are talking to the SSL endpoint? It could be a rogue SSL endpoint unless you do certificate validation.

--

pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Possible bug in logical replication.
Next
From: Heikki Linnakangas
Date:
Subject: Re: Memory unit GUC range checks