On 12/04/17 19:34, Heikki Linnakangas wrote:
> On 04/11/2017 02:32 PM, Álvaro Hernández Tortosa wrote:
>> So I still see your proposal more awkward and less clear, mixing
>> things that are separate. But again, your choice :)
>
> So, here's my more full-fledged proposal.
>
> The first patch refactors libpq code, by moving the responsibility of
> reading the GSS/SSPI/SASL/MD5 specific data from the authentication
> request packet, from the enormous switch-case construct in
> PQConnectPoll(), into pg_fe_sendauth(). This isn't strictly necessary,
> but I think it's useful cleanup anyway, and now that there's a bit
> more structure in the AuthenticationSASL message, the old way was
> getting awkward.
>
> The second patch contains the protocol changes, and adds the
> documentation for it.
>
> - Heikki
>
Hi Heikki.
Thanks for the patches :)
By looking at the them, and unless I'm missing something, I don't
see how the extra information for the future implementation of channel
binding would be added (without changing the protocol). Relevant part is:
The message body is a list of SASL authentication mechanisms, in the
server's order of preference. A zero byte is required as terminator after
the last authentication mechanism name. For each mechanism, there is the
following:
<variablelist>
<varlistentry>
<term> String
</term>
<listitem>
<para> Name of a SASL authentication mechanism.
</para>
</listitem>
</varlistentry>
</variablelist>
How do you plan to implement it, in future versions, without
modifying the AuthenticationSASL message? Or is it OK to add new fields
to a message in future PostgreSQL versions, without considering that a
protocol change?
On a side note, I'd mention that the list of SASL authentication
mechanisms contains valid IANA Registry SCRAM names
(https://www.iana.org/assignments/sasl-mechanisms/sasl-mechanisms.xhtml#scram)for
SCRAM authentication messages (making it more clear what values would be
expected there from the client).
I hope to start testing this from Java the coming weekend. I will
keep you posted.
Álvaro
--
Álvaro Hernández Tortosa
-----------
<8K>data
