Re: Letting the client choose the protocol to use during a SASL exchange

On 04/11/2017 11:55 AM, Álvaro Hernández Tortosa wrote:
> On 11/04/17 08:50, Heikki Linnakangas wrote:
>> Oh, I see. According to the SCRAM RFC, "tls-unique" is used by
>> default. I don't see us implementing anything else any time soon.
>> PostgreSQL doesn't support any other "channel type" than TLS, and
>> tls-unique is the only one that makes sense for TLS.
>>
>> If we do need it after all, the server could advertise the additional
>> channel binding types as additional SASL mechanisms in the
>> AuthenticationSASL message, maybe something like:
>>
>> "SCRAM-SHA-256"
>> "SCRAM-SHA-256-PLUS" (for tls-unique)
>> "SCRAM-SHA-256-PLUS ssh-unique" (for hypothetical ssh channel binding)
>>
>> The same trick can be used to advertise any other SASL mechanism
>> specific options, if needed in the future.
>
> Why not add an extra field to the message? This scheme has in my
> opinion some disadvantages:
>
> - You assume no extensibility. Maybe Postgres will implement other
> mechanisms for channel binding. Maybe not. But why limit ourselves?
> - Apart from tls-unique there are others today, like
> tls-server-end-point and who knows if in the future TLS 1.x comes with
> something like 'tls-unique-1.x'
> - Why have to parse the string (separated by spaces) when you could use
> different fields and have no parsing at all?

I don't think an option separated by space is any more difficult to
parse than a separate field. I'm envisioning that the "parsing" would be
simply:

if (strcmp(sasl_mechanism, "SCRAM-SHA-256") == 0) { ... }
else if (strcmp(sasl_mechanism, "SCRAM-SHA-256-PLUS") == 0) { ... }
else if (strcmp(sasl_mechanism, "SCRAM-SHA-256-PLUS tls-awesome") == 0)
{ ... }

This can be extended for more complicated options, if necessary.
Although if we find that we need a dozen different options, I think
we've done something wrong.

> - How do you advertise different SCRAM mechanisms with different channel
> binding types? And a mix of SCRAM mechanisms with and without channel
> binding? If I got it right, with your proposal it would be something like:
>
> Field 1: SCRAM-SHA-256,SCRAM-SHA-256-PLUS tls-unique,SCRAM-SHA-1-PLUS
> tls-unique,SCRAM-SHA-512-PLUS tls-unique
>
> (which is basically a CSV of pairs where the right part of the pair
> might be empty; too much IMHO for a single field)

Yes, except that in my proposal, the list is not a comma-separated
string, but a list of null-terminated strings, similar to how some other
lists of options in the FE/BE protocol are transmitted.

> vs
>
> Field 1:
> SCRAM-SHA-256,SCRAM-SHA-256-PLUS,SCRAM-SHA-1-PLUS,SCRAM-SHA-512-PLUS
> (simple CSV)
> Field 2: tls-unique (String)

What if tls-unique is only supported with SCRAM-SHA-256-PLUS, while
SCRAM-SHA-512-PLUS requires tls-awesome? What about possible other
options you might need to tack on to mechanisms? This seems less
flexible. I'm not saying that either of those cases is very likely, but
I don't think it's very likely we'll need extra options or different
channel binding types any time soon, anyway.

> Is there any reason not to use two fields? AuthenticationSASL is a
> new message, I guess we can freely choose its format, right?

Yes, we can choose the format freely.

In summary, I think the simple list of mechanism names is best, because:

* It's simple, and doesn't have any extra fields or options that are not
needed right now.
* It's flexible, for future extension. We can add new mechanism entries
with extra options, not just new channel binding types, if needed, and
existing clients (i.e. v10 clients) will ignore them.

>> Perhaps we should reserve a magic user name to mean "same as startup
>> message", in addition or instead of the empty string. We actually
>> discussed that already at [1], but we forgot about it since.
>
> That works. Please let me know what is the "magic constant" chosen ;P

I was hoping you'd have some good suggestions :-). Unfortunately there
is no reserved username prefix or anything like that, so whatever we
choose might also be a real username. That's OK, but it could be
confusing, if we ever tried to do something different with the SASL
username. How about "pg_same_as_startup_message"?

- Heikki