Re: patch: Client certificate requirements

Alex Hunsaker wrote:
> On Sat, Nov 15, 2008 at 17:39, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> "Alex Hunsaker" <badalex(at)gmail(dot)com> writes:
>>> Err that really should be ereport(FATAL,
>> I don't think that's a particularly user-friendly design.
>>
>> The behavior I'd expect to see is
>>
>> 1. Root cert file not there: issue existing LOG message. Maybe the user is
>> expecting client cert verification, and maybe he isn't, but it is a good
>> idea to put out the LOG message just to make sure he knows what will
>> happen.
>
> Right, pre patch I agree with you. The problem I noticed with the
> patch is we say nothing if its there and you can't read it. The whole
> point of the patch is to give you the option of forcing client certs.
> So why LOG every server start up just because I turned on ssl. If
> pg_hba is setup right you'll get the appropriate error message when
> the client connects.

Yeah, I agree with Alex on this. It makes sense not to log it in that case.

If it's there and we fail, we must log the details, but if it's just not
there, it can be logged at a later stage.

>> 2. Root cert file present but we fail to load it: FATAL is probably okay
>> here, but not with that hint message.
>
> Err, I was just trying to be congruent with HEAD. Currently that's
> the message you get if we could not "read" the root cert. (as a LOG,
> not FATAL). Should just drop the hint and keep the FATAL for this
> case?

Yes, I think so.

New version of the patch attached.

> Also we check that the private key is at least 0600, should we be
> doing the same for the root cert?

No need. The certificate is public information. The first thing we do on
an SSL connection is to send the thing to the client anyway.

We *could* check that it's not writable by anybody else - but do we
check that for our datafiles which contain the actual passwords and
such? If not, that would just be strange to do here, really..

//Magnus