From: Tom Lane <tgl@sss.pgh.pa.us>
To: Jeff Davis <jdavis@postgresql.org>
cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Add libpq parameter 'channel_binding'.
In-reply-to: <E1iCVWg-00057D-8U@gemulon.postgresql.org>
References: <E1iCVWg-00057D-8U@gemulon.postgresql.org>
Comments: In-reply-to Jeff Davis <jdavis@postgresql.org>
	message dated "Mon, 23 Sep 2019 21:05:42 -0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <24856.1569775891.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 29 Sep 2019 12:51:31 -0400
Message-ID: <24857.1569775891@sss.pgh.pa.us>
Precedence: bulk

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=3Dssltestuser cha=
nnel_bin
ding=3Drequire$$ -d dbname=3Dtrustdb sslmode=3Drequire sslcert=3Dinvalid s=
slrootcert=3Dinv
alid hostaddr=3D127.0.0.1 user=3Dssltestuser channel_binding=3Drequire
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=3Drequire

#   Failed test 'SCRAM with SSL and channel_binding=3Drequire'
#   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 certifica=
te
                 * can be created, and if channel_binding is not disabled.
                 */
-#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
                if (conn->channel_binding[0] !=3D 'd')    /* disable */
+               {
+#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
                    selected_mechanism =3D SCRAM_SHA_256_PLUS_NAME;
+#else
+                   printfPQExpBuffer(&conn->errorMessage,
+                                     libpq_gettext("client does not suppo=
rt SCRAM-SHA-256-PLUS authentication\n"));
+                   goto error;
+               }
 #endif
            }
            else

			regards, tom lane