WKWebView: Implemented recoverable SSL interstitials.

ios/web/web_state/ui/crw_wk_web_view_web_controller.mm

Index: ios/web/web_state/ui/crw_wk_web_view_web_controller.mm
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 726c4770e6ed33293bcce2a3fec08142abbc13fc..137201aaf804940622fceb944224b731c4b84523 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
@@ -6,6 +6,7 @@
 
 #import <WebKit/WebKit.h>
 
+#include "base/containers/mru_cache.h"
 #include "base/ios/ios_util.h"
 #include "base/ios/weak_nsobject.h"
 #include "base/json/json_reader.h"
@@ -20,6 +21,7 @@
 #import "ios/web/navigation/crw_session_entry.h"
 #include "ios/web/navigation/navigation_item_impl.h"
 #include "ios/web/navigation/web_load_params.h"
+#include "ios/web/net/cert_host_pair.h"
 #import "ios/web/net/crw_cert_verification_controller.h"
 #include "ios/web/public/cert_store.h"
 #include "ios/web/public/navigation_item.h"
@@ -43,12 +45,30 @@
 #import "ios/web/web_state/web_view_internal_creation_util.h"
 #import "ios/web/web_state/wk_web_view_security_util.h"
 #import "ios/web/webui/crw_web_ui_manager.h"
-#include "net/cert/x509_certificate.h"
 #import "net/base/mac/url_conversions.h"
+#include "net/cert/x509_certificate.h"
 #include "net/ssl/ssl_info.h"
 #include "url/url_constants.h"
 
 namespace {
+
+// Represents cert verification error, which happened inside
+// |webView:didReceiveAuthenticationChallenge:completionHandler:| and should
+// be checked inside |webView:didFailProvisionalNavigation:withError:|.
+struct CertVerificationError {
+  BOOL is_recoverable;
+  net::CertStatus status;
+};
+
+// Type of Cache object for storing cert verification errors.
+typedef base::MRUCache<web::CertHostPair, CertVerificationError>
+    CertVerificationErrorsCacheType;
+
+// Maximum number of errors to store in cert verification errors cache.
+// Cache holds errors only for pending navigation, so the actual number of
+// stored errors is not expected to be high.
+const CertVerificationErrorsCacheType::size_type kMaxCertErrorsCount = 100;
+
 // Extracts Referer value from WKNavigationAction request header.
 NSString* GetRefererFromNavigationAction(WKNavigationAction* action) {
   return [action.request valueForHTTPHeaderField:@"Referer"];
@@ -144,6 +164,13 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
   // Cancelled navigations should be simply discarded without handling any
   // specific error.
   BOOL _pendingNavigationCancelled;
+
+  // CertVerification errors which happened inside
+  // |webView:didReceiveAuthenticationChallenge:completionHandler:|.
+  // Key is leaf-cert/host pair. This storage is used to carry calculated
+  // cert status from |didReceiveAuthenticationChallenge:| to
+  // |didFailProvisionalNavigation:| delegate method.
+  scoped_ptr<CertVerificationErrorsCacheType> _certVerificationErrors;
 }
 
 // Response's MIME type of the last known navigation.
@@ -270,6 +297,14 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
 - (void)updateSSLStatusForCurrentNavigationItem;
 #endif
 
+// Used in webView:didReceiveAuthenticationChallenge:completionHandler: to reply
+// with NSURLSessionAuthChallengeDisposition and credentials.
+- (void)processAuthChallenge:(NSURLAuthenticationChallenge*)challenge
+         forCertAcceptPolicy:(web::CertAcceptPolicy)policy
+                  certStatus:(net::CertStatus)certStatus
+           completionHandler:(void (^)(NSURLSessionAuthChallengeDisposition,
+                                       NSURLCredential*))completionHandler;
+
 // Registers load request with empty referrer and link or client redirect
 // transition based on user interaction state.
 - (void)registerLoadRequest:(const GURL&)url;
@@ -319,6 +354,8 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
   if (self) {
     _certVerificationController.reset([[CRWCertVerificationController alloc]
         initWithBrowserState:browserState]);
+    _certVerificationErrors.reset(
+        new CertVerificationErrorsCacheType(kMaxCertErrorsCount));
   }
   return self;
 }
