On Mon, Mar 13, 2023 at 10:39 PM Michael Paquier <mich...@paquier.xyz> wrote:
> 0001 was looking fine enough seen from here, so applied it after
> tweaking a few comments.  That's enough to cover most of the needs of
> this thread.

Thank you very much!

> 0002 looks pretty simple as well, I think that's worth a look for this
> CF.

Cool. v17 just rebases the set over HEAD, then, for cfbot.

> I am not sure about 0003, to be honest, as I am wondering if
> there could be a better solution than tying more the mechanism names
> with the expected AUTH_REQ_* values..

Yeah, I'm not particularly excited about the approach I took. It'd be
easier if we had a second SASL method to verify the implementation...
I'd also proposed just adding an Assert, as a third option, to guide
the eventual SASL implementer back to this conversation?

--Jacob
From e2343c008916fe8dd9ecacfa71c60cc250fa208d Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Tue, 18 Oct 2022 16:55:36 -0700
Subject: [PATCH v17 2/2] require_auth: decouple SASL and SCRAM

Rather than assume that an AUTH_REQ_SASL* code refers to SCRAM-SHA-256,
future-proof by separating the single allowlist into a list of allowed
authentication request codes and a list of allowed SASL mechanisms.

The require_auth check is now separated into two tiers. The
AUTH_REQ_SASL* codes are always allowed. If the server sends one,
responsibility for the check then falls to pg_SASL_init(), which
compares the selected mechanism against the list of allowed mechanisms.
(Other SCRAM code is already responsible for rejecting unexpected or
out-of-order AUTH_REQ_SASL_* codes, so that's not explicitly handled
with this addition.)

Since there's only one recognized SASL mechanism, conn->sasl_mechs
currently only points at static hardcoded lists. Whenever a second
mechanism is added, the list will need to be managed dynamically.
---
 src/interfaces/libpq/fe-auth.c            | 34 +++++++++++++++++++
 src/interfaces/libpq/fe-connect.c         | 41 +++++++++++++++++++----
 src/interfaces/libpq/libpq-int.h          |  3 +-
 src/test/authentication/t/001_password.pl | 14 +++++---
 src/test/ssl/t/002_scram.pl               |  6 ++++
 5 files changed, 86 insertions(+), 12 deletions(-)

diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index b2497acdad..4ff49d8207 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -522,6 +522,40 @@ pg_SASL_init(PGconn *conn, int payloadlen)
 		goto error;
 	}
 
+	/*
+	 * Before going ahead with any SASL exchange, ensure that the user has
+	 * allowed (or, alternatively, has not forbidden) this particular mechanism.
+	 *
+	 * In a hypothetical future where a server responds with multiple SASL
+	 * mechanism families, we would need to instead consult this list up above,
+	 * during mechanism negotiation. We don't live in that world yet. The server
+	 * presents one auth method and we decide whether that's acceptable or not.
+	 */
+	if (conn->sasl_mechs)
+	{
+		const char **mech;
+		bool		found = false;
+
+		Assert(conn->require_auth);
+
+		for (mech = conn->sasl_mechs; *mech; mech++)
+		{
+			if (strcmp(*mech, selected_mechanism) == 0)
+			{
+				found = true;
+				break;
+			}
+		}
+
+		if ((conn->sasl_mechs_denied && found)
+			|| (!conn->sasl_mechs_denied && !found))
+		{
+			libpq_append_conn_error(conn, "auth method \"%s\" requirement failed: server requested unacceptable SASL mechanism \"%s\"",
+									conn->require_auth, selected_mechanism);
+			goto error;
+		}
+	}
+
 	if (conn->channel_binding[0] == 'r' &&	/* require */
 		strcmp(selected_mechanism, SCRAM_SHA_256_PLUS_NAME) != 0)
 	{
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index cbadb3f6af..a048793b46 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1258,12 +1258,25 @@ connectOptions2(PGconn *conn)
 					more;
 		bool		negated = false;
 
+		static const uint32 default_methods = (
+			1 << AUTH_REQ_SASL
+			| 1 << AUTH_REQ_SASL_CONT
+			| 1 << AUTH_REQ_SASL_FIN
+		);
+		static const char *no_mechs[] = { NULL };
+
 		/*
-		 * By default, start from an empty set of allowed options and add to
+		 * By default, start from a minimum set of allowed options and add to
 		 * it.
+		 *
+		 * NB: The SASL method codes are always "allowed" here. If the server
+		 * requests SASL auth, pg_SASL_init() will enforce adherence to the
+		 * sasl_mechs list, which by default is empty.
 		 */
 		conn->auth_required = true;
-		conn->allowed_auth_methods = 0;
+		conn->allowed_auth_methods = default_methods;
+		conn->sasl_mechs = no_mechs;
+		conn->sasl_mechs_denied = false;
 
 		for (first = true, more = true; more; first = false)
 		{
@@ -1290,6 +1303,9 @@ connectOptions2(PGconn *conn)
 					 */
 					conn->auth_required = false;
 					conn->allowed_auth_methods = -1;
+
+					/* conn->sasl_mechs is now a list of denied mechanisms. */
+					conn->sasl_mechs_denied = true;
 				}
 				else if (!negated)
 				{
@@ -1334,10 +1350,23 @@ connectOptions2(PGconn *conn)
 			}
 			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);
