Title: [230885] trunk/Source
Revision
230885
Author
carlo...@webkit.org
Date
2018-04-20 23:22:15 -0700 (Fri, 20 Apr 2018)

Log Message

[SOUP] Do TLS error checking on GTlsConnection::accept-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:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (230884 => 230885)


--- trunk/Source/WebCore/ChangeLog	2018-04-21 06:16:29 UTC (rev 230884)
+++ trunk/Source/WebCore/ChangeLog	2018-04-21 06:22:15 UTC (rev 230885)
@@ -1,3 +1,22 @@
+2018-04-20  Carlos Garcia Campos  <cgar...@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-04-20  Tim Horton  <timothy_hor...@apple.com>
 
         Adjust geolocation feature flag

Modified: trunk/Source/WebCore/platform/network/soup/ResourceError.h (230884 => 230885)


--- trunk/Source/WebCore/platform/network/soup/ResourceError.h	2018-04-21 06:16:29 UTC (rev 230884)
+++ trunk/Source/WebCore/platform/network/soup/ResourceError.h	2018-04-21 06:22:15 UTC (rev 230885)
@@ -56,7 +56,7 @@
     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*);
 

Modified: trunk/Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp (230884 => 230885)


--- trunk/Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp	2018-04-21 06:16:29 UTC (rev 230884)
+++ trunk/Source/WebCore/platform/network/soup/ResourceErrorSoup.cpp	2018-04-21 06:22:15 UTC (rev 230885)
@@ -75,10 +75,9 @@
         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;

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


--- trunk/Source/WebCore/platform/network/soup/SoupNetworkSession.cpp	2018-04-21 06:16:29 UTC (rev 230884)
+++ trunk/Source/WebCore/platform/network/soup/SoupNetworkSession.cpp	2018-04-21 06:22:15 UTC (rev 230885)
@@ -119,7 +119,7 @@
         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();
@@ -269,29 +269,19 @@
     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)

Modified: trunk/Source/WebCore/platform/network/soup/SoupNetworkSession.h (230884 => 230885)


--- trunk/Source/WebCore/platform/network/soup/SoupNetworkSession.h	2018-04-21 06:16:29 UTC (rev 230884)
+++ trunk/Source/WebCore/platform/network/soup/SoupNetworkSession.h	2018-04-21 06:22:15 UTC (rev 230885)
@@ -65,7 +65,7 @@
     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);

Modified: trunk/Source/WebKit/ChangeLog (230884 => 230885)


--- trunk/Source/WebKit/ChangeLog	2018-04-21 06:16:29 UTC (rev 230884)
+++ trunk/Source/WebKit/ChangeLog	2018-04-21 06:22:15 UTC (rev 230885)
@@ -1,3 +1,22 @@
+2018-04-20  Carlos Garcia Campos  <cgar...@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-20  Timothy Hatcher  <timo...@apple.com>
 
         NULL dereference crash sometimes under [super initWithCoder:] in WebView

Modified: trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp (230884 => 230885)


--- trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp	2018-04-21 06:16:29 UTC (rev 230884)
+++ trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp	2018-04-21 06:22:15 UTC (rev 230885)
@@ -164,7 +164,6 @@
     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);
@@ -406,28 +405,32 @@
     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)
@@ -1043,16 +1046,16 @@
     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) {
@@ -1075,6 +1078,9 @@
         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;

Modified: trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.h (230884 => 230885)


--- trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.h	2018-04-21 06:16:29 UTC (rev 230884)
+++ trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.h	2018-04-21 06:22:15 UTC (rev 230885)
@@ -66,8 +66,8 @@
     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 @@
     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
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to