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