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

ios/web/net/crw_cert_verification_controller.mm

 // Copyright 2015 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/net/crw_cert_verification_controller.h"
 
 #include "base/logging.h"
 #include "base/mac/bind_objc_block.h"
 #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"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace {
 
 // This class takes ownership of block and releases it on UI thread, even if
 // |BlockHolder| is destructed on a background thread.
 template <class T>
@@ skipping 41 matching lines @@
   // URLRequestContextGetter for obtaining net layer objects.
   net::URLRequestContextGetter* _contextGetter;
 }
 
 // Cert verification flags. Must be used on IO Thread.
 @property(nonatomic, readonly) int certVerifyFlags;
 
 // 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. |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 of worker task can't start.

[davidben, 2015/10/05 22:19:11]

of -> if the

[davidben, 2015/10/05 22:19:11]

As with the previous CL, the thread-hopping here gets really confusing. I think
it would be clearer if you kept the core logic on one thread. (Note that now
verifyCert must be callable on two different threads.)

You can make verifyCert's callback internally post back to the main thread
without changing anything since you're always posting back to the main thread
right now. Likewise, verifyTrust's ought to as well. It is, strictly speaking,
one extra thread hop, but only on invalid certificates and the result is the
code is much easier to reason about.

[Eugene But (OOO till 7-30), 2015/10/06 03:10:09]

verifyCert: and verifyTrust: will be called for making load/no-load decision for
bad SSL certs. I do understand that it's not a general case, but I still want to
avoid bouncing between threads for performance reasons.

For load-no load decision I can avoid 2 extra hops (marked with asterisk):
UI (called on UI) -> Worker (SecTrust) -> *UI* -> IO (CertVerifier) -> *UI* ->
IO (UserDecisionPolicy) -> UI (reply callback)

[Eugene But (OOO till 7-30), 2015/10/06 03:10:09]

Done.

+- (void)verifyTrust:(base::ScopedCFTypeRef<SecTrustRef>)trust
+    completionHandler:(void (^)(SecTrustResultType))completionHandler;
+
 @end
 
 @implementation CRWCertVerificationController
 
 #pragma mark - Superclass
 
 - (void)dealloc {
   DCHECK(!_certVerifier);
   [super dealloc];
 }
@@ skipping 12 matching lines @@
   if (self) {
     _contextGetter = browserState->GetRequestContext();
     DCHECK(_contextGetter);
     [self createCertVerifier];
   }
   return self;
 }
 
 - (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) {
         web::CertAcceptPolicy policy =
             web::CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR;
         if (error == net::OK) {
           policy = web::CERT_ACCEPT_POLICY_ALLOW;
         } else if (net::IsCertStatusError(result.cert_status)) {
           policy = net::IsCertStatusMinorError(result.cert_status)
                        ? web::CERT_ACCEPT_POLICY_ALLOW
                        : web::CERT_ACCEPT_POLICY_RECOVERABLE_ERROR;
         }
 
         dispatch_async(dispatch_get_main_queue(), ^{
           handlerHolder->call(policy, result.cert_status);
         });
       }];
 }
 
+- (void)querySSLStatusForTrust:(base::ScopedCFTypeRef<SecTrustRef>)trust
+                          host:(NSString*)host
+             completionHandler:(web::StatusQueryHandler)completionHandler {
+  DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI);
+
+  // Completion handler of |verifyCert:forHost:completionHandler:| will be

[davidben, 2015/10/05 22:19:10]

verifyCert:forHost -> verifyTrust?

[davidben, 2015/10/05 22:19:11]

Nit: Completion -> The completion

[Eugene But (OOO till 7-30), 2015/10/06 03:10:09]

Updated comments.

[Eugene But (OOO till 7-30), 2015/10/06 03:10:09]

Done.

+  // deallocated on IO 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));
+  scoped_refptr<net::X509Certificate> cert(web::CreateCertFromTrust(trust));
+  [self verifyTrust:trust
+      completionHandler:^(SecTrustResultType trustResult) {
+        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.
+        // TODO(eugenebut): Add UMA for CertVerifier and SecTrust verification
+        // mismatch (crbug.com/535699).
+        [self verifyCert:cert
+                      forHost:host

[davidben, 2015/10/05 22:19:11]

This will end up calling verifyCert if posting to the WorkerPool failed, but
that doesn't make a whole lot of sense since the certificate itself wasn't
invalid. There was just an internal error.

[Eugene But (OOO till 7-30), 2015/10/06 03:10:09]

Good point. Fixed.

+            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(^{
     // This block captures |self| delaying its deallocation and causing dealloc
     // to happen on either IO or UI thread (which is fine for this class).
     _certVerifier.reset();
   }));
 }
 
 #pragma mark - Private
@@ skipping 36 matching lines @@
 
         web::CertVerifierBlockAdapter::Params params(
             blockCert.Pass(), base::SysNSStringToUTF8(host));
         params.flags = self.certVerifyFlags;
         params.crl_set = net::SSLConfigService::GetCRLSet();
         // OCSP response is not provided by iOS API.
         _certVerifier->Verify(params, completionHandler);
       }));
 }
 
+- (void)verifyTrust:(base::ScopedCFTypeRef<SecTrustRef>)trust
+    completionHandler:(void (^)(SecTrustResultType))completionHandler {
+  DCHECK(completionHandler);
+  // SecTrustEvaluate performs trust evaluation synchronously, possibly making
+  // network requests. IO thread should not be blocked by that operation.

[davidben, 2015/10/05 22:19:11]

Nit: The IO thread

[Eugene But (OOO till 7-30), 2015/10/06 03:10:09]

Done.

+  bool result = base::WorkerPool::PostTask(FROM_HERE, base::BindBlock(^{
+    SecTrustResultType trustResult = kSecTrustResultInvalid;
+    if (SecTrustEvaluate(trust.get(), &trustResult) != errSecSuccess) {
+      trustResult = kSecTrustResultInvalid;
+    }
+    completionHandler(trustResult);
+  }), false /* task_is_slow */);
+
+  if (!result) {
+    completionHandler(kSecTrustResultInvalid);
+  }
+}
+
 @end