Message-ID: <86a05828-6396-314a-58c8-45e50eb2cfb8@pgmasters.net>
Date: Tue, 18 Jan 2022 16:44:29 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.5.0
Subject: Re: Allow root ownership of client certificate key
Content-Language: en-US
To: pgsql-hackers@lists.postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>,
 Stephen Frost <sfrost@snowman.net>
References: <f4b7bc55-97ac-9e69-7398-335e212f7743@pgmasters.net>
 <20211108190405.GL20998@tamriel.snowman.net>
 <716ece6d-7500-8665-05f0-488e75c5525d@pgmasters.net>
 <377058.1642538462@sss.pgh.pa.us>
From: David Steele <david@pgmasters.net>
In-Reply-To: <377058.1642538462@sss.pgh.pa.us>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/86a05828-6396-314a-58c8-45e50eb2cfb8%40pgmasters.net>
Precedence: bulk

On 1/18/22 15:41, Tom Lane wrote:
> David Steele <david@pgmasters.net> writes:
> 
> I took a quick look at this and agree with the proposed behavior
> change, but also with your self-criticisms:
> 
>> We may want to do the same on the server side to make the code blocks
>> look more similar.
>>
>> Also, on the server side the S_ISREG() check gets its own error and that
>> might be a good idea on the client side as well. As it is, the error
>> message on the client is going to be pretty confusing in this case.
> 
> Particularly, I think the S_ISREG check should happen before any
> ownership/permissions checks; it just seems saner that way.

I was worried about doing too much refactoring in this commit since I 
have hopes and dreams of it being back-patched. But I'll go ahead and do 
that and if any part of this can be back-patched we'll consider that 
separately.

> The only other nitpick I have is that I'd make the cross-references be
> to the two file names, ie like "Note that similar checks are performed
> in fe-secure-openssl.c ..."  References to the specific functions seem
> likely to bit-rot in the face of future code rearrangements.
> I suppose filename references could become obsolete too, but it
> seems less likely.

It's true that functions are more likely to be renamed, but when I 
rename a function I then search for all the places where it is used so I 
can update them. If the function name appears in a comment that gets 
updated as well.

If you would still prefer filenames I have no strong argument against 
that, just wanted to explain my logic.

Regards,
-David