Re: [PATCH] Sending root certificate for validation

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.

>> * 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?

> + if (!peerCerts)
> + peerCerts = SSL_get_peer_cert_chain(vcert.ssl);
> +

Thank you,

Alex.
Received on Mon Jun 10 2013 - 17:06:26 MDT