WKWebView(iOS9): correctly update SSL status for current navigation item

ios/web/web_state/ui/crw_wk_web_view_web_controller.mm

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ios/web/web_state/ui/crw_wk_web_view_web_controller.h"
 
 #import <WebKit/WebKit.h>
 
 #include "base/ios/ios_util.h"
 #include "base/ios/weak_nsobject.h"
@@ skipping 232 matching lines @@
 // Called when a load ends in an SSL error.
 - (void)handleSSLError:(NSError*)error;
 #endif
 
 // Adds an activity indicator tasks for this web controller.
 - (void)addActivityIndicatorTask;
 
 // Clears all activity indicator tasks for this web controller.
 - (void)clearActivityIndicatorTasks;
 
+// Obtains SSL status from given |certChain| and updates it for navigation items

[stuartmorgan, 2015/09/22 20:30:28]

"it" here would be the SSL status of the cert chain, but isn't in the navigation
items that are being updated?

[Eugene But (OOO till 7-30), 2015/09/22 22:43:04]

Yes, navigation items are updated. Fixed comment.

+// with given |certID|.
+- (void)updateSSLStatusForNavigationItemsWithCertID:(int)certID
+                                     usingCertChain:(NSArray*)certChain;
+
 #if !defined(ENABLE_CHROME_NET_STACK_FOR_WKWEBVIEW)
 // Updates SSL status for the current navigation item based on the information
 // provided by web view.
 - (void)updateSSLStatusForCurrentNavigationItem;
 #endif
 
 // Registers load request with empty referrer and link or client redirect
 // transition based on user interaction state.
 - (void)registerLoadRequest:(const GURL&)url;
 
@@ skipping 594 matching lines @@
 - (void)addActivityIndicatorTask {
   [[CRWNetworkActivityIndicatorManager sharedInstance]
       startNetworkTaskForGroup:[self activityIndicatorGroupID]];
 }
 
 - (void)clearActivityIndicatorTasks {
   [[CRWNetworkActivityIndicatorManager sharedInstance]
       clearNetworkTasksForGroup:[self activityIndicatorGroupID]];
 }
 
+- (void)updateSSLStatusForNavigationItemsWithCertID:(int)certID
+                                     usingCertChain:(NSArray*)certChain {
+  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;
+        }
+
+        web::NavigationManager* navigationManager =
+            [strongSelf webStateImpl]->GetNavigationManager();
+        int currentItemIndex = navigationManager->GetCurrentEntryIndex();
+
+        bool updatedCurrentItem = false;
+        for (int i = 0; i < navigationManager->GetEntryCount(); i++) {
+          web::NavigationItem* item = navigationManager->GetItemAtIndex(i);
+          web::SSLStatus& SSLStatus = item->GetSSL();
+          if (SSLStatus.cert_id == certID) {
+            web::SSLStatus previousSSLStatus = item->GetSSL();
+            SSLStatus.cert_status = certStatus;
+            SSLStatus.security_style = style;
+            if (currentItemIndex == i && !previousSSLStatus.Equals(SSLStatus)) {
+              updatedCurrentItem = true;
+            }
+          }
+        }
+
+        if (updatedCurrentItem) {
+          [strongSelf didUpdateSSLStatusForCurrentNavigationItem];
+        }
+      };
+
+  [_certVerificationController querySSLStatusForCertChain:certChain
+                                                     host:[_wkWebView URL].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();
-  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;
+  // Starting from iOS9 WKWebView blocks active mixed content, so if
+  // |hasOnlySecureContent| returns NO it means passive content.
+  // On iOS8 there is no way to determine if web view has active mixed content.
+  SSLStatus.content_status = [_wkWebView hasOnlySecureContent]
+                                 ? web::SSLStatus::NORMAL_CONTENT
+                                 : web::SSLStatus::DISPLAYED_INSECURE_CONTENT;
+
+  // Retrieve top level frame certificate.
+  scoped_refptr<net::X509Certificate> cert;
+  if (base::ios::IsRunningOnIOS9OrLater() &&
+      item->GetURL().SchemeIsCryptographic()) {
+    NSArray* chain = [_wkWebView performSelector:@selector(certificateChain)];

[stuartmorgan, 2015/09/22 20:30:28]

Why is this performSelector? (If it's an SDK issue, you should predeclare the
method in an SDK ifdef guard so it's easy to find and clean up once all bots
have the necessary SDK.)

[Eugene But (OOO till 7-30), 2015/09/22 22:43:04]

Instead of predeclaring I added ifdefs here, because it looks cleaner. I used
the smallest possible scope for this ifdef.

[stuartmorgan, 2015/09/24 14:01:43]

It may look cleaner, but it causes the code not to actually run on iOS 9 if
built with the iOS 8 SDK, which is not the outcome we want.

Predeclaring is our standard pattern for writing code against an SDK we don't
have completely rolled out yet (for exactly this reason).

[Eugene But (OOO till 7-30), 2015/09/24 18:50:28]

Done.

+    cert = web::CreateCertFromChain(chain);
+    if (cert) {
+      int oldCertID = SSLStatus.cert_id;
+      SSLStatus.cert_id = web::CertStore::GetInstance()->StoreCert(
+          cert.get(), self.certGroupID);
+      if (oldCertID != SSLStatus.cert_id) {
+        [self updateSSLStatusForNavigationItemsWithCertID:SSLStatus.cert_id
+                                           usingCertChain:chain];
       }
     }
-  } else {
-    SSLStatus.security_style = web::SECURITY_STYLE_UNAUTHENTICATED;
+  }
+
+  if (!cert) {
     SSLStatus.cert_id = 0;
+    if (!item->GetURL().SchemeIsCryptographic()) {
+      // HTTP or other non-secure connection.
+      SSLStatus.security_style = web::SECURITY_STYLE_UNAUTHENTICATED;
+    } else if (base::ios::IsRunningOnIOS9OrLater()) {
+      // HTTPS, iOS9 and no certificate (this use-case has not been observed).
+      // TODO(eugenebut): Add UMA action for this anomaly (crbug.com/528668).
+      SSLStatus.security_style = web::SECURITY_STYLE_UNKNOWN;
+    } else {
+      // HTTPS, iOS8.
+      // iOS8 cannot load unauthenticated HTTPS content.
+      SSLStatus.security_style = web::SECURITY_STYLE_AUTHENTICATED;
+    }
   }
 
   if (!previousSSLStatus.Equals(SSLStatus)) {
     [self didUpdateSSLStatusForCurrentNavigationItem];
   }
 }
+
 #endif  // !defined(ENABLE_CHROME_NET_STACK_FOR_WKWEBVIEW)
 
 - (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
   // by web controller or delegates.
   // TODO(stuartmorgan): Remove guesswork and replace with information from
   // decidePolicyForNavigationAction:.
   ui::PageTransition transition = self.userInteractionRegistered
                                       ? ui::PAGE_TRANSITION_LINK
@@ skipping 568 matching lines @@
                               placeholderText:defaultText
                                    requestURL:
             net::GURLWithNSURL(frame.request.URL)
                             completionHandler:completionHandler];
   } else if (completionHandler) {
     completionHandler(nil);
   }
 }
 
 @end