[webkit-changes] [230196] trunk/Source

Tue, 03 Apr 2018 00:10:24 -0700

Title: [230196] trunk/Source
Revision
230196
Author
carlo...@webkit.org
Date
2018-04-03 00:09:34 -0700 (Tue, 03 Apr 2018)

Log Message

[GTK] NetworkProcess from WebKitGtk+ 2.19.9x SIGSEVs in NetworkStorageSession (secret search callback)
https://bugs.webkit.org/show_bug.cgi?id=183346

Reviewed by Michael Catanzaro.

Source/WebCore:

This might happen if a request is cancelled right after the password request starts and before it finishes. We
should cancel the password search when the network request is cancelled, not only when the NetworkStorageSession
is destroyed.

* platform/network/NetworkStorageSession.h:
* platform/network/soup/NetworkStorageSessionSoup.cpp:
(WebCore::NetworkStorageSession::~NetworkStorageSession):
(WebCore::SecretServiceSearchData::SecretServiceSearchData): Helper struct to keep the request cancellable and
completion handler.
(WebCore::NetworkStorageSession::getCredentialFromPersistentStorage): Create a SecretServiceSearchData for the
request.
* platform/network/soup/ResourceHandleSoup.cpp:
(WebCore::ResourceHandle::didReceiveAuthenticationChallenge): Pass the request cancellable to
NetworkStorageSession::getCredentialFromPersistentStorage().

Source/WebKit:

Pass the request cancellable to NetworkStorageSession::getCredentialFromPersistentStorage().

* NetworkProcess/soup/NetworkDataTaskSoup.cpp:
(WebKit::NetworkDataTaskSoup::authenticate):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (230195 => 230196)


--- trunk/Source/WebCore/ChangeLog	2018-04-03 01:03:46 UTC (rev 230195)
+++ trunk/Source/WebCore/ChangeLog	2018-04-03 07:09:34 UTC (rev 230196)
@@ -1,3 +1,25 @@
+2018-04-03  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        [GTK] NetworkProcess from WebKitGtk+ 2.19.9x SIGSEVs in NetworkStorageSession (secret search callback)
+        https://bugs.webkit.org/show_bug.cgi?id=183346
+
+        Reviewed by Michael Catanzaro.
+
+        This might happen if a request is cancelled right after the password request starts and before it finishes. We
+        should cancel the password search when the network request is cancelled, not only when the NetworkStorageSession
+        is destroyed.
+
+        * platform/network/NetworkStorageSession.h:
+        * platform/network/soup/NetworkStorageSessionSoup.cpp:
+        (WebCore::NetworkStorageSession::~NetworkStorageSession):
+        (WebCore::SecretServiceSearchData::SecretServiceSearchData): Helper struct to keep the request cancellable and
+        completion handler.
+        (WebCore::NetworkStorageSession::getCredentialFromPersistentStorage): Create a SecretServiceSearchData for the
+        request.
+        * platform/network/soup/ResourceHandleSoup.cpp:
+        (WebCore::ResourceHandle::didReceiveAuthenticationChallenge): Pass the request cancellable to
+        NetworkStorageSession::getCredentialFromPersistentStorage().
+
 2018-04-02  Eric Carlson  <eric.carl...@apple.com>
 
         [Extra zoom mode] Replace video with a placeholder image during fullscreen transition

Modified: trunk/Source/WebCore/platform/network/NetworkStorageSession.h (230195 => 230196)


--- trunk/Source/WebCore/platform/network/NetworkStorageSession.h	2018-04-03 01:03:46 UTC (rev 230195)
+++ trunk/Source/WebCore/platform/network/NetworkStorageSession.h	2018-04-03 07:09:34 UTC (rev 230196)
@@ -124,7 +124,7 @@
     SoupCookieJar* cookieStorage() const;
     void setCookieStorage(SoupCookieJar*);
     void setCookieObserverHandler(Function<void ()>&&);
