Skip to content

Commit

Permalink
Remove unnecessary SPI for allowing untrusted TLS certificate chains
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=247807
rdar://102241157

Reviewed by Brady Eidson.

Now that rdar://30655740 is fixed, this SPI is no longer needed on Cocoa platforms.

* Source/WebCore/PAL/pal/spi/cf/CFNetworkSPI.h:
* Source/WebKit/NetworkProcess/NetworkProcess.h:
* Source/WebKit/NetworkProcess/NetworkProcess.messages.in:
* Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm:
(WebKit::NetworkProcess::allowSpecificHTTPSCertificateForHost): Deleted.
* Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.h:
* Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:
(-[WKNetworkSessionDelegate URLSession:task:didReceiveChallenge:completionHandler:]):
(WebKit::NetworkSessionCocoa::allowsSpecificHTTPSCertificateForHost): Deleted.
* Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm:
(-[WKWebsiteDataStore _allowTLSCertificateChain:forHost:]):
* Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:
* Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:
* Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h:

Canonical link: https://commits.webkit.org/266848@main
  • Loading branch information
achristensen07 committed Aug 12, 2023
1 parent 5ff67f8 commit bb1345f
Show file tree
Hide file tree
Showing 10 changed files with 9 additions and 31 deletions.
2 changes: 0 additions & 2 deletions Source/WebCore/PAL/pal/spi/cf/CFNetworkSPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,6 @@ typedef enum {
#endif

@interface NSURLRequest ()
+ (NSArray *)allowsSpecificHTTPSCertificateForHost:(NSString *)host;
+ (void)setAllowsSpecificHTTPSCertificate:(NSArray *)allow forHost:(NSString *)host;
+ (void)setDefaultTimeoutInterval:(NSTimeInterval)seconds;
- (NSArray *)contentDispositionEncodingFallbackArray;
- (CFMutableURLRequestRef)_CFURLRequest;
Expand Down
2 changes: 2 additions & 0 deletions Source/WebKit/NetworkProcess/NetworkProcess.h
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,9 @@ class NetworkProcess : public AuxiliaryProcess, private DownloadManager::Client,

void setCacheModel(CacheModel);
void setCacheModelSynchronouslyForTesting(CacheModel, CompletionHandler<void()>&&);
#if !PLATFORM(COCOA)
void allowSpecificHTTPSCertificateForHost(PAL::SessionID, const WebCore::CertificateInfo&, const String& host);
#endif
void allowTLSCertificateChainForLocalPCMTesting(PAL::SessionID, const WebCore::CertificateInfo&);
void setAllowsAnySSLCertificateForWebSocket(bool, CompletionHandler<void()>&&);

Expand Down
2 changes: 2 additions & 0 deletions Source/WebKit/NetworkProcess/NetworkProcess.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ messages -> NetworkProcess LegacyReceiver {

FlushCookies(PAL::SessionID sessionID) -> ()

#if !PLATFORM(COCOA)
AllowSpecificHTTPSCertificateForHost(PAL::SessionID sessionID, WebCore::CertificateInfo certificate, String host)
#endif
AllowTLSCertificateChainForLocalPCMTesting(PAL::SessionID sessionID, WebCore::CertificateInfo certificate)

SetCacheModel(enum:uint8_t WebKit::CacheModel cacheModel)
Expand Down
6 changes: 0 additions & 6 deletions Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,6 @@ static void initializeNetworkSettings()
}
}

void NetworkProcess::allowSpecificHTTPSCertificateForHost(PAL::SessionID, const WebCore::CertificateInfo& certificateInfo, const String& host)
{
// FIXME: Remove this once rdar://30655740 is fixed.
[NSURLRequest setAllowsSpecificHTTPSCertificate:(NSArray *)WebCore::CertificateInfo::certificateChainFromSecTrust(certificateInfo.trust().get()).get() forHost:host];
}

