Merge r230885 - [SOUP] Do TLS error checking on GTlsConnection::accep…

…t-certificate

https://bugs.webkit.org/show_bug.cgi?id=184480

Reviewed by Michael Catanzaro.

Source/WebCore:

* platform/network/soup/ResourceError.h: Change tlsError to recieve a failing URL instead of a SoupRequest,
since the request was only used to get the failing URL.
* platform/network/soup/ResourceErrorSoup.cpp:
(WebCore::ResourceError::tlsError): Use the given failing URL.
* platform/network/soup/SoupNetworkSession.cpp:
(WebCore::SoupNetworkSession::SoupNetworkSession): Use ssl-strict when creating the SoupSession to handle the
certificates ourselves by connecting to GTlsConnection::accept-certificate.
(WebCore::SoupNetworkSession::checkTLSErrors): Updated to receive a URL, certificate and errors instead of
receiving a SoupRequest and SoupMessage and extract the url, certirficate and errors from them. Also return the
optional error directly instead of using a completion handler since the function is always synchronous.
* platform/network/soup/SoupNetworkSession.h:

Source/WebKit:

Connect to GTlsConnection::accept-certificate signal instead of SoupMessage::notify::tls-errors to perform the
TLS errors check.

* NetworkProcess/soup/NetworkDataTaskSoup.cpp:
(WebKit::NetworkDataTaskSoup::createRequest): Do not connect to SoupMessage::notify::tls-errors.
(WebKit::NetworkDataTaskSoup::tlsConnectionAcceptCertificateCallback): Call tlsConnectionAcceptCertificate() is
the task is still ongoing.
(WebKit::NetworkDataTaskSoup::tlsConnectionAcceptCertificate): Check TLS errors here.
(WebKit::NetworkDataTaskSoup::networkEventCallback): Pass the stream to networkEvent.
(WebKit::NetworkDataTaskSoup::networkEvent): Connect to GTlsConnection::accept-certificate.
* NetworkProcess/soup/NetworkDataTaskSoup.h:

Source/WebCore/ChangeLog

@@ -1,3 +1,22 @@
+2018-04-20  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [SOUP] Do TLS error checking on GTlsConnection::accept-certificate
+        https://bugs.webkit.org/show_bug.cgi?id=184480
+
+        Reviewed by Michael Catanzaro.
+
+        * platform/network/soup/ResourceError.h: Change tlsError to recieve a failing URL instead of a SoupRequest,
+        since the request was only used to get the failing URL.
+        * platform/network/soup/ResourceErrorSoup.cpp:
+        (WebCore::ResourceError::tlsError): Use the given failing URL.
+        * platform/network/soup/SoupNetworkSession.cpp:
+        (WebCore::SoupNetworkSession::SoupNetworkSession): Use ssl-strict when creating the SoupSession to handle the
+        certificates ourselves by connecting to GTlsConnection::accept-certificate.
+        (WebCore::SoupNetworkSession::checkTLSErrors): Updated to receive a URL, certificate and errors instead of
+        receiving a SoupRequest and SoupMessage and extract the url, certirficate and errors from them. Also return the
+        optional error directly instead of using a completion handler since the function is always synchronous.
+        * platform/network/soup/SoupNetworkSession.h:
+
 2018-03-09  Zalan Bujtas  <zalan@apple.com>
 
         Turn off offset*/scroll* optimization for input elements with shadow content

Source/WebCore/platform/network/soup/ResourceError.h

@@ -56,7 +56,7 @@ class ResourceError : public ResourceErrorBase
     static ResourceError httpError(SoupMessage*, GError*, SoupRequest*);
     static ResourceError transportError(SoupRequest*, int statusCode, const String& reasonPhrase);
     static ResourceError genericGError(GError*, SoupRequest*);
-    static ResourceError tlsError(SoupRequest*, unsigned tlsErrors, GTlsCertificate*);
+    static ResourceError tlsError(const URL&, unsigned tlsErrors, GTlsCertificate*);
     static ResourceError timeoutError(const URL& failingURL);
     static ResourceError authenticationError(SoupMessage*);
 

Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp

@@ -75,10 +75,9 @@ ResourceError ResourceError::genericGError(GError* error, SoupRequest* request)
         failingURI(request), String::fromUTF8(error->message));
 }
 
-ResourceError ResourceError::tlsError(SoupRequest* request, unsigned tlsErrors, GTlsCertificate* certificate)
+ResourceError ResourceError::tlsError(const URL& failingURL, unsigned tlsErrors, GTlsCertificate* certificate)
 {
-    ResourceError resourceError(g_quark_to_string(SOUP_HTTP_ERROR), SOUP_STATUS_SSL_FAILED,
-        failingURI(request), unacceptableTLSCertificate());
+    ResourceError resourceError(g_quark_to_string(SOUP_HTTP_ERROR), SOUP_STATUS_SSL_FAILED, failingURL, unacceptableTLSCertificate());
     resourceError.setTLSErrors(tlsErrors);
     resourceError.setCertificate(certificate);
     return resourceError;

Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp

@@ -147,19 +147,26 @@ static bool isAuthenticationFailureStatusCode(int httpStatusCode)
     return httpStatusCode == SOUP_STATUS_PROXY_AUTHENTICATION_REQUIRED || httpStatusCode == SOUP_STATUS_UNAUTHORIZED;
 }
 
-static void tlsErrorsChangedCallback(SoupMessage* message, GParamSpec*, gpointer data)
+static gboolean tlsConnectionAcceptCertificateCallback(GTlsConnection* connection, GTlsCertificate* certificate, GTlsCertificateFlags errors, gpointer data)
 {
     RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(data);
     if (!handle || handle->cancelledOrClientless())
-        return;
+        return FALSE;
 
-    SoupNetworkSession::checkTLSErrors(handle->getInternal()->m_soupRequest.get(), message, [handle] (const ResourceError& error) {
-        if (error.isNull())
-            return;
+    ResourceHandleInternal* d = handle->getInternal();
 
-        handle->client()->didFail(handle.get(), error);
-        handle->cancel();
-    });
+    auto* connectionMessage = g_object_get_data(G_OBJECT(connection), "wk-soup-message");
+    if (connectionMessage != d->m_soupMessage.get())
+        return FALSE;
+
+    URL url(soup_request_get_uri(d->m_soupRequest.get()));
+    auto error = SoupNetworkSession::checkTLSErrors(url, certificate, errors);
+    if (!error)
+        return TRUE;
+
+    handle->client()->didFail(handle.get(), error.value());
+    handle->cancel();
+    return FALSE;
 }
 
 static void gotHeadersCallback(SoupMessage* message, gpointer data)
@@ -564,7 +571,7 @@ static void startingCallback(SoupMessage*, ResourceHandle* handle)
 }
 #endif // SOUP_CHECK_VERSION(2, 49, 91)
 
