From 4dc30c3af69ced946b2ceab8c6b8e003d3583a00 Mon Sep 17 00:00:00 2001
From: David Fetter <david@fetter.org>
Date: Thu, 2 Aug 2018 23:07:29 -0700
Subject: [PATCH] clientcert_verify_full v06
To: pgsql-hackers@postgresql.org

---
 doc/src/sgml/client-auth.sgml  | 15 +++++++---
 doc/src/sgml/runtime.sgml      | 51 +++++++++++++++++++++++++++-------
 src/backend/libpq/auth.c       | 39 ++++++++++++++++++++++++--
 src/backend/libpq/hba.c        | 28 ++++++++++++++-----
 src/include/libpq/hba.h        |  9 +++++-
 src/test/ssl/ServerSetup.pm    | 10 ++++++-
 src/test/ssl/t/001_ssltests.pl | 21 ++++++++++++++
 7 files changed, 147 insertions(+), 26 deletions(-)
 mode change 100644 => 100755 src/test/ssl/t/001_ssltests.pl

diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index c2114021c3..6b261aea92 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -563,10 +563,17 @@ hostnossl  <replaceable>database</replaceable>  <replaceable>user</replaceable>
       <para>
        In addition to the method-specific options listed below, there is one
        method-independent authentication option <literal>clientcert</literal>, which
-       can be specified in any <literal>hostssl</literal> record.  When set
-       to <literal>1</literal>, this option requires the client to present a valid
-       (trusted) SSL certificate, in addition to the other requirements of the
-       authentication method.
+       can be specified in any <literal>hostssl</literal> record.
+       This option can be set to <literal>verify-ca</literal> or
+       <literal>verify-full</literal>. Both options require the client
+       to present a valid (trusted) SSL certificate, while
+       <literal>verify-full</literal> additionally enforces that the
+       <literal>CN</literal> (Common Name) in the certificate matches
+       the username or an applicable mapping.
+       This behavior is similar to the cert authentication method
+       (see <xref linkend="auth-cert"/> ) but enables pairing
+       the verification of client certificates with any authentication
+       method that supports <literal>hostssl</literal> entries.
       </para>
      </listitem>
     </varlistentry>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 06c0ee7de2..ff6493d70e 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -2287,13 +2287,25 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
    (<acronym>CA</acronym>s) you trust in a file in the data
    directory, set the parameter <xref linkend="guc-ssl-ca-file"/> in
    <filename>postgresql.conf</filename> to the new file name, and add the
-   authentication option <literal>clientcert=1</literal> to the appropriate
+   authentication option <literal>clientcert=verify-ca</literal> or
+   <literal>clientcert=verify-full</literal> to the appropriate
    <literal>hostssl</literal> line(s) in <filename>pg_hba.conf</filename>.
    A certificate will then be requested from the client during SSL
    connection startup.  (See <xref linkend="libpq-ssl"/> for a description
-   of how to set up certificates on the client.)  The server will
-   verify that the client's certificate is signed by one of the trusted
-   certificate authorities.
+   of how to set up certificates on the client.)
+  </para>
+
+  <para>
+   For a <literal>hostssl</literal> entry with 
+   <literal>clientcert=verify-ca</literal>, the server will verify
+   that the client's certificate is signed by one of the trusted
+   certificate authorities. If <literal>clientcert=verify-full</literal>
+   is specified, the server will not only verify the certificate
+   chain, but it will also check whether the username or its mapping
+   matches the <literal>CN</literal> (Common Name) of the provided certificate.
+   Note that certificate chain validation is always ensured when the
+   <literal>cert</literal> authentication method is used
+   (see <xref linkend="auth-cert"/>).
   </para>
 
   <para>
@@ -2317,14 +2329,33 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
    client certificates against its CA file, if one is configured &mdash; but
    it will not insist that a client certificate be presented.
   </para>
+  </sect2>
+
+  <sect2 id="ssl-enforcing-client-certificates">
+   <title>Enforcing Client Certificates</title>
+  <para>
+   There are two approaches to enforce that users provide a certificate during login.
+  </para>
+
+  <para>
+   The first approach makes use of the <literal>cert</literal> authentication
+   method for <literal>hostssl</literal> entries in <filename>pg_hba.conf</filename>,
+   such that the certificate itself is used for authentication while also
+   providing ssl connection security. See <xref linkend="auth-cert"/> for details.
+   (It is not necessary to specify any <literal>clientcert</literal> options 
+   explicitly when using the <literal>cert</literal> authentication method.)
+   In this case, the <literal>CN</literal> (Common Name) provided in
+   the certificate is checked against the user name or an applicable mapping.
+  </para>
 
   <para>
