Diff
Modified: trunk/Source/WebKit/ChangeLog (260301 => 260302)
--- trunk/Source/WebKit/ChangeLog 2020-04-17 23:30:33 UTC (rev 260301)
+++ trunk/Source/WebKit/ChangeLog 2020-04-17 23:40:39 UTC (rev 260302)
@@ -1,3 +1,30 @@
+2020-04-17 Alex Christensen <[email protected]>
+
+ NetworkSessionCocoa should request client certificate only once per host/port
+ https://bugs.webkit.org/show_bug.cgi?id=210626
+ <rdar://problem/60340449>
+
+ Reviewed by Geoffrey Garen.
+
+ NSURLSession creates more than one TCP connection to a server when using HTTP 1.1.
+ Each TCP connection with TLS generates a didReceiveChallenge to do the server trust evaluation of the certificate chain.
+ If the server requests a client certificate in the TLS handshake, it also generates a didReceiveChallenge to request client
+ certificates as well. This is an implementation detail of our networking. We should not actually ask the WKNavigationDelegate
+ for client certificates more than once per host/port. We should remember the credential and give it to NSURLSession immediately
+ 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
+ handshake fails, we should not reuse that same certificate automatically.
+
+ * NetworkProcess/cocoa/NetworkSessionCocoa.h:
+ * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
+ (-[WKNetworkSessionDelegate URLSession:task:willPerformHTTPRedirection:newRequest:completionHandler:]):
+ (-[WKNetworkSessionDelegate URLSession:task:didReceiveChallenge:completionHandler:]):
+ (-[WKNetworkSessionDelegate URLSession:task:didCompleteWithError:]):
+ (-[WKNetworkSessionDelegate URLSession:dataTask:didReceiveResponse:completionHandler:]):
+ (WebKit::NetworkSessionCocoa::clientCertificateSuggestedForHost):
+ (WebKit::NetworkSessionCocoa::taskReceivedBytes):
+ (WebKit::NetworkSessionCocoa::taskFailed):
+ (WebKit::NetworkSessionCocoa::successfulClientCertificateForHost const):
+
2020-04-17 David Kilzer <[email protected]>
Bug 210646: REGRESSION (r260112): createArchiveList() leaks malloc memory on early returns due to an error
Modified: trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.h (260301 => 260302)
--- trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.h 2020-04-17 23:30:33 UTC (rev 260301)
+++ trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.h 2020-04-17 23:40:39 UTC (rev 260302)
@@ -48,6 +48,7 @@
enum class NegotiatedLegacyTLS : bool;
class LegacyCustomProtocolManager;
class NetworkSessionCocoa;
+using HostAndPort = std::pair<String, uint16_t>;
struct SessionWrapper : public CanMakeWeakPtr<SessionWrapper> {
void initialize(NSURLSessionConfiguration *, NetworkSessionCocoa&, WebCore::StoredCredentialsPolicy, NavigatingToAppBoundDomain);
@@ -100,6 +101,11 @@
bool isInAppBrowserPrivacyEnabled() const { return m_isInAppBrowserPrivacyEnabled; }
bool preventsSystemHTTPProxyAuthentication() const { return m_preventsSystemHTTPProxyAuthentication; }
+ void clientCertificateSuggestedForHost(NetworkDataTaskCocoa::TaskIdentifier, NSURLCredential *, const String& host, uint16_t port);
+ void taskReceivedBytes(NetworkDataTaskCocoa::TaskIdentifier);
+ void taskFailed(NetworkDataTaskCocoa::TaskIdentifier);
+ NSURLCredential *successfulClientCertificateForHost(const String& host, uint16_t port) const;
+
private:
void invalidateAndCancel() override;
void clearCredentials() override;
@@ -146,6 +152,14 @@
String m_dataConnectionServiceType;
bool m_isInAppBrowserPrivacyEnabled { false };
bool m_preventsSystemHTTPProxyAuthentication { false };
+
+ struct SuggestedClientCertificate {
+ String host;
+ uint16_t port { 0 };
+ RetainPtr<NSURLCredential> credential;
+ };
+ HashMap<NetworkDataTaskCocoa::TaskIdentifier, SuggestedClientCertificate> m_suggestedClientCertificates;
+ HashMap<HostAndPort, RetainPtr<NSURLCredential>> m_successfulClientCertificates;
};
} // namespace WebKit
Modified: trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm (260301 => 260302)
--- trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm 2020-04-17 23:30:33 UTC (rev 260301)
+++ trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm 2020-04-17 23:40:39 UTC (rev 260302)
@@ -529,6 +529,9 @@
if (auto* networkDataTask = [self existingTask:task]) {
auto completionHandlerCopy = Block_copy(completionHandler);
+ if (auto* sessionCocoa = [self sessionFromTask:task])
+ sessionCocoa->taskReceivedBytes(taskIdentifier);
+
bool shouldIgnoreHSTS = false;
#if ENABLE(RESOURCE_LOAD_STATISTICS)
if (auto* sessionCocoa = networkDataTask->networkSession()) {
@@ -692,6 +695,20 @@
#endif
}
}
+
+ if ([challenge.protectionSpace.authenticationMethod isEqualToString:NSURLAuthenticationMethodClientCertificate]) {
+ HostAndPort key { challenge.protectionSpace.host, challenge.protectionSpace.port };
+ if (auto* credential = sessionCocoa->successfulClientCertificateForHost(challenge.protectionSpace.host, challenge.protectionSpace.port))
+ return completionHandler(NSURLSessionAuthChallengeUseCredential, credential);
+ 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 {
+ NSURLCredential *nsCredential = credential.nsCredential();
+ if (disposition == WebKit::AuthenticationChallengeDisposition::UseCredential && nsCredential && weakSessionCocoa)
+ weakSessionCocoa->clientCertificateSuggestedForHost(taskIdentifier, nsCredential, key.first, key.second);
+ completionHandler(toNSURLSessionAuthChallengeDisposition(disposition), nsCredential);
+ });
+ return;
+ }
+
sessionCocoa->continueDidReceiveChallenge(*_sessionWrapper, challenge, negotiatedLegacyTLS, taskIdentifier, [self existingTask:task], [completionHandler = makeBlockPtr(completionHandler)] (WebKit::AuthenticationChallengeDisposition disposition, const WebCore::Credential& credential) mutable {
completionHandler(toNSURLSessionAuthChallengeDisposition(disposition), credential.nsCredential());
});
@@ -708,9 +725,11 @@
error = [NSError errorWithDomain:[error domain] code:[error code] userInfo:newUserInfo];
}
- if (auto* networkDataTask = [self existingTask:task])
+ if (auto* networkDataTask = [self existingTask:task]) {
+ if (auto* sessionCocoa = [self sessionFromTask:task])
+ sessionCocoa->taskFailed(task.taskIdentifier);
networkDataTask->didCompleteWithError(error, networkDataTask->networkLoadMetrics());
- else if (error) {
+ } else if (error) {
if (!_sessionWrapper)
return;
auto downloadID = _sessionWrapper->downloadMap.take(task.taskIdentifier);
@@ -840,6 +859,9 @@
if (auto* networkDataTask = [self existingTask:dataTask]) {
ASSERT(RunLoop::isMain());
+ if (auto* sessionCocoa = [self sessionFromTask:dataTask])
+ sessionCocoa->taskReceivedBytes(taskIdentifier);
+
NegotiatedLegacyTLS negotiatedLegacyTLS = NegotiatedLegacyTLS::No;
#if HAVE(TLS_PROTOCOL_VERSION_T)
NSURLSessionTaskTransactionMetrics *metrics = dataTask._incompleteTaskMetrics.transactionMetrics.lastObject;
@@ -1005,6 +1027,38 @@
return configuration;
}
+void NetworkSessionCocoa::clientCertificateSuggestedForHost(NetworkDataTaskCocoa::TaskIdentifier taskID, NSURLCredential *credential, const String& host, uint16_t port)
+{
+ m_suggestedClientCertificates.set(taskID, SuggestedClientCertificate { host, port, credential });
+}
+
+void NetworkSessionCocoa::taskReceivedBytes(NetworkDataTaskCocoa::TaskIdentifier identifier)
+{
+ if (LIKELY(m_suggestedClientCertificates.isEmpty()))
+ return;
+
+ auto suggestedClientCertificate = m_suggestedClientCertificates.take(identifier);
+ HostAndPort key { suggestedClientCertificate.host, suggestedClientCertificate.port };
+ if (suggestedClientCertificate.credential && decltype(m_successfulClientCertificates)::isValidKey(key))
+ m_successfulClientCertificates.add(key, suggestedClientCertificate.credential);
+}
+
+void NetworkSessionCocoa::taskFailed(NetworkDataTaskCocoa::TaskIdentifier identifier)
+{
+ if (LIKELY(m_suggestedClientCertificates.isEmpty()))
+ return;
+
+ m_suggestedClientCertificates.take(identifier);
+}
+
+NSURLCredential *NetworkSessionCocoa::successfulClientCertificateForHost(const String& host, uint16_t port) const
+{
+ HostAndPort key { host, port };
+ if (!decltype(m_successfulClientCertificates)::isValidKey(key))
+ return nil;
+ return m_successfulClientCertificates.get(key).get();
+}
+
const String& NetworkSessionCocoa::boundInterfaceIdentifier() const
{
return m_boundInterfaceIdentifier;
Modified: trunk/Tools/ChangeLog (260301 => 260302)
--- trunk/Tools/ChangeLog 2020-04-17 23:30:33 UTC (rev 260301)
+++ trunk/Tools/ChangeLog 2020-04-17 23:40:39 UTC (rev 260302)
@@ -1,3 +1,19 @@
+2020-04-17 Alex Christensen <[email protected]>
+
+ NetworkSessionCocoa should request client certificate only once per host/port
+ https://bugs.webkit.org/show_bug.cgi?id=210626
+ <rdar://problem/60340449>
+
+ Reviewed by Geoffrey Garen.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:
+ (TestWebKitAPI::clientCertServerWithCertVerifier):
+ (TestWebKitAPI::TEST):
+ * TestWebKitAPI/cocoa/HTTPServer.h:
+ (TestWebKitAPI::HTTPServer::HTTPResponse::HTTPResponse):
+ * TestWebKitAPI/cocoa/HTTPServer.mm:
+ (TestWebKitAPI::HTTPServer::HTTPServer):
+
2020-04-17 David Kilzer <[email protected]>
Bug 210645: REGRESSION (r211095): [iOS] TestRunnerWKWebView leaks @property accessibilitySpeakSelectionContent
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm (260301 => 260302)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm 2020-04-17 23:30:33 UTC (rev 260301)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm 2020-04-17 23:40:39 UTC (rev 260302)
@@ -29,6 +29,7 @@
#import "PlatformUtilities.h"
#import "TCPServer.h"
#import "Test.h"
+#import "TestNavigationDelegate.h"
#import "TestWKWebView.h"
#import "WKWebViewConfigurationExtras.h"
#import <WebKit/WKNavigationDelegate.h>
@@ -38,6 +39,7 @@
#import <WebKit/WebKit.h>
#import <WebKit/_WKErrorRecoveryAttempting.h>
#import <WebKit/_WKWebsiteDataStoreConfiguration.h>
+#import <wtf/BlockPtr.h>
#import <wtf/Platform.h>
#import <wtf/RetainPtr.h>
#import <wtf/spi/cocoa/SecuritySPI.h>
@@ -399,6 +401,102 @@
#endif
}
+// FIXME: Find out why these tests time out on Mojave.
+#if HAVE(NETWORK_FRAMEWORK) && (!PLATFORM(MAC) || __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500)
+
+static HTTPServer clientCertServer()
+{
+ Vector<LChar> chars(50000, 'a');
+ String longString(chars.data(), chars.size());
+ return HTTPServer({
+ { "/", { "<html><img src=''/><img src=''/><img src=''/><img src=''/><img src=''/><img src=''/></html>" } },
+ { "/1.png", { longString } },
+ { "/2.png", { longString } },
+ { "/3.png", { longString } },
+ { "/4.png", { longString } },
+ { "/5.png", { longString } },
+ { "/6.png", { longString } },
+ { "/redirectToError", { 301, {{ "Location", "/error" }} } },
+ { "/error", { HTTPServer::HTTPResponse::TerminateConnection::Yes } },
+ }, HTTPServer::Protocol::Https, [] (auto, auto, auto certificateAllowed) {
+ certificateAllowed(true);
+ });
+}
+
+static BlockPtr<void(WKWebView *, NSURLAuthenticationChallenge *, void (^)(NSURLSessionAuthChallengeDisposition, NSURLCredential *))> challengeHandler(Vector<RetainPtr<NSString>>& vector)
+{
+ return makeBlockPtr([&] (WKWebView *webView, NSURLAuthenticationChallenge *challenge, void (^completionHandler)(NSURLSessionAuthChallengeDisposition, NSURLCredential *)) {
+ NSString *method = challenge.protectionSpace.authenticationMethod;
+ vector.append(method);
+ if ([method isEqualToString:NSURLAuthenticationMethodClientCertificate])
+ return completionHandler(NSURLSessionAuthChallengeUseCredential, credentialWithIdentity().get());
+ if ([method isEqualToString:NSURLAuthenticationMethodServerTrust])
+ return completionHandler(NSURLSessionAuthChallengeUseCredential, [NSURLCredential credentialForTrust:challenge.protectionSpace.serverTrust]);
+ ASSERT_NOT_REACHED();
+ }).get();
+}
+
+static size_t countClientCertChallenges(Vector<RetainPtr<NSString>>& vector)
+{
+ vector.removeAllMatching([](auto& method) {
+ return ![method isEqualToString:NSURLAuthenticationMethodClientCertificate];
+ });
+ return vector.size();
+};
+
+TEST(MultipleClientCertificateConnections, Basic)
+{
+ auto server = clientCertServer();
+
+ Vector<RetainPtr<NSString>> methods;
+ auto delegate = adoptNS([TestNavigationDelegate new]);
+ delegate.get().didReceiveAuthenticationChallenge = challengeHandler(methods).get();
+
+ auto webView = adoptNS([WKWebView new]);
+ [webView setNavigationDelegate:delegate.get()];
+ [webView loadRequest:server.request()];
+ [delegate waitForDidFinishNavigation];
+ EXPECT_EQ(countClientCertChallenges(methods), 1u);
+}
+
+TEST(MultipleClientCertificateConnections, Redirect)
+{
+ auto server = clientCertServer();
+
+ Vector<RetainPtr<NSString>> methods;
+ auto delegate = adoptNS([TestNavigationDelegate new]);
+ delegate.get().didReceiveAuthenticationChallenge = challengeHandler(methods).get();
+
+ auto webView = adoptNS([WKWebView new]);
+ [webView setNavigationDelegate:delegate.get()];
+ [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"https://127.0.0.1:%d/redirectToError", server.port()]]]];
+ [delegate waitForDidFailProvisionalNavigation];
+ EXPECT_EQ(countClientCertChallenges(methods), 1u);
+ [webView loadRequest:server.request()];
+ [delegate waitForDidFinishNavigation];
+ EXPECT_EQ(countClientCertChallenges(methods), 1u);
+}
+
+TEST(MultipleClientCertificateConnections, Failure)
+{
+ auto server = clientCertServer();
+
+ Vector<RetainPtr<NSString>> methods;
+ auto delegate = adoptNS([TestNavigationDelegate new]);
+ delegate.get().didReceiveAuthenticationChallenge = challengeHandler(methods).get();
+
+ auto webView = adoptNS([WKWebView new]);
+ [webView setNavigationDelegate:delegate.get()];
+ [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"https://127.0.0.1:%d/error", server.port()]]]];
+ [delegate waitForDidFailProvisionalNavigation];
+ size_t certChallengesAfterInitialFailure = countClientCertChallenges(methods);
+ [webView loadRequest:server.request()];
+ [delegate waitForDidFinishNavigation];
+ EXPECT_EQ(countClientCertChallenges(methods), certChallengesAfterInitialFailure + 1);
+}
+
+#endif // HAVE(NETWORK_FRAMEWORK)
+
} // namespace TestWebKitAPI
#endif // HAVE(SSL)
Modified: trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.h (260301 => 260302)
--- trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.h 2020-04-17 23:30:33 UTC (rev 260301)
+++ trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.h 2020-04-17 23:40:39 UTC (rev 260302)
@@ -40,7 +40,7 @@
public:
struct HTTPResponse;
enum class Protocol : uint8_t { Http, Https, HttpsWithLegacyTLS };
- HTTPServer(std::initializer_list<std::pair<String, HTTPResponse>>, Protocol = Protocol::Http);
+ 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);
uint16_t port() const;
NSURLRequest *request() const;
size_t totalRequests() const { return m_totalRequests; }
@@ -50,20 +50,25 @@
RetainPtr<nw_listener_t> m_listener;
const Protocol m_protocol;
+ const Function<void(sec_protocol_metadata_t, sec_trust_t, sec_protocol_verify_complete_t)> m_certVerifier;
const HashMap<String, HTTPResponse> m_requestResponseMap;
size_t m_totalRequests { 0 };
};
struct HTTPServer::HTTPResponse {
- HTTPResponse(String&& body)
- : body(WTFMove(body)) { }
+ enum class TerminateConnection { No, Yes };
+
+ HTTPResponse(const String& body)
+ : body(body) { }
HTTPResponse(HashMap<String, String>&& headerFields, String&& body)
: headerFields(WTFMove(headerFields))
, body(WTFMove(body)) { }
- HTTPResponse(unsigned statusCode, HashMap<String, String>&& headerFields, String&& body = { })
+ HTTPResponse(unsigned statusCode, HashMap<String, String>&& headerFields = { }, String&& body = { })
: statusCode(statusCode)
, headerFields(WTFMove(headerFields))
, body(WTFMove(body)) { }
+ HTTPResponse(TerminateConnection terminateConnection)
+ : terminateConnection(terminateConnection) { }
HTTPResponse(const HTTPResponse&) = default;
HTTPResponse(HTTPResponse&&) = default;
@@ -74,6 +79,7 @@
unsigned statusCode { 200 };
HashMap<String, String> headerFields;
String body;
+ TerminateConnection terminateConnection { TerminateConnection::No };
};
} // namespace TestWebKitAPI
Modified: trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.mm (260301 => 260302)
--- trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.mm 2020-04-17 23:30:33 UTC (rev 260301)
+++ trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.mm 2020-04-17 23:40:39 UTC (rev 260302)
@@ -37,8 +37,9 @@
namespace TestWebKitAPI {
-HTTPServer::HTTPServer(std::initializer_list<std::pair<String, HTTPResponse>> responses, Protocol protocol)
+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)
: m_protocol(protocol)
+ , m_certVerifier(WTFMove(verify))
, m_requestResponseMap([](std::initializer_list<std::pair<String, HTTPServer::HTTPResponse>> list) {
HashMap<String, HTTPServer::HTTPResponse> map;
for (auto& pair : list)
@@ -53,9 +54,15 @@
sec_protocol_options_set_local_identity(options.get(), identity.get());
if (protocol == Protocol::HttpsWithLegacyTLS)
sec_protocol_options_set_max_tls_protocol_version(options.get(), tls_protocol_version_TLSv10);
+ if (m_certVerifier) {
+ sec_protocol_options_set_peer_authentication_required(options.get(), true);
+ sec_protocol_options_set_verify_block(options.get(), ^(sec_protocol_metadata_t metadata, sec_trust_t trust, sec_protocol_verify_complete_t completion) {
+ m_certVerifier(metadata, trust, completion);
+ }, dispatch_get_main_queue());
+ }
#else
UNUSED_PARAM(protocolOptions);
- ASSERT(protocol != Protocol::HttpsWithLegacyTLS);
+ ASSERT_UNUSED(protocol, protocol != Protocol::HttpsWithLegacyTLS);
#endif
};
auto parameters = adoptNS(nw_parameters_create_secure_tcp(configureTLS, NW_PARAMETERS_DEFAULT_CONFIGURATION));
@@ -83,6 +90,8 @@
return "OK"_s;
case 301:
return "Moved Permanently"_s;
+ case 404:
+ return "Not Found"_s;
}
ASSERT_NOT_REACHED();
return { };
@@ -121,6 +130,10 @@
CompletionHandler<void()> sendResponse = [this, connection = retainPtr(connection), context = retainPtr(context), path = WTFMove(path)] {
auto response = m_requestResponseMap.get(path);
+ if (response.terminateConnection == HTTPResponse::TerminateConnection::Yes) {
+ nw_connection_cancel(connection.get());
+ return;
+ }
StringBuilder responseBuilder;
responseBuilder.append("HTTP/1.1 ");
responseBuilder.appendNumber(response.statusCode);
