On Wed, Dec 20, 2017 at 1:19 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> I have committed 0001 and 0002 (renaming to scram_channel_binding).
Thanks!
> The 0003 patch looks mostly fine as well. The only concern I have is
> that the way it is set up now, we make the server compute the channel
> binding data for both tls-unique and tls-server-end-point, even though
> we only end up using one. This might need some restructuring so that we
> only get the data we need once we have learned which channel binding
> type the client requested.
The current patch is focused on simplicity and it has the advantage
that there is no need to depend on any SSL structures or Port* in
fe-auth-scram.c and auth-scram.c. So I would really like to keep the
code simple with this goal in mind.
Speaking of which, making the server-side code is going to be in my
opinion grotty, because the server only knows about the channel
binding type used by the client after reading its first message, so we
would need to call directly the SSL-related APIs in auth-scram.c, and
update scram_state with the appropriate data in the middle of the
exchange. This causes the addition of two dependencies to Port* and
the SSL APIs into auth-scram.c.
However, it is possible to simply optimize the frontend code as in
pg_SASL_init() we already know the channel binding type selected when
calling pgtls_get_finished() and pgtls_get_peer_certificate_hash(). So
while I agree with your point, my opinion is to keep the code as
simple as possible, and then just optimize the frontend code. What do
you think?
--
Michael