On Mon, Sep 30, 2019 at 03:45:39PM +0900, Michael Paquier wrote: > If the server publishes SCRAM-SHA-256-PLUS and the server does not > support channel binding, then we get this error message: > "channel binding is required, but server did not offer an > authentication method that supports channel binding." > So that's the part which is wrong. > > Now, I am not completely sure that the suggested change is completely > right either as we would get an error in this scenario when > channel_binding is "prefer" or "require". For "require", this error > message is fine. However, for "prefer", shouldn't we do what we do on > HEAD, aka *not* select SCRAM-SHA-256-PLUS and switch to SCRAM-SHA-256? > This would have the advantage to make the connection work with default > parameters.
So, something like the attached looks better to me. Using a server which publishes SCRAM-SHA-256-PLUS, I get the following over SSL: 1) client supports channel binding: 1-1) channel_binding = disable => OK, with SCRAM-SHA-256 1-2) channel_binding = prefer => OK, with SCRAM-SHA-256-PLUS 1-3) channel_binding = require => OK, with SCRAM-SHA-256-PLUS 2) client does not support channel binding 2-1) channel_binding = disable => OK, with SCRAM-SHA-256 2-2) channel_binding = prefer => OK, with SCRAM-SHA-256 2-3) channel_binding = require => failure with new error message, instead of the confusing one. The bug is with 2-3, and Tom's suggestion would have switched 2-2 to a failure (2-2 works on HEAD). -- Michael
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 04118d54e2..d6e4036641 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -471,14 +471,28 @@ pg_SASL_init(PGconn *conn, int payloadlen)
{
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, and if channel_binding is not disabled.
- */
+ /* The server has offered SCRAM-SHA-256-PLUS */
+
#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
+ /*
+ * The client supports channel binding, which is chosen if
+ * channel_binding is not disabled.
+ */
if (conn->channel_binding[0] != 'd') /* disable */
selected_mechanism = SCRAM_SHA_256_PLUS_NAME;
+#else
+ /*
+ * The client does not support channel binding. If it is
+ * required, complain immediately instead of the error below
+ * which would be confusing as the server is publishing
+ * SCRAM-SHA-256-PLUS.
+ */
+ if (conn->channel_binding[0] == 'r') /* require */
+ {
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("channel binding is required, but client does not support it\n"));
+ goto error;
+ }
#endif
}
else
signature.asc
Description: PGP signature
