On Sat, May 26, 2018 at 11:42:38PM +0900, Michael Paquier wrote:
> On Sat, May 26, 2018 at 09:08:50AM -0400, Bruce Momjian wrote:
>> On Sat, May 26, 2018 at 08:32:20AM +0900, Michael Paquier wrote:
>>>
>>> OK, I can live with that as well.  So we'll go in the direction of two
>>> parameters then:
>>> - scram_channel_binding, which can use "prefer" (default), "require" or
>>> "disable".
>>> - scram_channel_binding_name, developer option to choose the type of
>>> channel binding, with "tls-unique" (default) and "tls-server-end-point".
>>> We could also remove the prefix "scram_".  Ideas of names are welcome.
>> 
>> scram_channel_binding_method?
> 
> Or scram_channel_binding_type.  The first sentence of RFC 5929 uses this
> term.

I just went with scram_channel_binding_mode (require, disable and
prefer) and scram_channel_binding_type as parameter names, in the shape
of the attached patch.

>> Do we really know someone is going to want to actually specify the
>> channel binding type?  If it is only testing, maybe we don't need to
>> document this parameter.
> 
> Keeping everything documented is useful as well for new developers, as
> they need to guess less from the code.  So I would prefer seeing both
> connection parameters documented, and mentioning directly in the docs if
> a parameter is for developers or not.

So done this way.  Feel free to pick me up at PGcon this week if you
wish to discuss this issue.  Docs, tests and a commit message are
added.
--
Michael
From 34b68db28172458f69c59488a613d848939838a3 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 22 May 2018 17:03:48 +0900
Subject: [PATCH] Rework scram_channel_binding to protect from downgrade
 attacks

When a client attempts to connect to a PostgreSQL cluster, it may be
possible that it requested channel binding with SCRAM authentication,
but that the server tricks the cluster and forcibly downgrades the
authentication request.  For example, a v10 cluster supports SCRAM but
not channel binding so authentication could be achieved without channel
binding used.

In order to protect from such attacks, rework libpq handling of the
connection parameter scram_channel_binding as follows by splitting it
into two pieces:
- scram_channel_binding_mode, which can use as values "require",
"disable" or "prefer".
- scram_channel_binding_type, which is similar to the previous
scram_channel_binding, except that it does not accept anymore an empty
value to mean that channel binding is disabled.

"prefer", is the default and behaves so as channel binding is used if
available.  If the cluster does not support it then it is not used. This
does not protect from downgrade attacks.
"disable", which is the equivalent of the empty value previously,
disables channel binding for all exchanges.
"require" makes channel binding a hard requirement for the
authentication.

For "require", if the cluster does not support channel binding with
SCRAM (v10 server) or if SSL is not used, then the connection is refused
by libpq, which is something that can happen if SSL is not used
(prevention also for users forgetting to use sslmode=require at least in
connection strings).  This also allows users to enforce the use of SCRAM
with channel binding even if the targeted cluster downgrades to "trust"
or similar.

In order to achieve that, when receiving AUTH_REQ_OK from the server, we
check if a SASL exchange has happened and has finished, where the client
makes sure that the server knows the connection proof.  If the cluster
does not publish the -PLUS mechanism, then connection is also refused.

Discussion: https://postgr.es/m/20180519123557.gb2...@paquier.xyz
---
 doc/src/sgml/libpq.sgml              | 72 ++++++++++++++++++++++++----
 doc/src/sgml/release-11.sgml         |  2 +-
 src/interfaces/libpq/fe-auth-scram.c | 45 +++++++++++++----
 src/interfaces/libpq/fe-auth.c       | 54 +++++++++++++++++++--
 src/interfaces/libpq/fe-auth.h       |  1 +
 src/interfaces/libpq/fe-connect.c    | 51 +++++++++++++++++---
 src/interfaces/libpq/libpq-int.h     |  3 +-
 src/test/ssl/ServerSetup.pm          | 27 ++++++-----
 src/test/ssl/t/002_scram.pl          | 53 ++++++++++++++++----
 9 files changed, 257 insertions(+), 51 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 498b8df988..90e522b2af 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1238,24 +1238,76 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       </listitem>
      </varlistentry>
 
