Fix confusing error caused by connection parameter channel_binding

When using a client compiled without channel binding support (linking to
OpenSSL 1.0.1 or older) to connect to a server which supports channel
binding (linking to OpenSSL 1.0.2 or newer), libpq would generate a
confusing error message with channel_binding=require for an SSL
connection, where the server sends back SCRAM-SHA-256-PLUS:
"channel binding is required, but server did not offer an authentication
method that supports channel binding."

This is confusing because the server did send a SASL mechanism able to
support channel binding, but libpq was not able to detect that
properly.

The situation can be summarized as followed for the case described in
the previous paragraph for the SASL mechanisms used with the various
modes of channel_binding:
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.
This commit updates case 2-3 to generate a better error message.  Note
that the SSL TAP tests are not impacted as it is not possible to test
with mixed versions of OpenSSL for the backend and libpq.

Reported-by: Tom Lane
Author: Michael Paquier
Reviewed-by: Jeff Davis, Tom Lane
Discussion: https://postgr.es/m/24857.1569775891@sss.pgh.pa.us

diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 04118d54e2b6b71eb94db6d28021c07fb853fad1..8ca7b393264050ea714cabd13e6746d471038735 100644 (file)
--- 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. */
+
+#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
                                /*
-                                * 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 client supports channel binding, which is chosen if
+                                * channel_binding is not disabled.
                                 */
-#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
                                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