From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 2 Oct 2019 20:26:45 +0000 (+0200)
Subject: Accept SCRAM channel binding enabled clients
X-Git-Tag: pgbouncer_1_12_0~6
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2fbb0925161ef9dc566fdf3ad266d6bebef6023a;p=pgbouncer

Accept SCRAM channel binding enabled clients

Add support to the SCRAM exchange for clients that support channel
binding, such as PostgreSQL version 11 and beyond.  If such a client
encounters a PgBouncer server that does not support channel binding,
it will send a channel binding flag 'y', meaning the client supports
channel binding but thinks the server does not.  But PgBouncer
erroneously did not accept that flag.  This would cause connections to
fail if a PostgreSQL version 11 client connects to a PgBouncer with
SCRAM authentication over SSL.

adapted from PostgreSQL 218b024a7ec866ec62abb5c2fb4eb9108bb5fc0f

Reported-by: Andrew Dunstan
---

diff --git a/include/bouncer.h b/include/bouncer.h
index 5f8fd27..45795e0 100644
--- a/include/bouncer.h
+++ b/include/bouncer.h
@@ -396,6 +396,7 @@ struct PgSocket {
 		char *server_nonce;
 		char *server_first_message;
 		uint8_t	*SaltedPassword;
+		char cbind_flag;
 		int iterations;
 		char *salt;	/* base64-encoded */
 		uint8_t StoredKey[32];	/* SHA256_DIGEST_LENGTH */
diff --git a/include/scram.h b/include/scram.h
index c45ebec..b52c83a 100644
--- a/include/scram.h
+++ b/include/scram.h
@@ -57,6 +57,7 @@ bool verify_server_signature(ScramState *scram_state, const char *ServerSignatur
  */
 
 bool read_client_first_message(PgSocket *client, char *input,
+			       char *cbind_flag_p,
 			       char **client_first_message_bare_p,
 			       char **client_nonce_p);
 
diff --git a/src/client.c b/src/client.c
index 236b924..acaa9a1 100644
--- a/src/client.c
+++ b/src/client.c
@@ -532,6 +532,7 @@ static bool scram_client_first(PgSocket *client, uint32_t datalen, const uint8_t
 	input = ibuf;
 	slog_debug(client, "SCRAM client-first-message = \"%s\"", input);
 	if (!read_client_first_message(client, input,
+				       &client->scram_state.cbind_flag,
 				       &client->scram_state.client_first_message_bare,
 				       &client->scram_state.client_nonce))
 		goto failed;
diff --git a/src/scram.c b/src/scram.c
index 10df2f9..d4e0abf 100644
--- a/src/scram.c
+++ b/src/scram.c
@@ -560,12 +560,14 @@ bool verify_server_signature(ScramState *scram_state, const char *ServerSignatur
  */
 
 bool read_client_first_message(PgSocket *client, char *input,
+			       char *cbind_flag_p,
 			       char **client_first_message_bare_p,
 			       char **client_nonce_p)
 {
 	char *client_first_message_bare = NULL;
 	char *client_nonce = NULL;
 
+	*cbind_flag_p = *input;
 	switch (*input) {
 	case 'n':
 		/* Client does not support channel binding */
@@ -657,12 +659,15 @@ bool read_client_final_message(PgSocket *client, const uint8_t *raw_input, char
 	/*
 	 * Read channel-binding.  We don't support channel binding, so
 	 * it's expected to always be "biws", which is "n,,",
-	 * base64-encoded.
+	 * base64-encoded, or "eSws", which is "y,,".  We also have to
+	 * check whether the flag is the same one that the client
+	 * originally sent.
 	 */
 	channel_binding = read_attr_value(client, &input, 'c');
 	if (channel_binding == NULL)
 		goto failed;
-	if (strcmp(channel_binding, "biws") != 0) {
+	if (!(strcmp(channel_binding, "biws") == 0 && client->scram_state.cbind_flag == 'n') &&
+	    !(strcmp(channel_binding, "eSws") == 0 && client->scram_state.cbind_flag == 'y')) {
 		slog_error(client, "unexpected SCRAM channel-binding attribute in client-final-message");
 		goto failed;
 	}
diff --git a/test/ssl/test.sh b/test/ssl/test.sh
index 2f444b6..0cddf88 100755
--- a/test/ssl/test.sh
+++ b/test/ssl/test.sh
@@ -34,6 +34,21 @@ pgctl() {
 
 ulimit -c unlimited
 
+# System configuration checks
+SED_ERE_OP='-E'
+case `uname` in
+Linux)
+	SED_ERE_OP='-r'
+	;;
+esac
+
+pg_majorversion=$(initdb --version | sed -n $SED_ERE_OP 's/.* ([0-9]+).*/\1/p')
+if test $pg_majorversion -ge 10; then
+	pg_supports_scram=true
+else
+	pg_supports_scram=false
+fi
+
 stopit() {
 	local pid
 	if test -f "$1"; then
@@ -139,6 +154,9 @@ runtest() {
 	status=$?
 	if [ $status -eq 0 ]; then
 		echo "ok"
+	elif [ $status -eq 77 ]; then
+		echo "skipped"
+		status=0
 	else
 		echo "FAILED"
 		cat $LOGDIR/$1.log | sed 's/^/# /'
@@ -159,7 +177,7 @@ psql_pg() {
 }
 
 psql_bouncer() {
-	PGUSER=bouncer psql -X "$@"
+	PGUSER=bouncer PGPASSWORD=zzz psql -X "$@"
 }
 
 # server_lifetime
@@ -242,12 +260,27 @@ test_client_ssl_auth() {
 	return $rc
 }
 
+test_client_ssl_scram() {
+	$pg_supports_scram || return 77
+
+	reconf_bouncer "auth_type = scram-sha-256" "server_tls_sslmode = prefer" \
+		"client_tls_sslmode = require" \
+		"client_tls_key_file = TestCA1/sites/01-localhost.key" \
+		"client_tls_cert_file = TestCA1/sites/01-localhost.crt"
+	reconf_pgsql "ssl=on" "ssl_ca_file='root.crt'"
+	psql_bouncer -q -d "dbname=p0 sslmode=verify-full sslrootcert=TestCA1/ca.crt" -c "select 'client-ssl-connect'" | tee tmp/test.tmp 2>&1
+	grep -q "client-ssl-connect"  tmp/test.tmp
+	rc=$?
+	return $rc
+}
+
 testlist="
 test_server_ssl
 test_server_ssl_verify
 test_server_ssl_pg_auth
 test_client_ssl
 test_client_ssl_auth
+test_client_ssl_scram
 "
 if [ $# -gt 0 ]; then
 	testlist="$*"