Hi Thomas,

here's a rebased patch, with your observations corrected.

Thomas Munro wrote on 2018-07-13:
> +   In this case, the <literal>CN</literal> (nommon name) provided in
> "common name"
> +   <literal>CN</literal> (Common Name) in the certificate matches
> "common"? (why a capital letter here?)

I've resorted to "<literal>CN</literal> (Common Name)" on all occurences in 
this patch now.

Also, while writing this part of the docs, I tried to stay below 80 characters, 
but I've exceeded it in some places.
There are several other places (several in the .sgml files touched by this 
patch), where 80 characters are exceeded; How close should one adhere to that 
limit nowadays?


> This line isn't modified by your patch, but I saw it while in
> proof-reading mode:
>   *err_msg = "clientcert can not be set to 0 when using \"cert\"
> authentication";
> I think "can not" is usually written "cannot"?

I'm not sure about can not, cannot, can't... There are 56, respectively 12697, 
and 2024 occurrences in master right now.
We could touch those lines now and change them to the more common cannot, or we 
can leave it as is...


> Yeah.  The packages to install depend on your operating system, and in
> some cases (macOS, Windows?) which bolt-on package thingamajig you
> use, though.  Perhaps the READMEs could be improved with details for
> systems we have reports about (like the recently added "Requirements"
> section of src/test/ldap/README).

That would be nice, however I could only provide the package names for Fedora 
right now...
Would It make sense to add those on their own?
Or should somebody (maybe myself, when I'm less busy) gather those for most 
supported systems and commit them as a whole?

kind regards
Julian
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 656d5f94..40dc0ef7 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 1c92e7df..afdcc729 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>
-   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.)
+   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>
+   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 63f37902..0244a21f 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 1a65ec87..2c49b219 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 5f68f4c6..ce9a692b 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 3b451a36..7738ac3f 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
index e5502074..cc34ea37 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -8,7 +8,7 @@ use File::Copy;
 
 if ($ENV{with_openssl} eq 'yes')
 {
-	plan tests => 64;
+	plan tests => 68;
 }
 else
 {
@@ -348,6 +348,27 @@ test_connect_fails(
 	qr/SSL error/,
 	"certificate authorization fails with revoked client cert");
 
+# 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");
+
 # intermediate client_ca.crt is provided by client, and isn't in server's ssl_ca_file
 switch_server_cert($node, 'server-cn-only', 'root_ca');
 $common_connstr =

Reply via email to