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

On Sat, Apr 14, 2018 at 3:48 AM, Julian Markwort
<julian(dot)markwort(at)uni-muenster(dot)de> wrote:
> [a patch]

Hello Julian,

Could you please post a rebased patch?

I haven't reviewed or tested any code yet, but here's some proof-reading:

+ This behaviour is similar to the cert autentication method

"behavior" (our manual is written in en_US, "cd doc/src/sgml ; git
grep behavior | wc -l" -> 895, "git grep behaviour" -> 0).

<literal>cert</literal>

"authentication"

+ chain, but it will also check whether the username or it's
+ mapping match the common name (CN) of the provided certificate.

"its"

"matches"

+ Note that certificate chain validation is always ensured when
+ <literal>cert</literal> authentication method is used

extra space

when *the* ...

+ In this case, the <literal>CN</literal> (nommon name) provided in

"common name"

+ <literal>CN</literal> (Common Name) in the certificate matches

"common"? (why a capital letter here?)

This line isn't modified by your patch, but I saw it while in
proof-reading mode:

*err_msg = "clientcert can not be set to 0 when using \"cert\"
authentication";

I think "can not" is usually written "cannot"?

> slightly offtopic opinion:
> While creating the test cases, I stumbled upon the problem of missing
> depencies to run the tests...
> It's complicated enough that the binaries used by these perl tests are not
> named similar to the packages which provide them (the 'prove' binary is
> supplied by 'Test-Harness'), so maybe in the interest of providing a lower
> entry-barrier to running these tests, we could give a more detailed error
> message in the configure script, when using --enable-tap-tests ?

Yeah. The packages to install depend on your operating system, and in
some cases (macOS, Windows?) which bolt-on package thingamajig you
use, though. Perhaps the READMEs could be improved with details for
systems we have reports about (like the recently added "Requirements"
section of src/test/ldap/README).

--
Thomas Munro
http://www.enterprisedb.com