+				static const char *scram_mechs[] = {
+					SCRAM_SHA_256_NAME,
+					SCRAM_SHA_256_PLUS_NAME,
+					NULL /* list terminator */
+				};
+
+				/*
+				 * This currently assumes that SCRAM is the only SASL method.
+				 * Once a second mechanism is added, this code will need to add
+				 * to the list instead of replacing it wholesale.
+				 */
+				if (conn->sasl_mechs[0])
+					goto duplicate;
+				conn->sasl_mechs = scram_mechs;
+
+				free(part);
+				continue; /* avoid the bitmask manipulation below */
 			}
 			else if (strcmp(method, "creds") == 0)
 			{
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index f1f1d973cc..ab26292586 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -465,7 +465,8 @@ struct pg_conn
 										 * codes */
 	bool		client_finished_auth;	/* have we finished our half of the
 										 * authentication exchange? */
-
+	const char **sasl_mechs;	/* list of allowed/denied SASL mechanisms */
+	bool		sasl_mechs_denied;	/* is the sasl_mechs list forbidden? */
 
 	/* Transient state needed while establishing connection */
 	PGTargetServerType target_server_type;	/* desired session properties */
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index cba5d7d648..015532893c 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -301,30 +301,34 @@ $node->connect_fails(
 	"user=scram_role require_auth=password",
 	"password authentication required, fails with SCRAM auth",
 	expected_stderr =>
-	  qr/auth method "password" requirement failed: server requested SASL authentication/
+	  qr/server requested unacceptable SASL mechanism "SCRAM-SHA-256"/
 );
 $node->connect_fails(
 	"user=scram_role require_auth=md5",
 	"md5 authentication required, fails with SCRAM auth",
 	expected_stderr =>
-	  qr/auth method "md5" requirement failed: server requested SASL authentication/
+	  qr/server requested unacceptable SASL mechanism "SCRAM-SHA-256"/
 );
 $node->connect_fails(
 	"user=scram_role require_auth=none",
 	"all authentication forbidden, fails with SCRAM auth",
 	expected_stderr =>
-	  qr/auth method "none" requirement failed: server requested SASL authentication/
+	  qr/server requested unacceptable SASL mechanism "SCRAM-SHA-256"/
 );
 
 # Authentication fails if SCRAM authentication is forbidden.
 $node->connect_fails(
 	"user=scram_role require_auth=!scram-sha-256",
 	"SCRAM authentication forbidden, fails with SCRAM auth",
-	expected_stderr => qr/server requested SASL authentication/);
+	expected_stderr =>
+	  qr/server requested unacceptable SASL mechanism "SCRAM-SHA-256"/
+);
 $node->connect_fails(
 	"user=scram_role require_auth=!password,!md5,!scram-sha-256",
 	"multiple authentication types forbidden, fails with SCRAM auth",
-	expected_stderr => qr/server requested SASL authentication/);
+	expected_stderr =>
+	  qr/server requested unacceptable SASL mechanism "SCRAM-SHA-256"/
+);
 
 # Test that bad passwords are rejected.
 $ENV{"PGPASSWORD"} = 'badpass';
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 8038135697..173ac8d86b 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -157,6 +157,12 @@ if ($supports_tls_server_end_point)
 		"$common_connstr user=ssltestuser channel_binding=require require_auth=scram-sha-256",
 		"SCRAM with SSL, channel_binding=require, and require_auth=scram-sha-256"
 	);
