Re: [PATCH] Pull general SASL framework out of SCRAM - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: [PATCH] Pull general SASL framework out of SCRAM
Date
Msg-id YOU2xFL/WoHl6ny0@paquier.xyz
Whole thread Raw
In response to Re: [PATCH] Pull general SASL framework out of SCRAM  (Jacob Champion <pchampion@vmware.com>)
Responses Re: [PATCH] Pull general SASL framework out of SCRAM  (Jacob Champion <pchampion@vmware.com>)
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: [PATCH][postgres_fdw] Add push down of CASE WHEN clauses
Next
From: Andy Fan
Date:
Subject: Re: Keep notnullattrs in RelOptInfo (Was part of UniqueKey patch series)