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

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?