Title: [238805] trunk/Source/WebCore
Revision
238805
Author
[email protected]
Date
2018-12-03 11:11:28 -0800 (Mon, 03 Dec 2018)

Log Message

[SOUP] Use SoupSession instead of SoupSessionAsync
https://bugs.webkit.org/show_bug.cgi?id=107451

Reviewed by Carlos Garcia Campos.

With glib-networking 2.57.1, WebKit is no longer able to load TLS error pages. The problem
is a network process deadlock caused by a change in how glib-networking performs certificate
verification. Previously it verified certificates *after* the TLS handshake had completed,
which was weirdly late, but previously not problematic. But now that TLS 1.3 exists,
application data can be sent before certificate verification occurs, which is no good. So I
moved verification to occur during the handshake. I needed to do this regardless because I
need to add a new callback in GnuTLS for another feature. This introduced a deadlock in
WebKit:

 - glib-networking detects an unacceptable certificate, emits accept-certificate signal
 - NetworkDataTaskSoup::tlsConnectionAcceptCertificate calls
   NetworkDataTaskSoup::invalidateAndCancel calls NetworkDataTaskSoup::clearRequest
 - NetworkDataTaskSoup::clearRequest calls soup_session_cancel_message

The problem is that, in the deprecated SoupSessionAsync used by WebKit, cancellation is
always *synchronous* despite the name of the class. So soup_session_cancel_message winds up
doing its thing to close everything out, and that eventually ends up in a synchronous call
to g_tls_connection_gnutls_close. The close operation can't proceed until the TLS handshake
is finished, and the handshake is blocked waiting for WebKit to return from its
accept-certificate handler. So the close operation winds up polling forever waiting for the
handshake to finish. Deadlock.

The only changes required in WebKit to use the modern SoupSession are adjustments for the
new default property values. Most of the properties we used to set explicitly are now
defaults and should be removed. Additionally, SoupSession has default timeouts, which we
want to override to allow NetworkDataTaskSoup to implement its own timeouts.

No new tests because this is covered by TestSSL (which would be failing if run with the
newer glib-networking).

* platform/network/soup/SoupNetworkSession.cpp:
(WebCore::SoupNetworkSession::SoupNetworkSession):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (238804 => 238805)


--- trunk/Source/WebCore/ChangeLog	2018-12-03 17:57:51 UTC (rev 238804)
+++ trunk/Source/WebCore/ChangeLog	2018-12-03 19:11:28 UTC (rev 238805)
@@ -1,3 +1,43 @@
+2018-12-03  Michael Catanzaro  <[email protected]>
+
+        [SOUP] Use SoupSession instead of SoupSessionAsync
+        https://bugs.webkit.org/show_bug.cgi?id=107451
+
+        Reviewed by Carlos Garcia Campos.
+
+        With glib-networking 2.57.1, WebKit is no longer able to load TLS error pages. The problem
+        is a network process deadlock caused by a change in how glib-networking performs certificate
+        verification. Previously it verified certificates *after* the TLS handshake had completed,
+        which was weirdly late, but previously not problematic. But now that TLS 1.3 exists,
+        application data can be sent before certificate verification occurs, which is no good. So I
+        moved verification to occur during the handshake. I needed to do this regardless because I
+        need to add a new callback in GnuTLS for another feature. This introduced a deadlock in
+        WebKit:
+
+         - glib-networking detects an unacceptable certificate, emits accept-certificate signal
+         - NetworkDataTaskSoup::tlsConnectionAcceptCertificate calls
+           NetworkDataTaskSoup::invalidateAndCancel calls NetworkDataTaskSoup::clearRequest
+         - NetworkDataTaskSoup::clearRequest calls soup_session_cancel_message
+
+        The problem is that, in the deprecated SoupSessionAsync used by WebKit, cancellation is
+        always *synchronous* despite the name of the class. So soup_session_cancel_message winds up
+        doing its thing to close everything out, and that eventually ends up in a synchronous call
+        to g_tls_connection_gnutls_close. The close operation can't proceed until the TLS handshake
+        is finished, and the handshake is blocked waiting for WebKit to return from its
+        accept-certificate handler. So the close operation winds up polling forever waiting for the
+        handshake to finish. Deadlock.
+
+        The only changes required in WebKit to use the modern SoupSession are adjustments for the
+        new default property values. Most of the properties we used to set explicitly are now
+        defaults and should be removed. Additionally, SoupSession has default timeouts, which we
+        want to override to allow NetworkDataTaskSoup to implement its own timeouts.
+
+        No new tests because this is covered by TestSSL (which would be failing if run with the
+        newer glib-networking).
+
+        * platform/network/soup/SoupNetworkSession.cpp:
+        (WebCore::SoupNetworkSession::SoupNetworkSession):
+
 2018-12-03  Yusuke Suzuki  <[email protected]>
 
         Use WallTime for file time

Modified: trunk/Source/WebCore/platform/network/soup/SoupNetworkSession.cpp (238804 => 238805)


--- trunk/Source/WebCore/platform/network/soup/SoupNetworkSession.cpp	2018-12-03 17:57:51 UTC (rev 238804)
+++ trunk/Source/WebCore/platform/network/soup/SoupNetworkSession.cpp	2018-12-03 19:11:28 UTC (rev 238805)
@@ -105,7 +105,7 @@
 }
 
 SoupNetworkSession::SoupNetworkSession(PAL::SessionID sessionID, SoupCookieJar* cookieJar)
-    : m_soupSession(adoptGRef(soup_session_async_new()))
+    : m_soupSession(adoptGRef(soup_session_new()))
 {
     // Values taken from http://www.browserscope.org/ following
     // the rule "Do What Every Other Modern Browser Is Doing". They seem
@@ -123,13 +123,10 @@
     g_object_set(m_soupSession.get(),
         SOUP_SESSION_MAX_CONNS, maxConnections,
         SOUP_SESSION_MAX_CONNS_PER_HOST, maxConnectionsPerHost,
-        SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_CONTENT_DECODER,
+        SOUP_SESSION_TIMEOUT, 0,
+        SOUP_SESSION_IDLE_TIMEOUT, 0,
         SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_CONTENT_SNIFFER,
-        SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_PROXY_RESOLVER_DEFAULT,
         SOUP_SESSION_ADD_FEATURE, jar.get(),
-        SOUP_SESSION_USE_THREAD_CONTEXT, TRUE,
-        SOUP_SESSION_SSL_USE_SYSTEM_CA_FILE, TRUE,
-        SOUP_SESSION_SSL_STRICT, TRUE,
         nullptr);
 
     setupCustomProtocols();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to