Re: [JDBC] [HACKERS] Channel binding support for SCRAM-SHA-256

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