Re: SCRAM with channel binding downgrade attack

On Fri, May 18, 2018 at 09:30:22AM -0400, Bruce Momjian wrote:
> Good work, but I think the existance of both scram_channel_binding and
> channel_binding_mode in libpq is confusing. I am thinking we should
> have one channel binding parameter that can take values "prefer",
> "tls-unique", "tls-server-end-point", and "require". I don't see the
> point to having "none" and "allow" that sslmode supports. "tls-unique"
> and "tls-server-end-point" would _require_ those channel binding modes;
> the setting would never be ignored, e.g. if no SSL.

I can see the point you are coming at. Do you think that we should
worry about users willing to use specifically tls-server-end-point
(which is something actually in the backend protocol only for JDBC) and
manage a cluster of both v10 and v11 servers? In this case we assume
that "prefer" means always using tls-unique as channel binding, but
there is no way to say "I would prefer channel binding if available,
only with end-point". It would not be that hard to let the application
layer on top of libpq handle that complexity with channel binding
handling, though it makes the life of libpq consumers a bit harder.

Hence, I would actually eliminate "require", as that would be actually
the same as "tls-unique", and the possibility to use an empty value from
the list of options available but add "none" as that's actually the same
as the current empty value. This gives as list:
- tls-unique
- tls-server-end-point
- none
- prefer, which has the meaning of preferring tls-unique
- And prefer-end-point? But thinking why end-point has been added it
would not be worth it, and for tests only the option requiring end-point
is enough.

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