-   If you are setting up client certificates, you may wish to use
-   the <literal>cert</literal> authentication method, so that the certificates
-   control user authentication as well as providing connection security.
-   See <xref linkend="auth-cert"/> for details.  (It is not necessary to
-   specify <literal>clientcert=1</literal> explicitly when using
-   the <literal>cert</literal> authentication method.)
+   The second approach combines any authentication method for <literal>hostssl</literal>
+   entries with the verification of client certificates by setting the
+   <literal>clientcert</literal> authentication option to <literal>verify-ca</literal>
+   or <literal>verify-full</literal>. The former option only enforces that
+   the certificate is valid, while the latter also ensures that the
+   <literal>CN</literal> (Common Name) in the certificate matches
+   the user name or an applicable mapping.
   </para>
   </sect2>
 
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 63f37902e6..0244a21f38 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -347,6 +347,7 @@ void
 ClientAuthentication(Port *port)
 {
 	int			status = STATUS_ERROR;
+	int			status_verify_cert_full = STATUS_ERROR;
 	char	   *logdetail = NULL;
 
 	/*
@@ -364,7 +365,7 @@ ClientAuthentication(Port *port)
 	 * current connection, so perform any verifications based on the hba
 	 * options field that should be done *before* the authentication here.
 	 */
-	if (port->hba->clientcert)
+	if (port->hba->clientcert != clientCertOff)
 	{
 		/* If we haven't loaded a root certificate store, fail */
 		if (!secure_loaded_verify_locations())
@@ -600,10 +601,27 @@ ClientAuthentication(Port *port)
 			break;
 	}
 
+	if(port->hba->clientcert == clientCertFull && port->hba->auth_method!=uaCert)
+	{
+		/**
+		 * Make sure we only check the Certificate if it hasn't been checked
+		 * already due to the use of the uaCert authentication method.
+		 */
+#ifdef USE_SSL
+			status_verify_cert_full = CheckCertAuth(port);
+#else
+			Assert(false);
+#endif
+	}
+	else
+	{
+		status_verify_cert_full = STATUS_OK;
+	}
+
 	if (ClientAuthentication_hook)
 		(*ClientAuthentication_hook) (port, status);
 
-	if (status == STATUS_OK)
+	if (status == STATUS_OK && status_verify_cert_full == STATUS_OK)
 		sendAuthRequest(port, AUTH_REQ_OK, NULL, 0);
 	else
 		auth_failed(port, status, logdetail);
@@ -2758,6 +2776,7 @@ errdetail_for_ldap(LDAP *ldap)
 static int
 CheckCertAuth(Port *port)
 {
+	int status_check_usermap = STATUS_ERROR;
 	Assert(port->ssl);
 
 	/* Make sure we have received a username in the certificate */
@@ -2771,7 +2790,21 @@ CheckCertAuth(Port *port)
 	}
 
 	/* Just pass the certificate CN to the usermap check */
-	return check_usermap(port->hba->usermap, port->user_name, port->peer_cn, false);
+	status_check_usermap = check_usermap(port->hba->usermap, port->user_name, port->peer_cn, false);
+	if (status_check_usermap != STATUS_OK)
+	{
+		/*
+		 * If clientcert=verify-full was specified and the authentication method
+		 * is other than uaCert, log the reason for rejecting the authentication.
+		 */
+		if (port->hba->clientcert == clientCertFull && port->hba->auth_method!=uaCert)
+		{
+			ereport(LOG,
+				(errmsg("certificate validation (clientcert=verify-full) failed for user \"%s\": CN mismatch",
+						port->user_name)));
+		}
+	}
+	return status_check_usermap;
 }
 #endif
 
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 1a65ec87bd..2c49b21989 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -1609,7 +1609,7 @@ parse_hba_line(TokenizedLine *tok_line, int elevel)
 	 */
 	if (parsedline->auth_method == uaCert)
 	{
-		parsedline->clientcert = true;
+		parsedline->clientcert = clientCertOn;
 	}
 
 	return parsedline;
@@ -1675,11 +1675,16 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
 			*err_msg = "clientcert can only be configured for \"hostssl\" rows";
 			return false;
 		}
-		if (strcmp(val, "1") == 0)
+		if (strcmp(val, "1") == 0
+			|| strcmp(val, "verify-ca") == 0)
 		{
-			hbaline->clientcert = true;
+			hbaline->clientcert = clientCertOn;
 		}
-		else
+		else if(strcmp(val, "verify-full") == 0)
+		{
+			hbaline->clientcert = clientCertFull;
+		}
+		else if (strcmp(val, "0") == 0)
 		{
 			if (hbaline->auth_method == uaCert)
 			{
@@ -1691,7 +1696,16 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
 				*err_msg = "clientcert can not be set to 0 when using \"cert\" authentication";
 				return false;
 			}
-			hbaline->clientcert = false;
+			hbaline->clientcert = clientCertOff;
+		}
+		else
+		{
+			ereport(elevel,
+					(errcode(ERRCODE_CONFIG_FILE_ERROR),
+					 errmsg("invalid value for clientcert: \"%s\"", val),
+					 errcontext("line %d of configuration file \"%s\"",
+								line_num, HbaFileName)));
+			return false;
 		}
 	}
 	else if (strcmp(name, "pamservice") == 0)
