You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2019/06/24 06:46:20 UTC

Re: svn commit: r1861947 - in /httpd/httpd/trunk: modules/filters/mod_crypto.c modules/session/mod_session_crypto.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_init.c modules/ssl/ssl_private.h server/core.c


On 06/23/2019 11:10 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Sun Jun 23 21:10:23 2019
> New Revision: 1861947
> 
> URL: http://svn.apache.org/viewvc?rev=1861947&view=rev
> Log:
> After reinstatement of DSO support in APR/APR-util, revert r1837437,
> r1837435, r1834553, r1833598, r1833452, r1833383, r1833368.
> 
> Undoes the following:
> 
> mod_ssl: OpenSSL now initializes fully through APR, use that.
> 
> mod_ssl: build with LibreSSL.
> 
> LibreSSL seems to be openssl-1.1 API compatible only in version 2.8 (master).
> So use that for MODSSL_USE_OPENSSL_PRE_1_1_API instead of 2.7, the two 2.7
> compatibility-exceptions are handled explicitely but overall it's simpler.
> 
> Regarding CRYPTO_malloc_init vs OPENSSL_malloc_init, libreSSL uses none, the
> former used to be a no-op but depends is LIBRESSL_INTERNAL in latest versions,
> while the latter has never been (and will never be) defined. So don't call any
> with LibreSSL.
> 
> Follow up to r1833368: share openssl between modules.
> 
> Both libapr[-util], the core PRNG, mod_ssl, mod_crypto and mod_session_crypto
> can use the same crypto library (e.g. openssl), use the new APR crypto loading
> API so that they can work together and initialize/terminate the lib either once
> for all or on demand and reusable by the others.
> 
> Follow up to r1833368: apr_crypto_prng_after_fork() now used a PID.
> 
> Make use of the new apr_crypto_rng API if available.
> 
> Modified:
>     httpd/httpd/trunk/modules/filters/mod_crypto.c
>     httpd/httpd/trunk/modules/session/mod_session_crypto.c
>     httpd/httpd/trunk/modules/ssl/mod_ssl.c
>     httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
>     httpd/httpd/trunk/modules/ssl/ssl_private.h
>     httpd/httpd/trunk/server/core.c
> 

> Modified: httpd/httpd/trunk/modules/ssl/mod_ssl.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/mod_ssl.c?rev=1861947&r1=1861946&r2=1861947&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
> +++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Sun Jun 23 21:10:23 2019
                                apr_pool_t *plog,
