On 12 Jun 2018, at 22:11, yla...@apache.org wrote: > Author: ylavic > Date: Tue Jun 12 20:11:09 2018 > New Revision: 1833421 > > URL: http://svn.apache.org/viewvc?rev=1833421&view=rev > Log: > apr_crypto: follow up to r1833359. > > Link underlying crypto libraries (openssl, nss, and commoncrypto) with libapr > when the corresponding --with is configured, and allow to initialize, > terminate > or check whether initialized with respectively them with > apr_crypto_lib_init(), > apr_crypto_lib_term() or apr_crypto_lib_is_initialized(). > > This allows for users to control the (un)initialization of those libraries, > notably when they are also used independently in the user code and that doing > this multiple times can cause leaks or unexpected behaviour. > > The initialization code is moved from > "apr_crypto_{openssl,nss,commoncrypto}.c" > where previously loaded dynamically (DSO) to "apr_crypto_internal.c" which is > linked with libapr. > > Now apr_crypto_prng_init() can make sure the underlying crypto lib is ready.
Having looked at this in more detail, I am -1 on this change on the following basis: - We create a hard link between our crypto libraries and APR itself, which both breaks and renders pointless the existing DSO mechanism, and makes APR significantly more expensive and less attractive to use. - This doesn’t fix the problem for OpenSSL. The problem is related to legacy OpenSSL’s inability to de-init and re-init, which google shows me is a universal problem suffered by other libraries, not just us. The fix appears to be to not de-init the library in the DSO, but I;ve asked the openssl user’s list to be sure. In other words, this attempt to init is unnecessary, we should actually not de-init. - This change affects NSS, which doesn’t suffer from this problem as NSS supports proper reference counting and multiple inits. Touching the NSS code is unnecessary. - This problem appears to have been fixed in openssl v1.1.0, where no initialisation is required at all. I have asked on openssl-users for clarification on this. https://www.openssl.org/docs/man1.1.0/crypto/OPENSSL_init_crypto.html To be clear, I am only referring to this change here, this is unrelated to the PRNG patch which is a separate discussion. Regards, Graham —