-     <varlistentry id="libpq-scram-channel-binding" xreflabel="scram_channel_binding">
-      <term><literal>scram_channel_binding</literal></term>
+     <varlistentry id="libpq-scram-channel-binding-mode" xreflabel="scram_channel_binding_mode">
+      <term><literal>scram_channel_binding_mode</literal></term>
+      <listitem>
+       <para>
+        This option determines whether channel binding should be used
+        with <acronym>SCRAM</acronym> authentication.  While
+        <acronym>SCRAM</acronym> alone prevents the replay of transmitted
+        hashed passwords, channel binding also prevents man-in-the-middle
+        attacks.  There are three modes:
+        
+        <variablelist>
+         <varlistentry>
+          <term><literal>disable</literal></term>
+          <listitem>
+           <para>
+            Disable the use of channel binding for all connection attempts.
+           </para>
+          </listitem>
+         </varlistentry>
+         
+         <varlistentry>
+          <term><literal>prefer</literal> (default)</term>
+          <listitem>
+           <para>
+            Use channel binding if available.  If the cluster does not
+            support it, then it is not used.
+           </para>
+          </listitem>
+         </varlistentry>
+         
+         <varlistentry>
+          <term><literal>require</literal></term>
+          <listitem>
+           <para>
+            Require the presence of channel binding for authentication.
+            If the cluster does not support channel binding or if another
+            authentication protocol other than <acronym>SCRAM</acronym> is
+            used, then fail the connection.
+           </para>
+          </listitem>
+         </varlistentry>
+        </variablelist>
+       </para>
+
+       <para>
+        Channel binding is only supported on SSL connections.  However, if
+        the connection is not using SSL, then this setting is ignored except
+        if <literal>require</literal> is used, in which case the connection
+        to the cluster is refused even if SSL is not enabled or if the cluster
+        does not support channel binding as a mechanism as a prevention
+        against authentication downgrade attacks.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="libpq-scram-channel-binding-type" xreflabel="scram_channel_binding_type">
+      <term><literal>scram_channel_binding_type</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.
+        authentication.
        </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
+        listed in <xref linkend="sasl-authentication"/>.  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.
+        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>
diff --git a/doc/src/sgml/release-11.sgml b/doc/src/sgml/release-11.sgml
index 7599c6f7a7..15f21fd91a 100644
--- a/doc/src/sgml/release-11.sgml
+++ b/doc/src/sgml/release-11.sgml
@@ -1246,7 +1246,7 @@ same commits as above
         replay of transmitted hashed passwords in a later
         session, <acronym>SCRAM</acronym> with channel binding
         also prevents man-in-the-middle attacks.  The options are <link
-        linkend="libpq-scram-channel-binding"><option>scram_channel_binding=tls-unique</option></link>
+        linkend="libpq-scram-channel-binding-type"><option>scram_channel_binding=tls-unique</option></link>
         and <option>scram_channel_binding=tls-server-end-point</option>.
        </para>
       </listitem>
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index 8415bbb5c6..807a984f60 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -352,10 +352,10 @@ 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);
+		appendPQExpBuffer(&buf, "p=%s", conn->scram_channel_binding_type);
 	}
-	else if (conn->scram_channel_binding == NULL ||
-			 strlen(conn->scram_channel_binding) == 0)
+	else if (conn->scram_channel_binding_mode &&
+			 strcmp(conn->scram_channel_binding_mode, "disable") == 0)
 	{
 		/*
 		 * Client has chosen to not show to server that it supports channel
@@ -438,7 +438,8 @@ build_client_final_message(fe_scram_state *state)
 		char	   *cbind_input;
 		size_t		cbind_input_len;
 
-		if (strcmp(conn->scram_channel_binding, SCRAM_CHANNEL_BINDING_TLS_UNIQUE) == 0)
+		if (strcmp(conn->scram_channel_binding_type,
+				   SCRAM_CHANNEL_BINDING_TLS_UNIQUE) == 0)
 		{
 #ifdef USE_SSL
 			cbind_data = pgtls_get_finished(state->conn, &cbind_data_len);
@@ -446,7 +447,7 @@ build_client_final_message(fe_scram_state *state)
 				goto oom_error;
 #endif
 		}
-		else if (strcmp(conn->scram_channel_binding,
+		else if (strcmp(conn->scram_channel_binding_type,
 						SCRAM_CHANNEL_BINDING_TLS_END_POINT) == 0)
 		{
 			/* Fetch hash data of server's SSL certificate */
@@ -478,14 +479,14 @@ build_client_final_message(fe_scram_state *state)
 			termPQExpBuffer(&buf);
 			printfPQExpBuffer(&conn->errorMessage,
 							  libpq_gettext("empty channel binding data for channel binding type \"%s\"\n"),
-							  conn->scram_channel_binding);
+							  conn->scram_channel_binding_type);
 			return NULL;
 		}
 
 		appendPQExpBuffer(&buf, "c=");
 
 		/* p=type,, */