@@ -2250,9 +2264,9 @@ gethba_options(HbaLine *hba)
 		options[noptions++] =
 			CStringGetTextDatum(psprintf("map=%s", hba->usermap));
 
-	if (hba->clientcert)
+	if (hba->clientcert != clientCertOff)
 		options[noptions++] =
-			CStringGetTextDatum("clientcert=true");
+			CStringGetTextDatum(psprintf("clientcert=%s", (hba->clientcert == clientCertOn) ? "verify-ca" : "verify-full"));
 
 	if (hba->pamservice)
 		options[noptions++] =
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index 5f68f4c666..ce9a692b8d 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -58,6 +58,13 @@ typedef enum ConnType
 	ctHostNoSSL
 } ConnType;
 
+typedef enum ClientCertMode
+{
+	clientCertOff,
+	clientCertOn,
+	clientCertFull
+} ClientCertMode;
+
 typedef struct HbaLine
 {
 	int			linenumber;
@@ -86,7 +93,7 @@ typedef struct HbaLine
 	int			ldapscope;
 	char	   *ldapprefix;
 	char	   *ldapsuffix;
-	bool		clientcert;
+	ClientCertMode clientcert;
 	char	   *krb_realm;
 	bool		include_realm;
 	bool		compat_realm;
diff --git a/src/test/ssl/ServerSetup.pm b/src/test/ssl/ServerSetup.pm
index 3b451a360a..7738ac3f1b 100644
--- a/src/test/ssl/ServerSetup.pm
+++ b/src/test/ssl/ServerSetup.pm
@@ -93,8 +93,10 @@ sub configure_test_server_for_ssl
 	# Create test users and databases
 	$node->psql('postgres', "CREATE USER ssltestuser");
 	$node->psql('postgres', "CREATE USER anotheruser");
+	$node->psql('postgres', "CREATE USER yetanotheruser");
 	$node->psql('postgres', "CREATE DATABASE trustdb");
 	$node->psql('postgres', "CREATE DATABASE certdb");
+	$node->psql('postgres', "CREATE DATABASE verifydb");
 
 	# Update password of each user as needed.
 	if (defined($password))
@@ -173,11 +175,17 @@ sub configure_hba_for_ssl
 	# When connecting to certdb, also check the client certificate.
 	open my $hba, '>', "$pgdata/pg_hba.conf";
 	print $hba
-	  "# TYPE  DATABASE        USER            ADDRESS                 METHOD\n";
+	  "# TYPE  DATABASE        USER            ADDRESS                 METHOD             OPTIONS\n";
 	print $hba
 	  "hostssl trustdb         all             $serverhost/32            $authmethod\n";
 	print $hba
 	  "hostssl trustdb         all             ::1/128                 $authmethod\n";
+	print $hba
+	  "hostssl verifydb        ssltestuser     $serverhost/32          $authmethod        clientcert=verify-full\n";
+	print $hba
+	  "hostssl verifydb        anotheruser     $serverhost/32          $authmethod        clientcert=verify-full\n";
+	print $hba
+	  "hostssl verifydb        yetanotheruser  $serverhost/32          trust              clientcert=verify-ca\n";
 	print $hba
 	  "hostssl certdb          all             $serverhost/32            cert\n";
 	print $hba
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
old mode 100644
new mode 100755
index 2b875a3c95..2331dc7508
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -358,6 +358,27 @@ switch_server_cert($node, 'server-cn-only', 'root_ca');
 $common_connstr =
   "user=ssltestuser dbname=certdb sslkey=ssl/client_tmp.key sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
 
+# Check that connecting with auth-optionverify-full in pg_hba :
+# works, iff username matches Common Name
+# fails, iff username doesn't match Common Name.
+$common_connstr =
+"sslrootcert=ssl/root+server_ca.crt sslmode=require dbname=verifydb hostaddr=$SERVERHOSTADDR";
+
+test_connect_ok($common_connstr,
+				"user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
+				"auth_option clientcert=verify-full succeeds with matching username and Common Name");
+
+test_connect_fails($common_connstr,
+				   "user=anotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
+				   qr/FATAL/,
+				   "auth_option clientcert=verify-full fails with mismatching username and Common Name");
+
+# Check that connecting with auth-optionverify-ca in pg_hba :
+# works, when username doesn't match Common Name
+test_connect_ok($common_connstr,
+				"user=yetanotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key",
+				"auth_option clientcert=verify-ca succeeds with mismatching username and Common Name");
+
 test_connect_ok(
 	$common_connstr,
 	"sslmode=require sslcert=ssl/client+client_ca.crt",
-- 
2.17.1

