Bug 48253

Summary: Tomcat Native patch - adding dynamic locking callbacks for openssl engines
Product: Tomcat Native Reporter: Daniel Ruggeri <DRuggeri>
Component: LibraryAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: minor CC: jfclere
Priority: P2 Keywords: NeedsReleaseNote, PatchAvailable
Version: 1.1.20   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: Adds dynamic locking callbacks to TCNative for use by openssl engines
Updated patch - moves proposed changes from ssl_private.h to ssl.c

Description Daniel Ruggeri 2009-11-20 13:40:32 UTC
Created attachment 24576 [details]
Adds dynamic locking callbacks to TCNative for use by openssl engines

Hello;
   The attached patch adds dynamic locking callbacks needed by certain engines in OpenSSL (chil, specifically). Most of this code was poached from HTTPD 2.2.x mod_ssl (ssl_util.c). The notable differences to TCNative after applying the patch are that the call to ssl_thread_setup had to be moved before the engine is initialized since the callbacks must be set before engine init, and the dynamic callback functions were added to ssl_thread_setup.

The issue:
When utilizing an OpenSSL engine that requires the locking callback, no locks will be found causing the vendor's native library to exit. When using chil, this only happens when an assertion fails and detects that multiple threads are active, but no upcalls are provided. I am unsure what other engines require this functionality.

The solution:
Add the callback functions to use APR locks. Register them with OpenSSL via the CRYPTO_set_dyn..... functions.


Note:
This is the first TCNative patch I have submitted and was informed that there should be a xdocs/miscellaneous/changelog.xml file. This patch is against the tomcat-native-1.1.16-src.tar.gz file which does not include such a document. In any event, I think the CHANGELOG.txt entry should read:
Improvement: Add dynamic locking callbacks for openssl engines (druggeri)
Comment 1 Mladen Turk 2010-03-26 14:41:51 UTC
Patch looks good except that there shouldn't be any
static ... function/type declarations inside .h files.

Just copy that from ssl_private.h directly to the ssl.c
and only for the forward declarations.
Comment 2 Daniel Ruggeri 2010-03-29 14:07:12 UTC
(In reply to comment #1)
> Patch looks good except that there shouldn't be any
> static ... function/type declarations inside .h files.
> 
> Just copy that from ssl_private.h directly to the ssl.c
> and only for the forward declarations.

Thank you for your time, Mladen. I have updated the patch against the latest source distribution (1.1.20) and moved the proposed content of ssl_private.h to the lines immediately preceding the proposed functions in ssl.c. Please let me know if I misunderstood your directive, or if things are OK. Should I have also moved the CRYPTO_dynlock_value struct to ssl_private.h?
Comment 3 Daniel Ruggeri 2010-03-29 14:11:08 UTC
Created attachment 25206 [details]
Updated patch - moves proposed changes from ssl_private.h to ssl.c
Comment 4 jfclere 2011-01-07 03:01:23 UTC
Fixed by svn commit: r1055907