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