Re: [HACKERS] Letting the client choose the protocol to use during aSASL exchange - Mailing list pgsql-hackers

On 04/07/2017 01:13 AM, Michael Paquier wrote:

On Fri, Apr 7, 2017 at 5:15 AM, Álvaro Hernández Tortosa <aht@8kdata.com> wrote:

    I don't see it. The message AuthenticationSASL.String could contain a
CSV of the SCRAM protocols supported. This is specially important to support
channel binding (which is just another protocol name for this matter), which
is the really enhanced security mechanism of SCRAM. Since this message is
sent regardless, and the client replies with PasswordMessage, no extra round
trip is required. However, PasswordMessage needs to also include a field
with the name of the selected protocol (it is the client who picks). Or a
different message would need to be created, but no extra round-trips more
than those required by SCRAM itself (4 messages for SCRAM + 1 extra for the
server to tell the client it needs to use SCRAM).

Yes, it seems to me that the list of protocols to send should be done
by sendAuthRequest(). Then the client parses the received string, and
sends an extra 'p' message with its choice before sending the first
SCRAM message. So there is no need for any extra round trips.

I started writing down the protocol docs, based on the above idea. See attached. The AuthenticationSASL message now contains a list of mechanisms.

Does that seem clear to you? If so, I'll change the code to match the attached docs.

I added two new message formats to the docs, SASLResponse and SASLInitialResponse. Those use the same type byte as PasswordMessage, 'p', but I decided to describe them as separate messages for documentation purposes, since the content is completely different depending on whether the message is sent as part of SASL, GSS, md5, or password authentication. IOW, this is not a change in the implementation, only in the way it's documented.


While working on this, and reading the RFCs more carefully, I noticed one detail we should change, to be spec-complicant. The SASL spec specifies that a SASL authentication exchange consists of challenge-response pairs. There must be a response to each challenge. If the last message in the authentication mechanism (SCRAM in this case) goes in the server->client direction, then that message must sent as "additional data" in the server->client message that tells the client that the authentication was successful. That's AuthenticationOK in the PostgreSQL protocol. In the current implementation, the server-final-message is sent as an AuthenticationSASLContinue message, and the client doesn't respond to that.

We should change that, so that the server-final-message is sent as "additional data" in the AuthenticationOK message. The attached docs patch describes that, rather than what the current implementation does.

(For your convenience, I built the HTML docs with this patch, and put them up at http://hlinnaka.iki.fi/temp/scram-wip-docs/protocol.html for viewing)

- Heikki

    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.

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


    Álvaro

-- 

Álvaro Hernández Tortosa


-----------
<8K>data