Re: Negotiating the SCRAM channel binding type

On 07/08/18 17:34, Stephen Frost wrote:
> * Michael Paquier (michael(at)paquier(dot)xyz) wrote:
>> On Tue, Aug 07, 2018 at 02:32:27PM +0530, Robert Haas wrote:
>>> On Sun, Aug 5, 2018 at 4:30 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>>>> Well, it'd be useless for users, there is no reason to switch off channel
>>>> binding if both the client and server support it. It might not add any
>>>> security you care about, but it won't do any harm either. The
>>>> non-channel-binding codepath is still exercised with non-SSL connections.
>>>
>>> Is that true? What if it makes a connection fail that you wanted to
>>> succeed? Suppose we discover a bug that makes connections using
>>> channel binding fail on Thursdays.
>>
>> Well, as things stand today on HEAD, if channel binding has a bug, this
>> makes the SCRAM authentication not able to work over SSL, hence you need
>> to either drop SSL, SCRAM or patch libpq so as the client tells the
>> server that it does not want to use channel binding. None of those are
>> appealing. Before 7729113, the client still had the choice to enforce
>> channel binding to not be used over SSL, which I think is a win to
>> bypass any potential future bugs. On top of that, we can test
>> automatically for *any* future SSL implementations that (SSL + SCRAM +
>> no channel binding) actually works properly, which is, it seems at least
>> to me, a good thing to get more confidence when a new SSL implementation
>> is added.
>
> Uh, really? We can come up with all kinds of potential bugs that might
> exist in the world but I don't think we should be writing in options for
> everything that might fail due to some bug existing that we don't know
> about.

Yeah, if there's a bug, we'll fix it and move on, like with any other
feature.

> Now- if we thought that maybe there was some connection pooling solution
> that could be made to work with SSL+SCRAM if channel binding is turned
> off, then that's a use-case that maybe we should try and support, but
> this notion that we need to be able to turn it off because there might
> be a bug is hogwash, imv. Now, I haven't seen a pooling solution
> actually figure out a way to do SSL+SCRAM even without channel binding,
> and there might not even be a way, so I'm currently a -1 on adding an
> option to disable it, but if someone turned up tomorrow with an credible
> approach to doing that, then I'd +1 adding the option.

Now that's a lot more compelling argument for having an option.
Essentially, you might have a legitimate proxy or connection pooler that
acts like a Man-In-The-Middle.

The removed "channel_binding" libpq option wasn't very user-friendly,
and wasn't very good for dealing with that scenario anyway; wouldn't you
want to disable channel binding in the server rather than the client in
that scenario? So I have no regrets in removing it. But going forward,
we do need to put some thought in configuring this. We've talked a lot
about a libpq option to require channel binding, but we should also have
a server-side option to disable it.

- Heikki