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 3fa37486e966124d6400afbd3b69a440c5e7b45e..393afaaea4671f1f29c208a2fd695a4218d9319a 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
@@ -19,6 +19,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_verification_cache.h"
 #import "ios/web/net/crw_cert_verification_controller.h"
 #include "ios/web/public/cert_store.h"
 #include "ios/web/public/navigation_item.h"
@@ -48,6 +49,15 @@
 #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;
+};
+
 // Extracts Referer value from WKNavigationAction request header.
 NSString* GetRefererFromNavigationAction(WKNavigationAction* action) {
   return [action.request valueForHTTPHeaderField:@"Referer"];
@@ -145,6 +155,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.
+  web::CertVerificationCache<CertVerificationError> _certVerificationErrors;
 }
 
 // Response's MIME type of the last known navigation.
@@ -557,6 +574,7 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
 
 - (void)abortWebLoad {
   [_wkWebView stopLoading];
+  _certVerificationErrors.reset();
 }
 
 - (void)resetLoadState {
@@ -843,19 +861,49 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
 - (void)handleSSLError:(NSError*)error {
   DCHECK(web::IsWKWebViewSSLError(error));
 
-  net::SSLInfo sslInfo;
-  web::GetSSLInfoFromWKWebViewSSLError(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);
-
-  [self.delegate presentSSLError:sslInfo
-                    forSSLStatus:sslStatus
-                     recoverable:NO
-                        callback:nullptr];
+  net::SSLInfo SSLInfo;
+  web::GetSSLInfoFromWKWebViewSSLError(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);
+
+  NSArray* chain = error.userInfo[web::kNSErrorPeerCertificateChainKey];
+  NSString* host = [error.userInfo[web::kNSErrorFailingURLKey] host];
+  // Verification results are cached for leaf cert, because cert chain in
+  // |didReceiveAuthenticationChallenge:| maybe different from |chain|.
+  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.
+    leafCert = web::CreateCertFromChain(@[ [chain firstObject] ]);
+    if (leafCert) {
+      CertVerificationError error;
+      if (_certVerificationErrors.get(leafCert, base::SysNSStringToUTF8(host),
+                                      &error)) {
+        SSLStatus.cert_status = error.status;
+        recoverable = error.is_recoverable;
+      }
+    }
+  }
+  [self.delegate presentSSLError:SSLInfo
+                    forSSLStatus:SSLStatus
+                     recoverable:recoverable
+                        callback:^(BOOL proceed) {
+                          if (proceed) {
+                            // The interstitial will be removed during reload.
+                            [_certVerificationController

[stuartmorgan, 2015/09/23 17:36:09]

Are we okay with a strong reference to self here?

[Eugene But (OOO till 7-30), 2015/09/23 20:40:47]

Yes, displayed interstitial outlives web controller. Also the same approach is
used for UIWebView: 
https://code.google.com/p/chromium/codesearch#chromium/src/ios/web/web_state/...

+                                allowCert:leafCert
+                                  forHost:host
+                                   status:SSLStatus.cert_status];
+                            [self loadCurrentURL];
+                          }
+                        }];
+  _certVerificationErrors.reset();
 }
 #endif  // #if !defined(ENABLE_CHROME_NET_STACK_FOR_WKWEBVIEW)
 
@@ -1398,6 +1446,7 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
 - (void)webView:(WKWebView *)webView
     didCommitNavigation:(WKNavigation *)navigation {
   DCHECK_EQ(_wkWebView, webView);
+  _certVerificationErrors.reset();
   // 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]);
@@ -1448,15 +1497,46 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
   }
 
   SecTrustRef trust = challenge.protectionSpace.serverTrust;
-  scoped_refptr<net::X509Certificate> cert = web::CreateCertFromTrust(trust);
+  id handler = ^(web::CertAcceptPolicy policy, net::CertStatus status) {
+    if (policy == web::CERT_ACCEPT_POLICY_RECOVERABLE_ERROR_ACCEPTED_BY_USER) {
+      // cert is invalid, but user agreed to proceed.
+      completionHandler(NSURLSessionAuthChallengeUseCredential,
+                        [NSURLCredential credentialForTrust:trust]);
+    } else {
+      if (policy != web::CERT_ACCEPT_POLICY_ALLOW) {
+        // cert is invalid and user has not agreed to proceed.
+
+        // Cache cert verification result with _certVerificationErrors storage,
+        // so it can be later reused inside |didFailProvisionalNavigation:|.
+        // didFailProvisionalNavigation: does not receive full cert chain and it
+        // will not be possible to resulculate cert status there.
+        if (SecTrustGetCertificateCount(trust)) {
+          // Leaf cert (w/o any intermidiates) is used for caching.
+          // Chain inside |didFailProvisionalNavigation:| may be different and
+          // using intermidiates will result in keys mismatch.
+          scoped_refptr<net::X509Certificate> leafCert =
+              web::CreateCertFromChain(@[
+                static_cast<id>(SecTrustGetCertificateAtIndex(trust, 0))
+              ]);
+          if (leafCert) {
+            BOOL is_recoverable =
+                policy ==
+                web::CERT_ACCEPT_POLICY_RECOVERABLE_ERROR_NOT_ACCEPTED_BY_USER;
+            _certVerificationErrors.set(
+                leafCert,
+                base::SysNSStringToUTF8(challenge.protectionSpace.host),
+                {is_recoverable, status});
+          }
+        }
+      }
+      completionHandler(NSURLSessionAuthChallengeRejectProtectionSpace, nil);
+    }
+  };
+
   [_certVerificationController
-      decidePolicyForCert:cert
-                     host:challenge.protectionSpace.host
-        completionHandler:^(web::CertAcceptPolicy policy,
-                            net::CertStatus status) {
-          completionHandler(NSURLSessionAuthChallengeRejectProtectionSpace,
-                            nil);
-        }];
+      decideLoadPolicyForTrust:trust
+                          host:challenge.protectionSpace.host
+             completionHandler:handler];

[stuartmorgan, 2015/09/23 17:36:09]

Same question here; what's the ownership/lifetime guarantee here? Handing a
self-owning block to an ivar can be bad news.

[Eugene But (OOO till 7-30), 2015/09/23 20:40:48]

Good catch. CRWCertVerificationController has -close method, which cancels
CertVerifier requests, however -close does not cancel SecTrustEvaluate requests,
which retain this block.

This needs weak-strong dance.

 }
 
 - (void)webViewWebContentProcessDidTerminate:(WKWebView*)webView {