-static void networkEventCallback(SoupMessage*, GSocketClientEvent event, GIOStream*, gpointer data)
+static void networkEventCallback(SoupMessage*, GSocketClientEvent event, GIOStream* stream, gpointer data)
 {
     ResourceHandle* handle = static_cast<ResourceHandle*>(data);
     if (!handle)
@@ -604,6 +611,9 @@ static void networkEventCallback(SoupMessage*, GSocketClientEvent event, GIOStre
         break;
     case G_SOCKET_CLIENT_TLS_HANDSHAKING:
         d->m_response.deprecatedNetworkLoadMetrics().secureConnectionStart = deltaTime;
+        ASSERT(G_IS_TLS_CONNECTION(stream));
+        g_object_set_data(G_OBJECT(stream), "wk-soup-message", d->m_soupMessage.get());
+        g_signal_connect(stream, "accept-certificate", G_CALLBACK(tlsConnectionAcceptCertificateCallback), handle);
         break;
     case G_SOCKET_CLIENT_TLS_HANDSHAKED:
         break;
@@ -649,7 +659,6 @@ static bool createSoupMessageForHandleAndRequest(ResourceHandle* handle, const R
     if ((request.httpMethod() == "POST" || request.httpMethod() == "PUT") && !d->m_bodySize)
         soup_message_headers_set_content_length(soupMessage->request_headers, 0);
 
-    g_signal_connect(d->m_soupMessage.get(), "notify::tls-errors", G_CALLBACK(tlsErrorsChangedCallback), handle);
     g_signal_connect(d->m_soupMessage.get(), "got-headers", G_CALLBACK(gotHeadersCallback), handle);
     g_signal_connect(d->m_soupMessage.get(), "wrote-body-data", G_CALLBACK(wroteBodyDataCallback), handle);
 

Source/WebCore/platform/network/soup/SoupNetworkSession.cpp

@@ -138,7 +138,7 @@ SoupNetworkSession::SoupNetworkSession(PAL::SessionID sessionID, SoupCookieJar*
         SOUP_SESSION_ADD_FEATURE, jar.get(),
         SOUP_SESSION_USE_THREAD_CONTEXT, TRUE,
         SOUP_SESSION_SSL_USE_SYSTEM_CA_FILE, TRUE,
-        SOUP_SESSION_SSL_STRICT, FALSE,
+        SOUP_SESSION_SSL_STRICT, TRUE,
         nullptr);
 
     setupCustomProtocols();
@@ -293,29 +293,19 @@ void SoupNetworkSession::setShouldIgnoreTLSErrors(bool ignoreTLSErrors)
     gIgnoreTLSErrors = ignoreTLSErrors;
 }
 
-void SoupNetworkSession::checkTLSErrors(SoupRequest* soupRequest, SoupMessage* message, WTF::Function<void (const ResourceError&)>&& completionHandler)
+std::optional<ResourceError> SoupNetworkSession::checkTLSErrors(const URL& requestURL, GTlsCertificate* certificate, GTlsCertificateFlags tlsErrors)
 {
-    if (gIgnoreTLSErrors) {
-        completionHandler({ });
-        return;
-    }
+    if (gIgnoreTLSErrors)
+        return std::nullopt;
 
-    GTlsCertificate* certificate = nullptr;
-    GTlsCertificateFlags tlsErrors = static_cast<GTlsCertificateFlags>(0);
-    soup_message_get_https_status(message, &certificate, &tlsErrors);
-    if (!tlsErrors) {
-        completionHandler({ });
-        return;
-    }
+    if (!tlsErrors)
+        return std::nullopt;
 
-    URL url(soup_request_get_uri(soupRequest));
-    auto it = clientCertificates().find(url.host());
-    if (it != clientCertificates().end() && it->value.contains(certificate)) {
-        completionHandler({ });
-        return;
-    }
+    auto it = clientCertificates().find(requestURL.host());
+    if (it != clientCertificates().end() && it->value.contains(certificate))
+        return std::nullopt;
 
-    completionHandler(ResourceError::tlsError(soupRequest, tlsErrors, certificate));
+    return ResourceError::tlsError(requestURL, tlsErrors, certificate);
 }
 
 void SoupNetworkSession::allowSpecificHTTPSCertificateForHost(const CertificateInfo& certificateInfo, const String& host)

Source/WebCore/platform/network/soup/SoupNetworkSession.h

@@ -26,7 +26,7 @@
 #ifndef SoupNetworkSession_h
 #define SoupNetworkSession_h
 
-#include <glib-object.h>
+#include <gio/gio.h>
 #include <pal/SessionID.h>
 #include <wtf/Function.h>
 #include <wtf/Noncopyable.h>
@@ -65,7 +65,7 @@ class SoupNetworkSession {
     void setAcceptLanguages(const CString&);
 
     static void setShouldIgnoreTLSErrors(bool);
-    static void checkTLSErrors(SoupRequest*, SoupMessage*, WTF::Function<void (const ResourceError&)>&&);
+    static std::optional<ResourceError> checkTLSErrors(const URL&, GTlsCertificate*, GTlsCertificateFlags);
     static void allowSpecificHTTPSCertificateForHost(const CertificateInfo&, const String& host);
 
     static void setCustomProtocolRequestType(GType);

Source/WebKit/ChangeLog

@@ -1,3 +1,22 @@
+2018-04-20  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [SOUP] Do TLS error checking on GTlsConnection::accept-certificate
+        https://bugs.webkit.org/show_bug.cgi?id=184480
+
+        Reviewed by Michael Catanzaro.
+
+        Connect to GTlsConnection::accept-certificate signal instead of SoupMessage::notify::tls-errors to perform the
+        TLS errors check.
+
+        * NetworkProcess/soup/NetworkDataTaskSoup.cpp:
+        (WebKit::NetworkDataTaskSoup::createRequest): Do not connect to SoupMessage::notify::tls-errors.
+        (WebKit::NetworkDataTaskSoup::tlsConnectionAcceptCertificateCallback): Call tlsConnectionAcceptCertificate() is
+        the task is still ongoing.
+        (WebKit::NetworkDataTaskSoup::tlsConnectionAcceptCertificate): Check TLS errors here.
+        (WebKit::NetworkDataTaskSoup::networkEventCallback): Pass the stream to networkEvent.
+        (WebKit::NetworkDataTaskSoup::networkEvent): Connect to GTlsConnection::accept-certificate.
+        * NetworkProcess/soup/NetworkDataTaskSoup.h:
+
 2018-04-17  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [WPE][GTK] GObject introspection annotation fixes: BackForwardList, NetworkProxySettings

Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp

@@ -164,7 +164,6 @@ void NetworkDataTaskSoup::createRequest(ResourceRequest&& request)
     m_soupRequest = WTFMove(soupRequest);
     m_soupMessage = WTFMove(soupMessage);
 
-    g_signal_connect(m_soupMessage.get(), "notify::tls-errors", G_CALLBACK(tlsErrorsChangedCallback), this);
     g_signal_connect(m_soupMessage.get(), "got-headers", G_CALLBACK(gotHeadersCallback), this);
     g_signal_connect(m_soupMessage.get(), "wrote-body-data", G_CALLBACK(wroteBodyDataCallback), this);
     g_signal_connect(static_cast<NetworkSessionSoup&>(m_session.get()).soupSession(), "authenticate",  G_CALLBACK(authenticateCallback), this);
@@ -403,28 +402,32 @@ void NetworkDataTaskSoup::dispatchDidCompleteWithError(const ResourceError& erro
     m_client->didCompleteWithError(error, m_networkLoadMetrics);
 }
 
-void NetworkDataTaskSoup::tlsErrorsChangedCallback(SoupMessage* soupMessage, GParamSpec*, NetworkDataTaskSoup* task)
+gboolean NetworkDataTaskSoup::tlsConnectionAcceptCertificateCallback(GTlsConnection* connection, GTlsCertificate* certificate, GTlsCertificateFlags errors, NetworkDataTaskSoup* task)
 {
     if (task->state() == State::Canceling || task->state() == State::Completed || !task->m_client) {
         task->clearRequest();
-        return;
+        return FALSE;
     }
 
-    ASSERT(soupMessage == task->m_soupMessage.get());
-    task->tlsErrorsChanged();
+    auto* connectionMessage = g_object_get_data(G_OBJECT(connection), "wk-soup-message");
+    if (connectionMessage != task->m_soupMessage.get())
+        return FALSE;
+
+    return task->tlsConnectionAcceptCertificate(certificate, errors);
 }
 
-void NetworkDataTaskSoup::tlsErrorsChanged()
+bool NetworkDataTaskSoup::tlsConnectionAcceptCertificate(GTlsCertificate* certificate, GTlsCertificateFlags tlsErrors)
 {
     ASSERT(m_soupRequest);
-    SoupNetworkSession::checkTLSErrors(m_soupRequest.get(), m_soupMessage.get(), [this] (const ResourceError& error) {
-        if (error.isNull())
-            return;
+    URL url(soup_request_get_uri(m_soupRequest.get()));
+    auto error = SoupNetworkSession::checkTLSErrors(url, certificate, tlsErrors);
+    if (!error)
+        return true;
 
-        RefPtr<NetworkDataTaskSoup> protectedThis(this);
-        invalidateAndCancel();
-        dispatchDidCompleteWithError(error);
-    });
+    RefPtr<NetworkDataTaskSoup> protectedThis(this);
+    invalidateAndCancel();
+    dispatchDidCompleteWithError(error.value());
+    return false;
 }
 
 void NetworkDataTaskSoup::applyAuthenticationToRequest(ResourceRequest& request)
@@ -1040,16 +1043,16 @@ void NetworkDataTaskSoup::didFail(const ResourceError& error)
     dispatchDidCompleteWithError(error);
 }
 
-void NetworkDataTaskSoup::networkEventCallback(SoupMessage* soupMessage, GSocketClientEvent event, GIOStream*, NetworkDataTaskSoup* task)
+void NetworkDataTaskSoup::networkEventCallback(SoupMessage* soupMessage, GSocketClientEvent event, GIOStream* stream, NetworkDataTaskSoup* task)
 {
     if (task->state() == State::Canceling || task->state() == State::Completed || !task->m_client)
         return;
 
     ASSERT(task->m_soupMessage.get() == soupMessage);
-    task->networkEvent(event);
+    task->networkEvent(event, stream);
 }
 
-void NetworkDataTaskSoup::networkEvent(GSocketClientEvent event)
+void NetworkDataTaskSoup::networkEvent(GSocketClientEvent event, GIOStream* stream)
 {
     Seconds deltaTime = MonotonicTime::now() - m_startTime;
     switch (event) {
@@ -1072,6 +1075,9 @@ void NetworkDataTaskSoup::networkEvent(GSocketClientEvent event)
         break;
     case G_SOCKET_CLIENT_TLS_HANDSHAKING:
         m_networkLoadMetrics.secureConnectionStart = deltaTime;
+        RELEASE_ASSERT(G_IS_TLS_CONNECTION(stream));
+        g_object_set_data(G_OBJECT(stream), "wk-soup-message", m_soupMessage.get());
+        g_signal_connect(stream, "accept-certificate", G_CALLBACK(tlsConnectionAcceptCertificateCallback), this);
         break;
     case G_SOCKET_CLIENT_TLS_HANDSHAKED:
         break;

Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.h

@@ -66,8 +66,8 @@ class NetworkDataTaskSoup final : public NetworkDataTask {
     void dispatchDidReceiveResponse();
     void dispatchDidCompleteWithError(const WebCore::ResourceError&);
 
-    static void tlsErrorsChangedCallback(SoupMessage*, GParamSpec*, NetworkDataTaskSoup*);
-    void tlsErrorsChanged();
+    static gboolean tlsConnectionAcceptCertificateCallback(GTlsConnection*, GTlsCertificate*, GTlsCertificateFlags, NetworkDataTaskSoup*);
+    bool tlsConnectionAcceptCertificate(GTlsCertificate*, GTlsCertificateFlags);
 
     void applyAuthenticationToRequest(WebCore::ResourceRequest&);
     static void authenticateCallback(SoupSession*, SoupMessage*, SoupAuth*, gboolean retrying, NetworkDataTaskSoup*);
@@ -107,7 +107,7 @@ class NetworkDataTaskSoup final : public NetworkDataTask {
     void didFail(const WebCore::ResourceError&);
 
     static void networkEventCallback(SoupMessage*, GSocketClientEvent, GIOStream*, NetworkDataTaskSoup*);
-    void networkEvent(GSocketClientEvent);
+    void networkEvent(GSocketClientEvent, GIOStream*);
 #if SOUP_CHECK_VERSION(2, 49, 91)
     static void startingCallback(SoupMessage*, NetworkDataTaskSoup*);
 #else