> @@ -406,58 +394,29 @@ static int ssl_hook_pre_config(apr_pool_
>  #endif
>      modssl_running_statically = modssl_is_prelinked();
>  
> -#if USE_APR_CRYPTO_LIB_INIT
> -    {
> -        /* When mod_ssl is builtin, no need to unload openssl on restart,
> -         * so use pglobal.
> -         */
> -        apr_pool_t *p = modssl_running_statically ? ap_pglobal : pconf;
> -        apr_status_t rv = apr_crypto_lib_init("openssl", NULL, NULL, p);
> -        if (rv != APR_SUCCESS && rv != APR_EREINIT) {
> -            ap_log_perror(APLOG_MARK, APLOG_ERR, rv, pconf, APLOGNO(10155)
> -                          "mod_ssl: can't initialize OpenSSL library");
> -            return !OK;
> -        }
> -    }
> -#else /* USE_APR_CRYPTO_LIB_INIT */
> -    {
> -        /* We must register the library in full, to ensure our configuration
> -         * code can successfully test the SSL environment.
> -         */
> -/* Both undefined (or no-op) with LibreSSL */
> -#if !defined(LIBRESSL_VERSION_NUMBER)
> -#if MODSSL_USE_OPENSSL_PRE_1_1_API
> -        CRYPTO_malloc_init();
> -#else
> -        OPENSSL_malloc_init();
> -#endif
> -#endif
> -        ERR_load_crypto_strings();
> -#if HAVE_ENGINE_LOAD_BUILTIN_ENGINES
> -        ENGINE_load_builtin_engines();
> -#endif
> -        OpenSSL_add_all_algorithms();
> -        OPENSSL_load_builtin_modules();
> -
> -        SSL_load_error_strings();
> -        SSL_library_init();
> -
> -        /*
> -         * Let us cleanup the ssl library when the module is unloaded
> -         */
> -        apr_pool_cleanup_register(pconf, NULL, ssl_cleanup_pre_config,
> -                                               apr_pool_cleanup_null);
> -    }
> -
> -#if APR_HAS_THREADS && MODSSL_USE_OPENSSL_PRE_1_1_API
>      /* Some OpenSSL internals are allocated per-thread, make sure they
> -     * are associated to the/our same thread-id until cleaned up. Then
> -     * initialize all the thread locking stuff needed by the lib.
> +     * are associated to the/our same thread-id until cleaned up.
>       */
> +#if APR_HAS_THREADS && MODSSL_USE_OPENSSL_PRE_1_1_API
>      ssl_util_thread_id_setup(pconf);
> -    ssl_util_thread_setup(pconf);
>  #endif
> -#endif /* USE_APR_CRYPTO_LIB_INIT */
> +
> +    /* We must register the library in full, to ensure our configuration
> +     * code can successfully test the SSL environment.
> +     */
> +#if MODSSL_USE_OPENSSL_PRE_1_1_API || defined(LIBRESSL_VERSION_NUMBER)
> +    (void)CRYPTO_malloc_init();
> +#else
> +    OPENSSL_malloc_init();
> +#endif
> +    ERR_load_crypto_strings();
> +    SSL_load_error_strings();
> +    SSL_library_init();
> +#if HAVE_ENGINE_LOAD_BUILTIN_ENGINES
> +    ENGINE_load_builtin_engines();
> +#endif
> +    OpenSSL_add_all_algorithms();
> +    OPENSSL_load_builtin_modules();

Is there any reason why the order of the above 6 library calls has changed?
The order doesn't matter?

>  
>      if (OBJ_txt2nid("id-on-dnsSRV") == NID_undef) {
>          (void)OBJ_create("1.3.6.1.5.5.7.8.7", "id-on-dnsSRV",
> @@ -467,6 +426,12 @@ static int ssl_hook_pre_config(apr_pool_
>      /* Start w/o errors (e.g. OBJ_txt2nid() above) */
>      ERR_clear_error();
>  
> +    /*
> +     * Let us cleanup the ssl library when the module is unloaded
> +     */
> +    apr_pool_cleanup_register(pconf, NULL, ssl_cleanup_pre_config,
> +                                           apr_pool_cleanup_null);
> +
>      /* Register us to handle mod_log_config %c/%x variables */
>      ssl_var_log_config_register(pconf);
>  
> 
> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_init.c?rev=1861947&r1=1861946&r2=1861947&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_init.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_init.c Sun Jun 23 21:10:23 2019

> @@ -302,6 +301,10 @@ apr_status_t ssl_init_Module(apr_pool_t
>  #endif
>      }
>  
> +#if APR_HAS_THREADS && MODSSL_USE_OPENSSL_PRE_1_1_API
> +    ssl_util_thread_setup(p);
> +#endif
> +

Why moving it here and not leaving it in the old place?

>      /*
>       * SSL external crypto device ("engine") support
>       */


Regards

Rüdiger


Re: svn commit: r1861947 - in /httpd/httpd/trunk: modules/filters/mod_crypto.c modules/session/mod_session_crypto.c modules/ssl/mod_ssl.c modules/ssl/ssl_engine_init.c modules/ssl/ssl_private.h server/core.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 24 Jun 2019, at 08:46, Ruediger Pluem <rp...@apache.org> wrote:

>> URL: http://svn.apache.org/viewvc?rev=1861947&view=rev
>> Log:
>> After reinstatement of DSO support in APR/APR-util, revert r1837437,
>> r1837435, r1834553, r1833598, r1833452, r1833383, r1833368.

>> -#else /* USE_APR_CRYPTO_LIB_INIT */
>> -    {
>> -        /* We must register the library in full, to ensure our configuration
>> -         * code can successfully test the SSL environment.
>> -         */
>> -/* Both undefined (or no-op) with LibreSSL */
>> -#if !defined(LIBRESSL_VERSION_NUMBER)
>> -#if MODSSL_USE_OPENSSL_PRE_1_1_API
>> -        CRYPTO_malloc_init();
>> -#else
>> -        OPENSSL_malloc_init();
>> -#endif
>> -#endif
>> -        ERR_load_crypto_strings();
>> -#if HAVE_ENGINE_LOAD_BUILTIN_ENGINES
>> -        ENGINE_load_builtin_engines();
>> -#endif
>> -        OpenSSL_add_all_algorithms();
>> -        OPENSSL_load_builtin_modules();
>> -
>> -        SSL_load_error_strings();
>> -        SSL_library_init();
>> -
>> -        /*
>> -         * Let us cleanup the ssl library when the module is unloaded
>> -         */
>> -        apr_pool_cleanup_register(pconf, NULL, ssl_cleanup_pre_config,
>> -                                               apr_pool_cleanup_null);
>> -    }
>> -
>> -#if APR_HAS_THREADS && MODSSL_USE_OPENSSL_PRE_1_1_API
>>     /* Some OpenSSL internals are allocated per-thread, make sure they
>> -     * are associated to the/our same thread-id until cleaned up. Then
>> -     * initialize all the thread locking stuff needed by the lib.
>> +     * are associated to the/our same thread-id until cleaned up.
>>      */
>> +#if APR_HAS_THREADS && MODSSL_USE_OPENSSL_PRE_1_1_API
>>     ssl_util_thread_id_setup(pconf);
>> -    ssl_util_thread_setup(pconf);
>> #endif
>> -#endif /* USE_APR_CRYPTO_LIB_INIT */
>> +
>> +    /* We must register the library in full, to ensure our configuration
>> +     * code can successfully test the SSL environment.
>> +     */
>> +#if MODSSL_USE_OPENSSL_PRE_1_1_API || defined(LIBRESSL_VERSION_NUMBER)
>> +    (void)CRYPTO_malloc_init();
>> +#else
>> +    OPENSSL_malloc_init();
>> +#endif
>> +    ERR_load_crypto_strings();
>> +    SSL_load_error_strings();
>> +    SSL_library_init();
>> +#if HAVE_ENGINE_LOAD_BUILTIN_ENGINES
>> +    ENGINE_load_builtin_engines();
>> +#endif
>> +    OpenSSL_add_all_algorithms();
>> +    OPENSSL_load_builtin_modules();
> 
> Is there any reason why the order of the above 6 library calls has changed?
> The order doesn't matter?

I have no idea to be honest, to answer this we would need to pick apart the original reverted commits to see why.

>> @@ -302,6 +301,10 @@ apr_status_t ssl_init_Module(apr_pool_t
>> #endif
>>     }
>> 
>> +#if APR_HAS_THREADS && MODSSL_USE_OPENSSL_PRE_1_1_API
>> +    ssl_util_thread_setup(p);
>> +#endif
>> +
> 
> Why moving it here and not leaving it in the old place?

Again, this is a revert, the original changes would need to be analysed to answer this.

Regards,
Graham
—