Wrap OpenSSL library initialization to make it safe for loaded modules to also use OpenSSL.
Review Request #1006 - Created Nov. 11, 2010 and submitted
During the devcon after AstriCon 2010, we got a report that using PostgreSQL from within Asterisk, when the PostgreSQL connections are configured to use SSL/TLS to connect to the database server, can cause random crashes and other bizarre behavior. The reporter said this was known to be an issue with some other packages as well (notably Kamailio), and had to do with both Asterisk and the PostgreSQL libraries assuming they "owned" the OpenSSL libraries in the process' memory space, and thus calling initialization code twice (or worse). This patch addresses this problem by using dynamic linker functionality to *wrap* the real OpenSSL initialization functions (and some other dangerous ones) with versions that don't actually do anything, and then calling the real ones only *one* time during Asterisk startup. To make this work, the SSL functionality that is normally built into the main Asterisk binary now must be built into a dynamic library (libasteriskssl.so), which is installed into the standard dynamic library location on the system (this is *not* an Asterisk loadable module, just a regular dynamic library). As part of this patch, the usage of ASTLIBDIR throughout the build system to refer to the directory where Asterisk loadable modules are installed was changed to ASTMODDIR (which matches how it is referred to in the source code and in asterisk.conf), and a new definition of ASTLIBDIR was created to point to the system's dynamic library directory.
Compiles and runs on Linux x86-64 with no apparent change in behavior. The Makefile bits to install libasteriskssl.so in the right place will probably have to be checked by Solaris, Darwin and *BSD users to get them right.
Posted (Nov. 16, 2010, 8:35 a.m.)
This isn't really a message to Kevin, but more to everyone else: For those that have seen this problem before, can someone please open a report on issues.asterisk.org that discusses a situation where you have hit the problem? Also, could you please test this to verify that it solves the problems you have seen?
Review request changed
Updated (Jan. 18, 2012, 9:03 a.m.)
Update to current trunk, and ensure that the main Asterisk binary does not need to be linked directly to libssl or libcrypto at all (so that we can be sure that any of their functions used by code in the Asterisk binary will be directed through libasteriskssl.so). Unfortunately this does mean that any future changes to main/tcptls.c (or any other part of the main Asterisk binary) that involve usage of additional functions from libssl or libcrypto will require changes in libasteriskssl.c in order to provide 'wrapped' versions of those functions, but this will be caught at compile/link time, so it will be quite noticeable. Note: This patch renames main/ssl.c to main/libasteriskssl.c and also adds/changes a lot of code in that file; ReviewBoard doesn't know how to properly display this, so main/ssl.c shows up twice. The first instance in the review contains all the actual changes (and represents what will be main/libasteriskssl.c in the final tree), the second instance can be ignored.
Posted (Jan. 18, 2012, 12:30 p.m.)
This is a real problem and Asterisk is not the first one to hit it. It's for exactly this reason why e.g. Postgres offers PQinitSSL()/PQinitOpenSSL() to tell the PQ library that the application is going to initialize OpenSSL rather than PQ. Personally, I find the proposed solution with the shared library quite ugly and hackish. I'd prefer solving this by disabling OpenSSL initialization by the libraries that Asterisk uses (I can think of Postgres and curl; shouldn't be that many). Most should already offer such a way (like Postgres) since this is a common problem. If they don't, submit bugs and/or patches to them to solve this for the rest of the ecosystem :) If you insist with going with the libasteriskssl.so way, please at least a) add PQinitSSL(0) to the Postgres module anyway (shouldn't hurt), b) make the libasteriskssl.so feature optional via e.g. a ./configure option, so that people can disable it...
Posted (Jan. 19, 2012, 6:47 a.m.)
This changeset fails to compile on Snow Leopard. The problem is due to the prototypes not matching: [CCi] ssl.i -> ssl.o ssl.c:159: error: conflicting types for 'SSL_CTX_new' /usr/include/openssl/ssl.h:1313: error: previous declaration of 'SSL_CTX_new' was here ssl.c:271: error: conflicting types for 'SSLv23_client_method' /usr/include/openssl/ssl.h:1484: error: previous declaration of 'SSLv23_client_method' was here ssl.c:278: error: conflicting types for 'SSLv23_server_method' /usr/include/openssl/ssl.h:1483: error: previous declaration of 'SSLv23_server_method' was here ssl.c:285: error: conflicting types for 'SSLv3_client_method' /usr/include/openssl/ssl.h:1480: error: previous declaration of 'SSLv3_client_method' was here ssl.c:292: error: conflicting types for 'TLSv1_client_method' /usr/include/openssl/ssl.h:1488: error: previous declaration of 'TLSv1_client_method' was here make: *** [ssl.o] Error 1 make: *** [main] Error 2 make: *** [_cleantest_all] Error 2
Posted (Jan. 19, 2012, 10:23 a.m.)
If you want to build this code, please pull it from: http://svn.asterisk.org/svn/asterisk/team/libasteriskssl Grabbing the patch from ReviewBoard won't result in a buildable and linkable tree, due to limitations of what the patch and ReviewBoard can handle.
Review request changed
Updated (Jan. 25, 2012, 10:26 a.m.)
Updated to incorporate feedback from various people: * Building 'libasteriskssl' is now optional (enabled by default). It can be disabled using the "--disable-libasteriskssl" argument to the configure script. * On traditional Unix-type platforms, libasteriskssl.so is now versioned, for ABI safety. * Support for building libasterisskl.dylib on Mac OS/X (Darwin) is now included (contributed by Tilghman Lesher). * After researching how the static and dynamic linkers work together (specifically library load order and symbol resolution order), libasteriskssl.c was greatly simplified. It has returned to only wrapping the OpenSSL initialization/shutdown functions from libssl and libcrypto (there is no need to wrap other functions used by code in the main Asterisk binary, as that binary is once again also linked to libssl and libcrypto directly). * Installation (and uninstallation) of the main Asterisk binary and libasteriskssl has been relocated from the top-level Makefile to the Makefile in 'main' (where they are built).