-    void getCredentialFromPersistentStorage(const ProtectionSpace&, Function<void (Credential&&)> completionHandler);
+    void getCredentialFromPersistentStorage(const ProtectionSpace&, GCancellable*, Function<void (Credential&&)>&& completionHandler);
     void saveCredentialToPersistentStorage(const ProtectionSpace&, const Credential&);
 #elif USE(CURL)
     NetworkStorageSession(PAL::SessionID, NetworkingContext*);
@@ -162,10 +162,6 @@
     mutable std::unique_ptr<SoupNetworkSession> m_session;
     GRefPtr<SoupCookieJar> m_cookieStorage;
     Function<void ()> m_cookieObserverHandler;
-#if USE(LIBSECRET)
-    Function<void (Credential&&)> m_persisentStorageCompletionHandler;
-    GRefPtr<GCancellable> m_persisentStorageCancellable;
-#endif
 #elif USE(CURL)
     RefPtr<NetworkingContext> m_context;

Modified: trunk/Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp (230195 => 230196)


--- trunk/Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp	2018-04-03 01:03:46 UTC (rev 230195)
+++ trunk/Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp	2018-04-03 07:09:34 UTC (rev 230196)
@@ -61,10 +61,6 @@
 NetworkStorageSession::~NetworkStorageSession()
 {
     g_signal_handlers_disconnect_matched(m_cookieStorage.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
-
-#if USE(LIBSECRET)
-    g_cancellable_cancel(m_persisentStorageCancellable.get());
-#endif
 }
 
 static std::unique_ptr<NetworkStorageSession>& defaultSession()
@@ -187,9 +183,22 @@
     ASSERT_NOT_REACHED();
     return "unknown";
 }
+
+struct SecretServiceSearchData {
+    SecretServiceSearchData(GCancellable* cancellable, Function<void (Credential&&)>&& completionHandler)
+        : cancellable(cancellable)
+        , completionHandler(WTFMove(completionHandler))
+    {
+    }
+
+    ~SecretServiceSearchData() = default;
+
+    GRefPtr<GCancellable> cancellable;
+    Function<void (Credential&&)> completionHandler;
+};
 #endif // USE(LIBSECRET)
 
-void NetworkStorageSession::getCredentialFromPersistentStorage(const ProtectionSpace& protectionSpace, Function<void (Credential&&)> completionHandler)
+void NetworkStorageSession::getCredentialFromPersistentStorage(const ProtectionSpace& protectionSpace, GCancellable* cancellable, Function<void (Credential&&)>&& completionHandler)
 {
 #if USE(LIBSECRET)
     if (m_sessionID.isEphemeral()) {
@@ -215,28 +224,24 @@
         return;
     }
 
-    m_persisentStorageCancellable = adoptGRef(g_cancellable_new());
-    m_persisentStorageCompletionHandler = WTFMove(completionHandler);
+    auto data = "" WTFMove(completionHandler));
     secret_service_search(nullptr, SECRET_SCHEMA_COMPAT_NETWORK, attributes.get(),
-        static_cast<SecretSearchFlags>(SECRET_SEARCH_UNLOCK | SECRET_SEARCH_LOAD_SECRETS), m_persisentStorageCancellable.get(),
+        static_cast<SecretSearchFlags>(SECRET_SEARCH_UNLOCK | SECRET_SEARCH_LOAD_SECRETS), cancellable,
         [](GObject* source, GAsyncResult* result, gpointer userData) {
+            auto data = ""
             GUniqueOutPtr<GError> error;
             GUniquePtr<GList> elements(secret_service_search_finish(SECRET_SERVICE(source), result, &error.outPtr()));
-            if (g_error_matches (error.get(), G_IO_ERROR, G_IO_ERROR_CANCELLED))
+            if (g_cancellable_is_cancelled(data->cancellable.get()) || error || !elements || !elements->data) {
+                data->completionHandler({ });
                 return;
-
-            NetworkStorageSession* session = static_cast<NetworkStorageSession*>(userData);
-            auto completionHandler = std::exchange(session->m_persisentStorageCompletionHandler, nullptr);
-            if (error || !elements || !elements->data) {
-                completionHandler({ });
-                return;
             }
 
-            GRefPtr<SecretItem> secretItem = adoptGRef(static_cast<SecretItem*>(elements->data));
+            GRefPtr<SecretItem> secretItem = static_cast<SecretItem*>(elements->data);
+            g_list_foreach(elements.get(), reinterpret_cast<GFunc>(g_object_unref), nullptr);
             GRefPtr<GHashTable> attributes = adoptGRef(secret_item_get_attributes(secretItem.get()));
             String user = String::fromUTF8(static_cast<const char*>(g_hash_table_lookup(attributes.get(), "user")));
             if (user.isEmpty()) {
-                completionHandler({ });
+                data->completionHandler({ });
                 return;
             }
 
@@ -243,8 +248,8 @@
             size_t length;
             GRefPtr<SecretValue> secretValue = adoptGRef(secret_item_get_secret(secretItem.get()));
             const char* passwordData = secret_value_get(secretValue.get(), &length);
-            completionHandler(Credential(user, String::fromUTF8(passwordData, length), CredentialPersistencePermanent));
-    }, this);
+            data->completionHandler(Credential(user, String::fromUTF8(passwordData, length), CredentialPersistencePermanent));
+        }, data.release());
 #else
     UNUSED_PARAM(protectionSpace);
     completionHandler({ });

