Re: BUG #4869: No proper initialization of OpenSSL-Engine in libpq

Lars Kanis wrote:
>>> The following patch solves the problem:
>> This looks good in generael to me. I remember looking at the engine code
>> wondering why we didn't do that, but since I don't have a good
>> environment to test that part in, I forgot about it :(
>>
>> Shouldn't there be an ENGINE_free() in the error path of ENGINE_init()?
> In the patch it is already there, isn't it?

Eh. So it is. Don't know where I got the idea that it didn't :-)

>> Should we not also call ENGINE_finish() and ENGINE_free() in the success
>> path of this code? Your patch adds it to the case where we didn't get
>> the private key, but what if we did? I assume they should also go
>> outside the error path, per the attached patch - or will that break
>> their usage?
> That's right. I thought about it, but I don't know where the right place is.
>
>> Can you test that and verify that it doesn't break for you?
> It breaks with Segmentation fault in my smartcard-based setup. The pkey-handle
> is all we have from the engine, when client_cert_cb() is finished. Obviously
> the ref-counting of openssl does not take the pkey-handle into account, so we
> need to keep the engine_ptr for later freeing.

So we need to keep the engine initialized during this time? Ugh. We
don't currently carry around the engine pointer. I guess we have to.

> close_SSL() should be the right place for ENGINE_finish() and ENGINE_free() ?

Yup.

How about the attached patch? Does it work for you?

A question from that then, for others, is it Ok to add a field to the
PGconn structure during RC? :-) It's only in libpq-int.h, but? Comments?
Tom, perhaps?

--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/