void NetworkProcess::clearHSTSCache(PAL::SessionID sessionID, WallTime modifiedSince)
{
NSTimeInterval timeInterval = modifiedSince.secondsSinceEpoch().seconds();
Expand Down
1 change: 0 additions & 1 deletion Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ class NetworkSessionCocoa final : public NetworkSession {
const String& dataConnectionServiceType() const;
#endif

static bool allowsSpecificHTTPSCertificateForHost(const WebCore::AuthenticationChallenge&);
void setClientAuditToken(const WebCore::AuthenticationChallenge&);

void continueDidReceiveChallenge(SessionWrapper&, const WebCore::AuthenticationChallenge&, NegotiatedLegacyTLS, NetworkDataTaskCocoa::TaskIdentifier, RefPtr<NetworkDataTaskCocoa>, CompletionHandler<void(WebKit::AuthenticationChallengeDisposition, const WebCore::Credential&)>&&);
Expand Down
20 changes: 0 additions & 20 deletions Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -766,8 +766,6 @@ - (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didRece

if ([challenge.protectionSpace.authenticationMethod isEqualToString:NSURLAuthenticationMethodServerTrust]) {
sessionCocoa->setClientAuditToken(challenge);
if (NetworkSessionCocoa::allowsSpecificHTTPSCertificateForHost(challenge))
return completionHandler(NSURLSessionAuthChallengeUseCredential, [NSURLCredential credentialForTrust:challenge.protectionSpace.serverTrust]);

NSURLSessionTaskTransactionMetrics *metrics = task._incompleteTaskMetrics.transactionMetrics.lastObject;
auto tlsVersion = (tls_protocol_version_t)metrics.negotiatedTLSProtocolVersion.unsignedShortValue;
Expand Down Expand Up @@ -1814,24 +1812,6 @@ static void activateSessionCleanup(NetworkSessionCocoa& session, const NetworkSe
}
}

bool NetworkSessionCocoa::allowsSpecificHTTPSCertificateForHost(const WebCore::AuthenticationChallenge& challenge)
{
const String& host = challenge.protectionSpace().host();
NSArray *certificates = [NSURLRequest allowsSpecificHTTPSCertificateForHost:host];
if (!certificates)
return false;

bool requireServerCertificates = challenge.protectionSpace().authenticationScheme() == WebCore::ProtectionSpace::AuthenticationScheme::ServerTrustEvaluationRequested;
RetainPtr<SecPolicyRef> policy = adoptCF(SecPolicyCreateSSL(requireServerCertificates, host.createCFString().get()));

SecTrustRef trustRef = nullptr;
if (SecTrustCreateWithCertificates((CFArrayRef)certificates, policy.get(), &trustRef) != noErr)
return false;
RetainPtr<SecTrustRef> trust = adoptCF(trustRef);

return WebCore::certificatesMatch(trust.get(), challenge.nsURLAuthenticationChallenge().protectionSpace.serverTrust);
}

static CompletionHandler<void(WebKit::AuthenticationChallengeDisposition disposition, const WebCore::Credential& credential)> createChallengeCompletionHandler(Ref<NetworkProcess>&& networkProcess, PAL::SessionID sessionID, const WebCore::AuthenticationChallenge& challenge, const String& partition, uint64_t taskIdentifier, CompletionHandler<void(WebKit::AuthenticationChallengeDisposition, const WebCore::Credential&)>&& completionHandler)
{
WebCore::AuthenticationChallenge authenticationChallenge { challenge };
Expand Down
1 change: 0 additions & 1 deletion Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,6 @@ + (WKNotificationManagerRef)_sharedServiceWorkerNotificationManager

- (void)_allowTLSCertificateChain:(NSArray *)certificateChain forHost:(NSString *)host
{
_websiteDataStore->allowSpecificHTTPSCertificateForHost(WebCore::CertificateInfo(WebCore::CertificateInfo::secTrustFromCertificateChain((__bridge CFArrayRef)certificateChain)), host);
}

- (void)_trustServerForLocalPCMTesting:(SecTrustRef)serverTrust
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ typedef NS_OPTIONS(NSUInteger, _WKWebsiteDataStoreFetchOptions) {

+ (void)_setCachedProcessSuspensionDelayForTesting:(double)delayInSeconds WK_API_AVAILABLE(macos(13.0), ios(16.0));

- (void)_allowTLSCertificateChain:(NSArray *)certificateChain forHost:(NSString *)host WK_API_AVAILABLE(macos(12.0), ios(15.0));
- (void)_allowTLSCertificateChain:(NSArray *)certificateChain forHost:(NSString *)host WK_API_DEPRECATED_WITH_REPLACEMENT("WKNavigationDelegate.didReceiveAuthenticationChallenge", macos(12.0, WK_MAC_TBA), ios(15.0, WK_IOS_TBA));
- (void)_trustServerForLocalPCMTesting:(SecTrustRef)serverTrust WK_API_AVAILABLE(macos(13.0), ios(16.0));

- (void)_setPrivateClickMeasurementDebugModeEnabled:(BOOL)enabled WK_API_AVAILABLE(macos(14.0), ios(17.0));
Expand Down
2 changes: 2 additions & 0 deletions Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1541,10 +1541,12 @@ static String computeMediaKeyFile(const String& mediaKeyDirectory)
return FileSystem::pathByAppendingComponent(mediaKeyDirectory, "SecureStop.plist"_s);
}

#if !PLATFORM(COCOA)
void WebsiteDataStore::allowSpecificHTTPSCertificateForHost(const WebCore::CertificateInfo& certificate, const String& host)
{
networkProcess().send(Messages::NetworkProcess::AllowSpecificHTTPSCertificateForHost(sessionID(), certificate, host), 0);
}
#endif

void WebsiteDataStore::allowTLSCertificateChainForLocalPCMTesting(const WebCore::CertificateInfo& certificate)
{
Expand Down
2 changes: 2 additions & 0 deletions Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,9 @@ class WebsiteDataStore : public API::ObjectImpl<API::Object::Type::WebsiteDataSt

static void setCachedProcessSuspensionDelayForTesting(Seconds);

#if !PLATFORM(COCOA)
void allowSpecificHTTPSCertificateForHost(const WebCore::CertificateInfo&, const String& host);
#endif
void allowTLSCertificateChainForLocalPCMTesting(const WebCore::CertificateInfo&);

DeviceIdHashSaltStorage& deviceIdHashSaltStorage() { return m_deviceIdHashSaltStorage.get(); }
Expand Down

0 comments on commit bb1345f

Please sign in to comment.