Dear hackers, We (Julian and I) would like to show you the seventh version of this patch which includes all the things mentioned before. Unfortunately we did not find the time to do this earlier.
On 07/19/2018 03:00 AM, Thomas Munro wrote: > you could just have one common code path to reach CheckCertAuth() > for all auth methods after that switch statement, instead of the more > complicated conditional coding you have now with two ways to reach it. There is only one path now to call CheckCertAuth(). I don't think we have left too many complicated conditions. > That would result in a couple less LOC and a bit clearer conditionals, > I agree. > If there are no objections to make uaCert a quasi-alias of uaTrust > with clientcert=verify-full, I'll go ahead and change the code > accordingly. uaCert and uaTrust are handled the same within the switch statement. > I'll make sure that the error messages are still handled based on > the auth method and not only depending on the clientcert flag. As far as I know we already handled the error message based on the auth method and clientcert flag. On 07/30/2018 12:20, Julian Markwort wrote: > I'm open for suggestions, but in absence of objections I might just > capitalize all occurrences of CN. We decided to stick with the old style for now. So we changed all occurrences of cn to lower case. > Yes, we should adopt the new style in all places. > I'll rewrite that passage to indicate that cert authentication > essentially results in the same behavior as clientcert=verify-full. > The existing text is somewhat ambiguous anyway, in case somebody > decides to skip over the restriction described in the second sentence. We fixed that. Additionally we added the alias "no-verify" for clientcert=0 since it seems to be a good idea to have aliases for all three available values. > > What do you think about using clientCertCA for the enumerator name > > instead of clientCertOn? That would correspond better to the names > > "verify-ca" and "verify-full". > +1 > I'm not sure if Magnus had any other cases in mind when he named it > clientCertOn? We agree that clientCertCA is a better name for it. Since Magnus does not seem to have any concerns about it we changed that as well. Julian and I think the time has come for this patch to make some progress. After a few months I think there is not that much to discuss anymore. Kind regards, Marius Timmer -- Westfälische Wilhelms-Universität Münster (WWU) Zentrum für Informationsverarbeitung (ZIV) Röntgenstraße 7-13 Besucheradresse: Einsteinstraße 60 - Raum 101 48149 Münster +49 251 83 31158 marius.tim...@uni-muenster.de https://www.uni-muenster.de/ZIV
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index c2114021c3..411f1e1679 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>
@@ -1865,11 +1872,11 @@ host ... ldap ldapserver=ldap.example.net ldapbasedn="dc=example, dc=net" ldapse
<para>
In a <filename>pg_hba.conf</filename> record specifying certificate
authentication, the authentication option <literal>clientcert</literal> is
- assumed to be <literal>1</literal>, and it cannot be turned off since a client
- certificate is necessary for this method. What the <literal>cert</literal>
- method adds to the basic <literal>clientcert</literal> certificate validity test
- is a check that the <literal>cn</literal> attribute matches the database
- user name.
+ assumed to be <literal>verify-ca</literal> or <literal>verify-full</literal>,
+ and it cannot be turned off since a client certificate is necessary for this
+ method. What the <literal>cert</literal> method adds to the basic
+ <literal>clientcert</literal> certificate validity test is a check that the
+ <literal>cn</literal> attribute matches the database user name.
</para>
</sect1>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 8d9d40664b..ef515535a8 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>
@@ -2312,18 +2324,34 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
The <literal>clientcert</literal> authentication option is available for
all authentication methods, but only in <filename>pg_hba.conf</filename> lines
specified as <literal>hostssl</literal>. When <literal>clientcert</literal> is
- not specified or is set to 0, the server will still verify any presented
- client certificates against its CA file, if one is configured — but
- it will not insist that a client certificate be presented.
+ not specified or is set to <literal>no-verify</literal>, the server will still
+ verify any presented client certificates against its CA file, if one is
+ configured — but it will not insist that a client certificate be presented.
+ </para>
+
+ <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 8517565535..700313b7ab 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -364,7 +364,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())
@@ -582,24 +582,31 @@ ClientAuthentication(Port *port)
status = CheckLDAPAuth(port);
#else
Assert(false);
-#endif
- break;
-
- case uaCert:
-#ifdef USE_SSL
- status = CheckCertAuth(port);
-#else
- Assert(false);
#endif
break;
case uaRADIUS:
status = CheckRADIUSAuth(port);
break;
+ case uaCert:
case uaTrust:
status = STATUS_OK;
break;
}
+ if ((status == STATUS_OK && port->hba->clientcert == clientCertFull)
+ || port->hba->auth_method == uaCert)
+ {
+ /**
+ * Make sure we only check the certificate if we use the
+ * cert method or verify-full option.
+ */
+#ifdef USE_SSL
+ status = CheckCertAuth(port);
+#else
+ Assert(false);
+#endif
+ }
+
if (ClientAuthentication_hook)
(*ClientAuthentication_hook) (port, status);
@@ -2747,6 +2754,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 */
@@ -2759,8 +2767,22 @@ CheckCertAuth(Port *port)
return STATUS_ERROR;
}
- /* Just pass the certificate CN to the usermap check */
- return check_usermap(port->hba->usermap, port->user_name, port->peer_cn, false);
+ /* Just pass the certificate cn to the usermap check */
+ 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..e05719c316 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 = clientCertCA;
}
return parsedline;
@@ -1675,23 +1675,39 @@ 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 = clientCertCA;
}
- else
+ else if(strcmp(val, "2") == 0
+ || strcmp(val, "verify-full") == 0)
+ {
+ hbaline->clientcert = clientCertFull;
+ }
+ else if (strcmp(val, "0") == 0
+ || strcmp(val, "no-verify") == 0)
{
if (hbaline->auth_method == uaCert)
{
ereport(elevel,
(errcode(ERRCODE_CONFIG_FILE_ERROR),
- errmsg("clientcert can not be set to 0 when using \"cert\" authentication"),
+ errmsg("clientcert can not be set to \"no-verify\" when using \"cert\" authentication"),
errcontext("line %d of configuration file \"%s\"",
line_num, HbaFileName)));
- *err_msg = "clientcert can not be set to 0 when using \"cert\" authentication";
+ *err_msg = "clientcert can not be set to \"no-verify\" 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 +2266,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 == clientCertCA) ? "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..c65eb9dc8a 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,
+ clientCertCA,
+ 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
index 2b875a3c95..6c3f190a9e 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 => 65;
+ plan tests => 69;
}
else
{
@@ -353,6 +353,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 =
smime.p7s
Description: S/MIME Cryptographic Signature
