Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(18)

Unified Diff: ios/web/web_state/ui/crw_wk_web_view_web_controller.mm

Issue 1322193003: WKWebView(iOS9): correctly update SSL status for current navigation item (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@reland_cert_verification
Patch Set: Predeclare -certificateChain. Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 368587eab562249bab292466ff34a608391942d3..487f50b2c90fa2013c42cc60e4f5a37bcce5feeb 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
@@ -82,6 +82,14 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
} // namespace
+#if !defined(__IPHONE_9_0)
+// Predeclare iOS9 API so calls are compiled on iOS8 bots.
+// TODO(eugenebut): cleanup this (crbug.com/523365).
+@interface WKWebView (CRWIOS9API)
+@property(nonatomic, readonly, copy) NSArray* certificateChain;
+@end
+#endif
+
@interface CRWWKWebViewWebController () <WKNavigationDelegate,
WKScriptMessageHandler,
WKUIDelegate> {
@@ -250,6 +258,11 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
// Clears all activity indicator tasks for this web controller.
- (void)clearActivityIndicatorTasks;
+// Obtains SSL status from given |certChain| and updates navigation items 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.
@@ -864,12 +877,51 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
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) {
Ryan Sleevi 2015/09/24 22:25:32 BUG? Not just the same cert - the same cert & host
Eugene But (OOO till 7-30) 2015/09/25 00:28:10 Yes, this was a bug. Fixed.
+ 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)
@@ -877,34 +929,52 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
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 certificateChain];
+ cert = web::CreateCertFromChain(chain);
+ if (cert) {
+ int oldCertID = SSLStatus.cert_id;
Ryan Sleevi 2015/09/24 22:25:32 Same remarks re: hostname
Eugene But (OOO till 7-30) 2015/09/25 00:28:10 This can also be a problem in case of redirect. Fi
+ 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 {

Powered by Google App Engine
This is Rietveld 408576698