Attachment 346654 Details for Bug 188355 – Patch

[patch] Patch

bug-188355-20180806135720.patch (text/plain), 11.51 KB, created by Chris Dumez on 2018-08-06 13:57:20 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-08-06 13:57:20 PDT

Size: 11.51 KB

patch

obsolete

>Subversion Revision: 234602
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 4df3209a81b45b6f03d1ae414f04f04e325cc1d4..69d4acf8e0d007133855f95b893804484853e71f 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,40 @@
>+2018-08-06  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression(NetworkLoadChecker): CORS preflights are no longer able to deal with client certificate authentication
>+        https://bugs.webkit.org/show_bug.cgi?id=188355
>+        <rdar://problem/42546319>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Before we started using the NetworkLoadChecker to do CORS-preflighting in the Network process, challenges would
>+        use the NetworkLoad::completeAuthenticationChallenge() code path with isAllowedToAskUserForCredentials to set
>+        to false. This would call:
>+        1. completionHandler(AuthenticationChallengeDisposition::UseCredential, { }); for TLS handshakes (server trust
>+           evaluation & client certification authentication)
>+        2. NetworkProcess::singleton().authenticationManager().didReceiveAuthenticationChallenge() otherwise
>+
>+        However, NetworkCORSPreflightChecker::didReceiveChallenge() was behaving differently and calling:
>+        1. completionHandler(AuthenticationChallengeDisposition::RejectProtectionSpace, { }); for server trust evaluations
>+        2. completionHandler(AuthenticationChallengeDisposition::Cancel, { }); otherwise
>+
>+        Restore previous behavior by aligning NetworkCORSPreflightChecker::didReceiveChallenge() with
>+        NetworkLoad::completeAuthenticationChallenge() when isAllowedToAskUserForCredentials is set to false. This means
>+        we end up asking the AuthenticationManager for client certificate authentication instead or cancelling the
>+        preflight.
>+
>+        This fixes CORS-preflighting on some internal sites.
>+
>+        * NetworkProcess/NetworkCORSPreflightChecker.cpp:
>+        (WebKit::NetworkCORSPreflightChecker::didReceiveChallenge):
>+        * NetworkProcess/NetworkCORSPreflightChecker.h:
>+        * NetworkProcess/NetworkLoadChecker.cpp:
>+        (WebKit::NetworkLoadChecker::NetworkLoadChecker):
>+        (WebKit::NetworkLoadChecker::checkCORSRequestWithPreflight):
>+        * NetworkProcess/NetworkLoadChecker.h:
>+        * NetworkProcess/NetworkResourceLoader.cpp:
>+        * NetworkProcess/PingLoad.cpp:
>+        (WebKit::PingLoad::PingLoad):
>+
> 2018-08-06  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [iOS] Caret disappears after resigning and becoming first responder if active focus state is retained
>diff --git a/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp b/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp
>index 8e9733cf1880d34cc37d7ad9f9d760cbfbdb2dbf..fb6861fb4a4503612dba3848cb73e4cddcb30a2a 100644
>--- a/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp
>@@ -29,6 +29,7 @@
> #include "AuthenticationManager.h"
> #include "Logging.h"
> #include "NetworkLoadParameters.h"
>+#include "NetworkProcess.h"
> #include "SessionTracker.h"
> #include <WebCore/CrossOriginAccessControl.h>
> #include <WebCore/SecurityOrigin.h>
>@@ -90,15 +91,18 @@ void NetworkCORSPreflightChecker::willPerformHTTPRedirection(WebCore::ResourceRe
> 
> void NetworkCORSPreflightChecker::didReceiveChallenge(const WebCore::AuthenticationChallenge& challenge, ChallengeCompletionHandler&& completionHandler)
> {
>-    RELEASE_LOG_IF_ALLOWED("didReceiveChallenge");
>+    RELEASE_LOG_IF_ALLOWED("didReceiveChallenge, authentication scheme: %u", challenge.protectionSpace().authenticationScheme());
> 
>-    if (challenge.protectionSpace().authenticationScheme() == ProtectionSpaceAuthenticationSchemeServerTrustEvaluationRequested) {
>-        completionHandler(AuthenticationChallengeDisposition::RejectProtectionSpace, { });
>+    auto scheme = challenge.protectionSpace().authenticationScheme();
>+    bool isTLSHandshake = scheme == ProtectionSpaceAuthenticationSchemeServerTrustEvaluationRequested
>+        || scheme == ProtectionSpaceAuthenticationSchemeClientCertificateRequested;
>+
>+    if (!isTLSHandshake) {
>+        completionHandler(AuthenticationChallengeDisposition::UseCredential, { });
>         return;
>     }
> 
>-    completionHandler(AuthenticationChallengeDisposition::Cancel, { });
>-    m_completionCallback(ResourceError { errorDomainWebKitInternal, 0, m_parameters.originalRequest.url(), "Preflight response is not successful"_s, ResourceError::Type::AccessControl });
>+    NetworkProcess::singleton().authenticationManager().didReceiveAuthenticationChallenge(m_parameters.pageID, m_parameters.frameID, challenge, WTFMove(completionHandler));
> }
> 
> void NetworkCORSPreflightChecker::didReceiveResponseNetworkSession(WebCore::ResourceResponse&& response, ResponseCompletionHandler&& completionHandler)
>diff --git a/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.h b/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.h
>index e66ea6d33d54875fe5b859b263927388062d1a5d..5d6e6363d68bdf76c4e22adf95ee988bd971ba28 100644
>--- a/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.h
>+++ b/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.h
>@@ -47,6 +47,8 @@ public:
>         String referrer;
>         String userAgent;
>         PAL::SessionID sessionID;
>+        uint64_t pageID;
>+        uint64_t frameID;
>         WebCore::StoredCredentialsPolicy storedCredentialsPolicy;
>     };
>     using CompletionCallback = CompletionHandler<void(WebCore::ResourceError&&)>;
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>index a87bdeb2dca7dd59b6d95cfeeb984e91228abb91..804e7b5c5fd04119a80417ede51b90070d2aed3f 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>@@ -53,9 +53,11 @@ static inline bool isSameOrigin(const URL& url, const SecurityOrigin* origin)
>     return url.protocolIsData() || url.protocolIsBlob() || !origin || origin->canRequest(url);
> }
> 
>-NetworkLoadChecker::NetworkLoadChecker(FetchOptions&& options, PAL::SessionID sessionID, HTTPHeaderMap&& originalRequestHeaders, URL&& url, RefPtr<SecurityOrigin>&& sourceOrigin, PreflightPolicy preflightPolicy, String&& referrer, bool shouldCaptureExtraNetworkLoadMetrics)
>+NetworkLoadChecker::NetworkLoadChecker(FetchOptions&& options, PAL::SessionID sessionID, uint64_t pageID, uint64_t frameID, HTTPHeaderMap&& originalRequestHeaders, URL&& url, RefPtr<SecurityOrigin>&& sourceOrigin, PreflightPolicy preflightPolicy, String&& referrer, bool shouldCaptureExtraNetworkLoadMetrics)
>     : m_options(WTFMove(options))
>     , m_sessionID(sessionID)
>+    , m_pageID(pageID)
>+    , m_frameID(frameID)
>     , m_originalRequestHeaders(WTFMove(originalRequestHeaders))
>     , m_url(WTFMove(url))
>     , m_origin(WTFMove(sourceOrigin))
>@@ -355,6 +357,8 @@ void NetworkLoadChecker::checkCORSRequestWithPreflight(ResourceRequest&& request
>         request.httpReferrer(),
>         request.httpUserAgent(),
>         m_sessionID,
>+        m_pageID,
>+        m_frameID,
>         m_storedCredentialsPolicy
>     };
>     m_corsPreflightChecker = std::make_unique<NetworkCORSPreflightChecker>(WTFMove(parameters), m_shouldCaptureExtraNetworkLoadMetrics, [this, request = WTFMove(request), handler = WTFMove(handler), isRedirected = isRedirected()](auto&& error) mutable {
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>index 68017686a8b7f1ea6f224bb739e1535187244f13..1797d7e025a04dd23ce497c811af281a2597125d 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>@@ -47,7 +47,7 @@ class NetworkCORSPreflightChecker;
> 
> class NetworkLoadChecker : public CanMakeWeakPtr<NetworkLoadChecker> {
> public:
>-    NetworkLoadChecker(WebCore::FetchOptions&&, PAL::SessionID, WebCore::HTTPHeaderMap&&, WebCore::URL&&, RefPtr<WebCore::SecurityOrigin>&&, WebCore::PreflightPolicy, String&& referrer, bool shouldCaptureExtraNetworkLoadMetrics = false);
>+    NetworkLoadChecker(WebCore::FetchOptions&&, PAL::SessionID, uint64_t pageID, uint64_t frameID, WebCore::HTTPHeaderMap&&, WebCore::URL&&, RefPtr<WebCore::SecurityOrigin>&&, WebCore::PreflightPolicy, String&& referrer, bool shouldCaptureExtraNetworkLoadMetrics = false);
>     ~NetworkLoadChecker();
> 
>     using RequestOrError = Expected<WebCore::ResourceRequest, WebCore::ResourceError>;
>@@ -114,6 +114,8 @@ private:
>     WebCore::FetchOptions m_options;
>     WebCore::StoredCredentialsPolicy m_storedCredentialsPolicy;
>     PAL::SessionID m_sessionID;
>+    uint64_t m_pageID;
>+    uint64_t m_frameID;
>     WebCore::HTTPHeaderMap m_originalRequestHeaders; // Needed for CORS checks.
>     WebCore::HTTPHeaderMap m_firstRequestHeaders; // Needed for CORS checks.
>     WebCore::URL m_url;
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>index 1b1bf272a00b7694611d0edea8aab6733ab06924..77f898903e2d24d14df44b0f0c3cd87e6f3756e4 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>@@ -118,7 +118,7 @@ NetworkResourceLoader::NetworkResourceLoader(NetworkResourceLoadParameters&& par
>     }
> 
>     if (synchronousReply || parameters.shouldRestrictHTTPResponseAccess) {
>-        m_networkLoadChecker = std::make_unique<NetworkLoadChecker>(FetchOptions { m_parameters.options }, m_parameters.sessionID, HTTPHeaderMap { m_parameters.originalRequestHeaders }, URL { m_parameters.request.url() }, m_parameters.sourceOrigin.copyRef(), m_parameters.preflightPolicy, originalRequest().httpReferrer(), shouldCaptureExtraNetworkLoadMetrics());
>+        m_networkLoadChecker = std::make_unique<NetworkLoadChecker>(FetchOptions { m_parameters.options }, m_parameters.sessionID, m_parameters.webPageID, m_parameters.webFrameID, HTTPHeaderMap { m_parameters.originalRequestHeaders }, URL { m_parameters.request.url() }, m_parameters.sourceOrigin.copyRef(), m_parameters.preflightPolicy, originalRequest().httpReferrer(), shouldCaptureExtraNetworkLoadMetrics());
>         if (m_parameters.cspResponseHeaders)
>             m_networkLoadChecker->setCSPResponseHeaders(ContentSecurityPolicyResponseHeaders { m_parameters.cspResponseHeaders.value() });
> #if ENABLE(CONTENT_EXTENSIONS)
>diff --git a/Source/WebKit/NetworkProcess/PingLoad.cpp b/Source/WebKit/NetworkProcess/PingLoad.cpp
>index e96e8593208fcc7c915c50716cbff44affecc66c..40143bcea6a23f535bbc2c8074c14a8405436fa5 100644
>--- a/Source/WebKit/NetworkProcess/PingLoad.cpp
>+++ b/Source/WebKit/NetworkProcess/PingLoad.cpp
>@@ -42,7 +42,7 @@ PingLoad::PingLoad(NetworkResourceLoadParameters&& parameters, WTF::CompletionHa
>     : m_parameters(WTFMove(parameters))
>     , m_completionHandler(WTFMove(completionHandler))
>     , m_timeoutTimer(*this, &PingLoad::timeoutTimerFired)
>-    , m_networkLoadChecker(makeUniqueRef<NetworkLoadChecker>(FetchOptions { m_parameters.options}, m_parameters.sessionID, WTFMove(m_parameters.originalRequestHeaders), URL { m_parameters.request.url() }, m_parameters.sourceOrigin.copyRef(), m_parameters.preflightPolicy, m_parameters.request.httpReferrer()))
>+    , m_networkLoadChecker(makeUniqueRef<NetworkLoadChecker>(FetchOptions { m_parameters.options}, m_parameters.sessionID, m_parameters.webPageID, m_parameters.webFrameID, WTFMove(m_parameters.originalRequestHeaders), URL { m_parameters.request.url() }, m_parameters.sourceOrigin.copyRef(), m_parameters.preflightPolicy, m_parameters.request.httpReferrer()))
> {
>     m_networkLoadChecker->enableContentExtensionsCheck();
>     if (m_parameters.cspResponseHeaders)

Actions: View | Formatted Diff | Diff

Attachments on bug 188355: 346654