+	$node->connect_fails(
+		"$common_connstr user=ssltestuser channel_binding=require require_auth=password",
+		"SCRAM with SSL, channel_binding=require, and require_auth=password",
+		expected_stderr =>
+		  qr/server requested unacceptable SASL mechanism "SCRAM-SHA-256-PLUS"/
+	);
 }
 else
 {
-- 
2.25.1

From 8d93ca3792bd886f7ba50f21f9f6d8836b557a2b Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Fri, 24 Jun 2022 15:40:42 -0700
Subject: [PATCH v17 1/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 +-
 doc/src/sgml/libpq.sgml                  | 64 ++++++++++++++++++++++++
 meson.build                              |  3 +-
 src/include/pg_config.h.in               |  3 ++
 src/interfaces/libpq/fe-auth.c           | 19 +++++++
 src/interfaces/libpq/fe-connect.c        | 51 +++++++++++++++++++
 src/interfaces/libpq/fe-secure-openssl.c | 39 ++++++++++++++-
 src/interfaces/libpq/libpq-int.h         |  3 ++
 src/test/ssl/t/001_ssltests.pl           | 43 ++++++++++++++++
 src/tools/msvc/Solution.pm               |  9 ++++
 11 files changed, 240 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index e35769ea73..92896d2d83 100755
--- a/configure
+++ b/configure
@@ -12973,13 +12973,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 af23c15cb2..34af40f621 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1373,8 +1373,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/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 3706d349ab..ac1c379f2a 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1820,6 +1820,60 @@ 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 client does not send a certificate and
+            the server successfully authenticates the client anyway.
+           </para>
+          </listitem>
+         </varlistentry>
+        </variablelist>
+       </para>
+
+       <note>
+        <para>
+         <literal>sslcertmode=require</literal> doesn't add any additional
+         security, since there is no guarantee that the server is validating the
+         certificate correctly; PostgreSQL servers generally request TLS
+         certificates from clients whether they validate them or not. The option
+         may be useful when troubleshooting more complicated TLS setups.
+        </para>
+       </note>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="libpq-connect-sslrootcert" xreflabel="sslrootcert">
       <term><literal>sslrootcert</literal></term>
       <listitem>
@@ -7996,6 +8050,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/meson.build b/meson.build
index 2ebdf914c1..6a5e76af94 100644
--- a/meson.build
+++ b/meson.build
@@ -1219,8 +1219,9 @@ if sslopt in ['auto', 'openssl']
       ['CRYPTO_new_ex_data', {'required': true}],
       ['SSL_new', {'required': true}],
 
-      # Function introduced in OpenSSL 1.0.2.
+      # Functions introduced in OpenSSL 1.0.2. LibreSSL doesn't have all of these.
       ['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
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 20c82f5979..c5a6762fc1 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -394,6 +394,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 a3b80dc550..b2497acdad 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -862,6 +862,25 @@ check_expected_areq(AuthRequest areq, PGconn *conn)
 	StaticAssertDecl((sizeof(conn->allowed_auth_methods) * CHAR_BIT) > AUTH_REQ_MAX,
 					 "AUTH_REQ_MAX overflows the allowed_auth_methods bitmask");
 
+	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)
+		{
+			libpq_append_conn_error(conn, "server did not request a certificate");
+			return false;
+		}
+		else if (!conn->ssl_cert_sent)
+		{
+			libpq_append_conn_error(conn, "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
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index dd4b98e099..cbadb3f6af 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -125,8 +125,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"
@@ -283,6 +285,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)},
@@ -1510,6 +1516,51 @@ connectOptions2(PGconn *conn)
 		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;
+			libpq_append_conn_error(conn, "invalid %s value: \"%s\"",
+									"sslcertmode", conn->sslcertmode);
+			return false;
+		}
+#ifndef USE_SSL
+		if (strcmp(conn->sslcertmode, "require") == 0)
+		{
+			conn->status = CONNECTION_BAD;
+			libpq_append_conn_error(conn, "sslcertmode value \"%s\" invalid when SSL support is not compiled in",
+									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;
+			libpq_append_conn_error(conn, "sslcertmode value \"%s\" is not supported (check OpenSSL version)",
+									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 6a4431ddfe..b88d9da3e2 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -462,6 +462,33 @@ 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 use 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
@@ -953,6 +980,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);
 
@@ -1107,7 +1139,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 1dc264fe54..f1f1d973cc 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -384,6 +384,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 */
@@ -527,6 +528,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 3094e27af3..4617f06f86 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -42,6 +42,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;
 
@@ -191,6 +195,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
@@ -538,6 +558,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 5eaea6355e..1ec1bac552 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -327,6 +327,7 @@ sub GenerateFiles
 		HAVE_SETPROCTITLE_FAST                   => 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,
@@ -507,6 +508,14 @@ sub GenerateFiles
 			$define{HAVE_HMAC_CTX_NEW}          = 1;
 			$define{HAVE_OPENSSL_INIT_SSL}      = 1;
 		}
+
+		# Symbols needed with OpenSSL 1.0.2 and above.
+		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;
+		}
 	}
 
 	$self->GenerateConfigHeader('src/include/pg_config.h',     \%define, 1);
-- 
2.25.1

Reply via email to