From 4e2f010692aaef97d4e16822359e86073a53e96b Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Fri, 20 Jul 2018 20:02:17 +0300
Subject: [PATCH 1/2] Remove support for tls-unique channel binding.

There are some problems with the tls-unique channel binding type. It's not
supported by all SSL libraries, and strictly speaking it's not defined for
TLS 1.3 at all, even though at least in OpenSSL, the functions used for it
still seem to work with TLS 1.3 connections. And since we had no
mechanism to negotiate what channel binding type to use, there would be
awkward interoperability issues if a server only supported some channel
binding types. tls-server-end-point seems feasible to support with any SSL
library, so let's just stick to that.

This removes the scram_channel_binding libpq option altogether, since there
is now only one supported channel binding type.

This also removes all the channel binding tests from the SSL test suite.
It would be good to actually have some tests for channel binding, but these
tests were really just testing the scram_channel_binding option, which is
now gone.

I also removed the SCRAM_CHANNEL_BINDING_TLS_END_POINT constant. This is a
matter of taste, but IMO it's more readable to just use the
"tls-server-end-point" string.

Refactor the checks on whether the SSL library supports the functions
needed for tls-server-end-point channel binding. Now the server won't
advertise, and the client won't choose, the SCRAM-SHA-256-PLUS variant, if
compiled with an OpenSSL version too old to support it.

In the passing, add some sanity checks to check that the chosen SASL
mechanism, SCRAM-SHA-256 or SCRAM-SHA-256-PLUS, matches whether the SCRAM
exchange used channel binding or not. For example, if the client selects
the non-channel-binding variant SCRAM-SHA-256, but in the SCRAM message
uses channel binding anyway. It's harmless from a security point of view,
I believe, and I'm not sure if there are some other conditions that would
cause the connection to fail, but it seems better to be strict about these
things and check explicitly.

Discussion: https://www.postgresql.org/message-id/ec787074-2305-c6f4-86aa-6902f98485a4%40iki.fi
---
 doc/src/sgml/libpq.sgml                  |  28 ----
 doc/src/sgml/protocol.sgml               |  28 ++--
 doc/src/sgml/release-11.sgml             |   5 +-
 src/backend/libpq/auth-scram.c           | 218 +++++++++++++++++++++----------
 src/backend/libpq/auth.c                 |  61 +++------
 src/backend/libpq/be-secure-openssl.c    |  27 +---
 src/include/common/scram-common.h        |   4 -
 src/include/libpq/libpq-be.h             |  12 +-
 src/include/libpq/scram.h                |   4 +-
 src/interfaces/libpq/fe-auth-scram.c     |  73 +++--------
 src/interfaces/libpq/fe-auth.c           |  25 +++-
 src/interfaces/libpq/fe-connect.c        |   8 --
 src/interfaces/libpq/fe-secure-openssl.c |  28 +---
 src/interfaces/libpq/libpq-int.h         |  12 +-
 src/test/ssl/t/002_scram.pl              |  33 +----
 15 files changed, 233 insertions(+), 333 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index d67212b831..06a1fdef69 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1238,34 +1238,6 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       </listitem>
      </varlistentry>
 
-     <varlistentry id="libpq-scram-channel-binding" xreflabel="scram_channel_binding">
-      <term><literal>scram_channel_binding</literal></term>
-      <listitem>
-       <para>
-        Specifies the channel binding type to use with SCRAM
-        authentication.  While <acronym>SCRAM</acronym> alone prevents
-        the replay of transmitted hashed passwords, channel binding also
-        prevents man-in-the-middle attacks.
-       </para>
-
-       <para>
-        The list of channel binding types supported by the server are
-        listed in <xref linkend="sasl-authentication"/>.  An empty value
-        specifies that the client will not use channel binding.  If this
-        parameter is not specified, <literal>tls-unique</literal> is used,
-        if supported by both server and client.
-        Channel binding is only supported on SSL connections.  If the
-        connection is not using SSL, then this setting is ignored.
-       </para>
-
-       <para>
-        This parameter is mainly intended for protocol testing.  In normal
-        use, there should not be a need to choose a channel binding type other
-        than the default one.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="libpq-connect-replication" xreflabel="replication">
       <term><literal>replication</literal></term>
       <listitem>
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 46d7e19f10..b8e47fdbce 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1576,12 +1576,10 @@ the password is in.
   <para>
 <firstterm>Channel binding</firstterm> is supported in PostgreSQL builds with
 SSL support. The SASL mechanism name for SCRAM with channel binding is
-<literal>SCRAM-SHA-256-PLUS</literal>.  Two channel binding types are
-supported: <literal>tls-unique</literal> and
-<literal>tls-server-end-point</literal>, both defined in RFC 5929.  Clients
-should use <literal>tls-unique</literal> if they can support it.
-<literal>tls-server-end-point</literal> is intended for third-party clients
-that cannot support <literal>tls-unique</literal> for some reason.
+<literal>SCRAM-SHA-256-PLUS</literal>.  The channel binding type used by
+PostgreSQL is <literal>tls-server-end-point</literal>.  If other channel
+binding types are supported in the future, the server will advertise
+them as separate SASL mechanisms.
   </para>
 
   <para>
@@ -1596,19 +1594,11 @@ that cannot support <literal>tls-unique</literal> for some reason.
 
   <para>
    <acronym>SCRAM</acronym> with channel binding prevents such
-   man-in-the-middle attacks by mixing a value into the transmitted
-   password hash that cannot be retransmitted by a fake server.
-   In <acronym>SCRAM</acronym> with <literal>tls-unique</literal>
-   channel binding, the shared secret negotiated during the SSL session
-   is mixed into the user-supplied password hash.  The shared secret
-   is partly chosen by the server, but not directly transmitted, making
-   it impossible for a fake server to create an SSL connection with the
-   client that has the same shared secret it has with the real server.
-   <acronym>SCRAM</acronym> with <literal>tls-server-end-point</literal>
-   mixes a hash of the server's certificate into the user-supplied password
-   hash.  While a fake server can retransmit the real server's certificate,
-   it doesn't have access to the private key matching that certificate, and
-   therefore cannot prove it is the owner, causing SSL connection failure.
+   man-in-the-middle attacks by mixing the signature of the server's
+   certificate into the transmitted password hash. While a fake server can
+   retransmit the real server's certificate, it doesn't have access to the
+   private key matching that certificate, and therefore cannot prove it is
+   the owner, causing SSL connection failure.
   </para>
 
 <procedure>
diff --git a/doc/src/sgml/release-11.sgml b/doc/src/sgml/release-11.sgml
index 1b21794e5a..8c29ebc5d1 100644
--- a/doc/src/sgml/release-11.sgml
+++ b/doc/src/sgml/release-11.sgml
@@ -2636,10 +2636,7 @@ same commits as above
         the feature currently does not prevent man-in-the-middle
         attacks when using libpq and interfaces built using it.  It is
         expected that future versions of libpq and interfaces not built
-        using libpq, e.g. JDBC, will allow this capability.  The libpq
-        options to control the optional channel binding type are <link
-        linkend="libpq-scram-channel-binding"><option>scram_channel_binding=tls-unique</option></link>
-        and <option>scram_channel_binding=tls-server-end-point</option>.
+        using libpq, e.g. JDBC, will allow this capability.
        </para>
       </listitem>
 
diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index 48eb531d0f..7603c9a3a7 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -17,6 +17,19 @@
  *	 by the SASLprep profile, we skip the SASLprep pre-processing and use
  *	 the raw bytes in calculating the hash.
  *
+ * - If channel binding is used, the channel binding type is always
+ *	 "tls-server-end-point".  The spec says the default is "tls-unique"
+ *	 (RFC 5802, section 6.1. Default Channel Binding), but there are some
+ *	 problems with that.  Firstly, not all SSL libraries provide an API to
+ *	 get the TLS Finished message, required to use "tls-unique".  Secondly,
+ *	 "tls-unique" is not specified for TLS v1.3, and as of this writing,
+ *	 it's not clear if there will be a replacement.  We could support both
+ *	 "tls-server-end-point" and "tls-unique", but for our use case,
+ *	 "tls-unique" doesn't really have any advantages.  The main advantage
+ *	 of "tls-unique" in general is that it works even if the server doesn't
+ *	 have a certificate, but PostgreSQL requires a server certificate
+ *	 whenever SSL is used, anyway.
+ *
  *
  * The password stored in pg_authid consists of the iteration count, salt,
  * StoredKey and ServerKey.
@@ -111,8 +124,7 @@ typedef struct
 	const char *username;		/* username from startup packet */
 
 	Port	   *port;
-	char		cbind_flag;
-	char	   *channel_binding_type;
+	bool		channel_binding_in_use;
 
 	int			iterations;
 	char	   *salt;			/* base64-encoded */
@@ -120,6 +132,7 @@ typedef struct
 	uint8		ServerKey[SCRAM_KEY_LEN];
 
 	/* Fields of the first message from client */
+	char		cbind_flag;
 	char	   *client_first_message_bare;
 	char	   *client_username;
 	char	   *client_nonce;
@@ -155,22 +168,58 @@ static void mock_scram_verifier(const char *username, int *iterations,
 					char **salt, uint8 *stored_key, uint8 *server_key);
 static bool is_scram_printable(char *p);
 static char *sanitize_char(char c);
