On Sat, 2018-03-17 at 18:24 +0100, Magnus Hagander wrote: > > The error message "certificate authentication failed for user XYZ: > > > > client certificate contains no user name" is the result of calling > > > > CheckCertAuth when the user presented a certificate without a CN in > > it. > > That is arguably wrong, since it's actually password authentication > that fails. That is the authentication type that was picked in > pg_hba.conf. It's more "certificate validation" that failed.
I think we got confused about this; maybe I didn't graps it fully before: CheckCertAuth is currently only called when auth method cert is used. So it actually makes sense to say that certificate authentication failed, I think. > I agree that the log message is useful. Though it could be good to > clearly indicate that it was caused specifically because of the > verify-full, to differentiate it from other cases of the same > message. I've modified my patch so it still uses CheckCertAuth, but now a different message is written to the log when clientcert=verify-full was used. For auth method cert, the function should behave as before. > For example, what about the scenario where I use GSSAPI > authentication and clientcert=verify-full. Then we suddenly have > three usernames (gssapi, certificate and specified) -- how is the > user supposed to know which one came from the cert and which one came > from gssapi for example? The user will only see what's printed in the auth_failed() function in auth.c with the addition of the logdetail string, which I don't touch with this patch. As you said, it makes sense that more detailed information is only available in the server's log. I've attached an updated version of the patch. I'm not sure if it is preferred to keep patches as short as possible (mostly with respect to the changed lines in the documentation) or to organize changes so that the text matches the surrounding column width und text flow? Also, I've omitted mentions of the current usage 'clientcert=1' - this is still supported in code, but I think telling new users only about 'clientcert=verify-ca' and 'clientcert=verify- full' is clearer. Or am I wrong on this one? Greetings Julian
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 53832d0..11c5961 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 behaviour is similar to the cert autentication 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 587b430..a3eb180 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -2263,13 +2263,24 @@ 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 it's
+ mapping match the common name (CN) of the provided certificate.
+ Note that this is always ensured when <literal>cert</literal>
+ authentication method is used (See <xref linkend="auth-cert"/>).
</para>
<para>
@@ -2293,14 +2304,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 3014b17..86da258 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;
/*
@@ -600,10 +601,23 @@ ClientAuthentication(Port *port)
break;
}
+ if(port->hba->clientcert_verify_full && port->hba->auth_method!=uaCert)
+ {
+#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);
@@ -2756,6 +2770,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 */
@@ -2769,7 +2784,19 @@ 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, give a hint about clientcert=verify-full. */
+ if (port->hba->clientcert_verify_full && port->hba->auth_method!=uaCert)
+ {
+ ereport(LOG,
+ (errmsg("certificate validation failed for user \"%s\": common name in client certificate does not match user name or mapping, but clientcert=verify-full is enabled.",
+ port->user_name)));
+ }
+ }
+ return status_check_usermap;
}
#endif
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index acf625e..a5b0683 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -1675,10 +1675,17 @@ 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;
}
+ else if(strcmp(val, "2") == 0
+ || strcmp(val, "verify-full") == 0)
+ {
+ hbaline->clientcert = true;
+ hbaline->clientcert_verify_full = true;
+ }
else
{
if (hbaline->auth_method == uaCert)
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index 5f68f4c..309db47 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -87,6 +87,7 @@ typedef struct HbaLine
char *ldapprefix;
char *ldapsuffix;
bool clientcert;
+ bool clientcert_verify_full;
char *krb_realm;
bool include_realm;
bool compat_realm;
smime.p7s
Description: S/MIME cryptographic signature