-		cbind_header_len = 4 + strlen(conn->scram_channel_binding);
+		cbind_header_len = 4 + strlen(conn->scram_channel_binding_type);
 		cbind_input_len = cbind_header_len + cbind_data_len;
 		cbind_input = malloc(cbind_input_len);
 		if (!cbind_input)
@@ -494,7 +495,7 @@ build_client_final_message(fe_scram_state *state)
 			goto oom_error;
 		}
 		snprintf(cbind_input, cbind_input_len, "p=%s,,",
-				 conn->scram_channel_binding);
+				 conn->scram_channel_binding_type);
 		memcpy(cbind_input + cbind_header_len, cbind_data, cbind_data_len);
 
 		if (!enlargePQExpBuffer(&buf, pg_b64_enc_len(cbind_input_len)))
@@ -509,8 +510,8 @@ build_client_final_message(fe_scram_state *state)
 		free(cbind_data);
 		free(cbind_input);
 	}
-	else if (conn->scram_channel_binding == NULL ||
-			 strlen(conn->scram_channel_binding) == 0)
+	else if (conn->scram_channel_binding_mode &&
+			 strcmp(conn->scram_channel_binding_mode, "disable") == 0)
 		appendPQExpBuffer(&buf, "c=biws");	/* base64 of "n,," */
 	else if (conn->ssl_in_use)
 		appendPQExpBuffer(&buf, "c=eSws");	/* base64 of "y,," */
@@ -811,6 +812,30 @@ pg_fe_scram_build_verifier(const char *password)
 	return result;
 }
 
