On 04/10/2017 09:33 PM, Álvaro Hernández Tortosa wrote:
> Thanks for posting the patched HTML. In my opinion, all looks good
> except that:
>
> - I will add an extra String (a CSV) to AuthenticationSASL message for
> channel binding names, so that message format can remain without changes
> when channel binding is implemented. It can be empty.
Note that SCRAM-SHA-256 with channel binding has a different SASL
mechanism name, SRAM-SHA-256-PLUS. No need for a separate flag or string
for channel binding. When support for channel binding is added to the
server, it will advertise two SASL mechanisms in the AuthenticationSASL
message, SCRAM-SHA-256 and SCRAM-SHA-256-PLUS. (Or just
SCRAM-SHA-256-PLUS, if channel-binding is required).
> - If the username used is the one sent in the startup message, rather
> than leaving it empty in the client-first-message, I would force it to
> be the same as the used in the startuo message.
The problem with that is that the SCRAM spec dictates that the username
must be encoded in UTF-8, but PostgreSQL supports non-UTF-8 usernames.
Or did you mean that, if the username is sent, it must match the one in
the startup packet, but an empty string would always be allowed? That
would be reasonable.
> Otherwise we may confuse
> some client implementations which would probably consider that as an
> error; for one, my implementation would currently throw an error if
> username is empty, and I think that's correct.
I'm not sure I follow. The username is sent from client to server, and
currently, the server will ignore it. If you're writing a client
library, it can send whatever it wants. (Although again I would
recommend an empty string, to avoid confusion. Sending the same username
as in the startup packet, as long as it's in UTF-8, seems reasonable too.)
Thanks for reviewing this! I'll start hacking on code changes to go with
these docs.
- Heikki