From: Jacob Champion <jchampion@apache.org>
Date: Tue, 18 Apr 2017 00:19:32 +0000 (+0000)
Subject: PR60947: avoid CRYPTO_THREADID_set_callback() if possible
X-Git-Tag: 2.5.0-alpha~465^2~3
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7b2bdcea91c8db88f572bfb495619356ed9a8ed5;p=apache

PR60947: avoid CRYPTO_THREADID_set_callback() if possible

CRYPTO_THREADID_set_callback() is write-once, so if mod_ssl is unloaded
and reloaded into a different address, we'll crash if OpenSSL wasn't
also unloaded and reloaded at the same time. This can happen if another
module or library is using OpenSSL as well.

- OpenSSL 1.1.0 isn't affected.
- Certain platforms (Windows, BeOS, and POSIX-compliant systems) can
  make use of the default THREADID callback in OpenSSL 1.0.x.
- If the deprecated CRYPTO_set_id_callback() is available, we can use it
  instead of CRYPTO_THREADID_set_callback().

Otherwise, we have to fall back to CRYPTO_THREADID_set_callback(), but
hopefully that applies to a small percentage of users at this point.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/trunk-openssl-threadid@1791732 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/acinclude.m4 b/acinclude.m4
index 14c02a1e83..3ab82d726b 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -608,7 +608,8 @@ AC_DEFUN([APACHE_CHECK_OPENSSL],[
       liberrors=""
       AC_CHECK_HEADERS([openssl/engine.h])
       AC_CHECK_FUNCS([SSL_CTX_new], [], [liberrors="yes"])
-      AC_CHECK_FUNCS([ENGINE_init ENGINE_load_builtin_engines RAND_egd])
+      AC_CHECK_FUNCS([ENGINE_init ENGINE_load_builtin_engines RAND_egd \
+                      CRYPTO_set_id_callback])
       if test "x$liberrors" != "x"; then
         AC_MSG_WARN([OpenSSL libraries are unusable])
       fi
diff --git a/modules/ssl/ssl_util.c b/modules/ssl/ssl_util.c
index 45e8411a10..488df0e102 100644
--- a/modules/ssl/ssl_util.c
+++ b/modules/ssl/ssl_util.c
@@ -252,6 +252,13 @@ void ssl_asn1_table_unset(apr_hash_t *table,
  * To ensure thread-safetyness in OpenSSL - work in progress
  */
 
+#if (OPENSSL_VERSION_NUMBER >= 0x10000000L) && \
+        (_WIN32 || __BEOS__ || AP_OPENSSL_USE_ERRNO_THREADID)
+/* Windows and BeOS can use the default THREADID callback shipped with OpenSSL
+ * 1.0.x, as can any platform with a thread-safe errno. */
+# define DEFAULT_THREADID_IS_SAFE 1
+#endif
+
 static apr_thread_mutex_t **lock_cs;
 static int                  lock_num_locks;
 
@@ -363,6 +370,13 @@ static void ssl_dyn_destroy_function(struct CRYPTO_dynlock_value *l,
     apr_pool_destroy(l->pool);
 }
 
+#if DEFAULT_THREADID_IS_SAFE
+
+/* We don't need to set up a threadid callback on this platform. */
+void ssl_util_thread_id_setup(apr_pool_t *p) { }
+
+#else
+
 /**
  * Used by both versions of ssl_util_thr_id(). Returns an unsigned long that
  * should be unique to the currently executing thread.
@@ -385,7 +399,14 @@ static unsigned long ssl_util_thr_id_internal(void)
 #endif
 }
 
-#if OPENSSL_VERSION_NUMBER >= 0x10000000L
+#if HAVE_CRYPTO_SET_ID_CALLBACK
+
+static unsigned long ssl_util_thr_id(void)
+{
+    return ssl_util_thr_id_internal();
+}
+
+#else
 
 static void ssl_util_thr_id(CRYPTO_THREADID *id)
 {
@@ -396,27 +417,39 @@ static void ssl_util_thr_id(CRYPTO_THREADID *id)
     CRYPTO_THREADID_set_numeric(id, ssl_util_thr_id_internal());
 }
 
-#else
-
-static unsigned long ssl_util_thr_id(void)
-{
-    return ssl_util_thr_id_internal();
-}
-
-#endif
+#endif /* HAVE_CRYPTO_SET_ID_CALLBACK */
 
 static apr_status_t ssl_util_thr_id_cleanup(void *old)
 {
-#if OPENSSL_VERSION_NUMBER >= 0x10000000L
+#if HAVE_CRYPTO_SET_ID_CALLBACK
+    CRYPTO_set_id_callback(old);
+#else
     /* XXX This does nothing. The new-style THREADID callback is write-once. */
     CRYPTO_THREADID_set_callback(old);
-#else
-    CRYPTO_set_id_callback(old);
 #endif
 
     return APR_SUCCESS;
 }
 
+void ssl_util_thread_id_setup(apr_pool_t *p)
+{
+#if HAVE_CRYPTO_SET_ID_CALLBACK
+    /* This API is deprecated, but we prefer it to its replacement since it
+     * allows us to unset the callback when this module is being unloaded. */
+    CRYPTO_set_id_callback(ssl_util_thr_id);
+#else
+    /* This is a last resort. We can only set this once, which means that we'd
+     * better not get loaded into a different address during a restart. See
+     * PR60947. */
+    CRYPTO_THREADID_set_callback(ssl_util_thr_id);
+#endif
+
+    apr_pool_cleanup_register(p, NULL, ssl_util_thr_id_cleanup,
+                                       apr_pool_cleanup_null);
+}
+
+#endif /* DEFAULT_THREADID_IS_SAFE */
+
 static apr_status_t ssl_util_thread_cleanup(void *data)
 {
     CRYPTO_set_locking_callback(NULL);
@@ -457,15 +490,5 @@ void ssl_util_thread_setup(apr_pool_t *p)
                                        apr_pool_cleanup_null);
 }
 
-void ssl_util_thread_id_setup(apr_pool_t *p)
-{
-#if OPENSSL_VERSION_NUMBER >= 0x10000000L
-    CRYPTO_THREADID_set_callback(ssl_util_thr_id);
-#else
-    CRYPTO_set_id_callback(ssl_util_thr_id);
-#endif
-    apr_pool_cleanup_register(p, NULL, ssl_util_thr_id_cleanup,
-                                       apr_pool_cleanup_null);
-}
 #endif /* #if OPENSSL_VERSION_NUMBER < 0x10100000L */
 #endif /* #if APR_HAS_THREADS */