[asterisk-dev] [Code Review]: Wrap OpenSSL library initialization to make it safe for loaded modules to also use OpenSSL.

[Kevin Fleming]

> On Jan. 18, 2012, 6:30 p.m., paravoid wrote:
> > 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...

I agree that the proposed solution is not elegant, but it has one important benefit: it is 'fail-safe'. With this solution in place, it does not matter whether a module loaded into Asterisk (or any libraries it links to) brings in OpenSSL or not, or whether they offer optional initialization or not. I would also be fine with making it optional, and I'll update the patch to do that.

As far as improving libraries that are used to offer optional initialization, that's certainly a worthwhile goal, and it would be beneficial to have someone audit all the modules provided with Asterisk to learn which ones use OpenSSL or use libraries that (even optionally) use OpenSSL. With a comprehensive list of libraries, we could investigate each one to determine whether they can participate in this solution or not.

On a quick glance through the tree (Asterisk trunk), the following modules *could* have indirect usage of OpenSSL: res_config_mysql/app_mysql/cdr_mysql, cdr_radius/cel_radius, cdr_tds/cel_tds, res_calendar/submodules, res_curl, res_odbc, res_config_ldap, cdr_pgsql/cel_pgsql/res_config_pgsql, res_jabber, res_snmp. Note that there isn't any single "Postgres module", there are three of them (it hasn't been converted to have a res_pgsql master module like we have for ODBC, CURL and Jabber).

On a final note, it appears that the PostreSQL functions you referred to are fairly new, and they don't actually document what they control (other than 'initialization', which is pretty vague). In our case, not only do we intialize libssl and libcrypto, but we also provide callback functions for those libraries to obtain thread identifiers, and we setup a mutex lock pool so that usage of the libraries will be thread-safe. Finally, we disallow any of the *de-initialization* functions from being called, so that we can ensure that the loaded libraries continue to function properly, even if an Asterisk module gets unloaded. For all libraries brought in through Asterisk modules that have the ability to use OpenSSL, they would have to provide a guarantee that they don't do *any* of this before it would be safe to stop Asterisk from doing it (or the result is the situation we are in today).


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1006/#review5215
-----------------------------------------------------------


On Jan. 18, 2012, 3:03 p.m., Kevin Fleming wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1006/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2012, 3:03 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   /trunk/Makefile.moddir_rules 351448 
>   /trunk/build_tools/make_defaults_h 351448 
>   /trunk/build_tools/mkpkgconfig 351448 
>   /trunk/configure UNKNOWN 
>   /trunk/configure.ac 351448 
>   /trunk/include/asterisk.h 351448 
>   /trunk/include/asterisk/optional_api.h 351448 
>   /trunk/main/Makefile 351448 
>   /trunk/main/ssl.c 351409 
>   /trunk/main/ssl.c 351448 
>   /trunk/makeopts.in 351448 
>   /trunk/Makefile 351448 
> 
> Diff: https://reviewboard.asterisk.org/r/1006/diff
> 
> 
> Testing
> -------
> 
> 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.
> 
> 
> Thanks,
> 
> Kevin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120120/16428217/attachment-0001.htm>