On 11/18/17 06:32, Michael Paquier wrote:
> Here are some comments.
>
> + * The client requires channel biding. Channel binding type
> s/biding/binding/.
fixed
> if (!state->ssl_in_use)
> + /*
> + * Without SSL, we don't support channel binding.
> + *
> Better to add brackets if adding a comment.
done
> + * Read value provided by client; only tls-unique is supported
> + * for now. XXX Not sure whether it would be safe to print
> + * the name of an unsupported binding type in the error
> + * message. Pranksters could print arbitrary strings into the
> + * log that way.
> That's why I didn't show those strings in the error messages of the
> previous versions. Those are useless as well, except for debugging the
> feature and the protocol.
Right. I left the comment in there as a note to future hackers who want
to improve error messages (often myself).
> + cbind_header_len = 4 + strlen(state->channel_binding_type); /*
> p=type,, */
> + cbind_input_len = cbind_header_len + cbind_data_len;
> + cbind_input = malloc(cbind_input_len);
> + if (!cbind_input)
> + goto oom_error;
> + snprintf(cbind_input, cbind_input_len, "p=%s",
> state->channel_binding_type);
> + memcpy(cbind_input + cbind_header_len, cbind_data, cbind_data_len);
> By looking at RFC5802, a base64 encoding of cbind-input is used:
> cbind-input = gs2-header [ cbind-data ]
> gs2-cbind-flag "," [ authzid ] ","
> However you are missing two commands after p=%s, no?
fixed
I have committed the patch with the above fixes.
I'll be off for a week, so perhaps by that time you could make a rebased
version of the rest? I'm not sure how much more time I'll have, so
maybe it will end up being moved to the next CF.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
