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 — 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 =