Changeset 238805 in webkit


Ignore:
Timestamp:
Dec 3, 2018 11:11:28 AM (5 years ago)
Author:
Michael Catanzaro
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):

Location:
trunk/Source/WebCore
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r238802 r238805  
     12018-12-03  Michael Catanzaro  <mcatanzaro@igalia.com>
     2
     3        [SOUP] Use SoupSession instead of SoupSessionAsync
     4        https://bugs.webkit.org/show_bug.cgi?id=107451
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        With glib-networking 2.57.1, WebKit is no longer able to load TLS error pages. The problem
     9        is a network process deadlock caused by a change in how glib-networking performs certificate
     10        verification. Previously it verified certificates *after* the TLS handshake had completed,
     11        which was weirdly late, but previously not problematic. But now that TLS 1.3 exists,
     12        application data can be sent before certificate verification occurs, which is no good. So I
     13        moved verification to occur during the handshake. I needed to do this regardless because I
     14        need to add a new callback in GnuTLS for another feature. This introduced a deadlock in
     15        WebKit:
     16
     17         - glib-networking detects an unacceptable certificate, emits accept-certificate signal
     18         - NetworkDataTaskSoup::tlsConnectionAcceptCertificate calls
     19           NetworkDataTaskSoup::invalidateAndCancel calls NetworkDataTaskSoup::clearRequest
     20         - NetworkDataTaskSoup::clearRequest calls soup_session_cancel_message
     21
     22        The problem is that, in the deprecated SoupSessionAsync used by WebKit, cancellation is
     23        always *synchronous* despite the name of the class. So soup_session_cancel_message winds up
     24        doing its thing to close everything out, and that eventually ends up in a synchronous call
     25        to g_tls_connection_gnutls_close. The close operation can't proceed until the TLS handshake
     26        is finished, and the handshake is blocked waiting for WebKit to return from its
     27        accept-certificate handler. So the close operation winds up polling forever waiting for the
     28        handshake to finish. Deadlock.
     29
     30        The only changes required in WebKit to use the modern SoupSession are adjustments for the
     31        new default property values. Most of the properties we used to set explicitly are now
     32        defaults and should be removed. Additionally, SoupSession has default timeouts, which we
     33        want to override to allow NetworkDataTaskSoup to implement its own timeouts.
     34
     35        No new tests because this is covered by TestSSL (which would be failing if run with the
     36        newer glib-networking).
     37
     38        * platform/network/soup/SoupNetworkSession.cpp:
     39        (WebCore::SoupNetworkSession::SoupNetworkSession):
     40
    1412018-12-03  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
    242
  • trunk/Source/WebCore/platform/network/soup/SoupNetworkSession.cpp

    r238771 r238805  
    106106
    107107SoupNetworkSession::SoupNetworkSession(PAL::SessionID sessionID, SoupCookieJar* cookieJar)
    108     : m_soupSession(adoptGRef(soup_session_async_new()))
     108    : m_soupSession(adoptGRef(soup_session_new()))
    109109{
    110110    // Values taken from http://www.browserscope.org/ following
     
    124124        SOUP_SESSION_MAX_CONNS, maxConnections,
    125125        SOUP_SESSION_MAX_CONNS_PER_HOST, maxConnectionsPerHost,
    126         SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_CONTENT_DECODER,
     126        SOUP_SESSION_TIMEOUT, 0,
     127        SOUP_SESSION_IDLE_TIMEOUT, 0,
    127128        SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_CONTENT_SNIFFER,
    128         SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_PROXY_RESOLVER_DEFAULT,
    129129        SOUP_SESSION_ADD_FEATURE, jar.get(),
    130         SOUP_SESSION_USE_THREAD_CONTEXT, TRUE,
    131         SOUP_SESSION_SSL_USE_SYSTEM_CA_FILE, TRUE,
    132         SOUP_SESSION_SSL_STRICT, TRUE,
    133130        nullptr);
    134131
Note: See TracChangeset for help on using the changeset viewer.