Changeset 260302 in webkit
- Timestamp:
- Apr 17, 2020 4:40:39 PM (4 years ago)
- Location:
- trunk
- Files:
-
- 7 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebKit/ChangeLog
r260299 r260302 1 2020-04-17 Alex Christensen <achristensen@webkit.org> 2 3 NetworkSessionCocoa should request client certificate only once per host/port 4 https://bugs.webkit.org/show_bug.cgi?id=210626 5 <rdar://problem/60340449> 6 7 Reviewed by Geoffrey Garen. 8 9 NSURLSession creates more than one TCP connection to a server when using HTTP 1.1. 10 Each TCP connection with TLS generates a didReceiveChallenge to do the server trust evaluation of the certificate chain. 11 If the server requests a client certificate in the TLS handshake, it also generates a didReceiveChallenge to request client 12 certificates as well. This is an implementation detail of our networking. We should not actually ask the WKNavigationDelegate 13 for client certificates more than once per host/port. We should remember the credential and give it to NSURLSession immediately 14 if we have used this credential in the past for a task that has received bytes (either a response or a redirect). If the TLS 15 handshake fails, we should not reuse that same certificate automatically. 16 17 * NetworkProcess/cocoa/NetworkSessionCocoa.h: 18 * NetworkProcess/cocoa/NetworkSessionCocoa.mm: 19 (-[WKNetworkSessionDelegate URLSession:task:willPerformHTTPRedirection:newRequest:completionHandler:]): 20 (-[WKNetworkSessionDelegate URLSession:task:didReceiveChallenge:completionHandler:]): 21 (-[WKNetworkSessionDelegate URLSession:task:didCompleteWithError:]): 22 (-[WKNetworkSessionDelegate URLSession:dataTask:didReceiveResponse:completionHandler:]): 23 (WebKit::NetworkSessionCocoa::clientCertificateSuggestedForHost): 24 (WebKit::NetworkSessionCocoa::taskReceivedBytes): 25 (WebKit::NetworkSessionCocoa::taskFailed): 26 (WebKit::NetworkSessionCocoa::successfulClientCertificateForHost const): 27 1 28 2020-04-17 David Kilzer <ddkilzer@apple.com> 2 29 -
trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.h
r260031 r260302 49 49 class LegacyCustomProtocolManager; 50 50 class NetworkSessionCocoa; 51 using HostAndPort = std::pair<String, uint16_t>; 51 52 52 53 struct SessionWrapper : public CanMakeWeakPtr<SessionWrapper> { … … 101 102 bool preventsSystemHTTPProxyAuthentication() const { return m_preventsSystemHTTPProxyAuthentication; } 102 103 104 void clientCertificateSuggestedForHost(NetworkDataTaskCocoa::TaskIdentifier, NSURLCredential *, const String& host, uint16_t port); 105 void taskReceivedBytes(NetworkDataTaskCocoa::TaskIdentifier); 106 void taskFailed(NetworkDataTaskCocoa::TaskIdentifier); 107 NSURLCredential *successfulClientCertificateForHost(const String& host, uint16_t port) const; 108 103 109 private: 104 110 void invalidateAndCancel() override; … … 147 153 bool m_isInAppBrowserPrivacyEnabled { false }; 148 154 bool m_preventsSystemHTTPProxyAuthentication { false }; 155 156 struct SuggestedClientCertificate { 157 String host; 158 uint16_t port { 0 }; 159 RetainPtr<NSURLCredential> credential; 160 }; 161 HashMap<NetworkDataTaskCocoa::TaskIdentifier, SuggestedClientCertificate> m_suggestedClientCertificates; 162 HashMap<HostAndPort, RetainPtr<NSURLCredential>> m_successfulClientCertificates; 149 163 }; 150 164 -
trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm
r260031 r260302 530 530 auto completionHandlerCopy = Block_copy(completionHandler); 531 531 532 if (auto* sessionCocoa = [self sessionFromTask:task]) 533 sessionCocoa->taskReceivedBytes(taskIdentifier); 534 532 535 bool shouldIgnoreHSTS = false; 533 536 #if ENABLE(RESOURCE_LOAD_STATISTICS) … … 693 696 } 694 697 } 698 699 if ([challenge.protectionSpace.authenticationMethod isEqualToString:NSURLAuthenticationMethodClientCertificate]) { 700 HostAndPort key { challenge.protectionSpace.host, challenge.protectionSpace.port }; 701 if (auto* credential = sessionCocoa->successfulClientCertificateForHost(challenge.protectionSpace.host, challenge.protectionSpace.port)) 702 return completionHandler(NSURLSessionAuthChallengeUseCredential, credential); 703 sessionCocoa->continueDidReceiveChallenge(*_sessionWrapper, challenge, negotiatedLegacyTLS, taskIdentifier, [self existingTask:task], [completionHandler = makeBlockPtr(completionHandler), key, weakSessionCocoa = makeWeakPtr(sessionCocoa), taskIdentifier] (WebKit::AuthenticationChallengeDisposition disposition, const WebCore::Credential& credential) mutable { 704 NSURLCredential *nsCredential = credential.nsCredential(); 705 if (disposition == WebKit::AuthenticationChallengeDisposition::UseCredential && nsCredential && weakSessionCocoa) 706 weakSessionCocoa->clientCertificateSuggestedForHost(taskIdentifier, nsCredential, key.first, key.second); 707 completionHandler(toNSURLSessionAuthChallengeDisposition(disposition), nsCredential); 708 }); 709 return; 710 } 711 695 712 sessionCocoa->continueDidReceiveChallenge(*_sessionWrapper, challenge, negotiatedLegacyTLS, taskIdentifier, [self existingTask:task], [completionHandler = makeBlockPtr(completionHandler)] (WebKit::AuthenticationChallengeDisposition disposition, const WebCore::Credential& credential) mutable { 696 713 completionHandler(toNSURLSessionAuthChallengeDisposition(disposition), credential.nsCredential()); … … 709 726 } 710 727 711 if (auto* networkDataTask = [self existingTask:task]) 728 if (auto* networkDataTask = [self existingTask:task]) { 729 if (auto* sessionCocoa = [self sessionFromTask:task]) 730 sessionCocoa->taskFailed(task.taskIdentifier); 712 731 networkDataTask->didCompleteWithError(error, networkDataTask->networkLoadMetrics()); 713 else if (error) {732 } else if (error) { 714 733 if (!_sessionWrapper) 715 734 return; … … 840 859 if (auto* networkDataTask = [self existingTask:dataTask]) { 841 860 ASSERT(RunLoop::isMain()); 861 862 if (auto* sessionCocoa = [self sessionFromTask:dataTask]) 863 sessionCocoa->taskReceivedBytes(taskIdentifier); 842 864 843 865 NegotiatedLegacyTLS negotiatedLegacyTLS = NegotiatedLegacyTLS::No; … … 1004 1026 #endif 1005 1027 return configuration; 1028 } 1029 1030 void NetworkSessionCocoa::clientCertificateSuggestedForHost(NetworkDataTaskCocoa::TaskIdentifier taskID, NSURLCredential *credential, const String& host, uint16_t port) 1031 { 1032 m_suggestedClientCertificates.set(taskID, SuggestedClientCertificate { host, port, credential }); 1033 } 1034 1035 void NetworkSessionCocoa::taskReceivedBytes(NetworkDataTaskCocoa::TaskIdentifier identifier) 1036 { 1037 if (LIKELY(m_suggestedClientCertificates.isEmpty())) 1038 return; 1039 1040 auto suggestedClientCertificate = m_suggestedClientCertificates.take(identifier); 1041 HostAndPort key { suggestedClientCertificate.host, suggestedClientCertificate.port }; 1042 if (suggestedClientCertificate.credential && decltype(m_successfulClientCertificates)::isValidKey(key)) 1043 m_successfulClientCertificates.add(key, suggestedClientCertificate.credential); 1044 } 1045 1046 void NetworkSessionCocoa::taskFailed(NetworkDataTaskCocoa::TaskIdentifier identifier) 1047 { 1048 if (LIKELY(m_suggestedClientCertificates.isEmpty())) 1049 return; 1050 1051 m_suggestedClientCertificates.take(identifier); 1052 } 1053 1054 NSURLCredential *NetworkSessionCocoa::successfulClientCertificateForHost(const String& host, uint16_t port) const 1055 { 1056 HostAndPort key { host, port }; 1057 if (!decltype(m_successfulClientCertificates)::isValidKey(key)) 1058 return nil; 1059 return m_successfulClientCertificates.get(key).get(); 1006 1060 } 1007 1061 -
trunk/Tools/ChangeLog
r260298 r260302 1 2020-04-17 Alex Christensen <achristensen@webkit.org> 2 3 NetworkSessionCocoa should request client certificate only once per host/port 4 https://bugs.webkit.org/show_bug.cgi?id=210626 5 <rdar://problem/60340449> 6 7 Reviewed by Geoffrey Garen. 8 9 * TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm: 10 (TestWebKitAPI::clientCertServerWithCertVerifier): 11 (TestWebKitAPI::TEST): 12 * TestWebKitAPI/cocoa/HTTPServer.h: 13 (TestWebKitAPI::HTTPServer::HTTPResponse::HTTPResponse): 14 * TestWebKitAPI/cocoa/HTTPServer.mm: 15 (TestWebKitAPI::HTTPServer::HTTPServer): 16 1 17 2020-04-17 David Kilzer <ddkilzer@apple.com> 2 18 -
trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm
r255846 r260302 30 30 #import "TCPServer.h" 31 31 #import "Test.h" 32 #import "TestNavigationDelegate.h" 32 33 #import "TestWKWebView.h" 33 34 #import "WKWebViewConfigurationExtras.h" … … 39 40 #import <WebKit/_WKErrorRecoveryAttempting.h> 40 41 #import <WebKit/_WKWebsiteDataStoreConfiguration.h> 42 #import <wtf/BlockPtr.h> 41 43 #import <wtf/Platform.h> 42 44 #import <wtf/RetainPtr.h> … … 400 402 } 401 403 404 // FIXME: Find out why these tests time out on Mojave. 405 #if HAVE(NETWORK_FRAMEWORK) && (!PLATFORM(MAC) || __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) 406 407 static HTTPServer clientCertServer() 408 { 409 Vector<LChar> chars(50000, 'a'); 410 String longString(chars.data(), chars.size()); 411 return HTTPServer({ 412 { "/", { "<html><img src='1.png'/><img src='2.png'/><img src='3.png'/><img src='4.png'/><img src='5.png'/><img src='6.png'/></html>" } }, 413 { "/1.png", { longString } }, 414 { "/2.png", { longString } }, 415 { "/3.png", { longString } }, 416 { "/4.png", { longString } }, 417 { "/5.png", { longString } }, 418 { "/6.png", { longString } }, 419 { "/redirectToError", { 301, {{ "Location", "/error" }} } }, 420 { "/error", { HTTPServer::HTTPResponse::TerminateConnection::Yes } }, 421 }, HTTPServer::Protocol::Https, [] (auto, auto, auto certificateAllowed) { 422 certificateAllowed(true); 423 }); 424 } 425 426 static BlockPtr<void(WKWebView *, NSURLAuthenticationChallenge *, void (^)(NSURLSessionAuthChallengeDisposition, NSURLCredential *))> challengeHandler(Vector<RetainPtr<NSString>>& vector) 427 { 428 return makeBlockPtr([&] (WKWebView *webView, NSURLAuthenticationChallenge *challenge, void (^completionHandler)(NSURLSessionAuthChallengeDisposition, NSURLCredential *)) { 429 NSString *method = challenge.protectionSpace.authenticationMethod; 430 vector.append(method); 431 if ([method isEqualToString:NSURLAuthenticationMethodClientCertificate]) 432 return completionHandler(NSURLSessionAuthChallengeUseCredential, credentialWithIdentity().get()); 433 if ([method isEqualToString:NSURLAuthenticationMethodServerTrust]) 434 return completionHandler(NSURLSessionAuthChallengeUseCredential, [NSURLCredential credentialForTrust:challenge.protectionSpace.serverTrust]); 435 ASSERT_NOT_REACHED(); 436 }).get(); 437 } 438 439 static size_t countClientCertChallenges(Vector<RetainPtr<NSString>>& vector) 440 { 441 vector.removeAllMatching([](auto& method) { 442 return ![method isEqualToString:NSURLAuthenticationMethodClientCertificate]; 443 }); 444 return vector.size(); 445 }; 446 447 TEST(MultipleClientCertificateConnections, Basic) 448 { 449 auto server = clientCertServer(); 450 451 Vector<RetainPtr<NSString>> methods; 452 auto delegate = adoptNS([TestNavigationDelegate new]); 453 delegate.get().didReceiveAuthenticationChallenge = challengeHandler(methods).get(); 454 455 auto webView = adoptNS([WKWebView new]); 456 [webView setNavigationDelegate:delegate.get()]; 457 [webView loadRequest:server.request()]; 458 [delegate waitForDidFinishNavigation]; 459 EXPECT_EQ(countClientCertChallenges(methods), 1u); 460 } 461 462 TEST(MultipleClientCertificateConnections, Redirect) 463 { 464 auto server = clientCertServer(); 465 466 Vector<RetainPtr<NSString>> methods; 467 auto delegate = adoptNS([TestNavigationDelegate new]); 468 delegate.get().didReceiveAuthenticationChallenge = challengeHandler(methods).get(); 469 470 auto webView = adoptNS([WKWebView new]); 471 [webView setNavigationDelegate:delegate.get()]; 472 [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"https://127.0.0.1:%d/redirectToError", server.port()]]]]; 473 [delegate waitForDidFailProvisionalNavigation]; 474 EXPECT_EQ(countClientCertChallenges(methods), 1u); 475 [webView loadRequest:server.request()]; 476 [delegate waitForDidFinishNavigation]; 477 EXPECT_EQ(countClientCertChallenges(methods), 1u); 478 } 479 480 TEST(MultipleClientCertificateConnections, Failure) 481 { 482 auto server = clientCertServer(); 483 484 Vector<RetainPtr<NSString>> methods; 485 auto delegate = adoptNS([TestNavigationDelegate new]); 486 delegate.get().didReceiveAuthenticationChallenge = challengeHandler(methods).get(); 487 488 auto webView = adoptNS([WKWebView new]); 489 [webView setNavigationDelegate:delegate.get()]; 490 [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"https://127.0.0.1:%d/error", server.port()]]]]; 491 [delegate waitForDidFailProvisionalNavigation]; 492 size_t certChallengesAfterInitialFailure = countClientCertChallenges(methods); 493 [webView loadRequest:server.request()]; 494 [delegate waitForDidFinishNavigation]; 495 EXPECT_EQ(countClientCertChallenges(methods), certChallengesAfterInitialFailure + 1); 496 } 497 498 #endif // HAVE(NETWORK_FRAMEWORK) 499 402 500 } // namespace TestWebKitAPI 403 501 -
trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.h
r259392 r260302 41 41 struct HTTPResponse; 42 42 enum class Protocol : uint8_t { Http, Https, HttpsWithLegacyTLS }; 43 HTTPServer(std::initializer_list<std::pair<String, HTTPResponse>>, Protocol = Protocol::Http );43 HTTPServer(std::initializer_list<std::pair<String, HTTPResponse>>, Protocol = Protocol::Http, Function<void(sec_protocol_metadata_t, sec_trust_t, sec_protocol_verify_complete_t)>&& = nullptr); 44 44 uint16_t port() const; 45 45 NSURLRequest *request() const; … … 51 51 RetainPtr<nw_listener_t> m_listener; 52 52 const Protocol m_protocol; 53 const Function<void(sec_protocol_metadata_t, sec_trust_t, sec_protocol_verify_complete_t)> m_certVerifier; 53 54 const HashMap<String, HTTPResponse> m_requestResponseMap; 54 55 size_t m_totalRequests { 0 }; … … 56 57 57 58 struct HTTPServer::HTTPResponse { 58 HTTPResponse(String&& body) 59 : body(WTFMove(body)) { } 59 enum class TerminateConnection { No, Yes }; 60 61 HTTPResponse(const String& body) 62 : body(body) { } 60 63 HTTPResponse(HashMap<String, String>&& headerFields, String&& body) 61 64 : headerFields(WTFMove(headerFields)) 62 65 , body(WTFMove(body)) { } 63 HTTPResponse(unsigned statusCode, HashMap<String, String>&& headerFields , String&& body = { })66 HTTPResponse(unsigned statusCode, HashMap<String, String>&& headerFields = { }, String&& body = { }) 64 67 : statusCode(statusCode) 65 68 , headerFields(WTFMove(headerFields)) 66 69 , body(WTFMove(body)) { } 70 HTTPResponse(TerminateConnection terminateConnection) 71 : terminateConnection(terminateConnection) { } 67 72 68 73 HTTPResponse(const HTTPResponse&) = default; … … 75 80 HashMap<String, String> headerFields; 76 81 String body; 82 TerminateConnection terminateConnection { TerminateConnection::No }; 77 83 }; 78 84 -
trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.mm
r259392 r260302 38 38 namespace TestWebKitAPI { 39 39 40 HTTPServer::HTTPServer(std::initializer_list<std::pair<String, HTTPResponse>> responses, Protocol protocol )40 HTTPServer::HTTPServer(std::initializer_list<std::pair<String, HTTPResponse>> responses, Protocol protocol, Function<void(sec_protocol_metadata_t, sec_trust_t, sec_protocol_verify_complete_t)>&& verify) 41 41 : m_protocol(protocol) 42 , m_certVerifier(WTFMove(verify)) 42 43 , m_requestResponseMap([](std::initializer_list<std::pair<String, HTTPServer::HTTPResponse>> list) { 43 44 HashMap<String, HTTPServer::HTTPResponse> map; … … 54 55 if (protocol == Protocol::HttpsWithLegacyTLS) 55 56 sec_protocol_options_set_max_tls_protocol_version(options.get(), tls_protocol_version_TLSv10); 57 if (m_certVerifier) { 58 sec_protocol_options_set_peer_authentication_required(options.get(), true); 59 sec_protocol_options_set_verify_block(options.get(), ^(sec_protocol_metadata_t metadata, sec_trust_t trust, sec_protocol_verify_complete_t completion) { 60 m_certVerifier(metadata, trust, completion); 61 }, dispatch_get_main_queue()); 62 } 56 63 #else 57 64 UNUSED_PARAM(protocolOptions); 58 ASSERT (protocol != Protocol::HttpsWithLegacyTLS);65 ASSERT_UNUSED(protocol, protocol != Protocol::HttpsWithLegacyTLS); 59 66 #endif 60 67 }; … … 84 91 case 301: 85 92 return "Moved Permanently"_s; 93 case 404: 94 return "Not Found"_s; 86 95 } 87 96 ASSERT_NOT_REACHED(); … … 122 131 CompletionHandler<void()> sendResponse = [this, connection = retainPtr(connection), context = retainPtr(context), path = WTFMove(path)] { 123 132 auto response = m_requestResponseMap.get(path); 133 if (response.terminateConnection == HTTPResponse::TerminateConnection::Yes) { 134 nw_connection_cancel(connection.get()); 135 return; 136 } 124 137 StringBuilder responseBuilder; 125 138 responseBuilder.append("HTTP/1.1 ");
Note: See TracChangeset
for help on using the changeset viewer.