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

From: Julian Markwort <julian(dot)markwort(at)uni-muenster(dot)de>
To: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, 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-07-14 12:47:37
Message-ID: permail-201807141247373cc687ad000077fb-j_mark05@message-id.uni-muenster.de
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Hi Thomas,

here's a rebased patch, with your observations corrected.

Thomas Munro wrote on 2018-07-13:
> + 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?)

I've resorted to "<literal>CN</literal> (Common Name)" on all occurences in this patch now.

Also, while writing this part of the docs, I tried to stay below 80 characters, but I've exceeded it in some places.
There are several other places (several in the .sgml files touched by this patch), where 80 characters are exceeded; How close should one adhere to that limit nowadays?

> 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"?

I'm not sure about can not, cannot, can't... There are 56, respectively 12697, and 2024 occurrences in master right now.
We could touch those lines now and change them to the more common cannot, or we can leave it as is...

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

That would be nice, however I could only provide the package names for Fedora right now...
Would It make sense to add those on their own?
Or should somebody (maybe myself, when I'm less busy) gather those for most supported systems and commit them as a whole?

kind regards
Julian

Attachment Content-Type Size
clientcert_verify_full_v05.patch text/x-patch 13.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-07-14 14:29:23 Re: CVE-2017-7484-induced bugs, or, btree cmp functions are not leakproof?
Previous Message Michael Paquier 2018-07-14 12:13:45 Re: missing toast table for pg_policy