+static char *sanitize_str(const char *s);
 static char *scram_mock_salt(const char *username);
 
 /*
+ * pg_be_scram_get_mechanisms
+ *
+ * Get a list of SASL mechanisms that this module supports.
+ *
+ * For the convenience of building the FE/BE packet that lists the
+ * mechanisms, the names are appended to the given StringInfo buffer,
+ * separated by '\0' bytes.
+ */
+void
+pg_be_scram_get_mechanisms(Port *port, StringInfo buf)
+{
+	/*
+	 * Advertise the mechanisms in decreasing order of importance.  So the
+	 * channel-binding variants go first, if they are supported.  Channel
+	 * binding is only supported with SSL, and only if the SSL implementation
+	 * has a function to get the certificate's hash.
+	 */
+#ifdef HAVE_BE_TLS_GET_CERTIFICATE_HASH
+	if (port->ssl_in_use)
+	{
+		appendStringInfoString(buf, SCRAM_SHA_256_PLUS_NAME);
+		appendStringInfoChar(buf, '\0');
+	}
+#endif
+	appendStringInfoString(buf, SCRAM_SHA_256_NAME);
+	appendStringInfoChar(buf, '\0');
+}
+
+/*
  * pg_be_scram_init
  *
  * Initialize a new SCRAM authentication exchange status tracker.  This
  * needs to be called before doing any exchange.  It will be filled later
  * after the beginning of the exchange with verifier data.
  *
- * 'username' is the username provided by the client in the startup message.
+ * 'selected_mech' identifies the SASL mechanism that the client selected.
+ * It should be one of the mechanisms that we support, as returned by
+ * pg_be_scram_get_mechanisms().
+ *
  * 'shadow_pass' is the role's password verifier, from pg_authid.rolpassword.
- * If 'shadow_pass' is NULL, we still perform an authentication exchange, but
- * it will fail, as if an incorrect password was given.
+ * The username to  was provided by the client in the startup message, and is
+ * available in port->user_name.  If 'shadow_pass' is NULL, we still perform
+ * an authentication exchange, but  it will fail, as if an incorrect password
+ * was given.
  */
 void *
 pg_be_scram_init(Port *port,
+				 const char *selected_mech,
 				 const char *shadow_pass)
 {
 	scram_state *state;
@@ -179,7 +228,27 @@ pg_be_scram_init(Port *port,
 	state = (scram_state *) palloc0(sizeof(scram_state));
 	state->port = port;
 	state->state = SCRAM_AUTH_INIT;
-	state->channel_binding_type = NULL;
+
+	/*
+	 * Parse the selected mechanism.
+	 *
+	 * Note that if we don't support channel binding, either because the SSL
+	 * implementation doesn't support it or we're not using SSL at all, we
+	 * would not have advertised the PLUS variant in the first place.  If the
+	 * client nevertheless tries to select it, it's a protocol violation like
+	 * selecting any other SASL mechanism we don't support.
+	 */
+#ifdef HAVE_BE_TLS_GET_CERTIFICATE_HASH
+	if (strcmp(selected_mech, SCRAM_SHA_256_PLUS_NAME) == 0 && port->ssl_in_use)
+		state->channel_binding_in_use = true;
+	else
+#endif
+	if (strcmp(selected_mech, SCRAM_SHA_256_NAME) == 0)
+		state->channel_binding_in_use = false;
+	else
+		ereport(ERROR,
+				(errcode(ERRCODE_PROTOCOL_VIOLATION),
+				 errmsg("client selected an invalid SASL authentication mechanism")));
 
 	/*
 	 * Parse the stored password verifier.
@@ -656,6 +725,36 @@ sanitize_char(char c)
 }
 
 /*
+ * Convert an arbitrary string to printable form, for error messages.
+ *
+ * Anything that's not a printable ASCII character is replaced with
+ * '?', and the string is truncated at 50 characters.
+ *
+ * The returned pointer points to a static buffer.
+ */
+static char *
+sanitize_str(const char *s)
+{
+	static char buf[50 + 1];
+	int			i;
+
+	for (i = 0; i < sizeof(buf) - 1; i++)
+	{
+		char		c = s[i];
+
+		if (c == '\0')
+			break;
+
+		if (c >= 0x21 && c <= 0x7E)
+			buf[i] = c;
+		else
+			buf[i] = '?';
+	}
+	buf[i] = '\0';
+	return buf;
+}
+
+/*
  * Read the next attribute and value in a SCRAM exchange message.
  *
  * Returns NULL if there is attribute.
@@ -715,6 +814,8 @@ read_any_attr(char **input, char *attr_p)
 static void
 read_client_first_message(scram_state *state, char *input)
 {
+	char	   *channel_binding_type;
+
 	input = pstrdup(input);
 
 	/*------
@@ -790,6 +891,12 @@ read_client_first_message(scram_state *state, char *input)
 			 * The client does not support channel binding or has simply
 			 * decided to not use it.  In that case just let it go.
 			 */
+			if (state->channel_binding_in_use)
+				ereport(ERROR,
+						(errcode(ERRCODE_PROTOCOL_VIOLATION),
+						 errmsg("malformed SCRAM message"),
+						 errdetail("The client selected SCRAM-SHA-256-PLUS, but the SCRAM message does not include channel binding data.")));
+
 			input++;
 			if (*input != ',')
 				ereport(ERROR,
@@ -807,6 +914,12 @@ read_client_first_message(scram_state *state, char *input)
 			 * it supports channel binding, which in this implementation is
 			 * the case if a connection is using SSL.
 			 */
+			if (state->channel_binding_in_use)
+				ereport(ERROR,
+						(errcode(ERRCODE_PROTOCOL_VIOLATION),
+						 errmsg("malformed SCRAM message"),
+						 errdetail("The client selected SCRAM-SHA-256-PLUS, but the SCRAM message does not include channel binding data.")));
+
 			if (state->port->ssl_in_use)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
@@ -826,44 +939,25 @@ read_client_first_message(scram_state *state, char *input)
 
 			/*
 			 * The client requires channel binding.  Channel binding type
-			 * follows, e.g., "p=tls-unique".
+			 * follows, e.g., "p=tls-server-end-point".
 			 */
-			{
-				char	   *channel_binding_type;
-
-				if (!state->port->ssl_in_use)
-				{
-					/*
-					 * Without SSL, we don't support channel binding.
-					 *
-					 * RFC 5802 specifies a particular error code,
-					 * e=server-does-support-channel-binding, for this.  But
-					 * it can only be sent in the server-final message, and we
-					 * don't want to go through the motions of the
-					 * authentication, knowing it will fail, just to send that
-					 * error message.
-					 */
-					ereport(ERROR,
-							(errcode(ERRCODE_PROTOCOL_VIOLATION),
-							 errmsg("client requires SCRAM channel binding, but it is not supported")));
-				}
+			if (!state->channel_binding_in_use)
+				ereport(ERROR,
+						(errcode(ERRCODE_PROTOCOL_VIOLATION),
+						 errmsg("malformed SCRAM message"),
+						 errdetail("The client selected SCRAM-SHA-256 without channel binding, but the SCRAM message includes channel binding data,")));
 
-				/*
-				 * Read value provided by client.  (It is not safe to print
-				 * the name of an unsupported binding type in the error
-				 * message.  Pranksters could print arbitrary strings into the
-				 * log that way.)
-				 */
-				channel_binding_type = read_attr_value(&input, 'p');
-				if (strcmp(channel_binding_type, SCRAM_CHANNEL_BINDING_TLS_UNIQUE) != 0 &&
-					strcmp(channel_binding_type, SCRAM_CHANNEL_BINDING_TLS_END_POINT) != 0)
-					ereport(ERROR,
-							(errcode(ERRCODE_PROTOCOL_VIOLATION),
-							 (errmsg("unsupported SCRAM channel-binding type"))));
-
-				/* Save the name for handling of subsequent messages */
-				state->channel_binding_type = pstrdup(channel_binding_type);
-			}
+			channel_binding_type = read_attr_value(&input, 'p');
+
+			/*
+			 * The only channel binding type we support is
+			 * tls-server-end-point.
+			 */
+			if (strcmp(channel_binding_type, "tls-server-end-point") != 0)
+				ereport(ERROR,
+						(errcode(ERRCODE_PROTOCOL_VIOLATION),
+						 (errmsg("unsupported SCRAM channel-binding type \"%s\"",
+								 sanitize_str(channel_binding_type)))));
 			break;
 		default:
 			ereport(ERROR,
@@ -1096,8 +1190,9 @@ read_client_final_message(scram_state *state, char *input)
 	 * then followed by the actual binding data depending on the type.
 	 */
 	channel_binding = read_attr_value(&p, 'c');
-	if (state->channel_binding_type)
+	if (state->channel_binding_in_use)
 	{
+#ifdef USE_SSL
 		const char *cbind_data = NULL;
 		size_t		cbind_data_len = 0;
 		size_t		cbind_header_len;
@@ -1108,39 +1203,18 @@ read_client_final_message(scram_state *state, char *input)
 
 		Assert(state->cbind_flag == 'p');
 
-		/*
-		 * Fetch data appropriate for channel binding type
-		 */
-		if (strcmp(state->channel_binding_type, SCRAM_CHANNEL_BINDING_TLS_UNIQUE) == 0)
-		{
-#ifdef USE_SSL
-			cbind_data = be_tls_get_peer_finished(state->port, &cbind_data_len);
-#endif
-		}
-		else if (strcmp(state->channel_binding_type,
-						SCRAM_CHANNEL_BINDING_TLS_END_POINT) == 0)
-		{
-			/* Fetch hash data of server's SSL certificate */
-#ifdef USE_SSL
-			cbind_data = be_tls_get_certificate_hash(state->port,
-													 &cbind_data_len);
-#endif
-		}
-		else
-		{
-			/* should not happen */
-			elog(ERROR, "invalid channel binding type");
-		}
+		/* Fetch hash data of server's SSL certificate */
+		cbind_data = be_tls_get_certificate_hash(state->port,
+												 &cbind_data_len);
 
 		/* should not happen */
 		if (cbind_data == NULL || cbind_data_len == 0)
-			elog(ERROR, "empty channel binding data for channel binding type \"%s\"",
-				 state->channel_binding_type);
+			elog(ERROR, "could not get server certificate hash");
 
-		cbind_header_len = 4 + strlen(state->channel_binding_type); /* p=type,, */
+		cbind_header_len = strlen("p=tls-server-end-point,,");	/* p=type,, */
 		cbind_input_len = cbind_header_len + cbind_data_len;
 		cbind_input = palloc(cbind_input_len);
-		snprintf(cbind_input, cbind_input_len, "p=%s,,", state->channel_binding_type);
+		snprintf(cbind_input, cbind_input_len, "p=tls-server-end-point,,");
 		memcpy(cbind_input + cbind_header_len, cbind_data, cbind_data_len);
 
 		b64_message = palloc(pg_b64_enc_len(cbind_input_len) + 1);
@@ -1156,6 +1230,10 @@ read_client_final_message(scram_state *state, char *input)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
 					 (errmsg("SCRAM channel binding check failed"))));
+#else
+		/* shouldn't happen, because we checked this earlier already */
+		elog(ERROR, "channel binding not supported without SSL");
+#endif
 	}
 	else
 	{
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 63f37902e6..d0b344bc59 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -862,8 +862,7 @@ CheckMD5Auth(Port *port, char *shadow_pass, char **logdetail)
 static int
 CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
 {
-	char	   *sasl_mechs;
-	char	   *p;
+	StringInfoData sasl_mechs;
 	int			mtype;
 	StringInfoData buf;
 	void	   *scram_opaq;
@@ -890,41 +889,16 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
 	/*
 	 * Send the SASL authentication request to user.  It includes the list of
 	 * authentication mechanisms that are supported.  The order of mechanisms
-	 * is advertised in decreasing order of importance.  So the
-	 * channel-binding variants go first, if they are supported.  Channel
-	 * binding is only supported in SSL builds.
+	 * is advertised in decreasing order of importance.
 	 */
-	sasl_mechs = palloc(strlen(SCRAM_SHA_256_PLUS_NAME) +
-						strlen(SCRAM_SHA_256_NAME) + 3);
-	p = sasl_mechs;
-
-	if (port->ssl_in_use)
-	{
-		strcpy(p, SCRAM_SHA_256_PLUS_NAME);
-		p += strlen(SCRAM_SHA_256_PLUS_NAME) + 1;
-	}
-
-	strcpy(p, SCRAM_SHA_256_NAME);
-	p += strlen(SCRAM_SHA_256_NAME) + 1;
+	initStringInfo(&sasl_mechs);
 
+	pg_be_scram_get_mechanisms(port, &sasl_mechs);
 	/* Put another '\0' to mark that list is finished. */
-	p[0] = '\0';
-
-	sendAuthRequest(port, AUTH_REQ_SASL, sasl_mechs, p - sasl_mechs + 1);
-	pfree(sasl_mechs);
+	appendStringInfoChar(&sasl_mechs, '\0');
 
-	/*
-	 * Initialize the status tracker for message exchanges.
-	 *
-	 * If the user doesn't exist, or doesn't have a valid password, or it's
-	 * expired, we still go through the motions of SASL authentication, but
-	 * tell the authentication method that the authentication is "doomed".
-	 * That is, it's going to fail, no matter what.
-	 *
-	 * This is because we don't want to reveal to an attacker what usernames
-	 * are valid, nor which users have a valid password.
-	 */
-	scram_opaq = pg_be_scram_init(port, shadow_pass);
+	sendAuthRequest(port, AUTH_REQ_SASL, sasl_mechs.data, sasl_mechs.len);
+	pfree(sasl_mechs.data);
 
 	/*
 	 * Loop through SASL message exchange.  This exchange can consist of
@@ -973,13 +947,20 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
 			const char *selected_mech;
 
 			selected_mech = pq_getmsgrawstring(&buf);
-			if (strcmp(selected_mech, SCRAM_SHA_256_NAME) != 0 &&
-				strcmp(selected_mech, SCRAM_SHA_256_PLUS_NAME) != 0)
-			{
-				ereport(ERROR,
-						(errcode(ERRCODE_PROTOCOL_VIOLATION),
-						 errmsg("client selected an invalid SASL authentication mechanism")));
-			}
+
+			/*
+			 * Initialize the status tracker for message exchanges.
+			 *
+			 * If the user doesn't exist, or doesn't have a valid password, or
+			 * it's expired, we still go through the motions of SASL
+			 * authentication, but tell the authentication method that the
+			 * authentication is "doomed". That is, it's going to fail, no
+			 * matter what.
+			 *
+			 * This is because we don't want to reveal to an attacker what
+			 * usernames are valid, nor which users have a valid password.
+			 */
+			scram_opaq = pg_be_scram_init(port, selected_mech, shadow_pass);
 
 			inputlen = pq_getmsgint(&buf, 4);
 			if (inputlen == -1)
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 310e9ba348..1b659a5870 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -1105,28 +1105,10 @@ be_tls_get_peerdn_name(Port *port, char *ptr, size_t len)
 		ptr[0] = '\0';
 }
 
-char *
-be_tls_get_peer_finished(Port *port, size_t *len)
-{
-	char		dummy[1];
-	char	   *result;
-
-	/*
-	 * OpenSSL does not offer an API to directly get the length of the
-	 * expected TLS Finished message, so just do a dummy call to grab this
-	 * information to allow caller to do an allocation with a correct size.
-	 */
-	*len = SSL_get_peer_finished(port->ssl, dummy, sizeof(dummy));
-	result = palloc(*len);
-	(void) SSL_get_peer_finished(port->ssl, result, *len);
-
-	return result;
-}
-
+#ifdef HAVE_X509_GET_SIGNATURE_NID
 char *
 be_tls_get_certificate_hash(Port *port, size_t *len)
 {
-#ifdef HAVE_X509_GET_SIGNATURE_NID
 	X509	   *server_cert;
 	char	   *cert_hash;
 	const EVP_MD *algo_type = NULL;
@@ -1176,13 +1158,8 @@ be_tls_get_certificate_hash(Port *port, size_t *len)
 	*len = hash_size;
 
 	return cert_hash;
-#else
-	ereport(ERROR,
-			(errcode(ERRCODE_PROTOCOL_VIOLATION),
-			 errmsg("channel binding type \"tls-server-end-point\" is not supported by this build")));
-	return NULL;
-#endif
 }
+#endif
 
 /*
  * Convert an X509 subject name to a cstring.
diff --git a/src/include/common/scram-common.h b/src/include/common/scram-common.h
index dcb5d69078..2131303169 100644
--- a/src/include/common/scram-common.h
+++ b/src/include/common/scram-common.h
@@ -19,10 +19,6 @@
 #define SCRAM_SHA_256_NAME "SCRAM-SHA-256"
 #define SCRAM_SHA_256_PLUS_NAME "SCRAM-SHA-256-PLUS"	/* with channel binding */
 
-/* Channel binding types */
-#define SCRAM_CHANNEL_BINDING_TLS_UNIQUE    "tls-unique"
-#define SCRAM_CHANNEL_BINDING_TLS_END_POINT	"tls-server-end-point"
-
 /* Length of SCRAM keys (client and server) */
 #define SCRAM_KEY_LEN				PG_SHA256_DIGEST_LENGTH
 
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 7698cd1f88..6f24d43ebd 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -261,23 +261,19 @@ extern const char *be_tls_get_cipher(Port *port);
 extern void be_tls_get_peerdn_name(Port *port, char *ptr, size_t len);
 
 /*
- * Get the expected TLS Finished message information from the client, useful
- * for authorization when doing channel binding.
- *
- * Result is a palloc'd copy of the TLS Finished message with its size.
- */
-extern char *be_tls_get_peer_finished(Port *port, size_t *len);
-
-/*
  * Get the server certificate hash for SCRAM channel binding type
  * tls-server-end-point.
  *
  * The result is a palloc'd hash of the server certificate with its
  * size, and NULL if there is no certificate available.
  */
+#if defined(USE_OPENSSL) && defined(HAVE_X509_GET_SIGNATURE_NID)
+#define HAVE_BE_TLS_GET_CERTIFICATE_HASH
 extern char *be_tls_get_certificate_hash(Port *port, size_t *len);
 #endif
 
+#endif	/* USE_SSL */
+
 extern ProtocolVersion FrontendProtocol;
 
 /* TCP keepalives configuration. These are no-ops on an AF_UNIX socket. */
diff --git a/src/include/libpq/scram.h b/src/include/libpq/scram.h
index 91872fcd08..f7865ca5fc 100644
--- a/src/include/libpq/scram.h
+++ b/src/include/libpq/scram.h
@@ -13,6 +13,7 @@
 #ifndef PG_SCRAM_H
 #define PG_SCRAM_H
 
+#include "lib/stringinfo.h"
 #include "libpq/libpq-be.h"
 
 /* Status codes for message exchange */
@@ -21,7 +22,8 @@
 #define SASL_EXCHANGE_FAILURE		2
 
 /* Routines dedicated to authentication */
-extern void *pg_be_scram_init(Port *port, const char *shadow_pass);
+extern void pg_be_scram_get_mechanisms(Port *port, StringInfo buf);
+extern void *pg_be_scram_init(Port *port, const char *selected_mech, const char *shadow_pass);
 extern int pg_be_scram_exchange(void *opaq, char *input, int inputlen,
 					 char **output, int *outputlen, char **logdetail);
 
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index 8415bbb5c6..5cd1409938 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -352,16 +352,7 @@ build_client_first_message(fe_scram_state *state)
 	if (strcmp(state->sasl_mechanism, SCRAM_SHA_256_PLUS_NAME) == 0)
 	{
 		Assert(conn->ssl_in_use);
-		appendPQExpBuffer(&buf, "p=%s", conn->scram_channel_binding);
-	}
-	else if (conn->scram_channel_binding == NULL ||
-			 strlen(conn->scram_channel_binding) == 0)
-	{
-		/*
-		 * Client has chosen to not show to server that it supports channel
-		 * binding.
-		 */
-		appendPQExpBuffer(&buf, "n");
+		appendPQExpBuffer(&buf, "p=tls-server-end-point");
 	}
 	else if (conn->ssl_in_use)
 	{
@@ -432,60 +423,28 @@ build_client_final_message(fe_scram_state *state)
 	 */
 	if (strcmp(state->sasl_mechanism, SCRAM_SHA_256_PLUS_NAME) == 0)
 	{
+#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
 		char	   *cbind_data = NULL;
 		size_t		cbind_data_len = 0;
 		size_t		cbind_header_len;
 		char	   *cbind_input;
 		size_t		cbind_input_len;
 
-		if (strcmp(conn->scram_channel_binding, SCRAM_CHANNEL_BINDING_TLS_UNIQUE) == 0)
-		{
-#ifdef USE_SSL
-			cbind_data = pgtls_get_finished(state->conn, &cbind_data_len);
-			if (cbind_data == NULL)
-				goto oom_error;
-#endif
-		}
-		else if (strcmp(conn->scram_channel_binding,
-						SCRAM_CHANNEL_BINDING_TLS_END_POINT) == 0)
-		{
-			/* Fetch hash data of server's SSL certificate */
-#ifdef USE_SSL
-			cbind_data =
-				pgtls_get_peer_certificate_hash(state->conn,
-												&cbind_data_len);
-			if (cbind_data == NULL)
-			{
-				/* error message is already set on error */
-				return NULL;
-			}
-#endif
-		}
-		else
+		/* Fetch hash data of server's SSL certificate */
+		cbind_data =
+			pgtls_get_peer_certificate_hash(state->conn,
+											&cbind_data_len);
+		if (cbind_data == NULL)
 		{
-			/* should not happen */
+			/* error message is already set on error */
 			termPQExpBuffer(&buf);
-			printfPQExpBuffer(&conn->errorMessage,
-							  libpq_gettext("invalid channel binding type\n"));
-			return NULL;
-		}
-
-		/* should not happen */
-		if (cbind_data == NULL || cbind_data_len == 0)
-		{
-			if (cbind_data != NULL)
-				free(cbind_data);
-			termPQExpBuffer(&buf);
-			printfPQExpBuffer(&conn->errorMessage,
-							  libpq_gettext("empty channel binding data for channel binding type \"%s\"\n"),
-							  conn->scram_channel_binding);
 			return NULL;
 		}
 
 		appendPQExpBuffer(&buf, "c=");
 
 		/* p=type,, */
-		cbind_header_len = 4 + strlen(conn->scram_channel_binding);
+		cbind_header_len = strlen("p=tls-server-end-point,,");
 		cbind_input_len = cbind_header_len + cbind_data_len;
 		cbind_input = malloc(cbind_input_len);
 		if (!cbind_input)
@@ -493,8 +452,7 @@ build_client_final_message(fe_scram_state *state)
 			free(cbind_data);
 			goto oom_error;
 		}
-		snprintf(cbind_input, cbind_input_len, "p=%s,,",
-				 conn->scram_channel_binding);
+		memcpy(cbind_input, "p=tls-server-end-point,,", cbind_header_len);
 		memcpy(cbind_input + cbind_header_len, cbind_data, cbind_data_len);
 
 		if (!enlargePQExpBuffer(&buf, pg_b64_enc_len(cbind_input_len)))
@@ -508,10 +466,15 @@ build_client_final_message(fe_scram_state *state)
 
 		free(cbind_data);
 		free(cbind_input);
+#else
+		/*
+		 * Chose channel binding, but the SSL library doesn't support it.
+		 * Shouldn't happen.
+		 */
+		termPQExpBuffer(&buf);
+		return NULL;
+#endif	/* HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH */
 	}
-	else if (conn->scram_channel_binding == NULL ||
-			 strlen(conn->scram_channel_binding) == 0)
-		appendPQExpBuffer(&buf, "c=biws");	/* base64 of "n,," */
 	else if (conn->ssl_in_use)
 		appendPQExpBuffer(&buf, "c=eSws");	/* base64 of "y,," */
 	else
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 3b2073a47f..4589427747 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -530,11 +530,26 @@ pg_SASL_init(PGconn *conn, int payloadlen)
 		 * nothing else has already been picked.  If we add more mechanisms, a
 		 * more refined priority mechanism might become necessary.
 		 */
-		if (conn->ssl_in_use &&
-			conn->scram_channel_binding &&
-			strlen(conn->scram_channel_binding) > 0 &&
-			strcmp(mechanism_buf.data, SCRAM_SHA_256_PLUS_NAME) == 0)
-			selected_mechanism = SCRAM_SHA_256_PLUS_NAME;
+		if (strcmp(mechanism_buf.data, SCRAM_SHA_256_PLUS_NAME) == 0)
+		{
+			if (conn->ssl_in_use)
+				selected_mechanism = SCRAM_SHA_256_PLUS_NAME;
+			else
+			{
+				/*
+				 * The server offered SCRAM-SHA-256-PLUS, but the connection
+				 * is not SSL-encrypted. That's not sane. Perhaps SSL was
+				 * stripped by a proxy? There's no point in continuing,
+				 * because the server will reject the connection anyway if we
+				 * try authenticate without channel binding even though both
+				 * the client and server supported it. The SCRAM exchange
+				 * checks for that, to prevent downgrade attacks.
+				 */
+				printfPQExpBuffer(&conn->errorMessage,
+								  libpq_gettext("server offered SCRAM-SHA-256-PLUS authentication over a non-SSL connection\n"));
+				goto error;
+			}
+		}
 		else if (strcmp(mechanism_buf.data, SCRAM_SHA_256_NAME) == 0 &&
 				 !selected_mechanism)
 			selected_mechanism = SCRAM_SHA_256_NAME;
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index bd7dac120d..e4d5d696b4 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -123,7 +123,6 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
 #define DefaultOption	""
 #define DefaultAuthtype		  ""
 #define DefaultTargetSessionAttrs	"any"
-#define DefaultSCRAMChannelBinding	SCRAM_CHANNEL_BINDING_TLS_UNIQUE
 #ifdef USE_SSL
 #define DefaultSSLMode "prefer"
 #else
@@ -264,11 +263,6 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 		"TCP-Keepalives-Count", "", 10, /* strlen(INT32_MAX) == 10 */
 	offsetof(struct pg_conn, keepalives_count)},
 
-	{"scram_channel_binding", NULL, DefaultSCRAMChannelBinding, NULL,
-		"SCRAM-Channel-Binding", "D",
-		21,						/* sizeof("tls-server-end-point") == 21 */
-	offsetof(struct pg_conn, scram_channel_binding)},
-
 	/*
 	 * ssl options are allowed even without client SSL support because the
 	 * client can still handle SSL modes "disable" and "allow". Other
@@ -3476,8 +3470,6 @@ freePGconn(PGconn *conn)
 		free(conn->keepalives_interval);
 	if (conn->keepalives_count)
 		free(conn->keepalives_count);
-	if (conn->scram_channel_binding)
-		free(conn->scram_channel_binding);
 	if (conn->sslmode)
 		free(conn->sslmode);
 	if (conn->sslcert)
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 045405e92b..bbae8eff81 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -369,30 +369,10 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
 	return n;
 }
 
-char *
-pgtls_get_finished(PGconn *conn, size_t *len)
-{
-	char		dummy[1];
-	char	   *result;
-
-	/*
-	 * OpenSSL does not offer an API to get directly the length of the TLS
-	 * Finished message sent, so first do a dummy call to grab this
-	 * information and then do an allocation with the correct size.
-	 */
-	*len = SSL_get_finished(conn->ssl, dummy, sizeof(dummy));
-	result = malloc(*len);
-	if (result == NULL)
-		return NULL;
-	(void) SSL_get_finished(conn->ssl, result, *len);
-
-	return result;
-}
-
+#ifdef HAVE_X509_GET_SIGNATURE_NID
 char *
 pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
 {
-#ifdef HAVE_X509_GET_SIGNATURE_NID
 	X509	   *peer_cert;
 	const EVP_MD *algo_type;
 	unsigned char hash[EVP_MAX_MD_SIZE];	/* size for SHA-512 */
@@ -462,12 +442,8 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
 	*len = hash_size;
 
 	return cert_hash;
-#else
-	printfPQExpBuffer(&conn->errorMessage,
-					  libpq_gettext("channel binding type \"tls-server-end-point\" is not supported by this build\n"));
-	return NULL;
-#endif
 }
+#endif /* HAVE_X509_GET_SIGNATURE_NID */
 
 /* ------------------------------------------------------------ */
 /*						OpenSSL specific code					*/
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 9a586ff25a..3323679ce9 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -349,7 +349,6 @@ struct pg_conn
 										 * retransmits */
 	char	   *keepalives_count;	/* maximum number of TCP keepalive
 									 * retransmits */
-	char	   *scram_channel_binding;	/* SCRAM channel binding type */
 	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
 	char	   *sslcompression; /* SSL compression (0 or 1) */
 	char	   *sslkey;			/* client key filename */
@@ -716,21 +715,16 @@ extern bool pgtls_read_pending(PGconn *conn);
 extern ssize_t pgtls_write(PGconn *conn, const void *ptr, size_t len);
 
 /*
- * Get the TLS finish message sent during last handshake.
- *
- * This information is useful for callers doing channel binding during
- * authentication.
- */
-extern char *pgtls_get_finished(PGconn *conn, size_t *len);
-
-/*
  * Get the hash of the server certificate, for SCRAM channel binding type
  * tls-server-end-point.
  *
  * NULL is sent back to the caller in the event of an error, with an
  * error message for the caller to consume.
  */
+#if defined(USE_OPENSSL) && defined(HAVE_X509_GET_SIGNATURE_NID)
+#define HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
 extern char *pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len);
