On Sun, Feb 17, 2019 at 7:50 PM Michael Paquier <mich...@paquier.xyz> wrote:
> On Fri, Feb 15, 2019 at 08:03:24PM -0800, Andres Freund wrote:
> > I see you've marked the patch as needs review - but as the patch
> > previously was marked as ready-for-committer, and I assume nothing
> > substantial has changed, I think RFC might still be the accurate state?
>
> Yes, RFC sounds good to me. I think that I could look at this patch
> and get it into shape for PG12 as that's an area I am rather familiar
> with. After that it depends on Magnus as he is registered as the
> committer of the patch. I'll be fine to jump into the ship if need
> be.
>
Sorry, for some reason this entire thread ended up getting tracked in my
spam folder :/ Yay the new gmail antispam breakage.
I was definitely still planning to work on it!
The first thing I noticed is that it needed yet another rebase because of
the changes in the SSL tests. PFA an updated patch (very trivial rebase).
(I'll keep working on it, just wanted to throw the rebased patch out there)
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
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 7de26e98ad..d786ebfb71 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 d5115aad72..3a3c1aaf4d 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())
@@ -583,22 +583,30 @@ ClientAuthentication(Port *port)
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/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 2f6dfad23c..c766ff215d 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -13,7 +13,7 @@ use SSLServer;
if ($ENV{with_openssl} eq 'yes')
{
- plan tests => 71;
+ plan tests => 75;
}
else
{
@@ -378,6 +378,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 =
diff --git a/src/test/ssl/t/SSLServer.pm b/src/test/ssl/t/SSLServer.pm
index b1b5b7f0b3..d25c38dbbc 100644
--- a/src/test/ssl/t/SSLServer.pm
+++ b/src/test/ssl/t/SSLServer.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,12 +185,18 @@ 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
"hostssl certdb all ::1/128 cert\n";
