Re: [PATCH] Pull general SASL framework out of SCRAM

On Tue, Jul 06, 2021 at 06:20:49PM +0000, Jacob Champion wrote:
> On Mon, 2021-07-05 at 17:17 +0900, Michael Paquier wrote:
> Each name must be null-terminated, not just null-separated. That way
> the list of names ends with an empty string:
>
> name-one\0 <- added by the mechanism
> name-two\0 <- added by the mechanism
> \0 <- added by the framework
>
> The way it's worded now, I could see some implementers failing to
> terminate the final name because the framework adds a trailing null
> already -- but the framework is terminating the list, not the final
> name.

Good point. I have used ending with '\0' bytes instead.

>> + * init()
>> + *
>> + * Initializes mechanism-specific state for a connection. This
>> + * callback must return a pointer to its allocated state, which will
>> + * be passed as-is as the first argument to the other callbacks.
>> + * free() is called to release any state resources.
>
> Maybe say "The free() callback is called" to differentiate it from
> standard free()?

Yes, that could be confusing. Switched to your wording instead.

> It's possible for conn->sasl to be NULL here, say if the client has
> channel_binding=require but connects as a user with an MD5 secret. The
> SCRAM TAP tests have one such case.

Indeed.

>> Hmm. Does the RFCs tell us anything about that?
>
> Just in general terms:
>
>> Each authentication exchange consists of a message from the client to
>> the server requesting authentication via a particular mechanism,
>> followed by one or more pairs of challenges from the server and
>> responses from the client, followed by a message from the server
>> indicating the outcome of the authentication exchange. (Note:
>> exchanges may also be aborted as discussed in Section 3.5.)
>
> So a challenge must be met with a response, or the exchange must be
> aborted. (And I don't think our protocol implementation provides a
> client abort message; if something goes wrong, we just tear down the
> connection.)

Thanks. At the same time, section 3.5 also says that the client may
send a message to abort. So one can interpret that the client has
also the choice to abort without sending a response back to the
server? Or I am just interpreting incorrectly the use of "may" in
this context?
--
Michael