+/*
+ * Check if a SCRAM exchange has been finished or not.  This can
+ * be used for security-related checks.
+ */
+bool
+pg_fe_scram_exchange_finished(PGconn *conn)
+{
+	fe_scram_state *state;
+
+	if (conn == NULL ||
+		conn->sasl_state == NULL)
+		return false;
+
+	state = (fe_scram_state *) conn->sasl_state;
+
+	/*
+	 * A exchange is considered as complete only once the server
+	 * signature has been checked.
+	 */
+	if (state->state != FE_SCRAM_FINISHED)
+		return false;
+	return true;
+}
+
 /*
  * Random number generator.
  */
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 3b2073a47f..8c7d189924 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -526,13 +526,13 @@ pg_SASL_init(PGconn *conn, int payloadlen)
 
 		/*
 		 * Select the mechanism to use.  Pick SCRAM-SHA-256-PLUS over anything
-		 * else if a channel binding type is set.  Pick SCRAM-SHA-256 if
-		 * nothing else has already been picked.  If we add more mechanisms, a
+		 * else if a channel binding mode is not 'disable'.  Pick SCRAM-SHA-256
+		 * if 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 &&
+			conn->scram_channel_binding_mode &&
+			strcmp(conn->scram_channel_binding_mode, "disable") != 0 &&
 			strcmp(mechanism_buf.data, SCRAM_SHA_256_PLUS_NAME) == 0)
 			selected_mechanism = SCRAM_SHA_256_PLUS_NAME;
 		else if (strcmp(mechanism_buf.data, SCRAM_SHA_256_NAME) == 0 &&
@@ -547,6 +547,37 @@ pg_SASL_init(PGconn *conn, int payloadlen)
 		goto error;
 	}
 
+	/*
+	 * If client is willing to enforce the use the channel binding but
+	 * it has not been previously selected, because it was either not
+	 * published by the server or could not be selected, then complain
+	 * back.  If SSL is not used for this connection, still complain
+	 * similarly, as the client may want channel binding but forgot
+	 * to set it up to do so which could be the case with sslmode=prefer
+	 * for example.  This protects from any kind of downgrade attacks
+	 * from rogue servers attempting to bypass channel binding.
+	 */
+	if (conn->scram_channel_binding_mode &&
+		strcmp(conn->scram_channel_binding_mode, "require") == 0 &&
+		strcmp(selected_mechanism, SCRAM_SHA_256_PLUS_NAME) != 0)
+	{
+		printfPQExpBuffer(&conn->errorMessage,
+						  libpq_gettext("channel binding required for SASL authentication but no valid mechanism could be selected\n"));
+		goto error;
+	}
+
+	/*
+	 * Complain if channel binding mechanism is chosen but no channel
+	 * binding type is defined.
+	 */
+	if (strcmp(selected_mechanism, SCRAM_SHA_256_PLUS_NAME) == 0 &&
+		conn->scram_channel_binding_type == NULL)
+	{
+		printfPQExpBuffer(&conn->errorMessage,
+						  libpq_gettext("SASL authentication mechanism using channel binding supported but no channel binding type defined\n"));
+		goto error;
+	}
+
 	/*
 	 * Now that the SASL mechanism has been chosen for the exchange,
 	 * initialize its state information.
@@ -838,6 +869,21 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
 	switch (areq)
 	{
 		case AUTH_REQ_OK:
+			/*
+			 * If channel binding has been requested for authentication,
+			 * make sure that the client has not been tricked and that
+			 * a full SASL exchange has happened.  This happens at this
+			 * stage as it could be possible that a backend with "trust"
+			 * sends directly this message type.
+			 */
+			if (conn->scram_channel_binding_mode &&
+				strcmp(conn->scram_channel_binding_mode, "require") == 0 &&
+				!pg_fe_scram_exchange_finished(conn))
+			{
+				printfPQExpBuffer(&conn->errorMessage,
+								  libpq_gettext("channel binding required for authentication but invalid protocol was used\n"));
+				return STATUS_ERROR;
+			}
 			break;
 
 		case AUTH_REQ_KRB4:
diff --git a/src/interfaces/libpq/fe-auth.h b/src/interfaces/libpq/fe-auth.h
index a8a27c24a6..bf41bb4343 100644
--- a/src/interfaces/libpq/fe-auth.h
+++ b/src/interfaces/libpq/fe-auth.h
@@ -31,5 +31,6 @@ extern void pg_fe_scram_exchange(void *opaq, char *input, int inputlen,
 					 char **output, int *outputlen,
 					 bool *done, bool *success);
 extern char *pg_fe_scram_build_verifier(const char *password);
+extern bool pg_fe_scram_exchange_finished(PGconn *conn);
 
 #endif							/* FE_AUTH_H */
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index a7e969d7c1..a781464524 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -123,7 +123,8 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
 #define DefaultOption	""
 #define DefaultAuthtype		  ""
 #define DefaultTargetSessionAttrs	"any"
-#define DefaultSCRAMChannelBinding	SCRAM_CHANNEL_BINDING_TLS_UNIQUE
+#define DefaultSCRAMChannelBindingMode	"prefer"
+#define DefaultSCRAMChannelBindingType	SCRAM_CHANNEL_BINDING_TLS_UNIQUE
 #ifdef USE_SSL
 #define DefaultSSLMode "prefer"
 #else
