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. > >
