[PATCH] pg_hba.conf : new auth option : clientcert=verify-full
Dear Postgresql Hackers,
as of now, pg_hba.conf allows us to enable authentification by
certificate through the auth-method "cert", in which case the user must
provide a valid certificate with a certificate common name(CN) matching
the database user's name or an entry in a pg_ident map.
Additionaly, for every other auth-method it is possible to set the
auth-option "clientcert=1", so clients must present a valid certificate
at login. The logic behind this only checks the validity of the
certificate itself, but the certificate common name(CN) is not
relevant.
I wrote a very small patch that adds another auth-option:
- clientcert=verify-full (analogous to server certificates; you could
also use 2 instead of verify-full for backwards compatibility, or
verify-ca instead of 1)
which also checks the certificate common name,
so all 3 factors get checked:
1.) auth-method, e.g. scram or md5 password passes
2.) client cert is in truststore
3.) CN is correct.
(The patch simply makes use of the function that is used for auth-
method "cert" to avoid code duplication).
Have a nice weekend,
Julian Markwort
Attachments:
clientcert_verify-full_v01.patchtext/x-patch; charset=UTF-8; name=clientcert_verify-full_v01.patchDownload
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 3014b17a..5757aa99 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)
+ {
+#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);
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index acf625e4..a5b0683d 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 5f68f4c6..309db47d 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;
On Fri, Feb 16, 2018 at 4:45 PM, Julian Markwort <
julian.markwort@uni-muenster.de> wrote:
Dear Postgresql Hackers,
as of now, pg_hba.conf allows us to enable authentification by
certificate through the auth-method "cert", in which case the user must
provide a valid certificate with a certificate common name(CN) matching
the database user's name or an entry in a pg_ident map.Additionaly, for every other auth-method it is possible to set the
auth-option "clientcert=1", so clients must present a valid certificate
at login. The logic behind this only checks the validity of the
certificate itself, but the certificate common name(CN) is not
relevant.I wrote a very small patch that adds another auth-option:
- clientcert=verify-full (analogous to server certificates; you could
also use 2 instead of verify-full for backwards compatibility, or
verify-ca instead of 1)
which also checks the certificate common name,
so all 3 factors get checked:
1.) auth-method, e.g. scram or md5 password passes
2.) client cert is in truststore
3.) CN is correct.(The patch simply makes use of the function that is used for auth-
method "cert" to avoid code duplication).
I think this makes a lot of sense, and can definitely be a useful option.
However, the patch is completely lacking documentation, which obviously
make it a no-starter.
Also if I read it right, if the CN is not correct, it will give the error
message "certificate authentication failed for user ...". I realize this
comes from the re-use of the code, but I don't think this makes it very
useful. We need to separate these two things.
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
Hello Magnus,
I think this makes a lot of sense, and can definitely be a useful
option.
I was hesistant to write a long and elaborate patch as I wasn't certain
if there was any interest for such an addition, but I'm thankful for
your input.
However, the patch is completely lacking documentation, which
obviously make it a no-starter.
I'll write the missing documentation shortly.
Also if I read it right, if the CN is not correct, it will give the
error message "certificate authentication failed for user ...". I
realize this comes from the re-use of the code, but I don't think
this makes it very useful. We need to separate these two things.
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.
The error message that is presented to the user upon trying to connect
with a certificate containing a CN other than the username is:
---------------------
psql: FATAL: password authentication failed for user "nottestuser"
---------------------
The server's log contains the lines:
---------------------
2018-03-09 13:06:43.111 CET [3310] LOG: provided user name
(nottestuser) and authenticated user name (testuser) do not match
2018-03-09 13:06:43.111 CET [3310] FATAL: password authentication
failed for user "nottestuser"
2018-03-09 13:06:43.111 CET [3310] DETAIL: Connection matched
pg_hba.conf line 97: "hostssl all nottestuser 127.0.0.1/32 password
clientcert=verify-full"
---------------------
I'd argue that the message in the log file is consistent and useful,
however the message given by psql (or any libpq application for that
matter) leaves uncertainty regarding the correctness of a provided
password, for example.
I could attach the log message of CheckCertAuth to the logdetail,
however then I'd have issues if there is already something written to
the logdetail.
I could also use an additional ereport() call whenever clientcert was
set to verify-full and the user name didn't match the CN.
Kind regards
Julian
On Fri, Mar 9, 2018 at 2:11 PM, Julian Markwort <
julian.markwort@uni-muenster.de> wrote:
Hello Magnus,
I think this makes a lot of sense, and can definitely be a useful
option.I was hesistant to write a long and elaborate patch as I wasn't certain
if there was any interest for such an addition, but I'm thankful for
your input.However, the patch is completely lacking documentation, which
obviously make it a no-starter.I'll write the missing documentation shortly.
Great!
Also if I read it right, if the CN is not correct, it will give the
error message "certificate authentication failed for user ...". I
realize this comes from the re-use of the code, but I don't think
this makes it very useful. We need to separate these two things.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.
The error message that is presented to the user upon trying to connect
with a certificate containing a CN other than the username is:
---------------------
psql: FATAL: password authentication failed for user "nottestuser"
---------------------The server's log contains the lines:
---------------------
2018-03-09 13:06:43.111 CET [3310] LOG: provided user name
(nottestuser) and authenticated user name (testuser) do not match
2018-03-09 13:06:43.111 CET [3310] FATAL: password authentication
failed for user "nottestuser"
2018-03-09 13:06:43.111 CET [3310] DETAIL: Connection matched
pg_hba.conf line 97: "hostssl all nottestuser 127.0.0.1/32 password
clientcert=verify-full"
---------------------I'd argue that the message in the log file is consistent and useful,
however the message given by psql (or any libpq application for that
matter) leaves uncertainty regarding the correctness of a provided
password, for example.
I could attach the log message of CheckCertAuth to the logdetail,
however then I'd have issues if there is already something written to
the logdetail.
I could also use an additional ereport() call whenever clientcert was
set to verify-full and the user name didn't match the CN.
It's hard to do too much about the client connection one when failing,
without leaking too much. It's pretty common that you have to actually look
in the server logfile to figure out what actually went wrong. I think that
message is fine.
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.
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?
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
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
Attachments:
clientcert_verify_full_v02.patchtext/x-patch; charset=UTF-8; name=clientcert_verify_full_v02.patchDownload
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;
On Fri, Mar 23, 2018 at 3:45 PM, Julian Markwort <
julian.markwort@uni-muenster.de> wrote:
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 assume this is a patch that's intended to be applied on top of the
previous patch? If so, please submit the complete pach to make sure the
correct combination ends up actually being reviewed.
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?
I have not had time to look at the updated verison of the patch yet, but I
wanted to get a response in for your last question here.
Keeping patches as short as possible is not a good thing itself. The
important part is that the resulting code and documentation is the best
possible. Sometimes you might want to turn it into two patches submitted at
the same time if one is clearly just reorganisation, but avoiding code
restructure just to keep the lines of patch down is not helpful.
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
On 1. of April 2018 17:46:38 MESZ wrote Magnus Hagander <magnus@hagander.net>:
I assume this is a patch that's intended to be applied on top of the
previous patch? If so, please submit the complete pach to make sure the
correct combination ends up actually being reviewed.
The v02.patch attached to my last mail contains both source and documentation changes.
Keeping patches as short as possible is not a good thing itself. The
important part is that the resulting code and documentation is the best
possible. Sometimes you might want to turn it into two patches
submitted at
the same time if one is clearly just reorganisation, but avoiding code
restructure just to keep the lines of patch down is not helpful.
I think I've made the right compromises regarding readability of the documentation in my patch then.
A happy Easter, passover, or Sunday to you
Julian
On Sun, Apr 1, 2018 at 6:01 PM, Julian Markwort <
julian.markwort@uni-muenster.de> wrote:
On 1. of April 2018 17:46:38 MESZ wrote Magnus Hagander <
magnus@hagander.net>:I assume this is a patch that's intended to be applied on top of the
previous patch? If so, please submit the complete pach to make sure the
correct combination ends up actually being reviewed.The v02.patch attached to my last mail contains both source and
documentation changes.
Hmm. I think I may have been looking at the wrong file. Sorry!
Keeping patches as short as possible is not a good thing itself. The
important part is that the resulting code and documentation is the best
possible. Sometimes you might want to turn it into two patches
submitted at
the same time if one is clearly just reorganisation, but avoiding code
restructure just to keep the lines of patch down is not helpful.I think I've made the right compromises regarding readability of the
documentation in my patch then.A happy Easter, passover, or Sunday to you
You, too!
(I shall return to reviewing it after the holidays are over)
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
On Sun, Apr 1, 2018 at 6:07 PM, Magnus Hagander <magnus@hagander.net> wrote:
On Sun, Apr 1, 2018 at 6:01 PM, Julian Markwort <
julian.markwort@uni-muenster.de> wrote:On 1. of April 2018 17:46:38 MESZ wrote Magnus Hagander <
magnus@hagander.net>:I assume this is a patch that's intended to be applied on top of the
previous patch? If so, please submit the complete pach to make sure the
correct combination ends up actually being reviewed.The v02.patch attached to my last mail contains both source and
documentation changes.Hmm. I think I may have been looking at the wrong file. Sorry!
Keeping patches as short as possible is not a good thing itself. The
important part is that the resulting code and documentation is the best
possible. Sometimes you might want to turn it into two patches
submitted at
the same time if one is clearly just reorganisation, but avoiding code
restructure just to keep the lines of patch down is not helpful.I think I've made the right compromises regarding readability of the
documentation in my patch then.A happy Easter, passover, or Sunday to you
You, too!
(I shall return to reviewing it after the holidays are over)
I've been through this one again.
There is one big omission from it -- it fails to work with the view
pg_hba_file_rules. When fixing that, things started to look kind of ugly
with the "two booleans to indicate one thing", so I went ahead and changed
it to instead be an enum of 3 values.
Also, now when using verify-full or verify-ca, I figured its a lot easier
to misspell the value, and we don't want that to mean "off". Instead, I
made it give an error in this case instead of silently treating it as 0.
The option "2" for clientcert was never actually documented, and I think we
should get rid of that. We need to keep "1" for backwards compatibility
(which arguably was a bad choice originally, but that was many years ago),
but let's not add another one.
I also made a couple of very small changes to the docs.
Attached is an updated patch with these changes. I'd appreciate it if you
can run it through your tests to confirm that it didn't break any of those
usecases.
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
Attachments:
clientcert_verify_full_v03.patchtext/x-patch; charset=US-ASCII; name=clientcert_verify_full_v03.patchDownload
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 53832d08e2..5ee8cbe01a 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 587b430527..19af9bd7e0 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -2263,13 +2263,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 it's
+ mapping match the common name (CN) of the provided certificate.
+ Note that certificate chain validation is always ensured when
+ <literal>cert</literal> authentication method is used
+ (see <xref linkend="auth-cert"/>).
</para>
<para>
@@ -2293,14 +2305,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>
+ 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> (nommon 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 3014b17a7c..f6c3ff01b2 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,23 @@ ClientAuthentication(Port *port)
break;
}
+ if(port->hba->clientcert == clientCertFull && 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,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 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 acf625e4ec..d17682b55a 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 5f68f4c666..ce9a692b8d 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;
On Fri, 2018-04-06 at 20:31 +0200, Magnus Hagander wrote:
I've been through this one again.
Thanks for taking the time!
There is one big omission from it -- it fails to work with the view
pg_hba_file_rules. When fixing that, things started to look kind of
ugly with the "two booleans to indicate one thing", so I went ahead
and changed it to instead be an enum of 3 values.
Oh, I missed that view. To be honest, it wasn't even on my mind that
there could be a view depending on pg_hba....
Also, now when using verify-full or verify-ca, I figured its a lot
easier to misspell the value, and we don't want that to mean "off".
Instead, I made it give an error in this case instead of silently
treating it as 0.
Good catch!
The option "2" for clientcert was never actually documented, and I
think we should get rid of that. We need to keep "1" for backwards
compatibility (which arguably was a bad choice originally, but that
was many years ago), but let's not add another one.
I also made a couple of very small changes to the docs.Attached is an updated patch with these changes. I'd appreciate it if
you can run it through your tests to confirm that it didn't break any
of those usecases.
I've tested a couple of things with this and it seems to work as
expected. Unforunately, there are no tests for libpq, afaik. But
testing such features would become complicated quite quickly, with the
need to generate certificates and such...
I've noticed that the error message for mismatching CN is now printed
twice when using password prompts, as all authentication details are
checked upon initiation of a connection and again later, after sending
the password.
2018-04-10 13:54:19.882 CEST [8735] LOG: provided user name
(testuser) and authenticated user name (nottestuser) do not match
2018-04-10 13:54:19.882 CEST [8735] LOG: certificate validation
failed for user "testuser": common name in client certificate does
not match user name or mapping, but clientcert=verify-full is
enabled.
2018-04-10 13:54:21.515 CEST [8736] LOG: provided user name
(testuser) and authenticated user name (nottestuser) do not match
2018-04-10 13:54:21.515 CEST [8736] LOG: certificate validation
failed for user "testuser": common name in client certificate does
not match user name or mapping, but clientcert=verify-full is
enabled.
2018-04-10 13:54:21.515 CEST [8736] FATAL: password authentication
failed for user "testuser"
2018-04-10 13:54:21.515 CEST [8736] DETAIL: Connection matched
pg_hba.conf line 94: "hostssl all testuser
127.0.0.1/32 password clientcert=verify-full"
Also, as you can see, there are two LOG messages indicating the
mismatch -- "provided user name (testuser) and authenticated user name
(nottestuser) do not match" comes from hba.c:check_usermap() and
"certificate validation failed for user "testuser": common name in
client certificate does not match user name or mapping, but
clientcert=verify-full is enabled." is the message added in
auth.c:CheckCertAuth() by the patch.
Maybe we should shorten the latter LOG message?
regards
Julian
On 4/10/18 08:10, Julian Markwort wrote:
Attached is an updated patch with these changes. I'd appreciate it if
you can run it through your tests to confirm that it didn't break any
of those usecases.I've tested a couple of things with this and it seems to work as
expected. Unforunately, there are no tests for libpq, afaik. But testing
such features would become complicated quite quickly, with the need to
generate certificates and such...
There are tests in src/test/ssl/ that would probably be a good fit to
extend for this.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Apr 10, 2018 at 2:10 PM, Julian Markwort <
julian.markwort@uni-muenster.de> wrote:
On Fri, 2018-04-06 at 20:31 +0200, Magnus Hagander wrote:
I've been through this one again.
Thanks for taking the time!
There is one big omission from it -- it fails to work with the view
pg_hba_file_rules. When fixing that, things started to look kind of ugly
with the "two booleans to indicate one thing", so I went ahead and changed
it to instead be an enum of 3 values.Oh, I missed that view. To be honest, it wasn't even on my mind that there
could be a view depending on pg_hba....Also, now when using verify-full or verify-ca, I figured its a lot easier
to misspell the value, and we don't want that to mean "off". Instead, I
made it give an error in this case instead of silently treating it as 0.Good catch!
The option "2" for clientcert was never actually documented, and I think
we should get rid of that. We need to keep "1" for backwards compatibility
(which arguably was a bad choice originally, but that was many years ago),
but let's not add another one.
I also made a couple of very small changes to the docs.Attached is an updated patch with these changes. I'd appreciate it if you
can run it through your tests to confirm that it didn't break any of those
usecases.I've tested a couple of things with this and it seems to work as expected.
Unforunately, there are no tests for libpq, afaik. But testing such
features would become complicated quite quickly, with the need to generate
certificates and such...
As Peter mentionde, there are in src/test/ssl. I forgot about those, but
yes, it would be useful to have that.
I've noticed that the error message for mismatching CN is now printed twice
when using password prompts, as all authentication details are checked upon
initiation of a connection and again later, after sending the password.
That depends on your authenticaiton method. Specifically for passwords,
what happens is there are actually two separate connections -- the first
one with no password supplied, and the second one with a password in it.
We could directly reject the connection after the first one and not ask for
a password. I'm not sure if that's better though -- that would leak the
information on *why* we rejected the connection.
2018-04-10 13:54:19.882 CEST [8735] LOG: provided user name (testuser) and
authenticated user name (nottestuser) do not match
2018-04-10 13:54:19.882 CEST [8735] LOG: certificate validation failed
for user "testuser": common name in client certificate does not match user
name or mapping, but clientcert=verify-full is enabled.
2018-04-10 13:54:21.515 CEST [8736] LOG: provided user name (testuser)
and authenticated user name (nottestuser) do not match
2018-04-10 13:54:21.515 CEST [8736] LOG: certificate validation failed
for user "testuser": common name in client certificate does not match user
name or mapping, but clientcert=verify-full is enabled.
2018-04-10 13:54:21.515 CEST [8736] FATAL: password authentication failed
for user "testuser"
2018-04-10 13:54:21.515 CEST [8736] DETAIL: Connection matched
pg_hba.conf line 94: "hostssl all testuser 127.0.0.1/32 password
clientcert=verify-full"Also, as you can see, there are two LOG messages indicating the mismatch
-- "provided user name (testuser) and authenticated user name (nottestuser)
do not match" comes from hba.c:check_usermap() and "certificate validation
failed for user "testuser": common name in client certificate does not
match user name or mapping, but clientcert=verify-full is enabled." is the
message added in auth.c:CheckCertAuth() by the patch.
Maybe we should shorten the latter LOG message?
It might definitely be worth shorting it yeah. For one, we can just use
"cn" :)
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
On Tue, 2018-04-10 at 18:35 +0200, Magnus Hagander wrote:
As Peter mentionde, there are in src/test/ssl. I forgot about those,
but yes, it would be useful to have that.
I've added three tests:
- verify-full specified, CN and username match -- should connect ok
- verify-full specified, CN and username do not match -- should fail
- verify-ca specified, CN and username do not match -- should connect
ok (This is current behaviour)
That depends on your authenticaiton method. Specifically for
passwords, what happens is there are actually two separate
connections -- the first one with no password supplied, and the
second one with a password in it.
Makes sense.
We could directly reject the connection after the first one and not
ask for a password. I'm not sure if that's better though -- that
would leak the information on *why* we rejected the connection.
This wouldn't be desirable, I think...
Most applications will probably supply the password in the connection
string anyway, so there would be only one connection, right?
It might definitely be worth shorting it yeah. For one, we can just
use "cn" :)
I've shortened the clientcert=verify-full specific error-message to
say:
"certificate validation (clientcert=verify-full) failed for
user \"%s\": CN mismatch"
slightly offtopic opinion:
While creating the test cases, I stumbled upon the problem of missing
depencies to run the tests...
It's complicated enough that the binaries used by these perl tests are
not named similar to the packages which provide them (the 'prove'
binary is supplied by 'Test-Harness'), so maybe in the interest of
providing a lower entry-barrier to running these tests, we could give a
more detailed error message in the configure script, when using --
enable-tap-tests ?
Julian
Attachments:
clientcert_verify_full_v04.patchtext/x-patch; charset=UTF-8; name=clientcert_verify_full_v04.patchDownload
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 53832d0..5ee8cbe 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 330e38a..6502df3 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 it's
+ mapping match the common name (CN) of the provided certificate.
+ Note that certificate chain validation is always ensured when
+ <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> (nommon 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..8330f5c 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,23 @@ ClientAuthentication(Port *port)
break;
}
+ if(port->hba->clientcert == clientCertFull && 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,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 acf625e..d17682b 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 5f68f4c..ce9a692 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 e81f4df..4343a1a 100644
--- a/src/test/ssl/ServerSetup.pm
+++ b/src/test/ssl/ServerSetup.pm
@@ -82,8 +82,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))
@@ -157,12 +159,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 trust clientcert=verify-ca\n";
+ print $hba
"hostssl certdb all $serverhost/32 cert\n";
print $hba
"hostssl certdb all ::1/128 cert\n";
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 91feac6..b45999e 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
{
@@ -298,6 +298,27 @@ test_connect_fails($common_connstr,
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 =
On Sat, Apr 14, 2018 at 3:48 AM, Julian Markwort
<julian.markwort@uni-muenster.de> wrote:
[a patch]
Hello Julian,
Could you please post a rebased patch?
I haven't reviewed or tested any code yet, but here's some proof-reading:
+ This behaviour is similar to the cert autentication method
"behavior" (our manual is written in en_US, "cd doc/src/sgml ; git
grep behavior | wc -l" -> 895, "git grep behaviour" -> 0).
<literal>cert</literal>
"authentication"
+ chain, but it will also check whether the username or it's
+ mapping match the common name (CN) of the provided certificate.
"its"
"matches"
+ Note that certificate chain validation is always ensured when
+ <literal>cert</literal> authentication method is used
extra space
when *the* ...
+ 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?)
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"?
slightly offtopic opinion:
While creating the test cases, I stumbled upon the problem of missing
depencies to run the tests...
It's complicated enough that the binaries used by these perl tests are not
named similar to the packages which provide them (the 'prove' binary is
supplied by 'Test-Harness'), so maybe in the interest of providing a lower
entry-barrier to running these tests, we could give a more detailed error
message in the configure script, when using --enable-tap-tests ?
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).
--
Thomas Munro
http://www.enterprisedb.com
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
Attachments:
clientcert_verify_full_v05.patchtext/x-patchDownload
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 =
On Sun, Jul 15, 2018 at 12:47 AM, Julian Markwort
<julian.markwort@uni-muenster.de> wrote:
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?
Not sure if there is a strict policy, but I usually rewrap with Emacs
M-q unless I'm making a small edit in the middle of an existing
paragraph and want to minimise the diff for clarity. Here you are
replacing all or most of some paragraphs, so +1 for rewrapping.
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?
Let's save that for another patch. I can supply data for a couple of OSes.
Some more comments:
if (parsedline->auth_method == uaCert)
{
- parsedline->clientcert = true;
+ parsedline->clientcert = clientCertOn;
}
The "cert" method is technically redundant with this patch, because
it's equivalent to "trust clientcert=verify-full", right? That gave
me an idea: wouldn't the control flow be a bit nicer if you just
treated uaCert the same as uaTrust in the big switch statement, but
also enforced that clientcert = clientCertFull in the hunk above?
Then 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.
Would that be better?
In the "auth-cert" section there are already a couple of examples
using lower case "cn":
will be sent to the client. The <literal>cn</literal> (Common Name)
is a check that the <literal>cn</literal> attribute matches the database
I guess you should do the same, or if there is a good reason to use
upper case "CN" instead then you should change those to match.
There is still a place in the documentation where we refer to "1":
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.
Maybe we should use "verify-ca" here, though as I noted above I think
it should really "verify-full" and we should simplify the flow.
I think we should also document that "1" and "0" are older spellings
still accepted, where the setting names are introduced.
+typedef enum ClientCertMode
+{
+ clientCertOff,
+ clientCertOn,
+ clientCertFull
+} ClientCertMode;
+
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".
--
Thomas Munro
http://www.enterprisedb.com
On 07/19/2018 03:00 AM, Thomas Munro wrote:
Some more comments:
if (parsedline->auth_method == uaCert) { - parsedline->clientcert = true; + parsedline->clientcert = clientCertOn; }The "cert" method is technically redundant with this patch, because
it's equivalent to "trust clientcert=verify-full", right? That gave
me an idea: wouldn't the control flow be a bit nicer if you just
treated uaCert the same as uaTrust in the big switch statement, but
also enforced that clientcert = clientCertFull in the hunk above?
Then 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.
Would that be better?
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.
I'll make sure that the error messages are still handled based on the
auth method and not only depending on the clientcert flag.
In the "auth-cert" section there are already a couple of examples
using lower case "cn":will be sent to the client. The <literal>cn</literal> (Common Name)
is a check that the <literal>cn</literal> attribute matches the database
I guess you should do the same, or if there is a good reason to use
upper case "CN" instead then you should change those to match.
I see that there are currently no places that use <literal>CN</literal>
right now.
However, we refer to Certification Authority as CA in most cases
(without the literal tag surrounding it).
The different fields of certificates seem to be abbreviated with capital
letters in most literature; That was my reasoning for capitalizing it in
the first place.
I'm open for suggestions, but in absence of objections I might just
capitalize all occurrences of CN.
There is still a place in the documentation where we refer to "1":
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.Maybe we should use "verify-ca" here, though as I noted above I think
it should really "verify-full" and we should simplify the flow.
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 behaviour 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.
I think we should also document that "1" and "0" are older spellings
still accepted, where the setting names are introduced.+typedef enum ClientCertMode +{ + clientCertOff, + clientCertOn, + clientCertFull +} ClientCertMode; +
+1
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?
Should I mark this entry as "Needs review" in the commitfest? It seems
some discussion is still needed to get this commited...
kind regards
Julian
On Mon, Jul 30, 2018 at 02:20:43PM +0200, Julian Markwort wrote:
On 07/19/2018 03:00 AM, Thomas Munro wrote:
Some more comments:
if (parsedline->auth_method == uaCert) { - parsedline->clientcert = true; + parsedline->clientcert = clientCertOn; }The "cert" method is technically redundant with this patch, because
it's equivalent to "trust clientcert=verify-full", right? That gave
me an idea: wouldn't the control flow be a bit nicer if you just
treated uaCert the same as uaTrust in the big switch statement, but
also enforced that clientcert = clientCertFull in the hunk above?
Then 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.
Would that be better?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.
I'll make sure that the error messages are still handled based on the auth
method and not only depending on the clientcert flag.In the "auth-cert" section there are already a couple of examples
using lower case "cn":will be sent to the client. The <literal>cn</literal> (Common Name)
is a check that the <literal>cn</literal> attribute matches the database
I guess you should do the same, or if there is a good reason to use
upper case "CN" instead then you should change those to match.I see that there are currently no places that use <literal>CN</literal>�
right now.
However, we refer to Certification Authority as CA in most cases (without
the literal tag surrounding it).
The different fields of certificates seem to be abbreviated with capital
letters in most literature; That was my reasoning for capitalizing it in the
first place.
I'm open for suggestions, but in absence of objections I might just
capitalize all occurrences of CN.There is still a place in the documentation where we refer to "1":
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.Maybe we should use "verify-ca" here, though as I noted above I think
it should really "verify-full" and we should simplify the flow.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 behaviour 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.I think we should also document that "1" and "0" are older spellings
still accepted, where the setting names are introduced.+typedef enum ClientCertMode +{ + clientCertOff, + clientCertOn, + clientCertFull +} ClientCertMode; ++1
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?Should I mark this entry as "Needs review" in the commitfest? It seems some
discussion is still needed to get this commited...
I've rebased the patch atop master so it applies and passes 'make
check-world'. I didn't make any other changes.
Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachments:
0001-clientcert_verify_full-v06.patchtext/x-diff; charset=us-asciiDownload
From 4dc30c3af69ced946b2ceab8c6b8e003d3583a00 Mon Sep 17 00:00:00 2001
From: David Fetter <david@fetter.org>
Date: Thu, 2 Aug 2018 23:07:29 -0700
Subject: [PATCH] clientcert_verify_full v06
To: pgsql-hackers@postgresql.org
---
doc/src/sgml/client-auth.sgml | 15 +++++++---
doc/src/sgml/runtime.sgml | 51 +++++++++++++++++++++++++++-------
src/backend/libpq/auth.c | 39 ++++++++++++++++++++++++--
src/backend/libpq/hba.c | 28 ++++++++++++++-----
src/include/libpq/hba.h | 9 +++++-
src/test/ssl/ServerSetup.pm | 10 ++++++-
src/test/ssl/t/001_ssltests.pl | 21 ++++++++++++++
7 files changed, 147 insertions(+), 26 deletions(-)
mode change 100644 => 100755 src/test/ssl/t/001_ssltests.pl
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index c2114021c3..6b261aea92 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 06c0ee7de2..ff6493d70e 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>
+ 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 63f37902e6..0244a21f38 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 1a65ec87bd..2c49b21989 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 5f68f4c666..ce9a692b8d 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 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
old mode 100644
new mode 100755
index 2b875a3c95..2331dc7508
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -358,6 +358,27 @@ switch_server_cert($node, 'server-cn-only', 'root_ca');
$common_connstr =
"user=ssltestuser dbname=certdb sslkey=ssl/client_tmp.key sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
+# 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");
+
test_connect_ok(
$common_connstr,
"sslmode=require sslcert=ssl/client+client_ca.crt",
--
2.17.1
On 03.08.2018 at 08:09 David Fetter wrote:
I've rebased the patch atop master so it applies and passes 'make
check-world'. I didn't make any other changes.Best,
David.
Much appreciated!
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.timmer@uni-muenster.de
https://www.uni-muenster.de/ZIV
Attachments:
clientcert_verify_full_v07.patchtext/x-patch; name=clientcert_verify_full_v07.patchDownload
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 =
Hi,
after talking with Marius:
The last sentence in his mail concerning the progress
suffers from poor translation, and can safely be ignored ;-)
We didn't intend to push anybody.
VlG-(Marius Timmer &) Arne Scheffer
On 25.10.18 15:08, Marius Timmer wrote:
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
--
Arne Scheffer
Webanwendungen
Beratung und Service (mit R. Mersch)
Westfälische Wilhelms-Universität Münster (WWU)
Zentrum für Informationsverarbeitung (ZIV)
Röntgenstraße 7-13
Besucheradresse: Einsteinstraße 60, Raum 104
48149 Münster
+49 251 83 31581
arne.scheffer@uni-muenster.de
https://www.uni-muenster.de/ZIV
Attachments:
On Fri, Oct 26, 2018 at 2:08 AM Marius Timmer
<marius.timmer@uni-muenster.de> wrote:
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.
+ case uaCert:
case uaTrust:
Maybe add a note there that this will be treated as if
clientcert=verify-full below?
+ else if(strcmp(val, "2") == 0
The "1" is needed for backwards compatibility, but is there any need
for "2" as an alternative for "verify-full"?
+# Check that connecting with auth-optionverify-full in pg_hba :
Missing space.
+ "hostssl verifydb yetanotheruser $serverhost/32
trust clientcert=verify-ca\n";
Why did you put "trust" there instead of "$authmethod" like the previous lines?
The tests pass and show the feature working correctly. I think this
is getting close to committable. I see that Magnus has signed up as
committer.
--
Thomas Munro
http://www.enterprisedb.com
Hello Thomas,
thank you for reviewing our patch.
Why did you put "trust" there instead of "$authmethod" like the previous lines?
That is a good question in deed. We changed that accordingly.
The tests pass and show the feature working correctly. I think this
is getting close to committable.
We implemented all other suggestions in the attached patch version...
I see that Magnus has signed up a committer.
... and changed status to "Ready for Committer" :-).
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.timmer@uni-muenster.de
https://www.uni-muenster.de/ZIV
Attachments:
clientcert_verify_full_v07_2.patchtext/x-patch; name=clientcert_verify_full_v07_2.patchDownload
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..39160a55a3 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,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);
@@ -2747,6 +2755,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 +2768,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..57eb9b2c80 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)
@@ -2250,9 +2265,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..8c058cafab 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 $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 2b875a3c95..50a936b561 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-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 =
On Fri, Nov 30, 2018 at 12:24:04PM +1300, Thomas Munro wrote:
The tests pass and show the feature working correctly. I think this
is getting close to committable. I see that Magnus has signed up as
committer.
It has been one month since this message, and the patch is marked as
ready for committer. Magnus, are you planning to look at it?
--
Michael
On Tue, Dec 25, 2018 at 9:08 AM Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Nov 30, 2018 at 12:24:04PM +1300, Thomas Munro wrote:
The tests pass and show the feature working correctly. I think this
is getting close to committable. I see that Magnus has signed up as
committer.It has been one month since this message, and the patch is marked as
ready for committer. Magnus, are you planning to look at it?
Hi!
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!
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
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?
--
Michael
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.timmer@uni-muenster.de
https://www.uni-muenster.de/ZIV
Attachments:
clientcert_verify_full_v07_3.patchtext/x-patch; name=clientcert_verify_full_v07_3.patchDownload
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 =
Hi,
On 2019-02-06 15:19:56 +0000, Timmer, Marius wrote:
On Mon, Jan 04, 2019 at 03:06, Michael Paquier wrote:
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.
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?
- Andres
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.
--
Michael
On Sun, Feb 17, 2019 at 7:50 PM Michael Paquier <michael@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/>
Attachments:
clientcert_verify_full_v07_4.patchtext/x-patch; charset=US-ASCII; name=clientcert_verify_full_v07_4.patchDownload
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";
On Sat, Mar 9, 2019 at 11:03 AM Magnus Hagander <magnus@hagander.net> wrote:
On Sun, Feb 17, 2019 at 7:50 PM Michael Paquier <michael@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)
Patched pushed, with some very minor formatting changes and a pgindent run.
Apologies for the long processing time, and thanks again for the patch!
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>