@@ -565,6 +602,7 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
 
 - (void)abortWebLoad {
   [_wkWebView stopLoading];
+  _certVerificationErrors->Clear();
 }
 
 - (void)resetLoadState {
@@ -857,19 +895,55 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
 - (void)handleSSLCertError:(NSError*)error {
   DCHECK(web::IsWKWebViewSSLCertError(error));
 
-  net::SSLInfo sslInfo;
-  web::GetSSLInfoFromWKWebViewSSLCertError(error, &sslInfo);
-
-  web::SSLStatus sslStatus;
-  sslStatus.security_style = web::SECURITY_STYLE_AUTHENTICATION_BROKEN;
-  sslStatus.cert_status = sslInfo.cert_status;
-  sslStatus.cert_id = web::CertStore::GetInstance()->StoreCert(
-      sslInfo.cert.get(), self.certGroupID);
+  net::SSLInfo SSLInfo;
+  web::GetSSLInfoFromWKWebViewSSLCertError(error, &SSLInfo);
+
+  web::SSLStatus SSLStatus;
+  SSLStatus.security_style = web::SECURITY_STYLE_AUTHENTICATION_BROKEN;
+  SSLStatus.cert_status = SSLInfo.cert_status;
+  SSLStatus.cert_id = web::CertStore::GetInstance()->StoreCert(
+      SSLInfo.cert.get(), self.certGroupID);
+
+  // Retrieve verification results from _certVerificationErrors cache to avoid
+  // unnecessary recalculations. Verification results are cached for leaf cert,

[Ryan Sleevi, 2015/10/19 23:56:22]

s/for leaf cert/for the leaf cert/

[Eugene But (OOO till 7-30), 2015/10/21 04:01:02]

Done.

+  // because cert chain in |didReceiveAuthenticationChallenge:| is OS

[Ryan Sleevi, 2015/10/19 23:56:22]

s/is OS/is the OS/

[Ryan Sleevi, 2015/10/19 23:56:22]

s/because cert chain/because the cert chain/

[Eugene But (OOO till 7-30), 2015/10/21 04:01:02]

Done.

[Eugene But (OOO till 7-30), 2015/10/21 04:01:03]

Done.

+  // constructed chain, while |chain| is a chain from the server.

[Ryan Sleevi, 2015/10/19 23:56:22]

s/a chain/the chain/

[Eugene But (OOO till 7-30), 2015/10/21 04:01:02]

Done.

+  NSArray* chain = error.userInfo[web::kNSErrorPeerCertificateChainKey];
+  NSString* host = [error.userInfo[web::kNSErrorFailingURLKey] host];
+  scoped_refptr<net::X509Certificate> leafCert;
+  BOOL recoverable = NO;
+  if (chain.count && host.length) {
+    // Complete cert chain may not be available inside this method, so leaf
+    // cert is used as a key to retrieve _certVerificationErrors as well as for
+    // storing cert decision.

[Ryan Sleevi, 2015/10/19 23:56:22]

// The complete cert chain may not be available, so the leaf
// cert is used as a key to retrieve _certVerificationErrors, as well as
// for storing the cert decision.

[Eugene But (OOO till 7-30), 2015/10/21 04:01:02]

Done.

+    leafCert = web::CreateCertFromChain(@[ chain.firstObject ]);
+    if (leafCert) {
+      // This cache will be purged anyway so there is no need to use |Get|.

[Ryan Sleevi, 2015/10/19 23:56:22]

This is surprising/non-obvious, and doesn't seem to fit why why you're using
MRUCache.

It sounds like you're describing behaviour external to this file, and if so,
that's a layering concern; if it's in this file, where? Document.

[Eugene But (OOO till 7-30), 2015/10/21 04:01:02]

MRUCache allows cache to be bounded and evicts least recent errors, so It's
actually ideal.
Added more comments.

[Ryan Sleevi, 2015/10/28 21:28:50]

I suppose this wasn't clear, as I'm still uncertain why you're using Peek
instead of Get.

Are you trying to optimize? Why?

As clearly documented in Get(), if you Get() an error, it actually brings it to
the front of the MRU. That's how the "R" in MRU is calculated. So it's
surprising and non-obvious that you're invoking a behaviour that doesn't change
ordering at all, as it just makes it seem like you're using a bounded cache
size, rather than an MRU.

[Eugene But (OOO till 7-30), 2015/10/29 00:39:14]

_certVerificationErrors will be purged right after reading from it (see the list
line in webView:didFailProvisionalNavigation:withError:), so Peek was an
optimization.

However I clearly see how that optimization (more-likely negligible) made the
code confusing. Changing to Get.

+      auto error = _certVerificationErrors->Peek(
+          {leafCert, base::SysNSStringToUTF8(host)});
+      if (error != _certVerificationErrors->end()) {
+        SSLStatus.cert_status = error->second.status;
+        recoverable = error->second.is_recoverable;
+      } else {
+        // TODO(eugenebut): Report UMA with cache size (crbug.com/541736).
+      }
+    }
+  }
 
-  [self.delegate presentSSLError:sslInfo
-                    forSSLStatus:sslStatus
-                     recoverable:NO
-                        callback:nullptr];
+  // Present SSL interstitial.
+  [self.delegate presentSSLError:SSLInfo
+                    forSSLStatus:SSLStatus
+                     recoverable:recoverable
+                        callback:^(BOOL proceed) {
+                          if (proceed) {
+                            // The interstitial will be removed during reload.
+                            [_certVerificationController
+                                allowCert:leafCert
+                                  forHost:host
+                                   status:SSLStatus.cert_status];
+                            [self loadCurrentURL];
+                          }
+                        }];
 }
 #endif  // #if !defined(ENABLE_CHROME_NET_STACK_FOR_WKWEBVIEW)
 
@@ -1012,6 +1086,44 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
 
 #endif  // !defined(ENABLE_CHROME_NET_STACK_FOR_WKWEBVIEW)
 
+- (void)processAuthChallenge:(NSURLAuthenticationChallenge*)challenge
+         forCertAcceptPolicy:(web::CertAcceptPolicy)policy
+                  certStatus:(net::CertStatus)certStatus
+           completionHandler:(void (^)(NSURLSessionAuthChallengeDisposition,
+                                       NSURLCredential*))completionHandler {
+  SecTrustRef trust = challenge.protectionSpace.serverTrust;
+  if (policy == web::CERT_ACCEPT_POLICY_RECOVERABLE_ERROR_ACCEPTED_BY_USER) {
+    // Cert is invalid, but user agreed to proceed, override default behavior.
+    completionHandler(NSURLSessionAuthChallengeUseCredential,
+                      [NSURLCredential credentialForTrust:trust]);
+    return;
+  }
+
+  if (policy != web::CERT_ACCEPT_POLICY_ALLOW &&
+      SecTrustGetCertificateCount(trust)) {
+    // Cert is invalid and user has not agreed to proceed. Cache cert

[Ryan Sleevi, 2015/10/19 23:56:22]

// The cert is invalid and the user has not agreed to proceed. Cache
// the cert verification result in |_certVerificationErrors|, so that it
// can later be reused inside |didFailProvisionalNavigation:|.

[Eugene But (OOO till 7-30), 2015/10/21 04:01:03]

Done.

+    // verification result with _certVerificationErrors storage, so it can be
+    // later reused inside |didFailProvisionalNavigation:|.
+    // Leaf cert (w/o any intermidiates) is used as a key, because chain inside
+    // |didFailProvisionalNavigation:| differs (it will be server chain) and
+    // using intermidiates may result in keys mismatch.

[Ryan Sleevi, 2015/10/19 23:56:22]

// The leaf cert is used as the key, because the chain provided by
// |didFailProvisionalNavigation:| will differ (it is the server-supplied
// chain), thus if intermediates were considered, the keys would mismatch.

[Eugene But (OOO till 7-30), 2015/10/21 04:01:02]

Done.

+    scoped_refptr<net::X509Certificate> leafCert =
+        net::X509Certificate::CreateFromHandle(
+            SecTrustGetCertificateAtIndex(trust, 0),
+            net::X509Certificate::OSCertHandles());
+    if (leafCert) {
+      BOOL is_recoverable =
+          policy ==
+          web::CERT_ACCEPT_POLICY_RECOVERABLE_ERROR_UNDECIDED_BY_USER;
+      std::string host =
+          base::SysNSStringToUTF8(challenge.protectionSpace.host);
+      _certVerificationErrors->Put({leafCert, host},
+                                   {is_recoverable, certStatus});
+    }
+  }
+  completionHandler(NSURLSessionAuthChallengeRejectProtectionSpace, nil);
+}
+
 - (void)registerLoadRequest:(const GURL&)url {
   // If load request is registered via WKWebViewWebController, assume transition
   // is link or client redirect as other transitions will already be registered
@@ -1427,11 +1539,13 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
     [self handleLoadError:error inMainFrame:YES];
 
   [self discardPendingNavigationTypeForMainFrame];
+  _certVerificationErrors->Clear();
 }
 
 - (void)webView:(WKWebView *)webView
     didCommitNavigation:(WKNavigation *)navigation {
   DCHECK_EQ(_wkWebView, webView);
+  _certVerificationErrors->Clear();
   // This point should closely approximate the document object change, so reset
   // the list of injected scripts to those that are automatically injected.
   _injectedScriptManagers.reset([[NSMutableSet alloc] init]);
@@ -1476,13 +1590,14 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
             withError:(NSError *)error {
   [self handleLoadError:WKWebViewErrorWithSource(error, NAVIGATION)
             inMainFrame:YES];
+  _certVerificationErrors->Clear();
 }
 
-- (void)webView:(WKWebView *)webView
-    didReceiveAuthenticationChallenge:(NSURLAuthenticationChallenge *)challenge
+- (void)webView:(WKWebView*)webView
+    didReceiveAuthenticationChallenge:(NSURLAuthenticationChallenge*)challenge
                     completionHandler:
-        (void (^)(NSURLSessionAuthChallengeDisposition disposition,
-                  NSURLCredential *credential))completionHandler {
+                        (void (^)(NSURLSessionAuthChallengeDisposition,
+                                  NSURLCredential*))completionHandler {
   if (![challenge.protectionSpace.authenticationMethod
           isEqual:NSURLAuthenticationMethodServerTrust]) {
     completionHandler(NSURLSessionAuthChallengePerformDefaultHandling, nil);
@@ -1490,19 +1605,25 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
   }
 
   SecTrustRef trust = challenge.protectionSpace.serverTrust;
-  scoped_refptr<net::X509Certificate> cert = web::CreateCertFromTrust(trust);
-  // TODO(eugenebut): pass SecTrustRef instead of cert.
+  base::ScopedCFTypeRef<SecTrustRef> scopedTrust(trust,
+                                                 base::scoped_policy::RETAIN);
+  base::WeakNSObject<CRWWKWebViewWebController> weakSelf(self);
   [_certVerificationController
-      decidePolicyForCert:cert
-                     host:challenge.protectionSpace.host
-        completionHandler:^(web::CertAcceptPolicy policy,
-                            net::CertStatus status) {
-          completionHandler(NSURLSessionAuthChallengeRejectProtectionSpace,
-                            nil);
-        }];
+      decideLoadPolicyForTrust:scopedTrust
+                          host:challenge.protectionSpace.host
+             completionHandler:^(web::CertAcceptPolicy policy,
+                                 net::CertStatus status) {
+               base::scoped_nsobject<CRWWKWebViewWebController> strongSelf(
+                   [weakSelf retain]);
+               [strongSelf processAuthChallenge:challenge
+                            forCertAcceptPolicy:policy
+                                     certStatus:status
+                              completionHandler:completionHandler];
+             }];
 }
 
 - (void)webViewWebContentProcessDidTerminate:(WKWebView*)webView {
+  _certVerificationErrors->Clear();
   [self webViewWebProcessDidCrash];
 }