diff options
author | eugenebut <eugenebut@chromium.org> | 2015-10-16 13:14:46 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-10-16 20:15:31 +0000 |
commit | 8ac2913b9c37a0a310ed152dc6be9f60e72d1f2b (patch) | |
tree | b97102beaa0bf6fa12c2bbcd8adad30c27b348d2 | |
parent | dc847f2b403a5784a299f748c2a410c317274f5f (diff) | |
download | chromium_src-8ac2913b9c37a0a310ed152dc6be9f60e72d1f2b.zip chromium_src-8ac2913b9c37a0a310ed152dc6be9f60e72d1f2b.tar.gz chromium_src-8ac2913b9c37a0a310ed152dc6be9f60e72d1f2b.tar.bz2 |
WKWebView(iOS9): correctly update SSL status for current navigation item
SSL status affects the color of SSL padlock and OIB content. Previously
the code was written with the assumption that WKWebView can not load a
page with invalid SSL cert (which was always true on iOS 8). With iOS 9
it is possible to load pages with invalid cert. This CL adds support for
correct SSL status calculation.
This is precursory CL for implementing recoverable SSL warnings.
BUG=462427, 528668
Design Doc:
https://docs.google.com/document/d/1o8wzJkxzG1fvhvW9Gqf-RdGjhm0btCIehDtbxDfbLcw/edit#
Review URL: https://codereview.chromium.org/1322193003
Cr-Commit-Position: refs/heads/master@{#354575}
-rw-r--r-- | ios/web/net/crw_cert_verification_controller.h | 23 | ||||
-rw-r--r-- | ios/web/net/crw_cert_verification_controller.mm | 93 | ||||
-rw-r--r-- | ios/web/net/crw_cert_verification_controller_unittest.mm | 92 | ||||
-rw-r--r-- | ios/web/public/ssl_status.h | 8 | ||||
-rw-r--r-- | ios/web/web_state/ui/crw_wk_web_view_web_controller.mm | 156 | ||||
-rw-r--r-- | ios/web/web_state/wk_web_view_security_util.h | 11 | ||||
-rw-r--r-- | ios/web/web_state/wk_web_view_security_util.mm | 33 | ||||
-rw-r--r-- | ios/web/web_state/wk_web_view_security_util_unittest.mm | 70 |
8 files changed, 447 insertions, 39 deletions
diff --git a/ios/web/net/crw_cert_verification_controller.h b/ios/web/net/crw_cert_verification_controller.h index 06ff635..28a7a50 100644 --- a/ios/web/net/crw_cert_verification_controller.h +++ b/ios/web/net/crw_cert_verification_controller.h @@ -7,7 +7,9 @@ #import <Foundation/Foundation.h> +#include "base/mac/scoped_cftyperef.h" #import "base/memory/ref_counted.h" +#include "ios/web/public/security_style.h" #include "net/cert/cert_status_flags.h" namespace net { @@ -32,6 +34,8 @@ typedef NS_ENUM(NSInteger, CertAcceptPolicy) { // Completion handler called by decidePolicyForCert:host:completionHandler:. typedef void (^PolicyDecisionHandler)(web::CertAcceptPolicy, net::CertStatus); +// Completion handler called by querySSLStatusForTrust:host:completionHandler:. +typedef void (^StatusQueryHandler)(web::SecurityStyle, net::CertStatus); } // namespace web @@ -49,14 +53,25 @@ typedef void (^PolicyDecisionHandler)(web::CertAcceptPolicy, net::CertStatus); // TODO(eugenebut): add API for: // - accepting bad SSL cert using CertPolicyCache -// - querying SSL cert status for Navigation Item // Decides the policy for the given |cert| for the given |host| and calls -// |completionHandler| on completion. |completionHandler| cannot be null and -// will be called synchronously or asynchronously on UI thread. +// |completionHandler| on completion. |host| should be in ASCII compatible form +// (e.g. for "http://名がドメイン.com", it should be "xn--v8jxj3d1dzdz08w.com"). +// |completionHandler| cannot be null and will be called asynchronously on the +// UI thread. - (void)decidePolicyForCert:(const scoped_refptr<net::X509Certificate>&)cert host:(NSString*)host - completionHandler:(web::PolicyDecisionHandler)handler; + completionHandler:(web::PolicyDecisionHandler)completionHandler; + +// Asynchronously provides web::SecurityStyle and net::CertStatus for the given +// |serverTrust| and |host|. |host| should be in ASCII compatible form. +// Note: The web::SecurityStyle determines whether the certificate is trusted. +// It is possible for an untrusted certificate to return a net::CertStatus with +// no errors if the cause could not be determined. Callers must handle this case +// gracefully. +- (void)querySSLStatusForTrust:(base::ScopedCFTypeRef<SecTrustRef>)serverTrust + host:(NSString*)host + completionHandler:(web::StatusQueryHandler)completionHandler; // Cancels all pending verification requests. Completion handlers will not be // called after |shutDown| call. Must always be called before object's diff --git a/ios/web/net/crw_cert_verification_controller.mm b/ios/web/net/crw_cert_verification_controller.mm index a0180d0..c5dca87 100644 --- a/ios/web/net/crw_cert_verification_controller.mm +++ b/ios/web/net/crw_cert_verification_controller.mm @@ -9,9 +9,11 @@ #import "base/memory/ref_counted.h" #import "base/memory/scoped_ptr.h" #include "base/strings/sys_string_conversions.h" +#include "base/threading/worker_pool.h" #include "ios/web/net/cert_verifier_block_adapter.h" #include "ios/web/public/browser_state.h" #include "ios/web/public/web_thread.h" +#import "ios/web/web_state/wk_web_view_security_util.h" #include "net/cert/cert_verify_result.h" #include "net/ssl/ssl_config_service.h" #include "net/url_request/url_request_context.h" @@ -73,13 +75,21 @@ class BlockHolder : public base::RefCountedThreadSafe<BlockHolder<T>> { // Creates _certVerifier object on IO thread. - (void)createCertVerifier; -// Verifies the given |cert| for the given |host| and calls |completionHandler| -// on completion. |completionHandler| cannot be null and will be called -// synchronously or asynchronously on IO thread. +// Verifies the given |cert| for the given |host| using |net::CertVerifier| and +// calls |completionHandler| on completion. This method can be called on any +// thread. |completionHandler| cannot be null and will be called asynchronously +// on IO thread. - (void)verifyCert:(const scoped_refptr<net::X509Certificate>&)cert forHost:(NSString*)host completionHandler:(void (^)(net::CertVerifyResult, int))completionHandler; +// Verifies the given |trust| using SecTrustRef API. |completionHandler| cannot +// be null and will be either called asynchronously on Worker thread or +// synchronously on current thread if the worker task can't start (in this +// case |dispatched| argument will be NO). +- (void)verifyTrust:(base::ScopedCFTypeRef<SecTrustRef>)trust + completionHandler:(void (^)(SecTrustResultType, BOOL dispatched))handler; + @end @implementation CRWCertVerificationController @@ -112,16 +122,16 @@ class BlockHolder : public base::RefCountedThreadSafe<BlockHolder<T>> { - (void)decidePolicyForCert:(const scoped_refptr<net::X509Certificate>&)cert host:(NSString*)host - completionHandler:(web::PolicyDecisionHandler)handler { + completionHandler:(web::PolicyDecisionHandler)completionHandler { DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI); // completionHandler of |verifyCert:forHost:completionHandler:| is called on // IO thread and then bounces back to UI thread. As a result all objects // captured by completionHandler may be released on either UI or IO thread. - // Since |handler| can potentially capture multiple thread unsafe objects - // (like Web Controller) |handler| itself should never be released on - // background thread and |BlockHolder| ensures that. + // Since |completionHandler| can potentially capture multiple thread unsafe + // objects (like Web Controller) |completionHandler| itself should never be + // released on background thread and |BlockHolder| ensures that. __block scoped_refptr<BlockHolder<web::PolicyDecisionHandler>> handlerHolder( - new BlockHolder<web::PolicyDecisionHandler>(handler)); + new BlockHolder<web::PolicyDecisionHandler>(completionHandler)); [self verifyCert:cert forHost:host completionHandler:^(net::CertVerifyResult result, int error) { @@ -141,6 +151,55 @@ class BlockHolder : public base::RefCountedThreadSafe<BlockHolder<T>> { }]; } +- (void)querySSLStatusForTrust:(base::ScopedCFTypeRef<SecTrustRef>)trust + host:(NSString*)host + completionHandler:(web::StatusQueryHandler)completionHandler { + DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI); + + // The completion handlers of |verifyCert:forHost:completionHandler:| and + // |verifyTrust:completionHandler:| will be deallocated on background thread. + // |completionHandler| itself should never be released on background thread + // and |BlockHolder| ensures that. + __block scoped_refptr<BlockHolder<web::StatusQueryHandler>> handlerHolder( + new BlockHolder<web::StatusQueryHandler>(completionHandler)); + [self verifyTrust:trust + completionHandler:^(SecTrustResultType trustResult, BOOL dispatched) { + if (!dispatched) { + // CertVerification task did not start. + dispatch_async(dispatch_get_main_queue(), ^{ + handlerHolder->call(web::SECURITY_STYLE_UNKNOWN, net::CertStatus()); + }); + return; + } + + web::SecurityStyle securityStyle = + web::GetSecurityStyleFromTrustResult(trustResult); + if (securityStyle == web::SECURITY_STYLE_AUTHENTICATED) { + // SecTrust API considers this cert as valid. + dispatch_async(dispatch_get_main_queue(), ^{ + handlerHolder->call(securityStyle, net::CertStatus()); + }); + return; + } + + // Retrieve the net::CertStatus for invalid certificates to determine + // the rejection reason, it is possible that rejection reason could not + // be determined and |cert_status| will be empty. + // TODO(eugenebut): Add UMA for CertVerifier and SecTrust verification + // mismatch (crbug.com/535699). + scoped_refptr<net::X509Certificate> cert( + web::CreateCertFromTrust(trust)); + [self verifyCert:cert + forHost:host + completionHandler:^(net::CertVerifyResult certVerifierResult, int) { + dispatch_async(dispatch_get_main_queue(), ^{ + handlerHolder->call(securityStyle, + certVerifierResult.cert_status); + }); + }]; + }]; +} + - (void)shutDown { DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI); web::WebThread::PostTask(web::WebThread::IO, FROM_HERE, base::BindBlock(^{ @@ -197,4 +256,22 @@ class BlockHolder : public base::RefCountedThreadSafe<BlockHolder<T>> { })); } +- (void)verifyTrust:(base::ScopedCFTypeRef<SecTrustRef>)trust + completionHandler:(void (^)(SecTrustResultType, BOOL))completionHandler { + DCHECK(completionHandler); + // SecTrustEvaluate performs trust evaluation synchronously, possibly making + // network requests. The UI thread should not be blocked by that operation. + bool dispatched = base::WorkerPool::PostTask(FROM_HERE, base::BindBlock(^{ + SecTrustResultType trustResult = kSecTrustResultInvalid; + if (SecTrustEvaluate(trust.get(), &trustResult) != errSecSuccess) { + trustResult = kSecTrustResultInvalid; + } + completionHandler(trustResult, YES); + }), false /* task_is_slow */); + + if (!dispatched) { + completionHandler(kSecTrustResultInvalid, NO); + } +} + @end diff --git a/ios/web/net/crw_cert_verification_controller_unittest.mm b/ios/web/net/crw_cert_verification_controller_unittest.mm index ad3a7f3..6015b5e 100644 --- a/ios/web/net/crw_cert_verification_controller_unittest.mm +++ b/ios/web/net/crw_cert_verification_controller_unittest.mm @@ -9,6 +9,7 @@ #include "base/test/ios/wait_util.h" #include "ios/web/public/web_thread.h" #include "ios/web/test/web_test.h" +#import "ios/web/web_state/wk_web_view_security_util.h" #include "net/base/test_data_directory.h" #include "net/cert/mock_cert_verifier.h" #include "net/cert/x509_certificate.h" @@ -41,6 +42,12 @@ class CRWCertVerificationControllerTest : public web::WebTest { initWithBrowserState:browser_state]); cert_ = net::ImportCertFromFile(net::GetTestCertsDirectory(), kCertFileName); + ASSERT_TRUE(cert_); + + NSArray* chain = GetChain(cert_); + valid_trust_ = web::CreateServerTrustFromChain(chain, kHostName); + web::EnsureFutureTrustEvaluationSucceeds(valid_trust_.get()); + invalid_trust_ = web::CreateServerTrustFromChain(chain, kHostName); } void TearDown() override { @@ -48,6 +55,16 @@ class CRWCertVerificationControllerTest : public web::WebTest { web::WebTest::TearDown(); } + // Returns NSArray of SecCertificateRef objects for the given |cert|. + NSArray* GetChain(const scoped_refptr<net::X509Certificate>& cert) const { + NSMutableArray* result = [NSMutableArray + arrayWithObject:static_cast<id>(cert->os_cert_handle())]; + for (SecCertificateRef intermediate : cert->GetIntermediateCertificates()) { + [result addObject:static_cast<id>(intermediate)]; + } + return result; + } + // Synchronously returns result of decidePolicyForCert:host:completionHandler: // call. void DecidePolicy(const scoped_refptr<net::X509Certificate>& cert, @@ -68,17 +85,39 @@ class CRWCertVerificationControllerTest : public web::WebTest { }, base::MessageLoop::current(), base::TimeDelta()); } + // Synchronously returns result of + // querySSLStatusForTrust:host:completionHandler: call. + void QueryStatus(const base::ScopedCFTypeRef<SecTrustRef>& trust, + NSString* host, + SecurityStyle* style, + net::CertStatus* status) { + __block bool completion_handler_called = false; + [controller_ querySSLStatusForTrust:trust + host:host + completionHandler:^(SecurityStyle callback_style, + net::CertStatus callback_status) { + *style = callback_style; + *status = callback_status; + completion_handler_called = true; + }]; + base::test::ios::WaitUntilCondition(^{ + return completion_handler_called; + }, base::MessageLoop::current(), base::TimeDelta()); + } + scoped_refptr<net::X509Certificate> cert_; + base::ScopedCFTypeRef<SecTrustRef> valid_trust_; + base::ScopedCFTypeRef<SecTrustRef> invalid_trust_; net::MockCertVerifier cert_verifier_; base::scoped_nsobject<CRWCertVerificationController> controller_; }; // Tests cert policy with a valid cert. -TEST_F(CRWCertVerificationControllerTest, ValidCert) { +TEST_F(CRWCertVerificationControllerTest, PolicyForValidCert) { net::CertVerifyResult verify_result; verify_result.cert_status = net::CERT_STATUS_NO_REVOCATION_MECHANISM; verify_result.verified_cert = cert_; - cert_verifier_.AddResultForCertAndHost(cert_.get(), [kHostName UTF8String], + cert_verifier_.AddResultForCertAndHost(cert_.get(), kHostName.UTF8String, verify_result, net::OK); web::CertAcceptPolicy policy = CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR; net::CertStatus status; @@ -88,7 +127,7 @@ TEST_F(CRWCertVerificationControllerTest, ValidCert) { } // Tests cert policy with an invalid cert. -TEST_F(CRWCertVerificationControllerTest, InvalidCert) { +TEST_F(CRWCertVerificationControllerTest, PolicyForInvalidCert) { web::CertAcceptPolicy policy = CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR; net::CertStatus status; DecidePolicy(cert_, kHostName, &policy, &status); @@ -96,7 +135,7 @@ TEST_F(CRWCertVerificationControllerTest, InvalidCert) { } // Tests cert policy with null cert. -TEST_F(CRWCertVerificationControllerTest, NullCert) { +TEST_F(CRWCertVerificationControllerTest, PolicyForNullCert) { web::CertAcceptPolicy policy = CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR; net::CertStatus status; DecidePolicy(nullptr, kHostName, &policy, &status); @@ -104,11 +143,54 @@ TEST_F(CRWCertVerificationControllerTest, NullCert) { } // Tests cert policy with null cert and null host. -TEST_F(CRWCertVerificationControllerTest, NullHost) { +TEST_F(CRWCertVerificationControllerTest, PolicyForNullHost) { web::CertAcceptPolicy policy = CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR; net::CertStatus status; DecidePolicy(cert_, nil, &policy, &status); EXPECT_EQ(CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR, policy); } +// Tests SSL status with valid trust. +TEST_F(CRWCertVerificationControllerTest, SSLStatusForValidTrust) { + SecurityStyle style = SECURITY_STYLE_UNKNOWN; + net::CertStatus status = net::CERT_STATUS_ALL_ERRORS; + + QueryStatus(valid_trust_, kHostName, &style, &status); + EXPECT_EQ(SECURITY_STYLE_AUTHENTICATED, style); + EXPECT_FALSE(status); +} + +// Tests SSL status with invalid host. +TEST_F(CRWCertVerificationControllerTest, SSLStatusForInvalidHost) { + net::CertVerifyResult result; + result.cert_status = net::CERT_STATUS_COMMON_NAME_INVALID; + result.verified_cert = cert_; + cert_verifier_.AddResultForCertAndHost(cert_.get(), kHostName.UTF8String, + result, + net::ERR_CERT_COMMON_NAME_INVALID); + + SecurityStyle style = SECURITY_STYLE_UNKNOWN; + net::CertStatus status = net::CERT_STATUS_ALL_ERRORS; + + QueryStatus(invalid_trust_, kHostName, &style, &status); + EXPECT_EQ(SECURITY_STYLE_AUTHENTICATION_BROKEN, style); + EXPECT_EQ(status, net::CERT_STATUS_COMMON_NAME_INVALID); +} + +// Tests SSL status with expired cert. +TEST_F(CRWCertVerificationControllerTest, SSLStatusForExpiredTrust) { + net::CertVerifyResult result; + result.cert_status = net::CERT_STATUS_DATE_INVALID; + result.verified_cert = cert_; + cert_verifier_.AddResultForCertAndHost(cert_.get(), kHostName.UTF8String, + result, net::ERR_CERT_DATE_INVALID); + + SecurityStyle style = SECURITY_STYLE_UNKNOWN; + net::CertStatus status = net::CERT_STATUS_ALL_ERRORS; + + QueryStatus(invalid_trust_, kHostName, &style, &status); + EXPECT_EQ(SECURITY_STYLE_AUTHENTICATION_BROKEN, style); + EXPECT_EQ(net::CERT_STATUS_DATE_INVALID, status); +} + } // namespace web diff --git a/ios/web/public/ssl_status.h b/ios/web/public/ssl_status.h index 56f3d11..18549e9 100644 --- a/ios/web/public/ssl_status.h +++ b/ios/web/public/ssl_status.h @@ -5,6 +5,8 @@ #ifndef IOS_WEB_PUBLIC_SSL_STATUS_H_ #define IOS_WEB_PUBLIC_SSL_STATUS_H_ +#include <string> + #include "ios/web/public/security_style.h" #include "net/cert/cert_status_flags.h" @@ -33,6 +35,7 @@ struct SSLStatus { cert_status == status.cert_status && security_bits == status.security_bits && content_status == status.content_status; + // |cert_status_host| is not used for comparison intentionally. } web::SecurityStyle security_style; @@ -42,6 +45,11 @@ struct SSLStatus { int connection_status; // A combination of the ContentStatusFlags above. int content_status; + // Host which was used for |cert_status| calculation. It is not an actual part + // of SSL status, hence it's not taken into account in |Equals| method. + // Used to check if |cert_status| is still valid or needs to be recalculated + // (e.g. after redirect). + std::string cert_status_host; }; } // namespace web diff --git a/ios/web/web_state/ui/crw_wk_web_view_web_controller.mm b/ios/web/web_state/ui/crw_wk_web_view_web_controller.mm index db9dc92..726c477 100644 --- a/ios/web/web_state/ui/crw_wk_web_view_web_controller.mm +++ b/ios/web/web_state/ui/crw_wk_web_view_web_controller.mm @@ -11,6 +11,7 @@ #include "base/json/json_reader.h" #import "base/mac/scoped_nsobject.h" #include "base/macros.h" +#include "base/metrics/histogram_macros.h" #include "base/strings/sys_string_conversions.h" #include "base/values.h" #import "ios/net/http_response_headers_util.h" @@ -248,6 +249,21 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { // Clears all activity indicator tasks for this web controller. - (void)clearActivityIndicatorTasks; +// Updates |security_style| and |cert_status| for the NavigationItem with ID +// |navigationItemID|, if URL and certificate chain still match |host| and +// |certChain|. +- (void)updateSSLStatusForNavigationItemWithID:(int)navigationItemID + certChain:(NSArray*)chain + host:(NSString*)host + withSecurityStyle:(web::SecurityStyle)style + certStatus:(net::CertStatus)certStatus; + +// Asynchronously obtains SSL status from given |certChain| and |host| and +// updates current navigation item. Before scheduling update changes SSLStatus' +// cert_status and security_style to default. +- (void)scheduleSSLStatusUpdateUsingCertChain:(NSArray*)chain + host:(NSString*)host; + #if !defined(ENABLE_CHROME_NET_STACK_FOR_WKWEBVIEW) // Updates SSL status for the current navigation item based on the information // provided by web view. @@ -867,47 +883,133 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { clearNetworkTasksForGroup:[self activityIndicatorGroupID]]; } +- (void)updateSSLStatusForNavigationItemWithID:(int)navigationItemID + certChain:(NSArray*)chain + host:(NSString*)host + withSecurityStyle:(web::SecurityStyle)style + certStatus:(net::CertStatus)certStatus { + web::NavigationManager* navigationManager = + self.webStateImpl->GetNavigationManager(); + + // The searched item almost always be the last one, so walk backward rather + // than forward. + for (int i = navigationManager->GetEntryCount() - 1; 0 <= i; i--) { + web::NavigationItem* item = navigationManager->GetItemAtIndex(i); + if (item->GetUniqueID() != navigationItemID) + continue; + + // NavigationItem's UniqueID is preserved even after redirects, so + // checking that cert and URL match is necessary. + scoped_refptr<net::X509Certificate> cert(web::CreateCertFromChain(chain)); + int certID = + web::CertStore::GetInstance()->StoreCert(cert.get(), self.certGroupID); + std::string GURLHost = base::SysNSStringToUTF8(host); + web::SSLStatus& SSLStatus = item->GetSSL(); + if (SSLStatus.cert_id == certID && item->GetURL().host() == GURLHost) { + web::SSLStatus previousSSLStatus = item->GetSSL(); + SSLStatus.cert_status = certStatus; + SSLStatus.security_style = style; + if (navigationManager->GetCurrentEntryIndex() == i && + !previousSSLStatus.Equals(SSLStatus)) { + [self didUpdateSSLStatusForCurrentNavigationItem]; + } + } + return; + } +} + +- (void)scheduleSSLStatusUpdateUsingCertChain:(NSArray*)chain + host:(NSString*)host { + // Use Navigation Item's unique ID to locate requested item after + // obtaining cert status asynchronously. + int itemID = self.webStateImpl->GetNavigationManager() + ->GetLastCommittedItem() + ->GetUniqueID(); + + base::WeakNSObject<CRWWKWebViewWebController> weakSelf(self); + void (^SSLStatusResponse)(web::SecurityStyle, net::CertStatus) = + ^(web::SecurityStyle style, net::CertStatus certStatus) { + base::scoped_nsobject<CRWWKWebViewWebController> strongSelf( + [weakSelf retain]); + if (!strongSelf || [strongSelf isBeingDestroyed]) { + return; + } + [strongSelf updateSSLStatusForNavigationItemWithID:itemID + certChain:chain + host:host + withSecurityStyle:style + certStatus:certStatus]; + }; + + [_certVerificationController + querySSLStatusForTrust:web::CreateServerTrustFromChain(chain, host) + host:host + completionHandler:SSLStatusResponse]; +} + #if !defined(ENABLE_CHROME_NET_STACK_FOR_WKWEBVIEW) + - (void)updateSSLStatusForCurrentNavigationItem { if ([self isBeingDestroyed]) return; - DCHECK(self.webStateImpl); web::NavigationItem* item = self.webStateImpl->GetNavigationManagerImpl().GetLastCommittedItem(); if (!item) return; web::SSLStatus previousSSLStatus = item->GetSSL(); - web::SSLStatus& SSLStatus = item->GetSSL(); + + // Starting from iOS9 WKWebView blocks active mixed content, so if + // |hasOnlySecureContent| returns NO it means passive content. + item->GetSSL().content_status = + [_wkWebView hasOnlySecureContent] + ? web::SSLStatus::NORMAL_CONTENT + : web::SSLStatus::DISPLAYED_INSECURE_CONTENT; + + // Try updating SSLStatus for current NavigationItem asynchronously. + scoped_refptr<net::X509Certificate> cert; if (item->GetURL().SchemeIsCryptographic()) { - // TODO(eugenebut): Do not set security style to authenticated once - // proceeding with bad ssl cert is implemented. - SSLStatus.security_style = web::SECURITY_STYLE_AUTHENTICATED; - SSLStatus.content_status = [_wkWebView hasOnlySecureContent] - ? web::SSLStatus::NORMAL_CONTENT - : web::SSLStatus::DISPLAYED_INSECURE_CONTENT; - - if (base::ios::IsRunningOnIOS9OrLater()) { - scoped_refptr<net::X509Certificate> cert(web::CreateCertFromChain( - [_wkWebView performSelector:@selector(certificateChain)])); - if (cert) { - SSLStatus.cert_id = web::CertStore::GetInstance()->StoreCert( - cert.get(), self.certGroupID); - } else { - SSLStatus.security_style = web::SECURITY_STYLE_UNAUTHENTICATED; - SSLStatus.cert_id = 0; + NSArray* chain = [_wkWebView certificateChain]; + cert = web::CreateCertFromChain(chain); + if (cert) { + int oldCertID = item->GetSSL().cert_id; + std::string oldHost = item->GetSSL().cert_status_host; + item->GetSSL().cert_id = web::CertStore::GetInstance()->StoreCert( + cert.get(), self.certGroupID); + item->GetSSL().cert_status_host = _documentURL.host(); + // Only recompute the SSLStatus information if the certificate or host has + // since changed. Host can be changed in case of redirect. + if (oldCertID != item->GetSSL().cert_id || + oldHost != item->GetSSL().cert_status_host) { + // Real SSL status is unknown, reset cert status and security style. + // They will be asynchronously updated in + // |scheduleSSLStatusUpdateUsingCertChain|. + item->GetSSL().cert_status = net::CertStatus(); + item->GetSSL().security_style = web::SECURITY_STYLE_UNKNOWN; + + NSString* host = base::SysUTF8ToNSString(_documentURL.host()); + [self scheduleSSLStatusUpdateUsingCertChain:chain host:host]; } } - } else { - SSLStatus.security_style = web::SECURITY_STYLE_UNAUTHENTICATED; - SSLStatus.cert_id = 0; } - if (!previousSSLStatus.Equals(SSLStatus)) { + if (!cert) { + item->GetSSL().cert_id = 0; + if (!item->GetURL().SchemeIsCryptographic()) { + // HTTP or other non-secure connection. + item->GetSSL().security_style = web::SECURITY_STYLE_UNAUTHENTICATED; + } else { + // HTTPS, no certificate (this use-case has not been observed). + item->GetSSL().security_style = web::SECURITY_STYLE_UNKNOWN; + } + } + + if (!previousSSLStatus.Equals(item->GetSSL())) { [self didUpdateSSLStatusForCurrentNavigationItem]; } } + #endif // !defined(ENABLE_CHROME_NET_STACK_FOR_WKWEBVIEW) - (void)registerLoadRequest:(const GURL&)url { @@ -926,6 +1028,7 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { - (void)URLDidChangeWithoutDocumentChange:(const GURL&)newURL { DCHECK(newURL == net::GURLWithNSURL([_wkWebView URL])); + DCHECK_EQ(_documentURL.host(), newURL.host()); _documentURL = newURL; // If called during window.history.pushState or window.history.replaceState // JavaScript evaluation, only update the document URL. This callback does not @@ -1347,6 +1450,14 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { #if !defined(ENABLE_CHROME_NET_STACK_FOR_WKWEBVIEW) [self updateSSLStatusForCurrentNavigationItem]; #endif + + // Report cases where SSL cert is missing for a secure connection. + if (_documentURL.SchemeIsCryptographic()) { + scoped_refptr<net::X509Certificate> cert = + web::CreateCertFromChain([_wkWebView certificateChain]); + UMA_HISTOGRAM_BOOLEAN("WebController.WKWebViewHasCertForSecureConnection", + cert); + } } - (void)webView:(WKWebView *)webView @@ -1380,6 +1491,7 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { SecTrustRef trust = challenge.protectionSpace.serverTrust; scoped_refptr<net::X509Certificate> cert = web::CreateCertFromTrust(trust); + // TODO(eugenebut): pass SecTrustRef instead of cert. [_certVerificationController decidePolicyForCert:cert host:challenge.protectionSpace.host diff --git a/ios/web/web_state/wk_web_view_security_util.h b/ios/web/web_state/wk_web_view_security_util.h index 7dcce6a..15c3fe9 100644 --- a/ios/web/web_state/wk_web_view_security_util.h +++ b/ios/web/web_state/wk_web_view_security_util.h @@ -6,8 +6,11 @@ #define IOS_WEB_WEB_STATE_WK_WEB_VIEW_SECURITY_UTIL_H_ #import <Foundation/Foundation.h> +#include <Security/Security.h> +#include "base/mac/scoped_cftyperef.h" #include "base/memory/ref_counted.h" +#include "ios/web/public/security_style.h" namespace net { class SSLInfo; @@ -27,6 +30,11 @@ scoped_refptr<net::X509Certificate> CreateCertFromChain(NSArray* certs); // Returns null if trust is null or does not have any certs. scoped_refptr<net::X509Certificate> CreateCertFromTrust(SecTrustRef trust); +// Creates server trust object from an array of SecCertificateRef objects. +// Returns null if |certs| is null or empty. +base::ScopedCFTypeRef<SecTrustRef> CreateServerTrustFromChain(NSArray* certs, + NSString* host); + // Makes SecTrustEvaluate call to return kSecTrustResultProceed. // Should be called only if the user expilitely agreed to proceed with |trust| // or trust represents a valid certificate chain. @@ -41,6 +49,9 @@ BOOL IsWKWebViewSSLCertError(NSError* error); void GetSSLInfoFromWKWebViewSSLCertError(NSError* error, net::SSLInfo* ssl_info); +// Maps SecTrustResultType value to web::SecurityStyle. +SecurityStyle GetSecurityStyleFromTrustResult(SecTrustResultType result); + } // namespace web #endif // IOS_WEB_WEB_STATE_WK_WEB_VIEW_SECURITY_UTIL_H_ diff --git a/ios/web/web_state/wk_web_view_security_util.mm b/ios/web/web_state/wk_web_view_security_util.mm index 87c3271..7e5a9eb 100644 --- a/ios/web/web_state/wk_web_view_security_util.mm +++ b/ios/web/web_state/wk_web_view_security_util.mm @@ -73,6 +73,22 @@ scoped_refptr<net::X509Certificate> CreateCertFromTrust(SecTrustRef trust) { SecTrustGetCertificateAtIndex(trust, 0), intermediates); } +base::ScopedCFTypeRef<SecTrustRef> CreateServerTrustFromChain(NSArray* certs, + NSString* host) { + base::ScopedCFTypeRef<SecTrustRef> scoped_result; + if (certs.count == 0) + return scoped_result; + + base::ScopedCFTypeRef<SecPolicyRef> policy( + SecPolicyCreateSSL(TRUE, static_cast<CFStringRef>(host))); + SecTrustRef ref_result = nullptr; + if (SecTrustCreateWithCertificates(certs, policy, &ref_result) == + errSecSuccess) { + scoped_result.reset(ref_result); + } + return scoped_result; +} + void EnsureFutureTrustEvaluationSucceeds(SecTrustRef trust) { base::ScopedCFTypeRef<CFDataRef> exceptions(SecTrustCopyExceptions(trust)); SecTrustSetExceptions(trust, exceptions); @@ -108,4 +124,21 @@ void GetSSLInfoFromWKWebViewSSLCertError(NSError* error, error.userInfo[web::kNSErrorPeerCertificateChainKey]); } +SecurityStyle GetSecurityStyleFromTrustResult(SecTrustResultType result) { + switch (result) { + case kSecTrustResultInvalid: + return SECURITY_STYLE_UNKNOWN; + case kSecTrustResultProceed: + case kSecTrustResultUnspecified: + return SECURITY_STYLE_AUTHENTICATED; + case kSecTrustResultDeny: + case kSecTrustResultRecoverableTrustFailure: + case kSecTrustResultFatalTrustFailure: + case kSecTrustResultOtherError: + return SECURITY_STYLE_AUTHENTICATION_BROKEN; + } + NOTREACHED(); + return SECURITY_STYLE_UNKNOWN; +} + } // namespace web diff --git a/ios/web/web_state/wk_web_view_security_util_unittest.mm b/ios/web/web_state/wk_web_view_security_util_unittest.mm index 97d2cc7..d925363 100644 --- a/ios/web/web_state/wk_web_view_security_util_unittest.mm +++ b/ios/web/web_state/wk_web_view_security_util_unittest.mm @@ -16,12 +16,15 @@ #include "net/cert/x509_util.h" #include "net/ssl/ssl_info.h" #include "testing/gtest/include/gtest/gtest.h" +#include "testing/gtest_mac.h" #include "testing/platform_test.h" namespace web { namespace { // Subject for testing self-signed certificate. const char kTestSubject[] = "self-signed"; +// Hostname for testing SecTrustRef objects. +NSString* const kTestHost = @"www.example.com"; // Returns an autoreleased certificate chain for testing. Chain will contain a // single self-signed cert with |subject| as a subject. @@ -108,6 +111,46 @@ TEST_F(WKWebViewSecurityUtilTest, CreationCertFromNilTrust) { EXPECT_FALSE(CreateCertFromTrust(nil)); } +// Tests CreateServerTrustFromChain with valid input. +TEST_F(WKWebViewSecurityUtilTest, CreationServerTrust) { + // Create server trust. + NSArray* chain = MakeTestCertChain(kTestSubject); + base::ScopedCFTypeRef<SecTrustRef> server_trust( + CreateServerTrustFromChain(chain, kTestHost)); + EXPECT_TRUE(server_trust); + + // Verify chain. + EXPECT_EQ(static_cast<CFIndex>(chain.count), + SecTrustGetCertificateCount(server_trust)); + [chain enumerateObjectsUsingBlock:^(id expected_cert, NSUInteger i, BOOL*) { + id actual_cert = static_cast<id>(SecTrustGetCertificateAtIndex( + server_trust.get(), static_cast<CFIndex>(i))); + EXPECT_EQ(expected_cert, actual_cert); + }]; + + // Verify policies. + CFArrayRef policies = nullptr; + EXPECT_EQ(errSecSuccess, SecTrustCopyPolicies(server_trust.get(), &policies)); + EXPECT_EQ(1, CFArrayGetCount(policies)); + SecPolicyRef policy = (SecPolicyRef)CFArrayGetValueAtIndex(policies, 0); + base::ScopedCFTypeRef<CFDictionaryRef> properties( + SecPolicyCopyProperties(policy)); + NSString* name = static_cast<NSString*>( + CFDictionaryGetValue(properties.get(), kSecPolicyName)); + EXPECT_NSEQ(kTestHost, name); + CFRelease(policies); +} + +// Tests CreateServerTrustFromChain with nil chain. +TEST_F(WKWebViewSecurityUtilTest, CreationServerTrustFromNilChain) { + EXPECT_FALSE(CreateServerTrustFromChain(nil, kTestHost)); +} + +// Tests CreateServerTrustFromChain with empty chain. +TEST_F(WKWebViewSecurityUtilTest, CreationServerTrustFromEmptyChain) { + EXPECT_FALSE(CreateServerTrustFromChain(@[], kTestHost)); +} + // Tests that IsWKWebViewSSLCertError returns YES for NSError with // NSURLErrorDomain domain, NSURLErrorSecureConnectionFailed error code and // certificate chain. @@ -208,4 +251,31 @@ TEST_F(WKWebViewSecurityUtilTest, SSLInfoFromErrorWithCert) { EXPECT_TRUE(info.cert->subject().GetDisplayName() == kTestSubject); } +// Tests GetSecurityStyleFromTrustResult with bad SecTrustResultType result. +TEST_F(WKWebViewSecurityUtilTest, GetSecurityStyleFromBadResult) { + EXPECT_EQ(SECURITY_STYLE_AUTHENTICATION_BROKEN, + GetSecurityStyleFromTrustResult(kSecTrustResultDeny)); + EXPECT_EQ( + SECURITY_STYLE_AUTHENTICATION_BROKEN, + GetSecurityStyleFromTrustResult(kSecTrustResultRecoverableTrustFailure)); + EXPECT_EQ(SECURITY_STYLE_AUTHENTICATION_BROKEN, + GetSecurityStyleFromTrustResult(kSecTrustResultFatalTrustFailure)); + EXPECT_EQ(SECURITY_STYLE_AUTHENTICATION_BROKEN, + GetSecurityStyleFromTrustResult(kSecTrustResultOtherError)); +} + +// Tests GetSecurityStyleFromTrustResult with good SecTrustResultType result. +TEST_F(WKWebViewSecurityUtilTest, GetSecurityStyleFromGoodResult) { + EXPECT_EQ(SECURITY_STYLE_AUTHENTICATED, + GetSecurityStyleFromTrustResult(kSecTrustResultProceed)); + EXPECT_EQ(SECURITY_STYLE_AUTHENTICATED, + GetSecurityStyleFromTrustResult(kSecTrustResultUnspecified)); +} + +// Tests GetSecurityStyleFromTrustResult with invalid SecTrustResultType result. +TEST_F(WKWebViewSecurityUtilTest, GetSecurityStyleFromInvalidResult) { + EXPECT_EQ(SECURITY_STYLE_UNKNOWN, + GetSecurityStyleFromTrustResult(kSecTrustResultInvalid)); +} + } // namespace web |