WKWebView: Implemented recoverable SSL interstitials.

ios/web/net/crw_cert_verification_controller.h

Index: ios/web/net/crw_cert_verification_controller.h
diff --git a/ios/web/net/crw_cert_verification_controller.h b/ios/web/net/crw_cert_verification_controller.h
index 1f3b20a5a317a1a2056d05f8edc7fccda4b5aecf..08904dbf28c9b6ca1f0d0ceee6d36fee4a9d0c52 100644
--- a/ios/web/net/crw_cert_verification_controller.h
+++ b/ios/web/net/crw_cert_verification_controller.h
@@ -26,12 +26,15 @@ typedef NS_ENUM(NSInteger, CertAcceptPolicy) {
   CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR = 0,
   // Cert is not valid. Caller may present SSL warning and ask user if they
   // want to proceed with the load.
-  CERT_ACCEPT_POLICY_RECOVERABLE_ERROR,
+  CERT_ACCEPT_POLICY_RECOVERABLE_ERROR_NOT_ACCEPTED_BY_USER,
+  // Cert is not valid. However caller should proceed with the load, because
+  // user has decided to proceed with this invalid cert.

[Ryan Sleevi, 2015/09/24 22:48:39]

// Cert is not valid. However, caller should proceed with the load, because ...

(extra ,)

[Eugene But (OOO till 7-30), 2015/09/25 21:24:23]

Done.

+  CERT_ACCEPT_POLICY_RECOVERABLE_ERROR_ACCEPTED_BY_USER,
   // Cert is valid. Caller should proceed with the load.
   CERT_ACCEPT_POLICY_ALLOW,
 };
 
-// Completion handler called by decidePolicyForCert:host:completionHandler:.
+// Completion handler called by decideLoadPolicyForTrust:host:completionHandler.
 typedef void (^PolicyDecisionHandler)(web::CertAcceptPolicy, net::CertStatus);
 // Completion handler called by decidePolicyForCert:host:completionHandler:.
 typedef void (^StatusQueryHandler)(web::SecurityStyle, net::CertStatus);
@@ -50,17 +53,14 @@ typedef void (^StatusQueryHandler)(web::SecurityStyle, net::CertStatus);
 - (instancetype)initWithBrowserState:(web::BrowserState*)browserState
     NS_DESIGNATED_INITIALIZER;
 
-// TODO(eugenebut): add API for:
-// - accepting bad SSL cert using CertPolicyCache
-
-// Decides the policy for the given |cert| for the given |host| and calls
-// |completionHandler| on completion. |host| should be in DNS form
+// Decides the policy for the given |serverTrust| and the given |host| and calls
+// |completionHandler| on completion.  |host| should be in DNS form
 // (f.e. for "http://名がドメイン.com", it should be "xn--v8jxj3d1dzdz08w.com").
 // |completionHandler| cannot be null and will be called synchronously or
 // asynchronously on the UI thread.
-- (void)decidePolicyForCert:(const scoped_refptr<net::X509Certificate>&)cert
-                       host:(NSString*)host
-          completionHandler:(web::PolicyDecisionHandler)completionHandler;
+- (void)decideLoadPolicyForTrust:(SecTrustRef)serverTrust
+                            host:(NSString*)host
+               completionHandler:(web::PolicyDecisionHandler)completionHandler;
 
 // Asynchronously returns web::SecurityStyle and net::CertStatus for the given
 // |certificateChain| (an NSArray of SecSertificateRef objects) and |host|.
@@ -69,6 +69,13 @@ typedef void (^StatusQueryHandler)(web::SecurityStyle, net::CertStatus);
                               host:(NSString*)host
                  completionHandler:(web::StatusQueryHandler)completionHandler;
 
+// Records that |leafCert| is permitted to be used for |host| in the future.
+// |host| should be in DNS form. |leafCert| must not contain any intermidiate

[Ryan Sleevi, 2015/09/24 22:48:39]

It's unclear why "|leafCert| must not contain any intermediate certs" - seems
like an unnecessary API constraint?

[Ryan Sleevi, 2015/09/24 22:48:39]

same remarks re: DNS form

[Ryan Sleevi, 2015/09/24 22:48:39]

spelling: intermediate

[Eugene But (OOO till 7-30), 2015/09/25 21:24:23]

Done.

[Eugene But (OOO till 7-30), 2015/09/25 21:24:23]

didFailProvisionalNavigation: callback, where decision is stored may not provide
complete/valid cert chain. So per you suggestion CertPolicy::Judgment decision
will be stored and read for leaf cert and hostname.

This very API constraint is necessary. Otherwise reading CertPolicy::Judgment
decision will not work:
https://codereview.chromium.org/1357773002/diff/40001/ios/web/net/crw_cert_ve...

Does it make sense?

[Eugene But (OOO till 7-30), 2015/09/25 21:24:23]

Done.

[Ryan Sleevi, 2015/09/28 22:46:52]

No. It's not clear to me why you impose this constraint on your callers, rather
than 'fixing up' incoming certs to normalize the CertPolicy::Judgement (e.g.
stripping off intermediate certificates yourself)

That is, I find it weird to force callers to be aware of these semantics, a
result of the inconsistency between the two APIs you mentioned
(didFailProvisionalNavigation vs interstitials), rather than just fixing it
yourself and leaving it as an implementation detail as to why. My understanding
is that this method (allowCert) is tightly-coupled to the implementation of
decideLoadPolicyForTrust (which I assume is what didFail is calling), but you're
forcing callers to be aware of that coupling rather than handling it yourself
(e.g. having both allowCert and decideLoadPolicy only consider/store the leaf).
By handling it yourself, you provide a strong API contract to callers (these two
methods are complementary), without making them worry about the details.

[Eugene But (OOO till 7-30), 2015/09/29 18:29:07]

I see your point. Changed allowCert: to strip intermediate certs.

+// certs.
+- (void)allowCert:(scoped_refptr<net::X509Certificate>)leafCert
+          forHost:(NSString*)host
+           status:(net::CertStatus)status;
+
 // Cancels all pending verification requests. Completion handlers will not be
 // called after |shutDown| call. Must always be called before object's
 // deallocation.