Title: [234672] branches/safari-606.1.36.1-branch/Source/WebKit
Revision
234672
Author
kocsen_ch...@apple.com
Date
2018-08-07 14:29:25 -0700 (Tue, 07 Aug 2018)

Log Message

Cherry-pick r234626. rdar://problem/43009894

    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 Alex Christensen.

    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):

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234626 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Diff

Modified: branches/safari-606.1.36.1-branch/Source/WebKit/ChangeLog (234671 => 234672)


--- branches/safari-606.1.36.1-branch/Source/WebKit/ChangeLog	2018-08-07 21:29:20 UTC (rev 234671)
+++ branches/safari-606.1.36.1-branch/Source/WebKit/ChangeLog	2018-08-07 21:29:25 UTC (rev 234672)
@@ -1,5 +1,84 @@
 2018-08-07  Kocsen Chung  <kocsen_ch...@apple.com>
 
+        Cherry-pick r234626. rdar://problem/43009894
+
+    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 Alex Christensen.
+    
+    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):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234626 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2018-08-06  Chris Dumez  <cdu...@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 Alex Christensen.
+
+            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-07  Kocsen Chung  <kocsen_ch...@apple.com>
+
         Cherry-pick r234611. rdar://problem/43009901
 
     Fix IPC::Connection leak in StorageManager

Modified: branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp (234671 => 234672)


--- branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp	2018-08-07 21:29:20 UTC (rev 234671)
+++ branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp	2018-08-07 21:29:25 UTC (rev 234672)
@@ -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::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)

Modified: branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.h (234671 => 234672)


--- branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.h	2018-08-07 21:29:20 UTC (rev 234671)
+++ branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.h	2018-08-07 21:29:25 UTC (rev 234672)
@@ -47,6 +47,8 @@
         String referrer;
         String userAgent;
         PAL::SessionID sessionID;
+        uint64_t pageID;
+        uint64_t frameID;
         WebCore::StoredCredentialsPolicy storedCredentialsPolicy;
     };
     using CompletionCallback = CompletionHandler<void(WebCore::ResourceError&&)>;

Modified: branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp (234671 => 234672)


--- branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp	2018-08-07 21:29:20 UTC (rev 234671)
+++ branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp	2018-08-07 21:29:25 UTC (rev 234672)
@@ -53,9 +53,11 @@
     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 @@
         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 {

Modified: branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkLoadChecker.h (234671 => 234672)


--- branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkLoadChecker.h	2018-08-07 21:29:20 UTC (rev 234671)
+++ branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkLoadChecker.h	2018-08-07 21:29:25 UTC (rev 234672)
@@ -47,7 +47,7 @@
 
 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 @@
     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;

Modified: branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp (234671 => 234672)


--- branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp	2018-08-07 21:29:20 UTC (rev 234671)
+++ branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp	2018-08-07 21:29:25 UTC (rev 234672)
@@ -119,7 +119,7 @@
     }
 
     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)

Modified: branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/PingLoad.cpp (234671 => 234672)


--- branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/PingLoad.cpp	2018-08-07 21:29:20 UTC (rev 234671)
+++ branches/safari-606.1.36.1-branch/Source/WebKit/NetworkProcess/PingLoad.cpp	2018-08-07 21:29:25 UTC (rev 234672)
@@ -42,7 +42,7 @@
     : 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)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to