On Thu, Jul 16, 2020 at 09:30:12AM +0900, Kyotaro Horiguchi wrote:
> Hello.
> 
> The "Certificate Authentication" section in the doc for PG12 and later
> describes the relation ship with clientcert as follows.
> 
> > In a pg_hba.conf record specifying certificate authentication, the
> > authentication option clientcert is assumed to be verify-ca or
> > verify-full, and it cannot be turned off since a client certificate
> > is necessary for this method. What the cert method adds to the basic
> > clientcert certificate validity test is a check that the cn
> > attribute matches the database user name.
> 
> In reality, cert method is assumed as "verify-full" and does not add
> anything to verify-full and cannot be degraded or turned off. It seems
> to be a mistake on rewriting it when clientcert was changed to accept
> verify-ca/full at PG12.

Agreed.  I was able to test this patch and it does what you explained. 
I have slightly adjusted the doc part of the patch, attached.

> Related to that, pg_hba.conf accepts the combination of "cert" method
> and the option clientcert="verify-ca" but it is ignored. We should
> reject that combination the same way with "cert"+"no-verify".

Are you saying we should _require_ clientcert=verify-full when 'cert'
authentication is used?  I don't see the point of that --- I just
updated the docs to say doing so was duplicate behavior.

-- 
  Bruce Momjian  <br...@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee

diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 5cd88b462d..7bf12765cb 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -2043,12 +2043,9 @@ host ... radius radiusservers="server1,server2" radiussecrets="""secret one"",""
 
    <para>
     In a <filename>pg_hba.conf</filename> record specifying certificate
-    authentication, the authentication option <literal>clientcert</literal> is
-    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.
+    authentication, the only valid value for <literal>clientcert</literal>
+    is <literal>verify-full</literal>, and this has no affect since it
+    just duplicates <literal>client</literal> authentication's behavior.
    </para>
   </sect1>
 
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 9d63830553..18ef385405 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -1711,6 +1711,16 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
 		if (strcmp(val, "1") == 0
 			|| strcmp(val, "verify-ca") == 0)
 		{
+			if (hbaline->auth_method == uaCert)
+			{
+				ereport(elevel,
+						(errcode(ERRCODE_CONFIG_FILE_ERROR),
+						 errmsg("clientcert can not be set to \"verify-ca\" when using \"cert\" authentication"),
+						 errcontext("line %d of configuration file \"%s\"",
+									line_num, HbaFileName)));
+				*err_msg = "clientcert can not be set to \"verify-ca\" when using \"cert\" authentication";
+				return false;
+			}
 			hbaline->clientcert = clientCertCA;
 		}
 		else if (strcmp(val, "verify-full") == 0)

Reply via email to