@@ -264,10 +265,15 @@ 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",
+	{"scram_channel_binding_mode", NULL, DefaultSCRAMChannelBindingMode, NULL,
+		"SCRAM-Channel-Binding-Mode", "D",
+		8,						/* sizeof("require") == 8 */
+	offsetof(struct pg_conn, scram_channel_binding_mode)},
+
+	{"scram_channel_binding_type", NULL, DefaultSCRAMChannelBindingType, NULL,
+		"SCRAM-Channel-Binding-Type", "D",
 		21,						/* sizeof("tls-server-end-point") == 21 */
-	offsetof(struct pg_conn, scram_channel_binding)},
+	offsetof(struct pg_conn, scram_channel_binding_type)},
 
 	/*
 	 * ssl options are allowed even without client SSL support because the
@@ -1166,6 +1172,37 @@ connectOptions2(PGconn *conn)
 			goto oom_error;
 	}
 
+	/* validate scram_channel_binding_mode option */
+	if (conn->scram_channel_binding_mode)
+	{
+		if (strcmp(conn->scram_channel_binding_mode, "prefer") != 0 &&
+			strcmp(conn->scram_channel_binding_mode, "disable") != 0 &&
+			strcmp(conn->scram_channel_binding_mode, "require") != 0)
+		{
+			conn->status = CONNECTION_BAD;
+			printfPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("invalid scram_channel_binding_mode value: \"%s\"\n"),
+							  conn->scram_channel_binding_mode);
+			return false;
+		}
+	}
+
+	/* validate scram_channel_binding_type option */
+	if (conn->scram_channel_binding_type)
+	{
+		if (strcmp(conn->scram_channel_binding_type,
+				   SCRAM_CHANNEL_BINDING_TLS_UNIQUE) != 0 &&
+			strcmp(conn->scram_channel_binding_type,
+				   SCRAM_CHANNEL_BINDING_TLS_END_POINT) != 0)
+		{
+			conn->status = CONNECTION_BAD;
+			printfPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("invalid scram_channel_binding_type value: \"%s\"\n"),
+							  conn->scram_channel_binding_type);
+			return false;
+		}
+	}
+
 	/*
 	 * Resolve special "auto" client_encoding from the locale
 	 */
@@ -3476,8 +3513,10 @@ 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->scram_channel_binding_mode)
+		free(conn->scram_channel_binding_mode);
+	if (conn->scram_channel_binding_type)
+		free(conn->scram_channel_binding_type);
 	if (conn->sslmode)
 		free(conn->sslmode);
 	if (conn->sslcert)
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 9a586ff25a..1a76809403 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -349,7 +349,8 @@ struct pg_conn
 										 * retransmits */
 	char	   *keepalives_count;	/* maximum number of TCP keepalive
 									 * retransmits */
-	char	   *scram_channel_binding;	/* SCRAM channel binding type */
+	char	   *scram_channel_binding_mode;	/* SCRAM channel binding mode */
+	char	   *scram_channel_binding_type;	/* type of SCRAM channel binding */
 	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
 	char	   *sslcompression; /* SSL compression (0 or 1) */
 	char	   *sslkey;			/* client key filename */
diff --git a/src/test/ssl/ServerSetup.pm b/src/test/ssl/ServerSetup.pm
index 1cd3badaa1..db71632bd1 100644
--- a/src/test/ssl/ServerSetup.pm
+++ b/src/test/ssl/ServerSetup.pm
@@ -27,6 +27,7 @@ use Test::More;
 use Exporter 'import';
 our @EXPORT = qw(
   configure_test_server_for_ssl
+  configure_hba_for_ssl
   switch_server_cert
   test_connect_fails
   test_connect_ok
@@ -132,9 +133,8 @@ sub configure_test_server_for_ssl
 	$node->restart;
 
 	# Change pg_hba after restart because hostssl requires ssl=on
-	configure_hba_for_ssl($node, $serverhost, $authmethod);
-
-	return;
+	configure_hba_for_ssl($node, $serverhost, "hostssl", $authmethod);
+	return
 }
 
 # Change the configuration to use given server cert file, and reload
