On Mon, Jan 04, 2019 at 03:06, Michael Paquier wrote: > On Thu, Dec 27, 2018 at 12:14:03PM +0100, Magnus Hagander wrote: >> I definitely am. In fact, I was ages ago (was planning for early December, >> but hey, see wher that let me), so my apologies for failing at that. But it >> definitely remains on my list of things to get to! > > So, Magnus, where are we on this? > > I have moved the patch to next CF, waiting on author as the latest > patch does not apply. Could it be rebased? The patch is rebased and applies now.
Kind regards, Marius -- 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 1f78f6c956..36a45f7f0c 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -2316,13 +2316,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> @@ -2341,18 +2353,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 20fe98fb82..d092049329 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -363,7 +363,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()) @@ -581,24 +581,32 @@ 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: + // uaCert will be treated as if clientcert=verify-full (uaTrust) 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); @@ -2788,6 +2796,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 */ @@ -2800,8 +2809,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 b17c714735..398456e439 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,38 @@ 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, "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) @@ -2252,9 +2267,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 5acba52310..321740be01 100644 --- a/src/test/ssl/ServerSetup.pm +++ b/src/test/ssl/ServerSetup.pm @@ -103,8 +103,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)) @@ -183,11 +185,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 $authmethod 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 915007805e..7c31e8a146 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 => 71; + plan tests => 75; } else { @@ -373,6 +373,27 @@ test_connect_fails( qr/SSL error/, "certificate authorization fails with revoked client cert"); +# Check that connecting with auth-option verify-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 =