Changeset 260302 in webkit


Ignore:
Timestamp:
Apr 17, 2020 4:40:39 PM (4 years ago)
Author:
achristensen@apple.com
Message:

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.

Source/WebKit:

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

Tools:

  • TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:

(TestWebKitAPI::clientCertServerWithCertVerifier):
(TestWebKitAPI::TEST):

  • TestWebKitAPI/cocoa/HTTPServer.h:

(TestWebKitAPI::HTTPServer::HTTPResponse::HTTPResponse):

  • TestWebKitAPI/cocoa/HTTPServer.mm:

(TestWebKitAPI::HTTPServer::HTTPServer):

Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/ChangeLog

    r260299 r260302  
     12020-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
    1282020-04-17  David Kilzer  <ddkilzer@apple.com>
    229
  • trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.h

    r260031 r260302  
    4949class LegacyCustomProtocolManager;
    5050class NetworkSessionCocoa;
     51using HostAndPort = std::pair<String, uint16_t>;
    5152
    5253struct SessionWrapper : public CanMakeWeakPtr<SessionWrapper> {
     
    101102    bool preventsSystemHTTPProxyAuthentication() const { return m_preventsSystemHTTPProxyAuthentication; }
    102103   
     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
    103109private:
    104110    void invalidateAndCancel() override;
     
    147153    bool m_isInAppBrowserPrivacyEnabled { false };
    148154    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;
    149163};
    150164
  • trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm

    r260031 r260302  
    530530        auto completionHandlerCopy = Block_copy(completionHandler);
    531531
     532        if (auto* sessionCocoa = [self sessionFromTask:task])
     533            sessionCocoa->taskReceivedBytes(taskIdentifier);
     534
    532535        bool shouldIgnoreHSTS = false;
    533536#if ENABLE(RESOURCE_LOAD_STATISTICS)
     
    693696        }
    694697    }
     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
    695712    sessionCocoa->continueDidReceiveChallenge(*_sessionWrapper, challenge, negotiatedLegacyTLS, taskIdentifier, [self existingTask:task], [completionHandler = makeBlockPtr(completionHandler)] (WebKit::AuthenticationChallengeDisposition disposition, const WebCore::Credential& credential) mutable {
    696713        completionHandler(toNSURLSessionAuthChallengeDisposition(disposition), credential.nsCredential());
     
    709726    }
    710727
    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);
    712731        networkDataTask->didCompleteWithError(error, networkDataTask->networkLoadMetrics());
    713     else if (error) {
     732    } else if (error) {
    714733        if (!_sessionWrapper)
    715734            return;
     
    840859    if (auto* networkDataTask = [self existingTask:dataTask]) {
    841860        ASSERT(RunLoop::isMain());
     861
     862        if (auto* sessionCocoa = [self sessionFromTask:dataTask])
     863            sessionCocoa->taskReceivedBytes(taskIdentifier);
    842864
    843865        NegotiatedLegacyTLS negotiatedLegacyTLS = NegotiatedLegacyTLS::No;
     
    10041026#endif
    10051027    return configuration;
     1028}
     1029
     1030void 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
     1035void 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
     1046void NetworkSessionCocoa::taskFailed(NetworkDataTaskCocoa::TaskIdentifier identifier)
     1047{
     1048    if (LIKELY(m_suggestedClientCertificates.isEmpty()))
     1049        return;
     1050
     1051    m_suggestedClientCertificates.take(identifier);
     1052}
     1053
     1054NSURLCredential *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();
    10061060}
    10071061
  • trunk/Tools/ChangeLog

    r260298 r260302  
     12020-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
    1172020-04-17  David Kilzer  <ddkilzer@apple.com>
    218
  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm

    r255846 r260302  
    3030#import "TCPServer.h"
    3131#import "Test.h"
     32#import "TestNavigationDelegate.h"
    3233#import "TestWKWebView.h"
    3334#import "WKWebViewConfigurationExtras.h"
     
    3940#import <WebKit/_WKErrorRecoveryAttempting.h>
    4041#import <WebKit/_WKWebsiteDataStoreConfiguration.h>
     42#import <wtf/BlockPtr.h>
    4143#import <wtf/Platform.h>
    4244#import <wtf/RetainPtr.h>
     
    400402}
    401403
     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
     407static 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
     426static 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
     439static 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
     447TEST(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
     462TEST(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
     480TEST(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
    402500} // namespace TestWebKitAPI
    403501
  • trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.h

    r259392 r260302  
    4141    struct HTTPResponse;
    4242    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);
    4444    uint16_t port() const;
    4545    NSURLRequest *request() const;
     
    5151    RetainPtr<nw_listener_t> m_listener;
    5252    const Protocol m_protocol;
     53    const Function<void(sec_protocol_metadata_t, sec_trust_t, sec_protocol_verify_complete_t)> m_certVerifier;
    5354    const HashMap<String, HTTPResponse> m_requestResponseMap;
    5455    size_t m_totalRequests { 0 };
     
    5657
    5758struct 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) { }
    6063    HTTPResponse(HashMap<String, String>&& headerFields, String&& body)
    6164        : headerFields(WTFMove(headerFields))
    6265        , body(WTFMove(body)) { }
    63     HTTPResponse(unsigned statusCode, HashMap<String, String>&& headerFields, String&& body = { })
     66    HTTPResponse(unsigned statusCode, HashMap<String, String>&& headerFields = { }, String&& body = { })
    6467        : statusCode(statusCode)
    6568        , headerFields(WTFMove(headerFields))
    6669        , body(WTFMove(body)) { }
     70    HTTPResponse(TerminateConnection terminateConnection)
     71        : terminateConnection(terminateConnection) { }
    6772
    6873    HTTPResponse(const HTTPResponse&) = default;
     
    7580    HashMap<String, String> headerFields;
    7681    String body;
     82    TerminateConnection terminateConnection { TerminateConnection::No };
    7783};
    7884
  • trunk/Tools/TestWebKitAPI/cocoa/HTTPServer.mm

    r259392 r260302  
    3838namespace TestWebKitAPI {
    3939
    40 HTTPServer::HTTPServer(std::initializer_list<std::pair<String, HTTPResponse>> responses, Protocol protocol)
     40HTTPServer::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)
    4141    : m_protocol(protocol)
     42    , m_certVerifier(WTFMove(verify))
    4243    , m_requestResponseMap([](std::initializer_list<std::pair<String, HTTPServer::HTTPResponse>> list) {
    4344        HashMap<String, HTTPServer::HTTPResponse> map;
     
    5455        if (protocol == Protocol::HttpsWithLegacyTLS)
    5556            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        }
    5663#else
    5764        UNUSED_PARAM(protocolOptions);
    58         ASSERT(protocol != Protocol::HttpsWithLegacyTLS);
     65        ASSERT_UNUSED(protocol, protocol != Protocol::HttpsWithLegacyTLS);
    5966#endif
    6067    };
     
    8491    case 301:
    8592        return "Moved Permanently"_s;
     93    case 404:
     94        return "Not Found"_s;
    8695    }
    8796    ASSERT_NOT_REACHED();
     
    122131        CompletionHandler<void()> sendResponse = [this, connection = retainPtr(connection), context = retainPtr(context), path = WTFMove(path)] {
    123132            auto response = m_requestResponseMap.get(path);
     133            if (response.terminateConnection == HTTPResponse::TerminateConnection::Yes) {
     134                nw_connection_cancel(connection.get());
     135                return;
     136            }
    124137            StringBuilder responseBuilder;
    125138            responseBuilder.append("HTTP/1.1 ");
Note: See TracChangeset for help on using the changeset viewer.