On Wed, Dec 20, 2017 at 1:19 AM, Peter Eisentraut <peter.eisentr...@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