On Tue, May 22, 2018 at 05:22:19PM +0900, 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.
Yes, I never liked the 'empty value' idea since I don't know of any
other libpq settings that use that API. "disable" matches "sslmode"
too, which is nice.
> In order to make sure that a client is not tricked by a "trust"
> connection downgrade which sends back directly AUTH_REQ_OK, one way I
> have thought about is to check if the client has achieved with a server
> a full SASL exchange when receiving this message type, which is
> something that we know about as the exchange state is saved in
> PGconn->sasl_state.
I had not thought of 'trust'. I was more worried about the password
hash being downgraded in robustness or passed through a
man-in-the-middle, while the 'trust' does not require. However, you are
right that channel binding, when required, does require the other end to
know the same password as the client knows. Good point.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +
