[prev in list] [next in list] [prev in thread] [next in thread] 

List:       postgresql-hackers
Subject:    Re: [PoC] Let libpq reject unexpected authentication requests
From:       Jacob Champion <jchampion () timescale ! com>
Date:       2022-06-30 23:26:54
Message-ID: CAAWbhmiKUFtkzF5_UFrJNssyTh2QEB7qF70K8zC9KWG0jMxmGw () mail ! gmail ! com
[Download RAW message or body]

On Wed, Jun 29, 2022 at 6:36 AM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
> It's not strictly related to your patch, but maybe this hint has
> outlived its usefulness?  I mean, we don't list all available tables
> when you try to reference a table that doesn't exist.  And unordered on
> top of that.

Yeah, maybe it'd be better to tell the user the correct context for an
otherwise-valid option ("the 'sslcert' option may only be applied to
USER MAPPING"), and avoid the option dump entirely?

--

v7, attached, fixes configuration on Windows.

--Jacob

["since-v6.diff.txt" (text/plain)]

commit 326912a24350d10e509bf911b4452d48708021ab
Author: Jacob Champion <jchampion@timescale.com>
Date:   Thu Jun 30 14:22:02 2022 -0700

    squash! Add sslcertmode option for client certificates
    
    Fix Windows configuration.

diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index d30e8fcb11..d7e7a897b2 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -364,6 +364,7 @@ sub GenerateFiles
 		HAVE_SHM_OPEN                            => undef,
 		HAVE_SOCKLEN_T                           => 1,
 		HAVE_SPINLOCKS                           => 1,
+		HAVE_SSL_CTX_SET_CERT_CB                 => undef,
 		HAVE_STDBOOL_H                           => 1,
 		HAVE_STDINT_H                            => 1,
 		HAVE_STDLIB_H                            => 1,
@@ -553,6 +554,13 @@ sub GenerateFiles
 
 		my ($digit1, $digit2, $digit3) = $self->GetOpenSSLVersion();
 
+		if (   ($digit1 >= '3' && $digit2 >= '0' && $digit3 >= '0')
+			|| ($digit1 >= '1' && $digit2 >= '1' && $digit3 >= '0')
+			|| ($digit1 >= '1' && $digit2 >= '0' && $digit3 >= '2'))
+		{
+			$define{HAVE_SSL_CTX_SET_CERT_CB} = 1;
+		}
+
 		# More symbols are needed with OpenSSL 1.1.0 and above.
 		if (   ($digit1 >= '3' && $digit2 >= '0' && $digit3 >= '0')
 			|| ($digit1 >= '1' && $digit2 >= '1' && $digit3 >= '0'))

["v7-0002-Add-sslcertmode-option-for-client-certificates.patch" (text/x-patch)]

From e16a23b9cd2a2600f45636ca399d6239329f23c8 Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchampion@timescale.com>
Date: Fri, 24 Jun 2022 15:40:42 -0700
Subject: [PATCH v7 2/2] Add sslcertmode option for client certificates

The sslcertmode option controls whether the server is allowed and/or
required to request a certificate from the client. There are three
modes:

- "allow" is the default and follows the current behavior -- a
  configured  sslcert is sent if the server requests one (which, with
  the current implementation, will happen whenever TLS is negotiated).

- "disable" causes the client to refuse to send a client certificate
  even if an sslcert is configured.

- "require" causes the client to fail if a client certificate is never
  sent and the server opens a connection anyway. This doesn't add any
  additional security, since there is no guarantee that the server is
  validating the certificate correctly, but it may help troubleshoot
  more complicated TLS setups.

sslcertmode=require needs the OpenSSL implementation to support
SSL_CTX_set_cert_cb(). Notably, LibreSSL does not.
---
 configure                                     | 11 ++--
 configure.ac                                  |  4 +-
 .../postgres_fdw/expected/postgres_fdw.out    |  2 +-
 doc/src/sgml/libpq.sgml                       | 54 ++++++++++++++++++
 src/include/pg_config.h.in                    |  3 +
 src/interfaces/libpq/fe-auth.c                | 21 +++++++
 src/interfaces/libpq/fe-connect.c             | 55 +++++++++++++++++++
 src/interfaces/libpq/fe-secure-openssl.c      | 40 +++++++++++++-
 src/interfaces/libpq/libpq-int.h              |  3 +
 src/test/ssl/t/001_ssltests.pl                | 43 +++++++++++++++
 src/tools/msvc/Solution.pm                    |  8 +++
 11 files changed, 235 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index fb07cd27d9..9d5db2879f 100755
--- a/configure
+++ b/configure
@@ -13044,13 +13044,14 @@ else
 fi
 
   fi
-  # Function introduced in OpenSSL 1.0.2.
-  for ac_func in X509_get_signature_nid
+  # Functions introduced in OpenSSL 1.0.2. LibreSSL doesn't have all of these.
+  for ac_func in X509_get_signature_nid SSL_CTX_set_cert_cb
 do :
-  ac_fn_c_check_func "$LINENO" "X509_get_signature_nid" \
                "ac_cv_func_X509_get_signature_nid"
-if test "x$ac_cv_func_X509_get_signature_nid" = xyes; then :
+  as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
+ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
+if eval test \"x\$"$as_ac_var"\" = x"yes"; then :
   cat >>confdefs.h <<_ACEOF
-#define HAVE_X509_GET_SIGNATURE_NID 1
+#define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1
 _ACEOF
 
 fi
diff --git a/configure.ac b/configure.ac
index 6c6f997ee3..8110bcfc38 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1325,8 +1325,8 @@ if test "$with_ssl" = openssl ; then
      AC_SEARCH_LIBS(CRYPTO_new_ex_data, [eay32 crypto], [], [AC_MSG_ERROR([library \
                'eay32' or 'crypto' is required for OpenSSL])])
      AC_SEARCH_LIBS(SSL_new, [ssleay32 ssl], [], [AC_MSG_ERROR([library 'ssleay32' \
or 'ssl' is required for OpenSSL])])  fi
-  # Function introduced in OpenSSL 1.0.2.
-  AC_CHECK_FUNCS([X509_get_signature_nid])
+  # Functions introduced in OpenSSL 1.0.2. LibreSSL doesn't have all of these.
+  AC_CHECK_FUNCS([X509_get_signature_nid SSL_CTX_set_cert_cb])
   # Functions introduced in OpenSSL 1.1.0. We used to check for
   # OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
   # defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out \
b/contrib/postgres_fdw/expected/postgres_fdw.out index 493931e003..47df10119e 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -9529,7 +9529,7 @@ DO $d$
     END;
 $d$;
 ERROR:  invalid option "password"
-HINT:  Valid options in this context are: service, passfile, channel_binding, \
connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, \
keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, \
sslcompression, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, sslsni, requirepeer, \
require_auth, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, \
krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, \
fdw_tuple_cost, extensions, updatable, truncatable, fetch_size, batch_size, \
async_capable, parallel_commit, keep_connections +HINT:  Valid options in this \
context are: service, passfile, channel_binding, connect_timeout, dbname, host, \
hostaddr, port, options, application_name, keepalives, keepalives_idle, \
keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcompression, \
sslcert, sslkey, sslcertmode, sslrootcert, sslcrl, sslcrldir, sslsni, requirepeer, \
require_auth, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, \
krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, \
fdw_tuple_cost, extensions, updatable, truncatable, fetch_size, batch_size, \
async_capable, parallel_commit, keep_connections  CONTEXT:  SQL statement "ALTER \
SERVER loopback_nopw OPTIONS (ADD password 'dummypw')"  PL/pgSQL function \
                inline_code_block line 3 at EXECUTE
 -- If we add a password for our user mapping instead, we should get a different
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index cc831158b7..b4c5dedccb 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1820,6 +1820,50 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       </listitem>
      </varlistentry>
 
+     <varlistentry id="libpq-connect-sslcertmode" xreflabel="sslcertmode">
+      <term><literal>sslcertmode</literal></term>
+      <listitem>
+       <para>
+        This option determines whether a client certificate may be sent to the
+        server, and whether the server is required to request one. There are
+        three modes:
+
+        <variablelist>
+         <varlistentry>
+          <term><literal>disable</literal></term>
+          <listitem>
+           <para>
+            a client certificate is never sent, even if one is provided via
+            <xref linkend="libpq-connect-sslcert" />
+           </para>
+          </listitem>
+         </varlistentry>
+
+         <varlistentry>
+          <term><literal>allow</literal> (default)</term>
+          <listitem>
+           <para>
+            a certificate may be sent, if the server requests one and it has
+            been provided via <literal>sslcert</literal>
+           </para>
+          </listitem>
+         </varlistentry>
+
+         <varlistentry>
+          <term><literal>require</literal></term>
+          <listitem>
+           <para>
+            the server <emphasis>must</emphasis> request a certificate. The
+            connection will fail if the server authenticates the client despite
+            not requesting or receiving one.
+           </para>
+          </listitem>
+         </varlistentry>
+        </variablelist>
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="libpq-connect-sslrootcert" xreflabel="sslrootcert">
       <term><literal>sslrootcert</literal></term>
       <listitem>
@@ -7973,6 +8017,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
      </para>
     </listitem>
 
+    <listitem>
+     <para>
+      <indexterm>
+       <primary><envar>PGSSLCERTMODE</envar></primary>
+      </indexterm>
+      <envar>PGSSLCERTMODE</envar> behaves the same as the <xref
+      linkend="libpq-connect-sslcertmode"/> connection parameter.
+     </para>
+    </listitem>
+
     <listitem>
      <para>
       <indexterm>
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index cdd742cb55..0ecf40c6b3 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -514,6 +514,9 @@
 /* Define to 1 if you have spinlocks. */
 #undef HAVE_SPINLOCKS
 
+/* Define to 1 if you have the `SSL_CTX_set_cert_cb' function. */
+#undef HAVE_SSL_CTX_SET_CERT_CB
+
 /* Define to 1 if stdbool.h conforms to C99. */
 #undef HAVE_STDBOOL_H
 
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index f789bc7ec3..406440a201 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -891,6 +891,27 @@ check_expected_areq(AuthRequest areq, PGconn *conn)
 	bool		result = true;
 	char	   *reason = NULL;
 
+	if (conn->sslcertmode[0] == 'r' /* require */
+		&& areq == AUTH_REQ_OK)
+	{
+		/*
+		 * Trade off a little bit of complexity to try to get these error
+		 * messages as precise as possible.
+		 */
+		if (!conn->ssl_cert_requested)
+		{
+			appendPQExpBufferStr(&conn->errorMessage,
+								 libpq_gettext("server did not request a certificate"));
+			return false;
+		}
+		else if (!conn->ssl_cert_sent)
+		{
+			appendPQExpBufferStr(&conn->errorMessage,
+								 libpq_gettext("server accepted connection without a valid certificate"));
+			return false;
+		}
+	}
+
 	/*
 	 * If the user required a specific auth method, or specified an allowed set,
 	 * then reject all others here, and make sure the server actually completes
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 7d5bf337f6..92c5516abc 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -128,8 +128,10 @@ static int	ldapServiceLookup(const char *purl, PQconninfoOption \
*options,  #define DefaultTargetSessionAttrs	"any"
 #ifdef USE_SSL
 #define DefaultSSLMode "prefer"
+#define DefaultSSLCertMode "allow"
 #else
 #define DefaultSSLMode	"disable"
+#define DefaultSSLCertMode "disable"
 #endif
 #ifdef ENABLE_GSS
 #include "fe-gssapi-common.h"
@@ -286,6 +288,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 		"SSL-Client-Key", "", 64,
 	offsetof(struct pg_conn, sslkey)},
 
+	{"sslcertmode", "PGSSLCERTMODE", NULL, NULL,
+		"SSL-Client-Cert-Mode", "", 8, /* sizeof("disable") == 8 */
+	offsetof(struct pg_conn, sslcertmode)},
+
 	{"sslpassword", NULL, NULL, NULL,
 		"SSL-Client-Key-Password", "*", 20,
 	offsetof(struct pg_conn, sslpassword)},
@@ -1529,6 +1535,55 @@ duplicate:
 		return false;
 	}
 
+	/*
+	 * validate sslcertmode option
+	 */
+	if (conn->sslcertmode)
+	{
+		if (strcmp(conn->sslcertmode, "disable") != 0 &&
+			strcmp(conn->sslcertmode, "allow") != 0 &&
+			strcmp(conn->sslcertmode, "require") != 0)
+		{
+			conn->status = CONNECTION_BAD;
+			appendPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("invalid %s value: \"%s\"\n"),
+							  "sslcertmode",
+							  conn->sslcertmode);
+			return false;
+		}
+#ifndef USE_SSL
+		if (strcmp(conn->sslcertmode, "require") == 0)
+		{
+			conn->status = CONNECTION_BAD;
+			appendPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("sslcertmode value \"%s\" invalid when SSL support is not \
compiled in\n"), +							  conn->sslcertmode);
+			return false;
+		}
+#endif
+#ifndef HAVE_SSL_CTX_SET_CERT_CB
+		/*
+		 * Without a certificate callback, the current implementation can't
+		 * figure out if a certficate was actually requested, so "require" is
+		 * useless.
+		 */
+		if (strcmp(conn->sslcertmode, "require") == 0)
+		{
+			conn->status = CONNECTION_BAD;
+			appendPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("sslcertmode value \"%s\" is not supported (check OpenSSL \
version)\n"), +							  conn->sslcertmode);
+			return false;
+		}
+#endif
+	}
+	else
+	{
+		conn->sslcertmode = strdup(DefaultSSLCertMode);
+		if (!conn->sslcertmode)
+			goto oom_error;
+	}
+
 	/*
 	 * validate gssencmode option
 	 */
diff --git a/src/interfaces/libpq/fe-secure-openssl.c \
b/src/interfaces/libpq/fe-secure-openssl.c index fc91cae7a2..13f6a28605 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -477,6 +477,34 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
 	return ok;
 }
 
+#ifdef HAVE_SSL_CTX_SET_CERT_CB
+/*
+ * Certificate selection callback
+ *
+ * This callback lets us choose the client certificate we send to the server
+ * after seeing its CertificateRequest. We only support sending a single
+ * hard-coded certificate via sslcert, so we don't actually set any certificates
+ * here; we just it to record whether or not the server has actually asked for
+ * one and whether we have one to send.
+ */
+static int
+cert_cb(SSL *ssl, void *arg)
+{
+	PGconn *conn = arg;
+	conn->ssl_cert_requested = true;
+
+	/* Do we have a certificate loaded to send back? */
+	if (SSL_get_certificate(ssl))
+		conn->ssl_cert_sent = true;
+
+	/*
+	 * Tell OpenSSL that the callback succeeded; we're not required to actually
+	 * make any changes to the SSL handle.
+	 */
+	return 1;
+}
+#endif
+
 /*
  * OpenSSL-specific wrapper around
  * pq_verify_peer_name_matches_certificate_name(), converting the ASN1_STRING
@@ -971,6 +999,11 @@ initialize_SSL(PGconn *conn)
 		SSL_CTX_set_default_passwd_cb_userdata(SSL_context, conn);
 	}
 
+#ifdef HAVE_SSL_CTX_SET_CERT_CB
+	/* Set up a certificate selection callback. */
+	SSL_CTX_set_cert_cb(SSL_context, cert_cb, conn);
+#endif
+
 	/* Disable old protocol versions */
 	SSL_CTX_set_options(SSL_context, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
 
@@ -1134,7 +1167,12 @@ initialize_SSL(PGconn *conn)
 	else
 		fnbuf[0] = '\0';
 
-	if (fnbuf[0] == '\0')
+	if (conn->sslcertmode[0] == 'd') /* disable */
+	{
+		/* don't send a client cert even if we have one */
+		have_cert = false;
+	}
+	else if (fnbuf[0] == '\0')
 	{
 		/* no home directory, proceed without a client cert */
 		have_cert = false;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 3278196eea..7c5989457a 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -381,6 +381,7 @@ struct pg_conn
 	char	   *sslkey;			/* client key filename */
 	char	   *sslcert;		/* client certificate filename */
 	char	   *sslpassword;	/* client key file password */
+	char	   *sslcertmode;	/* client cert mode (require,allow,disable) */
 	char	   *sslrootcert;	/* root certificate filename */
 	char	   *sslcrl;			/* certificate revocation list filename */
 	char	   *sslcrldir;		/* certificate revocation list directory name */
@@ -522,6 +523,8 @@ struct pg_conn
 
 	/* SSL structures */
 	bool		ssl_in_use;
+	bool		ssl_cert_requested;	/* Did the server ask us for a cert? */
+	bool		ssl_cert_sent;		/* Did we send one in reply? */
 
 #ifdef USE_SSL
 	bool		allow_ssl_try;	/* Allowed to try SSL negotiation */
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 707f4005af..357ac08110 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -38,6 +38,10 @@ my $SERVERHOSTADDR = '127.0.0.1';
 # This is the pattern to use in pg_hba.conf to match incoming connections.
 my $SERVERHOSTCIDR = '127.0.0.1/32';
 
+# Determine whether build supports sslcertmode=require.
+my $supports_sslcertmode_require =
+  check_pg_config("#define HAVE_SSL_CTX_SET_CERT_CB 1");
+
 # Allocation of base connection string shared among multiple tests.
 my $common_connstr;
 
@@ -187,6 +191,22 @@ $node->connect_ok(
 	"$common_connstr sslrootcert=ssl/both-cas-2.crt sslmode=verify-ca",
 	"cert root file that contains two certificates, order 2");
 
+# sslcertmode=allow and =disable should both work without a client certificate.
+$node->connect_ok(
+	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require \
sslcertmode=disable", +	"connect with sslcertmode=disable");
+$node->connect_ok(
+	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require \
sslcertmode=allow", +	"connect with sslcertmode=allow");
+
+# sslcertmode=require, however, should fail.
+$node->connect_fails(
+	"$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require \
sslcertmode=require", +	"connect with sslcertmode=require fails without a client \
certificate", +	expected_stderr => $supports_sslcertmode_require
+		? qr/server accepted connection without a valid certificate/
+		: qr/sslcertmode value "require" is not supported/);
+
 # CRL tests
 
 # Invalid CRL filename is the same as no CRL, succeeds
@@ -534,6 +554,29 @@ $node->connect_ok(
 	"certificate authorization succeeds with correct client cert in encrypted DER \
format"  );
 
+# correct client cert with required/allowed certificate authentication
+if ($supports_sslcertmode_require)
+{
+	$node->connect_ok(
+		"$common_connstr user=ssltestuser sslcertmode=require sslcert=ssl/client.crt "
+		  . sslkey('client.key'),
+		"certificate authorization succeeds with sslcertmode=require"
+	);
+}
+$node->connect_ok(
+	"$common_connstr user=ssltestuser sslcertmode=allow sslcert=ssl/client.crt "
+	  . sslkey('client.key'),
+	"certificate authorization succeeds with sslcertmode=allow"
+);
+
+# client cert isn't sent if certificate authentication is disabled
+$node->connect_fails(
+	"$common_connstr user=ssltestuser sslcertmode=disable sslcert=ssl/client.crt "
+	  . sslkey('client.key'),
+	"certificate authorization fails with sslcertmode=disable",
+	expected_stderr => qr/connection requires a valid client certificate/
+);
+
 # correct client cert in encrypted PEM with wrong password
 $node->connect_fails(
 	"$common_connstr user=ssltestuser sslcert=ssl/client.crt "
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index d30e8fcb11..d7e7a897b2 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -364,6 +364,7 @@ sub GenerateFiles
 		HAVE_SHM_OPEN                            => undef,
 		HAVE_SOCKLEN_T                           => 1,
 		HAVE_SPINLOCKS                           => 1,
+		HAVE_SSL_CTX_SET_CERT_CB                 => undef,
 		HAVE_STDBOOL_H                           => 1,
 		HAVE_STDINT_H                            => 1,
 		HAVE_STDLIB_H                            => 1,
@@ -553,6 +554,13 @@ sub GenerateFiles
 
 		my ($digit1, $digit2, $digit3) = $self->GetOpenSSLVersion();
 
+		if (   ($digit1 >= '3' && $digit2 >= '0' && $digit3 >= '0')
+			|| ($digit1 >= '1' && $digit2 >= '1' && $digit3 >= '0')
+			|| ($digit1 >= '1' && $digit2 >= '0' && $digit3 >= '2'))
+		{
+			$define{HAVE_SSL_CTX_SET_CERT_CB} = 1;
+		}
+
 		# More symbols are needed with OpenSSL 1.1.0 and above.
 		if (   ($digit1 >= '3' && $digit2 >= '0' && $digit3 >= '0')
 			|| ($digit1 >= '1' && $digit2 >= '1' && $digit3 >= '0'))
-- 
2.25.1


["v7-0001-libpq-let-client-reject-unexpected-auth-methods.patch" (text/x-patch)]

From 423d7f5d7628b7f26921d04afe9f768d35676e25 Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchampion@vmware.com>
Date: Mon, 28 Feb 2022 09:40:43 -0800
Subject: [PATCH v7 1/2] libpq: let client reject unexpected auth methods

The require_auth connection option allows the client to choose a list of
acceptable authentication types for use with the server. There is no
negotiation: if the server does not present one of the allowed
authentication requests, the connection fails. Additionally, all methods
in the list may be negated, e.g. '!password', in which case the server
must NOT use the listed authentication type. The special method "none"
allows/disallows the use of unauthenticated connections (but it does not
govern transport-level authentication via TLS or GSSAPI).

Internally, the patch expands the role of check_expected_areq() to
ensure that the incoming request is compatible with conn->require_auth.
It also introduces a new flag, conn->client_finished_auth, which is set
by various authentication routines when the client side of the handshake
is finished. This signals to check_expected_areq() that an OK message
from the server is expected, and allows the client to complain if the
server forgoes authentication entirely.

(Since the client can't generally prove that the server is actually
doing the work of authentication, this last part is mostly useful for
SCRAM without channel binding. It could also provide a client with a
decent signal that, at the very least, it's not connecting to a database
with trust auth, and so the connection can be tied to the client in a
later audit.)

Deficiencies:
- This feature currently doesn't prevent unexpected certificate exchange
  or unexpected GSSAPI encryption.
- This is unlikely to be very forwards-compatible at the moment,
  especially with SASL/SCRAM.
- SSPI support is "implemented" but untested.
- require_auth=none,scram-sha-256 currently allows the server to leave a
  SCRAM exchange unfinished. This is not net-new behavior but may be
  surprising.
---
 .../postgres_fdw/expected/postgres_fdw.out    |   2 +-
 doc/src/sgml/libpq.sgml                       | 105 +++++++++++
 src/include/libpq/pqcomm.h                    |   1 +
 src/interfaces/libpq/fe-auth-scram.c          |   1 +
 src/interfaces/libpq/fe-auth.c                | 138 +++++++++++++++
 src/interfaces/libpq/fe-connect.c             | 164 ++++++++++++++++++
 src/interfaces/libpq/fe-secure-openssl.c      |   1 -
 src/interfaces/libpq/libpq-int.h              |   7 +
 src/test/authentication/t/001_password.pl     | 149 ++++++++++++++++
 src/test/kerberos/t/001_auth.pl               |  26 +++
 src/test/ldap/t/001_auth.pl                   |   6 +
 src/test/ssl/t/002_scram.pl                   |  25 +++
 12 files changed, 623 insertions(+), 2 deletions(-)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out \
b/contrib/postgres_fdw/expected/postgres_fdw.out index 44457f930c..493931e003 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -9529,7 +9529,7 @@ DO $d$
     END;
 $d$;
 ERROR:  invalid option "password"
-HINT:  Valid options in this context are: service, passfile, channel_binding, \
connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, \
keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, \
sslcompression, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, sslsni, requirepeer, \
ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, \
target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, \
extensions, updatable, truncatable, fetch_size, batch_size, async_capable, \
parallel_commit, keep_connections +HINT:  Valid options in this context are: service, \
passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, \
application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, \
tcp_user_timeout, sslmode, sslcompression, sslcert, sslkey, sslrootcert, sslcrl, \
sslcrldir, sslsni, requirepeer, require_auth, ssl_min_protocol_version, \
ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, \
use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, \
truncatable, fetch_size, batch_size, async_capable, parallel_commit, keep_connections \
CONTEXT:  SQL statement "ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')" \
                PL/pgSQL function inline_code_block line 3 at EXECUTE
 -- If we add a password for our user mapping instead, we should get a different
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 37ec3cb4e5..cc831158b7 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1221,6 +1221,101 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       </listitem>
      </varlistentry>
 
+     <varlistentry id="libpq-connect-require-auth" xreflabel="require_auth">
+      <term><literal>require_auth</literal></term>
+      <listitem>
+      <para>
+        Specifies the authentication method that the client requires from the
+        server. If the server does not use the required method to authenticate
+        the client, or if the authentication handshake is not fully completed by
+        the server, the connection will fail. A comma-separated list of methods
+        may also be provided, of which the server must use exactly one in order
+        for the connection to succeed. By default, any authentication method is
+        accepted, and the server is free to skip authentication altogether.
+      </para>
+      <para>
+        Methods may be negated with the addition of a <literal>!</literal>
+        prefix, in which case the server must <emphasis>not</emphasis> attempt
+        the listed method; any other method is accepted, and the server is free
+        not to authenticate the client at all. If a comma-separated list is
+        provided, the server may not attempt <emphasis>any</emphasis> of the
+        listed negated methods. Negated and non-negated forms may not be
+        combined in the same setting.
+      </para>
+      <para>
+        As a final special case, the <literal>none</literal> method requires the
+        server not to use an authentication challenge. (It may also be negated,
+        to require some form of authentication.)
+      </para>
+      <para>
+        The following methods may be specified:
+
+        <variablelist>
+         <varlistentry>
+          <term><literal>password</literal></term>
+          <listitem>
+           <para>
+            The server must request plaintext password authentication.
+           </para>
+          </listitem>
+         </varlistentry>
+
+         <varlistentry>
+          <term><literal>md5</literal></term>
+          <listitem>
+           <para>
+            The server must request MD5 hashed password authentication.
+           </para>
+          </listitem>
+         </varlistentry>
+
+         <varlistentry>
+          <term><literal>gss</literal></term>
+          <listitem>
+           <para>
+            The server must either request a Kerberos handshake via
+            <acronym>GSSAPI</acronym> or establish a
+            <acronym>GSS</acronym>-encrypted channel (see also
+            <xref linkend="libpq-connect-gssencmode" />).
+           </para>
+          </listitem>
+         </varlistentry>
+
+         <varlistentry>
+          <term><literal>sspi</literal></term>
+          <listitem>
+           <para>
+            The server must request Windows <acronym>SSPI</acronym>
+            authentication.
+           </para>
+          </listitem>
+         </varlistentry>
+
+         <varlistentry>
+          <term><literal>scram-sha-256</literal></term>
+          <listitem>
+           <para>
+            The server must successfully complete a SCRAM-SHA-256 authentication
+            exchange with the client.
+           </para>
+          </listitem>
+         </varlistentry>
+
+         <varlistentry>
+          <term><literal>none</literal></term>
+          <listitem>
+           <para>
+            The server must not prompt the client for an authentication
+            exchange. (This does not prohibit client certificate authentication
+			via TLS, nor GSS authentication via its encrypted transport.)
+           </para>
+          </listitem>
+         </varlistentry>
+        </variablelist>
+      </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="libpq-connect-channel-binding" xreflabel="channel_binding">
       <term><literal>channel_binding</literal></term>
       <listitem>
@@ -7761,6 +7856,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
      </para>
     </listitem>
 
+    <listitem>
+     <para>
+      <indexterm>
+       <primary><envar>PGREQUIREAUTH</envar></primary>
+      </indexterm>
+      <envar>PGREQUIREAUTH</envar> behaves the same as the <xref
+      linkend="libpq-connect-require-auth"/> connection parameter.
+     </para>
+    </listitem>
+
     <listitem>
      <para>
       <indexterm>
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index b418283d5f..a47f7b2d91 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -161,6 +161,7 @@ extern PGDLLIMPORT bool Db_user_namespace;
 #define AUTH_REQ_SASL	   10	/* Begin SASL authentication */
 #define AUTH_REQ_SASL_CONT 11	/* Continue SASL authentication */
 #define AUTH_REQ_SASL_FIN  12	/* Final SASL message */
+#define AUTH_REQ_MAX	   AUTH_REQ_SASL_FIN	/* maximum AUTH_REQ_* value */
 
 typedef uint32 AuthRequest;
 
diff --git a/src/interfaces/libpq/fe-auth-scram.c \
b/src/interfaces/libpq/fe-auth-scram.c index e616200704..d918e8b87f 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -289,6 +289,7 @@ scram_exchange(void *opaq, char *input, int inputlen,
 			}
 			*done = true;
 			state->state = FE_SCRAM_FINISHED;
+			state->conn->client_finished_auth = true;
 			break;
 
 		default:
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 0a072a36dc..f789bc7ec3 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -138,7 +138,10 @@ pg_GSS_continue(PGconn *conn, int payloadlen)
 	}
 
 	if (maj_stat == GSS_S_COMPLETE)
+	{
+		conn->client_finished_auth = true;
 		gss_release_name(&lmin_s, &conn->gtarg_nam);
+	}
 
 	return STATUS_OK;
 }
@@ -328,6 +331,9 @@ pg_SSPI_continue(PGconn *conn, int payloadlen)
 		FreeContextBuffer(outbuf.pBuffers[0].pvBuffer);
 	}
 
+	if (r == SEC_E_OK)
+		conn->client_finished_auth = true;
+
 	/* Cleanup is handled by the code in freePGconn() */
 	return STATUS_OK;
 }
@@ -836,6 +842,44 @@ pg_password_sendauth(PGconn *conn, const char *password, \
AuthRequest areq)  return ret;
 }
 
+/*
+ * Translate an AuthRequest into a human-readable description.
+ */
+static const char *
+auth_description(AuthRequest areq)
+{
+	switch (areq)
+	{
+		case AUTH_REQ_PASSWORD:
+			return libpq_gettext("a cleartext password");
+		case AUTH_REQ_MD5:
+			return libpq_gettext("a hashed password");
+		case AUTH_REQ_GSS:
+		case AUTH_REQ_GSS_CONT:
+			return libpq_gettext("GSSAPI authentication");
+		case AUTH_REQ_SSPI:
+			return libpq_gettext("SSPI authentication");
+		case AUTH_REQ_SCM_CREDS:
+			return libpq_gettext("UNIX socket credentials");
+		case AUTH_REQ_SASL:
+		case AUTH_REQ_SASL_CONT:
+		case AUTH_REQ_SASL_FIN:
+			return libpq_gettext("SASL authentication");
+	}
+
+	return libpq_gettext("an unknown authentication type");
+}
+
+/*
+ * Convenience macro for checking the allowed_auth_methods bitmask. Caller must
+ * ensure that type is not greater than 31 (high bit of the bitmask).
+ */
+#define auth_allowed(conn, type) \
+	(((conn)->allowed_auth_methods & (1 << (type))) != 0)
+
+StaticAssertDecl(AUTH_REQ_MAX < CHAR_BIT * \
sizeof(((PGconn){0}).allowed_auth_methods), +				 "AUTH_REQ_MAX overflows the \
allowed_auth_methods bitmask"); +
 /*
  * Verify that the authentication request is expected, given the connection
  * parameters. This is especially important when the client wishes to
@@ -845,6 +889,97 @@ static bool
 check_expected_areq(AuthRequest areq, PGconn *conn)
 {
 	bool		result = true;
+	char	   *reason = NULL;
+
+	/*
+	 * If the user required a specific auth method, or specified an allowed set,
+	 * then reject all others here, and make sure the server actually completes
+	 * an authentication exchange.
+	 */
+	if (conn->require_auth)
+	{
+		switch (areq)
+		{
+			case AUTH_REQ_OK:
+				/*
+				 * Check to make sure we've actually finished our exchange (or
+				 * else that the user has allowed an authentication-less
+				 * connection).
+				 *
+				 * TODO: how should !auth_required interact with an incomplete
+				 * SCRAM exchange?
+				 */
+				if (!conn->auth_required || conn->client_finished_auth)
+					break;
+
+				/*
+				 * No explicit authentication request was made by the server --
+				 * or perhaps it was made and not completed, in the case of
+				 * SCRAM -- but there is one special case to check. If the user
+				 * allowed "gss", then a GSS-encrypted channel also satisfies
+				 * the check.
+				 */
+#ifdef ENABLE_GSS
+				if (auth_allowed(conn, AUTH_REQ_GSS) && conn->gssenc)
+				{
+					/*
+					 * If implicit GSS auth has already been performed via GSS
+					 * encryption, we don't need to have performed an
+					 * AUTH_REQ_GSS exchange.
+					 *
+					 * TODO: check this assumption. What mutual auth guarantees
+					 * are made in this case?
+					 */
+				}
+				else
+#endif
+				{
+					reason = libpq_gettext("server did not complete authentication"),
+					result = false;
+				}
+
+				break;
+
+			case AUTH_REQ_PASSWORD:
+			case AUTH_REQ_MD5:
+			case AUTH_REQ_GSS:
+			case AUTH_REQ_SSPI:
+			case AUTH_REQ_GSS_CONT:
+			case AUTH_REQ_SASL:
+			case AUTH_REQ_SASL_CONT:
+			case AUTH_REQ_SASL_FIN:
+				/*
+				 * We don't handle these with the default case, to avoid
+				 * bit-shifting past the end of the allowed_auth_methods mask if
+				 * the server sends an unexpected AuthRequest.
+				 */
+				result = auth_allowed(conn, areq);
+				break;
+
+			default:
+				result = false;
+				break;
+		}
+	}
+
+	if (!result)
+	{
+		if (reason)
+		{
+			appendPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("auth method \"%s\" requirement failed: %s\n"),
+							  conn->require_auth, reason);
+		}
+		else
+		{
+			appendPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("auth method \"%s\" required, but server requested %s\n"),
+							  conn->require_auth,
+							  auth_description(areq));
+		}
+
+		return result;
+	}
 
 	/*
 	 * When channel_binding=require, we must protect against two cases: (1) we
@@ -1046,6 +1181,9 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
 										 "fe_sendauth: error sending password authentication\n");
 					return STATUS_ERROR;
 				}
+
+				/* We expect no further authentication requests. */
+				conn->client_finished_auth = true;
 				break;
 			}
 
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 6e936bbff3..7d5bf337f6 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -310,6 +310,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 		"Require-Peer", "", 10,
 	offsetof(struct pg_conn, requirepeer)},
 
+	{"require_auth", "PGREQUIREAUTH", NULL, NULL,
+		"Require-Auth", "", 14, /* sizeof("scram-sha-256") == 14 */
+	offsetof(struct pg_conn, require_auth)},
+
 	{"ssl_min_protocol_version", "PGSSLMINPROTOCOLVERSION", "TLSv1.2", NULL,
 		"SSL-Minimum-Protocol-Version", "", 8,	/* sizeof("TLSv1.x") == 8 */
 	offsetof(struct pg_conn, ssl_min_protocol_version)},
@@ -1255,6 +1259,166 @@ connectOptions2(PGconn *conn)
 		}
 	}
 
+	/*
+	 * parse and validate require_auth option
+	 */
+	if (conn->require_auth)
+	{
+		char	   *s = conn->require_auth;
+		bool		first, more;
+		bool		negated = false;
+
+		/*
+		 * By default, start from an empty set of allowed options and add to it.
+		 */
+		conn->auth_required = true;
+		conn->allowed_auth_methods = 0;
+
+		for (first = true, more = true; more; first = false)
+		{
+			char	   *method, *part;
+			uint32		bits;
+
+			part = parse_comma_separated_list(&s, &more);
+			if (part == NULL)
+				goto oom_error;
+
+			/*
+			 * Check for negation, e.g. '!password'. If one element is negated,
+			 * they all have to be.
+			 */
+			method = part;
+			if (*method == '!')
+			{
+				if (first)
+				{
+					/*
+					 * Switch to a permissive set of allowed options, and
+					 * subtract from it.
+					 */
+					conn->auth_required = false;
+					conn->allowed_auth_methods = -1;
+				}
+				else if (!negated)
+				{
+					conn->status = CONNECTION_BAD;
+					appendPQExpBuffer(&conn->errorMessage,
+									  libpq_gettext("negative require_auth method \"%s\" cannot be mixed with \
non-negative methods"), +									  method);
+
+					free(part);
+					return false;
+				}
+
+				negated = true;
+				method++;
+			}
+			else if (negated)
+			{
+				conn->status = CONNECTION_BAD;
+				appendPQExpBuffer(&conn->errorMessage,
+								  libpq_gettext("require_auth method \"%s\" cannot be mixed with negative \
methods"), +								  method);
+
+				free(part);
+				return false;
+			}
+
+			if (strcmp(method, "password") == 0)
+			{
+				bits = (1 << AUTH_REQ_PASSWORD);
+			}
+			else if (strcmp(method, "md5") == 0)
+			{
+				bits = (1 << AUTH_REQ_MD5);
+			}
+			else if (strcmp(method, "gss") == 0)
+			{
+				bits = (1 << AUTH_REQ_GSS);
+				bits |= (1 << AUTH_REQ_GSS_CONT);
+			}
+			else if (strcmp(method, "sspi") == 0)
+			{
+				bits = (1 << AUTH_REQ_SSPI);
+				bits |= (1 << AUTH_REQ_GSS_CONT);
+			}
+			else if (strcmp(method, "scram-sha-256") == 0)
+			{
+				/* This currently assumes that SCRAM is the only SASL method. */
+				bits = (1 << AUTH_REQ_SASL);
+				bits |= (1 << AUTH_REQ_SASL_CONT);
+				bits |= (1 << AUTH_REQ_SASL_FIN);
+			}
+			else if (strcmp(method, "none") == 0)
+			{
+				/*
+				 * Special case: let the user explicitly allow (or disallow)
+				 * connections where the server does not send an explicit
+				 * authentication challenge, such as "trust" and "cert" auth.
+				 */
+				if (negated) /* "!none" */
+				{
+					if (conn->auth_required)
+						goto duplicate;
+
+					conn->auth_required = true;
+				}
+				else /* "none" */
+				{
+					if (!conn->auth_required)
+						goto duplicate;
+
+					conn->auth_required = false;
+				}
+
+				free(part);
+				continue; /* avoid the bitmask manipulation below */
+			}
+			else
+			{
+				conn->status = CONNECTION_BAD;
+				appendPQExpBuffer(&conn->errorMessage,
+								  libpq_gettext("invalid require_auth method: \"%s\""),
+								  method);
+
+				free(part);
+				return false;
+			}
+
+			/* Update the bitmask. */
+			if (negated)
+			{
+				if ((conn->allowed_auth_methods & bits) == 0)
+					goto duplicate;
+
+				conn->allowed_auth_methods &= ~bits;
+			}
+			else
+			{
+				if ((conn->allowed_auth_methods & bits) == bits)
+					goto duplicate;
+
+				conn->allowed_auth_methods |= bits;
+			}
+
+			free(part);
+			continue;
+
+duplicate:
+			/*
+			 * A duplicated method probably indicates a typo in a setting where
+			 * typos are extremely risky.
+			 */
+			conn->status = CONNECTION_BAD;
+			appendPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("require_auth method \"%s\" is specified more than once"),
+							  part);
+
+			free(part);
+			return false;
+		}
+	}
+
 	/*
 	 * validate channel_binding option
 	 */
diff --git a/src/interfaces/libpq/fe-secure-openssl.c \
b/src/interfaces/libpq/fe-secure-openssl.c index 8117cbd40f..fc91cae7a2 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -477,7 +477,6 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
 	return ok;
 }
 
-
 /*
  * OpenSSL-specific wrapper around
  * pq_verify_peer_name_matches_certificate_name(), converting the ASN1_STRING
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 3db6a17db4..3278196eea 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -393,6 +393,7 @@ struct pg_conn
 	char	   *ssl_min_protocol_version;	/* minimum TLS protocol version */
 	char	   *ssl_max_protocol_version;	/* maximum TLS protocol version */
 	char	   *target_session_attrs;	/* desired session properties */
+	char	   *require_auth;	/* name of the expected auth method */
 
 	/* Optional file to write trace info to */
 	FILE	   *Pfdebug;
@@ -454,6 +455,9 @@ struct pg_conn
 	bool		write_failed;	/* have we had a write failure on sock? */
 	char	   *write_err_msg;	/* write error message, or NULL if OOM */
 
+	bool		auth_required;	/* require an authentication challenge from the server? */
+	uint32		allowed_auth_methods;	/* bitmask of acceptable AuthRequest codes */
+
 	/* Transient state needed while establishing connection */
 	PGTargetServerType target_server_type;	/* desired session properties */
 	bool		try_next_addr;	/* time to advance to next address/host? */
@@ -509,6 +513,9 @@ struct pg_conn
 	bool		error_result;	/* do we need to make an ERROR result? */
 	PGresult   *next_result;	/* next result (used in single-row mode) */
 
+	bool		client_finished_auth; /* have we finished our half of the
+									   * authentication exchange? */
+
 	/* Assorted state for SASL, SSL, GSS, etc */
 	const pg_fe_sasl_mech *sasl;
 	void	   *sasl_state;
diff --git a/src/test/authentication/t/001_password.pl \
b/src/test/authentication/t/001_password.pl index 3e3079c824..473a93d6db 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -82,6 +82,74 @@ test_role($node, 'scram_role', 'trust', 0,
 test_role($node, 'md5_role', 'trust', 0,
 	log_unlike => [qr/connection authenticated:/]);
 
+# All positive require_auth options should fail...
+$node->connect_fails("user=scram_role require_auth=gss",
+	"GSS authentication can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+$node->connect_fails("user=scram_role require_auth=sspi",
+	"SSPI authentication can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+$node->connect_fails("user=scram_role require_auth=password",
+	"password authentication can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+$node->connect_fails("user=scram_role require_auth=md5",
+	"md5 authentication can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+$node->connect_fails("user=scram_role require_auth=scram-sha-256",
+	"SCRAM authentication can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+$node->connect_fails("user=scram_role require_auth=password,scram-sha-256",
+	"multiple authentication types can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+
+# ...and negative require_auth options should succeed.
+$node->connect_ok("user=scram_role require_auth=!gss",
+	"GSS authentication can be forbidden: succeeds with trust auth");
+$node->connect_ok("user=scram_role require_auth=!sspi",
+	"SSPI authentication can be forbidden: succeeds with trust auth");
+$node->connect_ok("user=scram_role require_auth=!password",
+	"password authentication can be forbidden: succeeds with trust auth");
+$node->connect_ok("user=scram_role require_auth=!md5",
+	"md5 authentication can be forbidden: succeeds with trust auth");
+$node->connect_ok("user=scram_role require_auth=!scram-sha-256",
+	"SCRAM authentication can be forbidden: succeeds with trust auth");
+$node->connect_ok("user=scram_role require_auth=!password,!scram-sha-256",
+	"multiple authentication types can be forbidden: succeeds with trust auth");
+
+# require_auth=[!]none should interact correctly with trust auth.
+$node->connect_ok("user=scram_role require_auth=none",
+	"all authentication can be forbidden: succeeds with trust auth");
+$node->connect_fails("user=scram_role require_auth=!none",
+	"any authentication can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+
+# Negative and positive require_auth options can't be mixed.
+$node->connect_fails("user=scram_role require_auth=scram-sha-256,!md5",
+	"negative require_auth methods can't be mixed with positive",
+	expected_stderr => qr/negative require_auth method "!md5" cannot be mixed with \
non-negative methods/); +$node->connect_fails("user=scram_role \
require_auth=!password,scram-sha-256", +	"positive require_auth methods can't be \
mixed with negative", +	expected_stderr => qr/require_auth method "scram-sha-256" \
cannot be mixed with negative methods/); +
+# require_auth methods can't be duplicated.
+$node->connect_fails("user=scram_role require_auth=password,md5,password",
+	"require_auth methods can't be duplicated: positive case",
+	expected_stderr => qr/require_auth method "password" is specified more than once/);
+$node->connect_fails("user=scram_role require_auth=!password,!md5,!password",
+	"require_auth methods can't be duplicated: negative case",
+	expected_stderr => qr/require_auth method "!password" is specified more than \
once/); +$node->connect_fails("user=scram_role require_auth=none,md5,none",
+	"require_auth methods can't be duplicated: none case",
+	expected_stderr => qr/require_auth method "none" is specified more than once/);
+$node->connect_fails("user=scram_role require_auth=!none,!md5,!none",
+	"require_auth methods can't be duplicated: !none case",
+	expected_stderr => qr/require_auth method "!none" is specified more than once/);
+
+# Unknown require_auth methods are caught.
+$node->connect_fails("user=scram_role require_auth=none,abcdefg",
+	"unknown require_auth methods are rejected",
+	expected_stderr => qr/invalid require_auth method: "abcdefg"/);
+
 # For plain "password" method, all users should also be able to connect.
 reset_pg_hba($node, 'password');
 test_role($node, 'scram_role', 'password', 0,
@@ -91,6 +159,33 @@ test_role($node, 'md5_role', 'password', 0,
 	log_like =>
 	  [qr/connection authenticated: identity="md5_role" method=password/]);
 
+# require_auth should succeed with a plaintext password...
+$node->connect_ok("user=scram_role require_auth=password",
+	"password authentication can be required: works with password auth");
+$node->connect_ok("user=scram_role require_auth=!none",
+	"any authentication can be required: works with password auth");
+$node->connect_ok("user=scram_role require_auth=scram-sha-256,password,md5",
+	"multiple authentication types can be required: works with password auth");
+
+# ...fail for other auth types...
+$node->connect_fails("user=scram_role require_auth=md5",
+	"md5 authentication can be required: fails with password auth",
+	expected_stderr => qr/server requested a cleartext password/);
+$node->connect_fails("user=scram_role require_auth=scram-sha-256",
+	"SCRAM authentication can be required: fails with password auth",
+	expected_stderr => qr/server requested a cleartext password/);
+$node->connect_fails("user=scram_role require_auth=none",
+	"all authentication can be forbidden: fails with password auth",
+	expected_stderr => qr/server requested a cleartext password/);
+
+# ...and allow password authentication to be prohibited.
+$node->connect_fails("user=scram_role require_auth=!password",
+	"password authentication can be forbidden: fails with password auth",
+	expected_stderr => qr/server requested a cleartext password/);
+$node->connect_fails("user=scram_role require_auth=!password,!md5,!scram-sha-256",
+	"multiple authentication types can be forbidden: fails with password auth",
+	expected_stderr => qr/server requested a cleartext password/);
+
 # For "scram-sha-256" method, user "scram_role" should be able to connect.
 reset_pg_hba($node, 'scram-sha-256');
 test_role(
@@ -104,6 +199,33 @@ test_role(
 test_role($node, 'md5_role', 'scram-sha-256', 2,
 	log_unlike => [qr/connection authenticated:/]);
 
+# require_auth should succeed with SCRAM...
+$node->connect_ok("user=scram_role require_auth=scram-sha-256",
+	"SCRAM authentication can be required: works with SCRAM auth");
+$node->connect_ok("user=scram_role require_auth=!none",
+	"any authentication can be required: works with SCRAM auth");
+$node->connect_ok("user=scram_role require_auth=password,scram-sha-256,md5",
+	"multiple authentication types can be required: works with SCRAM auth");
+
+# ...fail for other auth types...
+$node->connect_fails("user=scram_role require_auth=password",
+	"password authentication can be required: fails with SCRAM auth",
+	expected_stderr => qr/server requested SASL authentication/);
+$node->connect_fails("user=scram_role require_auth=md5",
+	"md5 authentication can be required: fails with SCRAM auth",
+	expected_stderr => qr/server requested SASL authentication/);
+$node->connect_fails("user=scram_role require_auth=none",
+	"all authentication can be forbidden: fails with SCRAM auth",
+	expected_stderr => qr/server requested SASL authentication/);
+
+# ...and allow SCRAM authentication to be prohibited.
+$node->connect_fails("user=scram_role require_auth=!scram-sha-256",
+	"SCRAM authentication can be forbidden: fails with SCRAM auth",
+	expected_stderr => qr/server requested SASL authentication/);
+$node->connect_fails("user=scram_role require_auth=!password,!md5,!scram-sha-256",
+	"multiple authentication types can be forbidden: fails with SCRAM auth",
+	expected_stderr => qr/server requested SASL authentication/);
+
 # Test that bad passwords are rejected.
 $ENV{"PGPASSWORD"} = 'badpass';
 test_role($node, 'scram_role', 'scram-sha-256', 2,
@@ -120,6 +242,33 @@ test_role($node, 'md5_role', 'md5', 0,
 	log_like =>
 	  [qr/connection authenticated: identity="md5_role" method=md5/]);
 
+# require_auth should succeed with MD5...
+$node->connect_ok("user=md5_role require_auth=md5",
+	"MD5 authentication can be required: works with MD5 auth");
+$node->connect_ok("user=md5_role require_auth=!none",
+	"any authentication can be required: works with MD5 auth");
+$node->connect_ok("user=md5_role require_auth=md5,scram-sha-256,password",
+	"multiple authentication types can be required: works with MD5 auth");
+
+# ...fail for other auth types...
+$node->connect_fails("user=md5_role require_auth=password",
+	"password authentication can be required: fails with MD5 auth",
+	expected_stderr => qr/server requested a hashed password/);
+$node->connect_fails("user=md5_role require_auth=scram-sha-256",
+	"SCRAM authentication can be required: fails with MD5 auth",
+	expected_stderr => qr/server requested a hashed password/);
+$node->connect_fails("user=md5_role require_auth=none",
+	"all authentication can be forbidden: fails with MD5 auth",
+	expected_stderr => qr/server requested a hashed password/);
+
+# ...and allow MD5 authentication to be prohibited.
+$node->connect_fails("user=md5_role require_auth=!md5",
+	"password authentication can be forbidden: fails with MD5 auth",
+	expected_stderr => qr/server requested a hashed password/);
+$node->connect_fails("user=md5_role require_auth=!password,!md5,!scram-sha-256",
+	"multiple authentication types can be forbidden: fails with MD5 auth",
+	expected_stderr => qr/server requested a hashed password/);
+
 # Tests for channel binding without SSL.
 # Using the password authentication method; channel binding can't work
 reset_pg_hba($node, 'password');
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index 62e0542639..de4ddcbf69 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -307,6 +307,24 @@ test_query(
 	'gssencmode=require',
 	'sending 100K lines works');
 
+# require_auth=gss should succeed...
+$node->connect_ok(
+	$node->connstr('postgres') . " user=test1 host=$host hostaddr=$hostaddr \
gssencmode=disable require_auth=gss", +	"GSS authentication can be requested: works \
with GSS auth without encryption"); +$node->connect_ok(
+	$node->connstr('postgres') . " user=test1 host=$host hostaddr=$hostaddr \
gssencmode=require require_auth=gss", +	"GSS authentication can be requested: works \
with GSS auth with encryption"); +
+# ...and require_auth=sspi should fail.
+$node->connect_fails(
+	$node->connstr('postgres') . " user=test1 host=$host hostaddr=$hostaddr \
gssencmode=disable require_auth=sspi", +	"SSPI authentication can be requested: fails \
with GSS auth without encryption", +	expected_stderr => qr/server requested GSSAPI \
authentication/); +$node->connect_fails(
+	$node->connstr('postgres') . " user=test1 host=$host hostaddr=$hostaddr \
gssencmode=require require_auth=sspi", +	"SSPI authentication can be requested: fails \
with GSS auth with encryption", +	expected_stderr => qr/server did not complete \
authentication/); +
 unlink($node->data_dir . '/pg_hba.conf');
 $node->append_conf('pg_hba.conf',
 	qq{hostgssenc all all $hostaddr/32 gss map=mymap});
@@ -335,6 +353,14 @@ test_access(
 test_access($node, 'test1', 'SELECT true', 2, 'gssencmode=disable',
 	'fails with GSS encryption disabled and hostgssenc hba');
 
+# require_auth=gss should succeed.
+$node->connect_ok(
+	$node->connstr('postgres') . " user=test1 host=$host hostaddr=$hostaddr \
gssencmode=require require_auth=gss", +	"GSS authentication can be requested: works \
with GSS encryption"); +$node->connect_ok(
+	$node->connstr('postgres') . " user=test1 host=$host hostaddr=$hostaddr \
gssencmode=require require_auth=gss,scram-sha-256", +	"multiple authentication types \
can be requested: works with GSS encryption"); +
 unlink($node->data_dir . '/pg_hba.conf');
 $node->append_conf('pg_hba.conf',
 	qq{hostnogssenc all all $hostaddr/32 gss map=mymap});
diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl
index 86dff8bd1f..e7c67920a1 100644
--- a/src/test/ldap/t/001_auth.pl
+++ b/src/test/ldap/t/001_auth.pl
@@ -216,6 +216,12 @@ test_access(
 		qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/
 	],);
 
+# require_auth=password should complete successfully; other methods should fail.
+$node->connect_ok("user=test1 require_auth=password",
+	"password authentication can be required: works with ldap auth");
+$node->connect_fails("user=test1 require_auth=scram-sha-256",
+	"SCRAM authentication can be required: fails with ldap auth");
+
 note "search+bind";
 
 unlink($node->data_dir . '/pg_hba.conf');
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 588f47a39b..9957a96e69 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -132,4 +132,29 @@ $node->connect_ok(
 		qr/connection authenticated: identity="ssltestuser" method=scram-sha-256/
 	]);
 
+# channel_binding should continue to function independently of require_auth.
+$node->connect_ok("$common_connstr user=ssltestuser channel_binding=disable \
require_auth=scram-sha-256", +	"SCRAM with SSL, channel_binding=disable, and \
require_auth=scram-sha-256"); +$node->connect_fails(
+	"$common_connstr user=md5testuser require_auth=md5 channel_binding=require",
+	"channel_binding can fail even when require_auth succeeds",
+	expected_stderr =>
+	  qr/channel binding required but not supported by server's authentication request/
+);
+if ($supports_tls_server_end_point)
+{
+	$node->connect_ok(
+		"$common_connstr user=ssltestuser channel_binding=require \
require_auth=scram-sha-256", +		"SCRAM with SSL, channel_binding=require, and \
require_auth=scram-sha-256"); +}
+else
+{
+	$node->connect_fails(
+		"$common_connstr user=ssltestuser channel_binding=require \
require_auth=scram-sha-256", +		"SCRAM with SSL, channel_binding=require, and \
require_auth=scram-sha-256", +		expected_stderr =>
+		  qr/channel binding is required, but server did not offer an authentication \
method that supports channel binding/ +	);
+}
+
 done_testing();
-- 
2.25.1



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic