WKWebView: Added cert verification API to web controller.

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/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 "ios/web/net/cert_verifier_block_adapter.h"
+#include "ios/web/public/browser_state.h"
+#include "ios/web/public/web_thread.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"
+

[davidben, 2015/09/01 22:30:38]

namespace {

[Eugene But (OOO till 7-30), 2015/09/02 20:17:45]

Done.

+// This class takes ownerhip of block and releases it on UI thread, even if

[davidben, 2015/09/01 22:30:38]

Nit: ownerhip -> ownership

[Eugene But (OOO till 7-30), 2015/09/02 20:17:45]

Done.

+// |BlockHolder| is destructed on a background thread.
+template <class T>
+class BlockHolder : public base::RefCounted<BlockHolder<T>> {

[davidben, 2015/09/01 22:30:39]

RefCountedThreadSafe

[Eugene But (OOO till 7-30), 2015/09/02 20:17:45]

Done.

+ public:
+  // Takes ownership of |block|, which must not be null.
+  explicit BlockHolder(T block) : block_([block copy]) { DCHECK(block_); }

[davidben, 2015/09/01 22:30:39]

I probably wouldn't bother templating this since it isn't used outside the one
type.

[Eugene But (OOO till 7-30), 2015/09/02 20:17:45]

I will use BlockHolder for another public method of
CRWCertVerificationController, that's why I need template.
I will refactor this code to remove BlockHolder and CertVerifierBlockAdapter
after I have CLs for SSL Lock Coloring and Recoverable Interstitials.

+
+  // Calls underlying block with the given variadic arguments.
+  template <typename... Arguments>

[davidben, 2015/09/01 22:30:39]

Ditto.

[Eugene But (OOO till 7-30), 2015/09/02 20:17:45]

Acknowledged.

+  void call(Arguments... Args) {

[davidben, 2015/09/01 22:30:38]

Style: Call?

[Eugene But (OOO till 7-30), 2015/09/02 20:17:45]

Both |Call| and |call| are correct names according to Google C++ Style guide:

"You may also use lowercase letters for other very short inlined functions."
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Names

+    block_(Args...);
+  }
+
+ private:
+  BlockHolder() = delete;
+  friend class base::RefCounted<BlockHolder>;
+
+  // Releases the given block, must be called on UI thread.
+  static void ReleaseBlock(id block) {
+    DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI);
+    [block release];
+  }
+
+  // Releases underlying |block_| on UI thread.
+  ~BlockHolder() {
+    if (web::WebThread::CurrentlyOn(web::WebThread::UI)) {
+      ReleaseBlock(block_);
+    } else {
+      web::WebThread::PostTask(web::WebThread::UI, FROM_HERE,
+                               base::Bind(&BlockHolder::ReleaseBlock, block_));
+    }
+  }
+
+  T block_;

[davidben, 2015/09/01 22:30:38]

I assume automatic reference-counting doesn't kick in here? I'm really not clear
on how ARC works. (Or do we not use ARC? I sort of figured we did given blocks
capturing stuff, but maybe that's orthogonal? I really don't know anything about
ObjC. :-) )

[Eugene But (OOO till 7-30), 2015/09/02 20:17:45]

Fortunately for this CL we don't support ARC :)

+};

[davidben, 2015/09/01 22:30:38]

}  // namespace

[Eugene But (OOO till 7-30), 2015/09/02 20:17:45]

Done.

+
+@interface CRWCertVerificationController () {
+  // Cert verification object which wraps |net::CertVerifier|. Must be created,
+  // used and destroyed on IO Thread.
+  scoped_ptr<web::CertVerifierBlockAdapter> _certVerifier;
+
+  // 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.
+- (void)verifyCert:(const scoped_refptr<net::X509Certificate>&)cert
+              forHost:(NSString*)host
+    completionHandler:(void (^)(net::CertVerifyResult, int))completionHandler;
+
+@end
+
+@implementation CRWCertVerificationController
+
+#pragma mark - Superclass
+
+- (void)dealloc {
+  DCHECK(!_certVerifier);  // This is not a thread safe check.

[davidben, 2015/09/01 22:30:39]

What's this comment for?

[Eugene But (OOO till 7-30), 2015/09/02 20:17:45]

Removed.

+  [super dealloc];
+}
+
+#pragma mark - Public
+
+- (instancetype)initWithBrowserState:(web::BrowserState*)browserState {
+  DCHECK(browserState);
+  DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI);
+  self = [super init];
+  if (self) {
+    _contextGetter = browserState->GetRequestContext();
+    DCHECK(_contextGetter);
+    [self createCertVerifier];
+  }
+  return self;
+}
+
+- (void)decidePolicyForCert:(const scoped_refptr<net::X509Certificate>&)cert
+                       host:(NSString*)host
+          completionHandler:(void (^)(web::CertAcceptPolicy))handler {
+  DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI);
+  // completionHandler of |verifyCert:forHost:completionHandler:| is called on
+  // IO thread and then bounses back to UI thread. As a result all objects

