Re: "cert" + clientcert=verify-ca in pg_hba.conf?

At Mon, 24 Aug 2020 20:01:26 -0400, Bruce Momjian <bruce@momjian.us> wrote in 
> 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.

Thanks.

     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.

I read it as "it can be specified (without an error), but actually
does nothing". If it is the correct reading, I prefer to mention that
incompatible values cause an error.

> > 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.

I don't suggest changing the current behavior. I'm saying it is the
way it is working and we should correctly error-out that since it
doesn't work as specified.

auth.c:608
    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
    }

regard.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center