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

Unified Diff: ios/web/net/crw_cert_verification_controller.mm

Issue 1357773002: WKWebView: Implemented recoverable SSL interstitials. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@lock_coloring
Patch Set: Resolved Stuart's review comments 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/net/crw_cert_verification_controller.mm
diff --git a/ios/web/net/crw_cert_verification_controller.mm b/ios/web/net/crw_cert_verification_controller.mm
index a2ac38e4ee041708f60d2fd585574a1a700946d0..557626a6595923ba446ea825c7a6d148cdcc830b 100644
--- a/ios/web/net/crw_cert_verification_controller.mm
+++ b/ios/web/net/crw_cert_verification_controller.mm
@@ -12,6 +12,7 @@
#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/certificate_policy_cache.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"
@@ -60,6 +61,9 @@ class BlockHolder : public base::RefCountedThreadSafe<BlockHolder<T>> {
} // namespace
+using web::CERT_ACCEPT_POLICY_RECOVERABLE_ERROR_ACCEPTED_BY_USER;
+using web::CERT_ACCEPT_POLICY_RECOVERABLE_ERROR_NOT_ACCEPTED_BY_USER;
+
@interface CRWCertVerificationController () {
// Cert verification object which wraps |net::CertVerifier|. Must be created,
// used and destroyed on IO Thread.
@@ -67,6 +71,9 @@ class BlockHolder : public base::RefCountedThreadSafe<BlockHolder<T>> {
// URLRequestContextGetter for obtaining net layer objects.
net::URLRequestContextGetter* _contextGetter;
+
+ // Used to remember decisions about how to handle problematic certs.
+ scoped_refptr<web::CertificatePolicyCache> _certPolicyCache;
}
// Cert verification flags. Must be used on IO Thread.
@@ -113,14 +120,17 @@ class BlockHolder : public base::RefCountedThreadSafe<BlockHolder<T>> {
if (self) {
_contextGetter = browserState->GetRequestContext();
DCHECK(_contextGetter);
+ _certPolicyCache =
+ web::BrowserState::GetCertificatePolicyCache(browserState);
+
[self createCertVerifier];
}
return self;
}
-- (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 {
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
@@ -130,23 +140,52 @@ class BlockHolder : public base::RefCountedThreadSafe<BlockHolder<T>> {
// released on background thread and |BlockHolder| ensures that.
__block scoped_refptr<BlockHolder<web::PolicyDecisionHandler>> handlerHolder(
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;
- }
+ web::WebThread::PostTask(web::WebThread::IO, FROM_HERE, base::BindBlock(^{
+ base::ScopedCFTypeRef<SecTrustRef> trust(
+ serverTrust, base::scoped_policy::RETAIN);
+ [self verifyTrust:trust completionHandler:^(SecTrustResultType result) {
+ if (result == kSecTrustResultProceed ||
+ result == kSecTrustResultUnspecified) {
+ // SecTrust API considers this cert as valid.
dispatch_async(dispatch_get_main_queue(), ^{
- handlerHolder->call(policy, result.cert_status);
+ handlerHolder->call(web::CERT_ACCEPT_POLICY_ALLOW, net::CertStatus());
});
- }];
+ return;
+ }
+
+ // SecTrust API considers this cert as invalid. Check the reason and
+ // whether or not user has decided to proceed with this bad cert.
+ scoped_refptr<net::X509Certificate> cert(web::CreateCertFromTrust(trust));
+ [self verifyCert:cert
+ forHost:host
+ completionHandler:^(net::CertVerifyResult certVerifierResult, int) {
+ web::CertAcceptPolicy policy =
+ web::CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR;
+ scoped_refptr<net::X509Certificate> leafCert;
+ if (result == kSecTrustResultRecoverableTrustFailure &&
+ SecTrustGetCertificateCount(trust)) {
+ // Check if user has decided to proceed with this bad cert.
+ leafCert = web::CreateCertFromChain(@[
+ static_cast<id>(SecTrustGetCertificateAtIndex(trust, 0))
+ ]);
+
+ web::CertPolicy::Judgment judgment =
+ _certPolicyCache->QueryPolicy(
+ leafCert.get(), base::SysNSStringToUTF8(host),
+ certVerifierResult.cert_status);
+
+ policy = (judgment == web::CertPolicy::ALLOWED)
+ ? CERT_ACCEPT_POLICY_RECOVERABLE_ERROR_ACCEPTED_BY_USER
+ : CERT_ACCEPT_POLICY_RECOVERABLE_ERROR_NOT_ACCEPTED_BY_USER;
+ }
+
+ dispatch_async(dispatch_get_main_queue(), ^{
+ handlerHolder->call(policy, certVerifierResult.cert_status);
+ });
+ }];
+ }];
+ }));
}
- (void)querySSLStatusForCertChain:(NSArray*)certChain
@@ -160,36 +199,53 @@ class BlockHolder : public base::RefCountedThreadSafe<BlockHolder<T>> {
// 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::CreateCertFromChain(certChain));
- // Retrieve the net::CertStatus to allow Chromium-specific policies to be
- // enacted. This is called for all certificates (valid and invalid alike),
- // since the net::CertVerifier may reject or downgrade certificates that
- // SecTrust accepts.
- [self verifyCert:cert
- forHost:host
- completionHandler:^(net::CertVerifyResult certVerifierResult, int) {
- [self verifyTrust:web::CreateServerTrustFromChain(certChain, host)
- completionHandler:^(SecTrustResultType trustResult) {
- // If SecTrust has accepted a certificate, clear any error
- // statuses returned by net::CertVerifier, since the CertVerifier
- // may not have access to the same trust information
- // (e.g. custom-installed roots). Note, this also disables
- // net::CertVerifier's ability to reject certificates, such as via
- // CRLSets. However, copy any supplemental status bits over, since
- // those may be used as part of policy logic, such as deprecating
- // SHA-1 or requiring Certificate Transparency.
- web::SecurityStyle security_style =
- web::GetSecurityStyleFromTrustResult(trustResult);
- net::CertStatus cert_status = certVerifierResult.cert_status;
- if (security_style == web::SECURITY_STYLE_AUTHENTICATED) {
- cert_status &= ~net::CERT_STATUS_ALL_ERRORS;
- }
-
- dispatch_async(dispatch_get_main_queue(), ^{
- handlerHolder->call(security_style, cert_status);
- });
- }];
- }];
+ web::WebThread::PostTask(web::WebThread::IO, FROM_HERE, base::BindBlock(^{
+ scoped_refptr<net::X509Certificate> cert(
+ web::CreateCertFromChain(certChain));
+ // Retrieve the net::CertStatus to allow Chromium-specific policies to
+ // be enacted. This is called for all certificates (valid and invalid
+ // alike), since the net::CertVerifier may reject or downgrade certificates
+ // that SecTrust accepts.
+ [self verifyCert:cert
+ forHost:host
+ completionHandler:^(net::CertVerifyResult certVerifierResult, int) {
+ [self verifyTrust:web::CreateServerTrustFromChain(certChain, host)
+ completionHandler:^(SecTrustResultType trustResult) {
+ // If SecTrust has accepted a certificate, clear any error
+ // statuses returned by net::CertVerifier, since the
+ // CertVerifier may not have access to the same trust
+ // information (e.g. custom-installed roots). Note, this also
+ // disables net::CertVerifier's ability to reject certificates,
+ // such as via CRLSets. However, copy any supplemental status
+ // bits over, since those may be used as part of policy logic,
+ // such as deprecating SHA-1 or requiring Certificate
+ // Transparency.
+ web::SecurityStyle security_style =
+ web::GetSecurityStyleFromTrustResult(trustResult);
+ net::CertStatus cert_status =
+ certVerifierResult.cert_status;
+ if (security_style == web::SECURITY_STYLE_AUTHENTICATED) {
+ cert_status &= ~net::CERT_STATUS_ALL_ERRORS;
+ }
+
+ dispatch_async(dispatch_get_main_queue(), ^{
+ handlerHolder->call(security_style, cert_status);
+ });
+ }];
+ }];
+ }));
+}
+
+- (void)allowCert:(scoped_refptr<net::X509Certificate>)cert
+ forHost:(NSString*)host
+ status:(net::CertStatus)status {
+ DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI);
+ DCHECK(cert);
+ DCHECK_EQ(0U, cert->GetIntermediateCertificates().size());
+ web::WebThread::PostTask(web::WebThread::IO, FROM_HERE, base::BindBlock(^{
+ _certPolicyCache->AllowCertForHost(
+ cert.get(), base::SysNSStringToUTF8(host), status);
+ }));
}
- (void)shutDown {
@@ -228,24 +284,19 @@ class BlockHolder : public base::RefCountedThreadSafe<BlockHolder<T>> {
- (void)verifyCert:(const scoped_refptr<net::X509Certificate>&)cert
forHost:(NSString*)host
completionHandler:(void (^)(net::CertVerifyResult, int))completionHandler {
+ DCHECK(web::WebThread::CurrentlyOn(web::WebThread::IO));
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);
- }));
+ if (!_certVerifier) {
+ completionHandler(net::CertVerifyResult(), net::ERR_FAILED);
+ return;
+ }
+
+ web::CertVerifierBlockAdapter::Params params(cert,
+ 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

Powered by Google App Engine
This is Rietveld 408576698