channel-binding-additions-michael.patch

diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index 3b40c4b083..61f9512544 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -127,8 +127,20 @@ pg_fe_scram_channel_bound(void *opaq)
 {
 	fe_scram_state *state = (fe_scram_state *) opaq;
 
-	return (state != NULL && state->state == FE_SCRAM_FINISHED &&
-			strcmp(state->sasl_mechanism, SCRAM_SHA_256_PLUS_NAME) == 0);
+	/* no SCRAM exchange done */
+	if (state == NULL)
+		return false;
+
+	/* SCRAM exchange not completed */
+	if (state->state != FE_SCRAM_FINISHED)
+		return false;
+
+	/* channel binding mechanism not used */
+	if (strcmp(state->sasl_mechanism, SCRAM_SHA_256_PLUS_NAME) != 0)
+		return false;
+
+	/* all clear! */
+	return true;
 }
 
 /*
@@ -368,7 +380,8 @@ build_client_first_message(fe_scram_state *state)
 		appendPQExpBufferStr(&buf, "p=tls-server-end-point");
 	}
 #ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
-	else if (conn->ssl_in_use && conn->channel_binding[0] != 'd')
+	else if (conn->channel_binding[0] != 'd' && /* disable */
+			 conn->ssl_in_use)
 	{
 		/*
 		 * Client supports channel binding, but thinks the server does not.
@@ -508,7 +521,8 @@ build_client_final_message(fe_scram_state *state)
 #endif							/* HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH */
 	}
 #ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
-	else if (conn->ssl_in_use && conn->channel_binding[0] != 'd')
+	else if (conn->channel_binding[0] != 'd' && /* disable */
+			 conn->ssl_in_use)
 		appendPQExpBufferStr(&buf, "c=eSws");	/* base64 of "y,," */
 #endif
 	else
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 0ef8fa89d0..0a70e0ea4a 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -423,7 +423,7 @@ pg_SASL_init(PGconn *conn, int payloadlen)
 
 	initPQExpBuffer(&mechanism_buf);
 
-	if (conn->channel_binding[0] == 'r' && /* require */
+	if (conn->channel_binding[0] == 'r' &&	/* require */
 		!conn->ssl_in_use)
 	{
 		printfPQExpBuffer(&conn->errorMessage,
@@ -462,8 +462,8 @@ 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 and if the client supports
-		 * it (and did not set channel_binding=disable). Pick SCRAM-SHA-256 if
+		 * else if a channel binding type is set and if the client supports it
+		 * (and did not set channel_binding=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.
 		 */
@@ -477,7 +477,7 @@ pg_SASL_init(PGconn *conn, int payloadlen)
 				 * can be created, and if channel_binding is not disabled.
 				 */
 #ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
-				if (conn->channel_binding[0] != 'd') /* disable */
+				if (conn->channel_binding[0] != 'd')	/* disable */
 					selected_mechanism = SCRAM_SHA_256_PLUS_NAME;
 #endif
 			}
@@ -502,7 +502,7 @@ pg_SASL_init(PGconn *conn, int payloadlen)
 			selected_mechanism = SCRAM_SHA_256_NAME;
 	}
 
-	if (conn->channel_binding[0] == 'r' && /* require */
+	if (conn->channel_binding[0] == 'r' &&	/* require */
 		strcmp(selected_mechanism, SCRAM_SHA_256_PLUS_NAME) != 0)
 	{
 		printfPQExpBuffer(&conn->errorMessage,
@@ -799,7 +799,7 @@ pg_password_sendauth(PGconn *conn, const char *password, AuthRequest areq)
 static bool
 check_expected_areq(AuthRequest areq, PGconn *conn)
 {
-	bool result = true;
+	bool		result = true;
 
 	/*
 	 * When channel_binding=require, we must protect against two cases: (1) we
@@ -808,7 +808,7 @@ check_expected_areq(AuthRequest areq, PGconn *conn)
 	 * AUTH_REQ_OK, we still must ensure that channel binding has happened in
 	 * order to authenticate the server.
 	 */
-	if (conn->channel_binding[0] == 'r' /* require */)
+	if (conn->channel_binding[0] == 'r' /* require */ )
 	{
 		switch (areq)
 		{
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index a7db473598..f91f0f2efe 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -217,7 +217,7 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 	offsetof(struct pg_conn, pgpassfile)},
 
 	{"channel_binding", "PGCHANNELBINDING", NULL, NULL,
-	 "Channel-Binding", "", 7, /* sizeof("require") */
+		"Channel-Binding", "", 7,	/* sizeof("require") */
 	offsetof(struct pg_conn, channel_binding)},
 
 	{"connect_timeout", "PGCONNECT_TIMEOUT", NULL, NULL,
@@ -3517,10 +3517,11 @@ keep_going:						/* We will come back to here until there is
 		case CONNECTION_SETENV:
 			{
 				/*
-				 * Do post-connection housekeeping (only needed in protocol 2.0).
+				 * Do post-connection housekeeping (only needed in protocol
+				 * 2.0).
 				 *
-				 * We pretend that the connection is OK for the duration of these
-				 * queries.
+				 * We pretend that the connection is OK for the duration of
+				 * these queries.
 				 */
 				conn->status = CONNECTION_OK;
 
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 0eb4132767..64468ab4da 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -347,7 +347,8 @@ struct pg_conn
 	char	   *pguser;			/* Postgres username and password, if any */
 	char	   *pgpass;
 	char	   *pgpassfile;		/* path to a file containing password(s) */
-	char	   *channel_binding;	/* channel binding mode (require,prefer) */
+	char	   *channel_binding;	/* channel binding mode
+									 * (require,prefer,disable) */
 	char	   *keepalives;		/* use TCP keepalives? */
 	char	   *keepalives_idle;	/* time between TCP keepalives */
 	char	   *keepalives_interval;	/* time between TCP keepalive
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index 3a3b0eb7e8..aae6de8b34 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -17,7 +17,7 @@ if ($windows_os)
 }
 else
 {
-	plan tests => 8;
+	plan tests => 10;
 }
 
 
@@ -86,3 +86,13 @@ test_role($node, 'md5_role',   'scram-sha-256', 2);
 reset_pg_hba($node, 'md5');
 test_role($node, 'scram_role', 'md5', 0);
 test_role($node, 'md5_role',   'md5', 0);
+
+# Tests for channel binding without SSL.
+# Using the password authentication method; channel binding can't work
+reset_pg_hba($node, 'password');
+$ENV{"PGCHANNELBINDING"} = 'require';
+test_role($node, 'scram_role', 'scram-sha-256', 2);
+# SSL not in use; channel binding still can't work
+reset_pg_hba($node, 'scram-sha-256');
+$ENV{"PGCHANNELBINDING"} = 'require';
+test_role($node, 'scram_role', 'scram-sha-256', 2);
diff --git a/src/test/authentication/t/002_saslprep.pl b/src/test/authentication/t/002_saslprep.pl
index 37f6d19c3a..c4b335c45f 100644
--- a/src/test/authentication/t/002_saslprep.pl
+++ b/src/test/authentication/t/002_saslprep.pl
@@ -14,7 +14,7 @@ if ($windows_os)
 }
 else
 {
-	plan tests => 14;
+	plan tests => 12;
 }
 
 # Delete pg_hba.conf from the given node, add a new entry to it
@@ -104,13 +104,3 @@ test_login($node, 'saslpreptest6_role', "foobar",     2);
 test_login($node, 'saslpreptest7_role', "foo\xd8\xa71bar", 0);
 test_login($node, 'saslpreptest7_role', "foo1\xd8\xa7bar", 2);
 test_login($node, 'saslpreptest7_role', "foobar",          2);
-
-# using the password authentication method; channel binding can't work
-reset_pg_hba($node, 'password');
-$ENV{"PGCHANNELBINDING"} = 'require';
-test_login($node, 'saslpreptest4a_role', "a", 2);
-
-# SSL not in use; channel binding still can't work
-reset_pg_hba($node, 'scram-sha-256');
-$ENV{"PGCHANNELBINDING"} = 'require';
-test_login($node, 'saslpreptest4a_role', "a", 2);
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index af508e74f5..71e5d771aa 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -18,7 +18,7 @@ if ($ENV{with_openssl} ne 'yes')
 	plan skip_all => 'SSL not supported by this build';
 }
 
-my $number_of_tests = 5;
+my $number_of_tests = 7;
 
 # This is the hostname used to connect to the server.
 my $SERVERHOSTADDR = '127.0.0.1';
@@ -42,22 +42,34 @@ $node->start;
 configure_test_server_for_ssl($node, $SERVERHOSTADDR, "scram-sha-256",
 	"pass", "scram-sha-256");
 switch_server_cert($node, 'server-cn-only');
-$ENV{PGUSER} = "ssltestuser";
 $ENV{PGPASSWORD} = "pass";
 $common_connstr =
   "dbname=trustdb sslmode=require sslcert=invalid sslrootcert=invalid hostaddr=$SERVERHOSTADDR";
 
 # Default settings
-test_connect_ok($common_connstr, '', "Basic SCRAM authentication with SSL");
+test_connect_ok($common_connstr, "user=ssltestuser",
+	"Basic SCRAM authentication with SSL");
 
 # Test channel_binding
-$ENV{PGCHANNELBINDING} = "disable";
-test_connect_ok($common_connstr, '', "SCRAM with SSL and channel_binding=disable");
-$ENV{PGCHANNELBINDING} = "require";
-test_connect_ok($common_connstr, '', "SCRAM with SSL and channel_binding=require");
+test_connect_fails(
+	$common_connstr,
+	"user=ssltestuser channel_binding=invalid_value",
+	qr/invalid channel_binding value: "invalid_value"/,
+	"SCRAM with SSL and channel_binding=invalid_value");
+test_connect_ok(
+	$common_connstr,
+	"user=ssltestuser channel_binding=disable",
+	"SCRAM with SSL and channel_binding=disable");
+test_connect_ok(
+	$common_connstr,
+	"user=ssltestuser channel_binding=require",
+	"SCRAM with SSL and channel_binding=require");
 
 # Now test when the user has an MD5-encrypted password; should fail
-$ENV{PGUSER} = "md5testuser";
-test_connect_fails($common_connstr, '', qr/Channel binding required but not supported by server's authentication request/, "MD5 with SSL and channel_binding=require");
+test_connect_fails(
+	$common_connstr,
+	"user=md5testuser channel_binding=require",
+	qr/Channel binding required but not supported by server's authentication request/,
+	"MD5 with SSL and channel_binding=require");
 
 done_testing($number_of_tests);
diff --git a/src/test/ssl/t/SSLServer.pm b/src/test/ssl/t/SSLServer.pm
index 9288b42811..005955a2ff 100644
--- a/src/test/ssl/t/SSLServer.pm
+++ b/src/test/ssl/t/SSLServer.pm
@@ -114,7 +114,7 @@ sub configure_test_server_for_ssl
 	{
 		$node->psql('postgres',
 			"SET password_encryption='$password_enc'; ALTER USER ssltestuser PASSWORD '$password';"
-		  );
+		);
 		# A special user that always has an md5-encrypted password
 		$node->psql('postgres',
 			"SET password_encryption='md5'; ALTER USER md5testuser PASSWORD '$password';"