On Thu, Feb 21, 2019 at 08:32:01PM +0100, Peter Eisentraut wrote: > On 2019-02-21 05:47, Michael Paquier wrote: >> if (conn->ssl_in_use) >> + { >> + /* >> + * The server has offered SCRAM-SHA-256-PLUS, which is only >> + * supported by the client if a hash of the peer certificate >> + * can be created. >> + */ >> +#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH >> selected_mechanism = SCRAM_SHA_256_PLUS_NAME; >> +#endif >> + } > > Is that right? Won't we then just select nothing if the macro is not > defined? In the context of an SSL connection, the server would send both SCRAM and SCRAM_PLUS as valid mechanisms if it supports channel binding (HAVE_BE_TLS_GET_CERTIFICATE_HASH). If the server does not support channel binding, then only SCRAM is sent. So you have the following possible cases for an SSL connection: 1) Server supports channel binding, sends SCRAM_PLUS and SCRAM as allowed mechanisms. 1-1) Client supports channel binding, and it chooses SCRAM_PLUS. 1-2) Client does not support channel binding, and it chooses SCRAM. 2) Server does not support channel binding, sends SCRAM as allow mechanism. 2-1) Client supports channel binding, still it has no choice but to choose SCRAM. 2-2) Client does not support channel binding, it chooses SCRAM. On HEAD, the bug you have spotted would cause case 1-2) to fail as the client thinks that it should choose SCRAM_PLUS even if it does not support it, causing a failure. My patch changes things so as SCRAM gets chosen instead of SCRAM_PLUS as the server sends both SCRAM and SCRAM_PLUS to the client as possible choices, still the client is not able to handle SCRAM_PLUS. Does this explanation make sense? -- Michael
Attachment:
signature.asc
Description: PGP signature