Re: pgsql: Add libpq parameter 'channel_binding'. - Mailing list pgsql-committers

From Tom Lane
Subject Re: pgsql: Add libpq parameter 'channel_binding'.
Date
Msg-id 24857.1569775891@sss.pgh.pa.us
Whole thread Raw
In response to pgsql: Add libpq parameter 'channel_binding'.  (Jeff Davis <jdavis@postgresql.org>)
Responses Re: pgsql: Add libpq parameter 'channel_binding'.  (Michael Paquier <michael@paquier.xyz>)
List pgsql-committers
Jeff Davis <jdavis@postgresql.org> writes:
> Add libpq parameter 'channel_binding'.

I found out the hard way that the added ssl tests fall over on a
platform that doesn't HAVE_X509_GET_SIGNATURE_NID:

# Running: psql -X -A -t -c SELECT $$connected with user=ssltestuser channel_bin
ding=require$$ -d dbname=trustdb sslmode=require sslcert=invalid sslrootcert=inv
alid hostaddr=127.0.0.1 user=ssltestuser channel_binding=require
psql: error: could not connect to server: channel binding is required, but serve
r did not offer an authentication method that supports channel binding
not ok 5 - SCRAM with SSL and channel_binding=require

#   Failed test 'SCRAM with SSL and channel_binding=require'
#   at t/002_scram.pl line 63.

I don't think that it's acceptable for the test to fail on a platform
that we're willing to compile on.  Maybe just skip these tests if we
lack X509_get_signature_nid?

Another point is that this error message is misleading --- or at least
would be misleading if the server had X509_get_signature_nid and the
client didn't.  Perhaps do something like

diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 04118d5..4289551 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -476,9 +476,15 @@ pg_SASL_init(PGconn *conn, int payloadlen)
                 * supported by the client if a hash of the peer certificate
                 * can be created, and if channel_binding is not disabled.
                 */
-#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
                if (conn->channel_binding[0] != 'd')    /* disable */
+               {
+#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
                    selected_mechanism = SCRAM_SHA_256_PLUS_NAME;
+#else
+                   printfPQExpBuffer(&conn->errorMessage,
+                                     libpq_gettext("client does not support SCRAM-SHA-256-PLUS authentication\n"));
+                   goto error;
+               }
 #endif
            }
            else

            regards, tom lane



pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: pgsql: Fix bogus order of error checks in new channel_binding code.
Next
From: Peter Eisentraut
Date:
Subject: pgsql: doc: Further clarify how recovery target parameters are applied