@@ -160,7 +160,7 @@ sub switch_server_cert
 
 sub configure_hba_for_ssl
 {
-	my ($node, $serverhost, $authmethod) = @_;
+	my ($node, $serverhost, $hosttype, $authmethod) = @_;
 	my $pgdata = $node->data_dir;
 
 	# Only accept SSL connections from localhost. Our tests don't depend on this
@@ -171,15 +171,20 @@ sub configure_hba_for_ssl
 	print $hba
 	  "# TYPE  DATABASE        USER            ADDRESS                 METHOD\n";
 	print $hba
-	  "hostssl trustdb         all             $serverhost/32            $authmethod\n";
+	  "$hosttype trustdb         all             $serverhost/32            $authmethod\n";
 	print $hba
-	  "hostssl trustdb         all             ::1/128                 $authmethod\n";
-	print $hba
-	  "hostssl certdb          all             $serverhost/32            cert\n";
-	print $hba
-	  "hostssl certdb          all             ::1/128                 cert\n";
+	  "$hosttype trustdb         all             ::1/128                 $authmethod\n";
+	# Certificate authentication is only authorized with hostssl
+	# as entry type.
+	if ($hosttype eq "hostssl")
+	{
+		print $hba
+		  "$hosttype certdb          all             $serverhost/32            cert\n";
+		print $hba
+		  "$hosttype certdb          all             ::1/128                 cert\n";
+	}
 	close $hba;
 	return;
 }
 
-1;
\ No newline at end of file
+1;
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 52a8f458cb..2f1fb8ba5c 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 = 13;
 
 # This is the hostname used to connect to the server.
 my $SERVERHOSTADDR = '127.0.0.1';
@@ -52,30 +52,67 @@ test_connect_ok($common_connstr, '',
 # Channel binding settings
 test_connect_ok(
 	$common_connstr,
-	"scram_channel_binding=tls-unique",
+	"scram_channel_binding_mode='require' scram_channel_binding_type=tls-unique",
 	"SCRAM authentication with tls-unique as channel binding");
-test_connect_ok($common_connstr, "scram_channel_binding=''",
+test_connect_ok($common_connstr, "scram_channel_binding_mode='disable'",
 	"SCRAM authentication without channel binding");
 if ($supports_tls_server_end_point)
 {
 	test_connect_ok(
 		$common_connstr,
-		"scram_channel_binding=tls-server-end-point",
+		"scram_channel_binding_mode='require' scram_channel_binding_type=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",
+		"scram_channel_binding_mode='require' scram_channel_binding_type=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");
+	"scram_channel_binding_mode=not-exists",
+	qr/invalid scram_channel_binding_mode value/,
+	"SCRAM authentication with invalid channel binding mode");
+test_connect_fails(
+	$common_connstr,
+	"scram_channel_binding_type=not-exists",
+	qr/invalid scram_channel_binding_type value/,
+	"SCRAM authentication with invalid channel binding type");
+
+# Downgrade attack tests
+# Update server's pg_hba.conf with "trust" level and try to
+# connect to the server with tls-unique.
+configure_hba_for_ssl($node, $SERVERHOSTADDR, "hostssl", "trust");
+$node->reload;
+test_connect_fails(
+	$common_connstr,
+	"scram_channel_binding_mode=require",
+	qr/channel binding required for authentication but invalid protocol was used/,
+	"Connection to trust server refused with mode require");
+
+# Now test "md5" level, which should pass as user has a SCRAM verifier.
+configure_hba_for_ssl($node, $SERVERHOSTADDR, "hostssl", "md5");
+$node->reload;
+test_connect_ok($common_connstr, "scram_channel_binding_mode=require",
+	"SCRAM authentication with channel binding required");
+
+# Now a tricky one, update pg_hba.conf so as non-SSL connections
+# are authorized.  Requiring channel binding should fail, with
+# incorrect SASL mechanism received from server.  Note that this
+# can trigger only with SCRAM used without SSL, or pre-10 servers
+# which have no channel binding support.
+configure_hba_for_ssl($node, $SERVERHOSTADDR, "hostnossl", "scram-sha-256");
+$node->reload;
+$common_connstr =
+  "user=ssltestuser dbname=trustdb sslmode=disable hostaddr=$SERVERHOSTADDR";
+test_connect_fails(
+	$common_connstr,
+	"scram_channel_binding_mode=require",
+	qr/channel binding required for SASL authentication but no valid mechanism could be selected/,
+	"Connection to trust server refused with required tls-unique");
 
 done_testing($number_of_tests);
-- 
2.17.0

Attachment: signature.asc
Description: PGP signature

Reply via email to