You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2018/08/04 17:17:03 UTC

svn commit: r1837435 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c ssl_engine_init.c

Author: ylavic
Date: Sat Aug  4 17:17:03 2018
New Revision: 1837435

URL: http://svn.apache.org/viewvc?rev=1837435&view=rev
Log:
mod_ssl: OpenSSL now initializes fully through APR, use that.

Follow up to r1833368 and r1833452.

Modified:
    httpd/httpd/trunk/modules/ssl/mod_ssl.c
    httpd/httpd/trunk/modules/ssl/ssl_engine_init.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=1837435&r1=1837434&r2=1837435&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
+++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Sat Aug  4 17:17:03 2018
@@ -342,6 +342,7 @@ static int modssl_is_prelinked(void)
     return 0;
 }
 
+#if !USE_APR_CRYPTO_LIB_INIT
 static apr_status_t ssl_cleanup_pre_config(void *data)
 {
     /*
@@ -397,47 +398,31 @@ static apr_status_t ssl_cleanup_pre_conf
      */
     return APR_SUCCESS;
 }
+#endif /* !USE_APR_CRYPTO_LIB_INIT */
 
 static int ssl_hook_pre_config(apr_pool_t *pconf,
                                apr_pool_t *plog,
                                apr_pool_t *ptemp)
 {
-#if USE_APR_CRYPTO_LIB_INIT
-    apr_status_t rv;
-#endif
-
 #if HAVE_VALGRIND
     ssl_running_on_valgrind = RUNNING_ON_VALGRIND;
 #endif
     modssl_running_statically = modssl_is_prelinked();
 
-    /* Some OpenSSL internals are allocated per-thread, make sure they
-     * 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);
-#endif
-
 #if USE_APR_CRYPTO_LIB_INIT
-    /* When mod_ssl is builtin, no need to unload openssl on restart */
-    rv = apr_crypto_lib_init("openssl", NULL, NULL,
-                             modssl_running_statically ? ap_pglobal : pconf);
-    if (rv == APR_SUCCESS || rv == APR_EREINIT) {
-        /* apr_crypto inits libcrypto only, so in any case init libssl here,
-         * each time if openssl is unloaded with pconf, but only once if
-         * mod_ssl is builtin.
+    {
+        /* When mod_ssl is builtin, no need to unload openssl on restart,
+         * so use pglobal.
          */
-        if (!modssl_running_statically
-                || !ap_retained_data_get("ssl_hook_pre_config")) {
-            if (modssl_running_statically) {
-                ap_retained_data_create("ssl_hook_pre_config", 1);
-            }
-            SSL_load_error_strings();
-            SSL_library_init();
+        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()
+                          "mod_ssl: can't initialize OpenSSL library");
+            return !OK;
         }
     }