+#endif
 
 /*
  * Verify that the server certificate matches the host name we connected to.
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 52a8f458cb..164823a4e6 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -13,7 +13,7 @@ if ($ENV{with_openssl} ne 'yes')
 	plan skip_all => 'SSL not supported by this build';
 }
 
-my $number_of_tests = 6;
+my $number_of_tests = 1;
 
 # This is the hostname used to connect to the server.
 my $SERVERHOSTADDR = '127.0.0.1';
@@ -47,35 +47,6 @@ $common_connstr =
 
 # Default settings
 test_connect_ok($common_connstr, '',
-	"SCRAM authentication with default channel binding");
-
-# Channel binding settings
-test_connect_ok(
-	$common_connstr,
-	"scram_channel_binding=tls-unique",
-	"SCRAM authentication with tls-unique as channel binding");
-test_connect_ok($common_connstr, "scram_channel_binding=''",
-	"SCRAM authentication without channel binding");
-if ($supports_tls_server_end_point)
-{
-	test_connect_ok(
-		$common_connstr,
-		"scram_channel_binding=tls-server-end-point",
-		"SCRAM authentication with tls-server-end-point as channel binding");
-}
-else
-{
-	test_connect_fails(
-		$common_connstr,
-		"scram_channel_binding=tls-server-end-point",
-		qr/channel binding type "tls-server-end-point" is not supported by this build/,
-		"SCRAM authentication with tls-server-end-point as channel binding");
-	$number_of_tests++;
-}
-test_connect_fails(
-	$common_connstr,
-	"scram_channel_binding=not-exists",
-	qr/unsupported SCRAM channel-binding type/,
-	"SCRAM authentication with invalid channel binding");
+	"Basic SCRAM authentication");
 
 done_testing($number_of_tests);
-- 
2.11.0

