Attachment 339340 Details for Bug 185138 – Fix potential security issue

[patch] Fix potential security issue

185138.diff (text/plain), 4.85 KB, created by Basuke Suzuki on 2018-05-02 14:29:32 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Basuke Suzuki

Created: 2018-05-02 14:29:32 PDT

Size: 4.85 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 94ad2070a4e..23813c4fdb3 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,31 @@
>+2018-05-02  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        [Curl] Add OpenSSL/LibreSSL multi-threading support
>+        https://bugs.webkit.org/show_bug.cgi?id=185138
>+
>+        The older OpenSSL manual says the locking_function and threadid_funcion should
>+        set when use it in multi-threading environment. This applies to LibreSSL also.
>+        https://www.openssl.org/docs/man1.0.2/crypto/threads.html
>+
>+        For unix and other similar os, the default threadId_function implementation is
>+        good enough. We'll set custom callback only for Windows OS.
>+
>+        Note it's not required for OpenSSL 1.1.0 and after.
>+        https://www.openssl.org/blog/blog/2017/02/21/threads/
>+
>+        Reviewed by Per Arne Vollan.
>+
>+        * platform/network/curl/CurlSSLHandle.cpp:
>+        (WebCore::CurlSSLHandle::CurlSSLHandle):
>+        (WebCore::CurlSSLHandle::ThreadSupport::ThreadSupport):
>+        (WebCore::CurlSSLHandle::ThreadSupport::lockingCallback):
>+        (WebCore::CurlSSLHandle::ThreadSupport::threadIdCallback):
>+        * platform/network/curl/CurlSSLHandle.h:
>+        (WebCore::CurlSSLHandle::ThreadSupport::setup):
>+        (WebCore::CurlSSLHandle::ThreadSupport::singleton):
>+        (WebCore::CurlSSLHandle::ThreadSupport::lock):
>+        (WebCore::CurlSSLHandle::ThreadSupport::unlock):
>+
> 2018-04-30  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         [GTK] Webkit should spoof as Safari on a Mac when on Chase.com
>diff --git a/Source/WebCore/platform/network/curl/CurlSSLHandle.cpp b/Source/WebCore/platform/network/curl/CurlSSLHandle.cpp
>index 3c7fd20548d..abd02bc7002 100644
>--- a/Source/WebCore/platform/network/curl/CurlSSLHandle.cpp
>+++ b/Source/WebCore/platform/network/curl/CurlSSLHandle.cpp
>@@ -37,6 +37,10 @@
> #include <wtf/RetainPtr.h>
> #endif
> 
>+#if NEED_OPENSSL_THREAD_SUPPORT && OS(WINDOWS)
>+#include <wtf/Threading.h>
>+#endif
>+
> namespace WebCore {
> 
> CurlSSLHandle::CurlSSLHandle()
>@@ -45,6 +49,13 @@ CurlSSLHandle::CurlSSLHandle()
>     char* ignoreSSLErrors = getenv("WEBKIT_IGNORE_SSL_ERRORS");
>     if (ignoreSSLErrors)
>         m_ignoreSSLErrors = true;
>+
>+#if NEED_OPENSSL_THREAD_SUPPORT
>+    static std::once_flag onceFlag;
>+    std::call_once(onceFlag, [] {
>+        ThreadSupport::setup();
>+    });
>+#endif
> }
> 
> CString CurlSSLHandle::getCACertPathEnv()
>@@ -120,6 +131,38 @@ std::optional<CurlSSLHandle::ClientCertificate> CurlSSLHandle::getSSLClientCerti
>     return it->value;
> }
> 
>+#if NEED_OPENSSL_THREAD_SUPPORT
>+
>+CurlSSLHandle::ThreadSupport::ThreadSupport()
>+{
>+    CRYPTO_set_locking_callback(lockingCallback);
>+#if OS(WINDOWS)
>+    CRYPTO_THREADID_set_callback(threadIdCallback);
>+#endif
>+}
>+
>+void CurlSSLHandle::ThreadSupport::lockingCallback(int mode, int type, const char*, int)
>+{
>+    RELEASE_ASSERT(type >= 0 && type < CRYPTO_NUM_LOCKS);
>+    auto& locker = ThreadSupport::singleton();
>+
>+    if (mode & CRYPTO_LOCK)
>+        locker.lock(type);
>+    else
>+        locker.unlock(type);
>+}
>+
>+#if OS(WINDOWS)
>+
>+void CurlSSLHandle::ThreadSupport::threadIdCallback(CRYPTO_THREADID* threadId)
>+{
>+    CRYPTO_THREADID_set_numeric(threadId, static_cast<unsigned long>(Thread::currentID()));
>+}
>+
>+#endif
>+
>+#endif
>+
> }
> 
> #endif
>diff --git a/Source/WebCore/platform/network/curl/CurlSSLHandle.h b/Source/WebCore/platform/network/curl/CurlSSLHandle.h
>index 088e64a6f34..6c2829f5a00 100644
>--- a/Source/WebCore/platform/network/curl/CurlSSLHandle.h
>+++ b/Source/WebCore/platform/network/curl/CurlSSLHandle.h
>@@ -26,11 +26,19 @@
> 
> #pragma once
> 
>+#include <openssl/crypto.h>
> #include <wtf/HashMap.h>
> #include <wtf/ListHashSet.h>
> #include <wtf/Noncopyable.h>
> #include <wtf/text/StringHash.h>
> 
>+// all version of LibreSSL and OpenSSL prior to 1.1.0
>+#if defined(LIBRESSL_VERSION_NUMBER) || (defined(OPENSSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER < 0x1010000fL)
>+#define NEED_OPENSSL_THREAD_SUPPORT 1
>+#else
>+#define NEED_OPENSSL_THREAD_SUPPORT 0
>+#endif
>+
> namespace WebCore {
> 
> class CurlSSLHandle {
>@@ -55,6 +63,32 @@ public:
> private:
>     CString getCACertPathEnv();
> 
>+#if NEED_OPENSSL_THREAD_SUPPORT
>+    class ThreadSupport {
>+    public:
>+        static void setup() {
>+            singleton();
>+        }
>+
>+    private:
>+        static ThreadSupport& singleton() {
>+            static ThreadSupport shared;
>+            return shared;
>+        }
>+
>+        ThreadSupport();
>+        void lock(int type) { m_locks[type].lock(); }
>+        void unlock(int type) { m_locks[type].unlock(); }
>+
>+        Lock m_locks[CRYPTO_NUM_LOCKS];
>+
>+        static void lockingCallback(int mode, int type, const char* file, int line);
>+#if OS(WINDOWS)
>+        static void threadIdCallback(CRYPTO_THREADID*);
>+#endif
>+    };
>+#endif
>+
>     bool m_ignoreSSLErrors { false };
>     CString m_caCertPath;
>

Actions: View | Formatted Diff | Diff

Attachments on bug 185138: 339130 | 339278 | 339279 | 339340 | 339353 | 339391 | 339392