Re: [PATCH] pg_hba.conf : new auth option : clientcert=verify-full

On Fri, Mar 9, 2018 at 2:11 PM, Julian Markwort <
julian(dot)markwort(at)uni-muenster(dot)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/>