-    else
-#endif
+#else /* USE_APR_CRYPTO_LIB_INIT */
     {
         /* We must register the library in full, to ensure our configuration
          * code can successfully test the SSL environment.
@@ -456,6 +441,7 @@ static int ssl_hook_pre_config(apr_pool_
 #endif
         OpenSSL_add_all_algorithms();
         OPENSSL_load_builtin_modules();
+
         SSL_load_error_strings();
         SSL_library_init();
 
@@ -466,6 +452,16 @@ static int ssl_hook_pre_config(apr_pool_
                                                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.
+     */
+    ssl_util_thread_id_setup(pconf);
+    ssl_util_thread_setup(pconf);
+#endif
+#endif /* USE_APR_CRYPTO_LIB_INIT */
+
     if (OBJ_txt2nid("id-on-dnsSRV") == NID_undef) {
         (void)OBJ_create("1.3.6.1.5.5.7.8.7", "id-on-dnsSRV",
                          "SRVName otherName form");

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=1837435&r1=1837434&r2=1837435&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_init.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_init.c Sat Aug  4 17:17:03 2018
@@ -294,10 +294,6 @@ 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
-
     /*
      * SSL external crypto device ("engine") support
      */



Re: svn commit: r1837435 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c ssl_engine_init.c

Posted by Stefan Eissing <st...@greenbytes.de>.
Hi Yann,

I am not under any time constraints here. Most people do not seem to run into this issue. So, stay hydrated in this weather and enjoy your upcoming vacation.

I think having some common *ssl lib init/lifetime handling in httpd is a Good Thing. However, it sounds as if we would not backport such an extra module to 2.4.x and then it is good to rely on good APR support, require something new enough for a trunk httpd and be done with it.

At least it sounds like this on a first read to me.

Cheers,
Stefan



> Am 06.08.2018 um 16:14 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> Hi Stefan,
> 
> On Mon, Aug 6, 2018 at 1:47 PM, stefan@eissing.org <st...@eissing.org> wrote:
>> Yann and Rüdiger, looking at this, I am thinking about how to improve
>> mod_md's init of openssl. Basically, if mod_ssl + mod_md is loaded,
>> mod_md does not have to do anything, it seems.
>> 
>> However, there is a slim chance that someone has another ssl module
>> (or none?) and what should mod_md do then? I can copy the crypto
>> parts of the pre_config and cleanup code, but given the ever
>> increasing version number checks...is there a better way?
> 
> I'm currently working on changing APR's apu_dso_load() so that it can
> work on the given pool (user controlled lifetime) and also
> (optionally) init/deinit the lib.
> This is needed (I think) for APR's self handling of its DSOs (focusing
> on apr_crypto and openssl for now).
> With this in e.g. APR 1.7 (at best), we can simply call
> apr_crypto_load_lib("openssl") in each module using openssl like we
> currently do in trunk (the dynamic linking part in APR trunk is
> currently being disputed, so I'm on the DSO way now...).
> 
> Anyway we can't mandate APR 1.7 (at best) in httpd, so I was thinking
> of copying interesting bits of apu_dso_load() into e.g. ap_dso_load()
> (or a more openssl only version of it), so that it works in httpd with
> earlier APR versions too.
> So we'd configure, build and install something like
> "mod_ssl_openssl.so/dll" (itself dynamically linked with
> libcrypto/ssl), that is when a module needs it. Thus we'd
> ap_dso_load() it in mod_ssl/md/session/... at pre_config time so that
> the load/unload and init/deinit work once for all according to the
> given pool (usually the same one, pconf).
> 
> Would that work for you?
> I'm not too far from having something for APR, and expecting the ap_
> part to be quite straight forward (and mostly copy/paste).
> Possibly something this week, otherwise after my vacations (more about
> septembre)...
> 
> Regards,
> Yann.


Re: svn commit: r1837435 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c ssl_engine_init.c

Posted by Yann Ylavic <yl...@gmail.com>.
Hi Stefan,

On Mon, Aug 6, 2018 at 1:47 PM, stefan@eissing.org <st...@eissing.org> wrote:
> Yann and Rüdiger, looking at this, I am thinking about how to improve
> mod_md's init of openssl. Basically, if mod_ssl + mod_md is loaded,
> mod_md does not have to do anything, it seems.
>
> However, there is a slim chance that someone has another ssl module
> (or none?) and what should mod_md do then? I can copy the crypto
> parts of the pre_config and cleanup code, but given the ever
> increasing version number checks...is there a better way?

I'm currently working on changing APR's apu_dso_load() so that it can
work on the given pool (user controlled lifetime) and also
(optionally) init/deinit the lib.
This is needed (I think) for APR's self handling of its DSOs (focusing
on apr_crypto and openssl for now).
With this in e.g. APR 1.7 (at best), we can simply call
apr_crypto_load_lib("openssl") in each module using openssl like we
currently do in trunk (the dynamic linking part in APR trunk is
currently being disputed, so I'm on the DSO way now...).

Anyway we can't mandate APR 1.7 (at best) in httpd, so I was thinking
of copying interesting bits of apu_dso_load() into e.g. ap_dso_load()
(or a more openssl only version of it), so that it works in httpd with
earlier APR versions too.
So we'd configure, build and install something like
"mod_ssl_openssl.so/dll" (itself dynamically linked with
libcrypto/ssl), that is when a module needs it. Thus we'd
ap_dso_load() it in mod_ssl/md/session/... at pre_config time so that
the load/unload and init/deinit work once for all according to the
given pool (usually the same one, pconf).

Would that work for you?
I'm not too far from having something for APR, and expecting the ap_
part to be quite straight forward (and mostly copy/paste).
Possibly something this week, otherwise after my vacations (more about
septembre)...

Regards,
Yann.

Re: svn commit: r1837435 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c ssl_engine_init.c

Posted by "stefan@eissing.org" <st...@eissing.org>.
Yann and Rüdiger, looking at this, I am thinking about how to improve mod_md's init of openssl. Basically, if mod_ssl + mod_md is loaded, mod_md does not have to do anything, it seems.

However, there is a slim chance that someone has another ssl module (or none?) and what should mod_md do then? I can copy the crypto parts of the pre_config and cleanup code, but given the ever increasing version number checks...is there a better way?

-Stefan



> Am 04.08.2018 um 19:17 schrieb ylavic@apache.org:
> 
> Author: ylavic
> Date: Sat Aug  4 17:17:03 2018
> New Revision: 1837435
> 
> URL: http://svn.apache.org/viewvc?rev=1837435&view=rev
> Log:
> mod_ssl: OpenSSL now initializes fully through APR, use that.
> 
> Follow up to r1833368 and r1833452.
> 
> Modified:
>    httpd/httpd/trunk/modules/ssl/mod_ssl.c
>    httpd/httpd/trunk/modules/ssl/ssl_engine_init.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=1837435&r1=1837434&r2=1837435&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
> +++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Sat Aug  4 17:17:03 2018
> @@ -342,6 +342,7 @@ static int modssl_is_prelinked(void)
>     return 0;
> }
> 
> +#if !USE_APR_CRYPTO_LIB_INIT
> static apr_status_t ssl_cleanup_pre_config(void *data)
> {
>     /*
> @@ -397,47 +398,31 @@ static apr_status_t ssl_cleanup_pre_conf
>      */
>     return APR_SUCCESS;
> }
> +#endif /* !USE_APR_CRYPTO_LIB_INIT */
> 
> static int ssl_hook_pre_config(apr_pool_t *pconf,
>                                apr_pool_t *plog,
>                                apr_pool_t *ptemp)
> {
> -#if USE_APR_CRYPTO_LIB_INIT
> -    apr_status_t rv;
> -#endif
> -
> #if HAVE_VALGRIND
>     ssl_running_on_valgrind = RUNNING_ON_VALGRIND;
> #endif
>     modssl_running_statically = modssl_is_prelinked();
> 
> -    /* Some OpenSSL internals are allocated per-thread, make sure they
> -     * 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);
> -#endif
> -
> #if USE_APR_CRYPTO_LIB_INIT
> -    /* When mod_ssl is builtin, no need to unload openssl on restart */
> -    rv = apr_crypto_lib_init("openssl", NULL, NULL,
> -                             modssl_running_statically ? ap_pglobal : pconf);
> -    if (rv == APR_SUCCESS || rv == APR_EREINIT) {
> -        /* apr_crypto inits libcrypto only, so in any case init libssl here,
> -         * each time if openssl is unloaded with pconf, but only once if
> -         * mod_ssl is builtin.
> +    {
> +        /* When mod_ssl is builtin, no need to unload openssl on restart,
> +         * so use pglobal.
>          */
> -        if (!modssl_running_statically
> -                || !ap_retained_data_get("ssl_hook_pre_config")) {
> -            if (modssl_running_statically) {
> -                ap_retained_data_create("ssl_hook_pre_config", 1);
> -            }
> -            SSL_load_error_strings();
> -            SSL_library_init();
> +        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()
> +                          "mod_ssl: can't initialize OpenSSL library");
> +            return !OK;
>         }
>     }
> -    else
> -#endif
> +#else /* USE_APR_CRYPTO_LIB_INIT */
>     {
>         /* We must register the library in full, to ensure our configuration
>          * code can successfully test the SSL environment.
> @@ -456,6 +441,7 @@ static int ssl_hook_pre_config(apr_pool_
> #endif
>         OpenSSL_add_all_algorithms();
>         OPENSSL_load_builtin_modules();
> +
>         SSL_load_error_strings();
>         SSL_library_init();
> 
> @@ -466,6 +452,16 @@ static int ssl_hook_pre_config(apr_pool_
>                                                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.
> +     */
> +    ssl_util_thread_id_setup(pconf);
> +    ssl_util_thread_setup(pconf);
> +#endif
> +#endif /* USE_APR_CRYPTO_LIB_INIT */
> +
>     if (OBJ_txt2nid("id-on-dnsSRV") == NID_undef) {
>         (void)OBJ_create("1.3.6.1.5.5.7.8.7", "id-on-dnsSRV",
>                          "SRVName otherName form");
> 
> 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=1837435&r1=1837434&r2=1837435&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_init.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_init.c Sat Aug  4 17:17:03 2018
> @@ -294,10 +294,6 @@ 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
> -
>     /*
>      * SSL external crypto device ("engine") support
>      */
> 
>