Re: [PATCH] Sending root certificate for validation

On 06/10/2013 08:06 PM, Alex Rousskov wrote:
> On 06/10/2013 10:31 AM, Tsantilas Christos wrote:
>> On 06/10/2013 03:16 PM, Amos Jeffries wrote:
>>> On 8/06/2013 4:20 a.m., Tsantilas Christos wrote:
>>>> This patch modify squid cert validation subsystem to sent to cert
>>>> validator helper the complete certificates chain, not only the
>>>> certificates sent by web server. This is may not be possible in all
>>>> cases, for example in cases where the root certificate is not stored
>>>> locally.
>
>
>>> in globals.h:
>>> * please do not add any new entries in globals.h - we are trying to
>>> remove things from there.
>
>> I am suggesting to leave it here for this patch, and then with a
>> separate patch remove all similar entries from global.h to ssl/support.h
>> file (or the opposite, move from global.h similar entries and then apply
>> this patch)
>
>
> If there are already similar entries in global.h, then I agree that the
> Christos suggestion is the best way forward. It allows this change to
> stay focused and consistent while also implementing a desirable cleanup.

Yes there are a number of similar entries, the indexes of attached
objects to SSL.

>
>
>
>>> * can you please also use the wrapper types provided in
>>> src/ssl/gadgets.h. ie X509_STACK_Pointer for things like STACK_OF(X509)
>>> and avoiding use of raw pointers to OpenSSL internal types?
>>
>> Amos, it will not help anywhere, and also my sense is that it will make
>> the code worst ...
>
>
> Various SSL _Pointer types are needed to free unused SSL objects (they
> are all TidyPointers) and also unlock some of the locked SSL objects
> after use. They are _not_ appropriate for temporary no-storage access to
> objects stored by OpenSSL because they do not add safety but will
> accidentally _delete_ those internal objects if we are not very careful.
>
> Amos, are any of your Pointer concerns related to objects that Christos
> stores or creates? If not (i.e., all uses are temporary convenience
> variables), we should not use TidyPointers for them.
>
>
> Christos, the following code does _not_ create a new STACK_OF(X509)
> object and does _not_ increment the lock count on an old object, right?

Right.
The STACK_OF does not support locking.

>
>> + if (!peerCerts)
>> + peerCerts = SSL_get_peer_cert_chain(vcert.ssl);
>> +
>
>
> Thank you,
>
> Alex.
>
>
Received on Mon Jun 10 2013 - 17:51:02 MDT