[davidben, 2015/09/01 22:30:39]

Nit: bounces

[Eugene But (OOO till 7-30), 2015/09/02 20:17:45]

Done.

+  // captured by completionHandler may be realeases on either UI or IO thread.

[davidben, 2015/09/01 22:30:38]

Nit: released

[Eugene But (OOO till 7-30), 2015/09/02 20:17:45]

Done.

+  // 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.
+  __block scoped_refptr<BlockHolder<void (^)(web::CertAcceptPolicy)>>
+      handlerHolder(new BlockHolder<void (^)(web::CertAcceptPolicy)>(handler));
+  [self verifyCert:cert
+                forHost:host
+      completionHandler:^(net::CertVerifyResult result, int error) {
+        web::CertAcceptPolicy policy = web::CERT_ACCEPT_POLICY_ERROR;
+        if (error == net::OK) {
+          policy = web::CERT_ACCEPT_POLICY_ALLOW;
+        } else if (net::IsCertStatusError(result.cert_status)) {
+          // TODO(eugenebut): Check CertPolicyCache for user's decision.
+          policy = net::IsCertStatusMinorError(result.cert_status)
+                       ? web::CERT_ACCEPT_POLICY_ALLOW
+                       : web::CERT_ACCEPT_POLICY_DENY;
+        }
+
+        dispatch_async(dispatch_get_main_queue(), ^{
+          handlerHolder->call(policy);
+        });
+      }];
+}
+
+- (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 possible causing
+    // dealloc to happen on IO thread (which is fine for this class).
+    _certVerifier.reset();
+  }));
+}
+
+#pragma mark - Private
+
+- (int)certVerifyFlags {
+  DCHECK(web::WebThread::CurrentlyOn(web::WebThread::IO));
+  DCHECK(_contextGetter);
+  // |net::URLRequestContextGetter| lifetime is expected to be at least the same
+  // or longer than |BrowserState| lifetime.
+  net::URLRequestContext* context = _contextGetter->GetURLRequestContext();
+  DCHECK(context);
+  net::SSLConfigService* SSLConfigService = context->ssl_config_service();

[davidben, 2015/09/01 22:30:38]

Style: ssl_config_service

[Eugene But (OOO till 7-30), 2015/09/02 20:17:45]

This is Objective-C method and for that reason should follow Objective-C style
guide.
And in Objective-C style guide all acronyms are all caps :)

[davidben, 2015/09/03 18:34:27]

Oh, I see. This is camelCase + first word happens to be an acronym. I would have
thought the "first word is lowercase" rule trumps the "acronyms are uppercase"
rule. (In which case, it'd be sslConfigService.) But I don't know ObjC style, so
I'll defer to you and Stuart here.

[Eugene But (OOO till 7-30), 2015/09/03 18:59:05]

Yes, this is not something that is very intuitive. And rationale for starting
variable names with all caps is the following:

1.) In method/property name acronyms must always be all caps (Apple frameworks
are very consistent with that).
2.) Since property is all caps, then instance variable that backs that property
up should be _SSLConfigService (or SSLConfigService). Otherwise Key Value Coding
technology will not work.
3.) Finally local variables should have the same style as ivars so they may be
started with all caps.

In Chromium code you will probably see sslConfigService more often than
SSLConfigService.

+  DCHECK(SSLConfigService);
+  net::SSLConfig config;
+  SSLConfigService->GetSSLConfig(&config);
+  return config.GetCertVerifyFlags();
+}
+
+- (void)createCertVerifier {
+  web::WebThread::PostTask(web::WebThread::IO, FROM_HERE, base::BindBlock(^{
+    net::URLRequestContext* context = _contextGetter->GetURLRequestContext();
+    _certVerifier.reset(new web::CertVerifierBlockAdapter(
+        context->cert_verifier(), context->net_log()));
+  }));
+}
+
+- (void)verifyCert:(const scoped_refptr<net::X509Certificate>&)cert
+              forHost:(NSString*)host
+    completionHandler:(void (^)(net::CertVerifyResult, int))completionHandler {
+  DCHECK(completionHandler);
+  __block scoped_refptr<net::X509Certificate> blockCert = cert;
+  web::WebThread::PostTask(
+      web::WebThread::IO, FROM_HERE, base::BindBlock(^{
+        // WeakNSObject does not work across different threads, hence this block
+        // retains self.
+        if (!_certVerifier) {
+          completionHandler(net::CertVerifyResult(), net::ERR_FAILED);
+          return;
+        }
+
+        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);
+      }));
+}
+
+@end