Modified: trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp (230195 => 230196)


--- trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp	2018-04-03 01:03:46 UTC (rev 230195)
+++ trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp	2018-04-03 07:09:34 UTC (rev 230196)
@@ -834,7 +834,7 @@
     // use HTTP authentication. In the end, this doesn't matter much, because persistent credentials
     // will become session credentials after the first use.
     if (useCredentialStorage && d->m_context && d->m_context->isValid()) {
-        d->m_context->storageSession().getCredentialFromPersistentStorage(challenge.protectionSpace(), [this, protectedThis = makeRef(*this)] (Credential&& credential) {
+        d->m_context->storageSession().getCredentialFromPersistentStorage(challenge.protectionSpace(), d->m_cancellable.get(), [this, protectedThis = makeRef(*this)] (Credential&& credential) {
             continueDidReceiveAuthenticationChallenge(WTFMove(credential));
         });
         return;

Modified: trunk/Source/WebKit/ChangeLog (230195 => 230196)


--- trunk/Source/WebKit/ChangeLog	2018-04-03 01:03:46 UTC (rev 230195)
+++ trunk/Source/WebKit/ChangeLog	2018-04-03 07:09:34 UTC (rev 230196)
@@ -1,3 +1,15 @@
+2018-04-03  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        [GTK] NetworkProcess from WebKitGtk+ 2.19.9x SIGSEVs in NetworkStorageSession (secret search callback)
+        https://bugs.webkit.org/show_bug.cgi?id=183346
+
+        Reviewed by Michael Catanzaro.
+
+        Pass the request cancellable to NetworkStorageSession::getCredentialFromPersistentStorage().
+
+        * NetworkProcess/soup/NetworkDataTaskSoup.cpp:
+        (WebKit::NetworkDataTaskSoup::authenticate):
+
 2018-04-02  Brady Eidson  <beid...@apple.com>
 
         Process swapping on navigation needs to handle server redirects.

Modified: trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp (230195 => 230196)


--- trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp	2018-04-03 01:03:46 UTC (rev 230195)
+++ trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp	2018-04-03 07:09:34 UTC (rev 230196)
@@ -502,7 +502,7 @@
     // will become session credentials after the first use.
     if (m_storedCredentialsPolicy == StoredCredentialsPolicy::Use) {
         auto protectionSpace = challenge.protectionSpace();
-        m_session->networkStorageSession().getCredentialFromPersistentStorage(protectionSpace,
+        m_session->networkStorageSession().getCredentialFromPersistentStorage(protectionSpace, m_cancellable.get(),
             [this, protectedThis = makeRef(*this), authChallenge = WTFMove(challenge)] (Credential&& credential) mutable {
                 if (m_state == State::Canceling || m_state == State::Completed || !m_client) {
                     clearRequest();

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to