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

From: Julian Markwort <julian(dot)markwort(at)uni-muenster(dot)de>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, arne(dot)scheffer(at)uni-muenster(dot)de
Subject: Re: [PATCH] pg_hba.conf : new auth option : clientcert=verify-full
Date: 2018-04-10 12:10:32
Message-ID: 1523362232.8971.14.camel@uni-muenster.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2018-04-10 12:11:55 Re: [HACKERS] path toward faster partition pruning
Previous Message David Rowley 2018-04-10 12:02:55 Re: [HACKERS] path toward faster partition pruning