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

On Sun, Apr 1, 2018 at 6:07 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:

> On Sun, Apr 1, 2018 at 6:01 PM, Julian Markwort <
> julian(dot)markwort(at)uni-muenster(dot)de> wrote:
>
>> On 1. of April 2018 17:46:38 MESZ wrote Magnus Hagander <
>> magnus(at)hagander(dot)net>:
>>
>> >I assume this is a patch that's intended to be applied on top of the
>> >previous patch? If so, please submit the complete pach to make sure the
>> >correct combination ends up actually being reviewed.
>>
>> The v02.patch attached to my last mail contains both source and
>> documentation changes.
>>
>
> Hmm. I think I may have been looking at the wrong file. Sorry!
>
>
> >Keeping patches as short as possible is not a good thing itself. The
>> >important part is that the resulting code and documentation is the best
>> >possible. Sometimes you might want to turn it into two patches
>> >submitted at
>> >the same time if one is clearly just reorganisation, but avoiding code
>> >restructure just to keep the lines of patch down is not helpful.
>>
>> I think I've made the right compromises regarding readability of the
>> documentation in my patch then.
>>
>> A happy Easter, passover, or Sunday to you
>>
>
> You, too!
>
> (I shall return to reviewing it after the holidays are over)
>
>
I've been through this one again.

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.

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.

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.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>