From bd6ed3f209bb72148118a118d9d176a1912df178 Mon Sep 17 00:00:00 2001 From: thestig Date: Thu, 3 Sep 2015 19:26:07 -0700 Subject: [PATCH] Revert of WKWebView: Added cert verification API to web controller. (patchset #26 id:490001 of https://codereview.chromium.org/1230033005/ ) Reason for revert: Compile failed ios/web/net/cert_verifier_block_adapter.cc:28:26: error: no member named 'SOURCE_IOS_WEB_VIEW_CERT_VERIFIER' in 'net::NetLog' Original issue's description: > WKWebView: Added cert verification API to web controller. > > This code is just a skeleton for verification and verification method > is not used for making security decisions or presenting security UI. > > The decision to use CertVerifier instead of iOS cert verification API > has not been made yet. But using CertVerifier is easier for > -[WKWebView certificateChain] verification, so this CL uses > CertVerifier. > > BUG=462427,462425 > > Committed: https://crrev.com/dc5f11025e1db3b7766bf5faeacc316969b519b2 > Cr-Commit-Position: refs/heads/master@{#347302} TBR=rsleevi@chromium.org,stuartmorgan@chromium.org,davidben@chromium.org,eugenebut@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=462427,462425 Review URL: https://codereview.chromium.org/1306733006 Cr-Commit-Position: refs/heads/master@{#347304} --- ios/web/ios_web.gyp | 2 - ios/web/ios_web_unittests.gyp | 2 - ios/web/net/cert_verifier_block_adapter.cc | 200 ++++++------ ios/web/net/cert_verifier_block_adapter.h | 65 ++-- .../net/cert_verifier_block_adapter_unittest.cc | 357 ++++++++++++--------- ios/web/net/crw_cert_verification_controller.h | 68 ---- ios/web/net/crw_cert_verification_controller.mm | 194 ----------- .../crw_cert_verification_controller_unittest.mm | 114 ------- ios/web/public/test/test_browser_state.cc | 5 +- .../web_state/ui/crw_wk_web_view_web_controller.mm | 39 +-- 10 files changed, 331 insertions(+), 715 deletions(-) rewrite ios/web/net/cert_verifier_block_adapter.cc (70%) rewrite ios/web/net/cert_verifier_block_adapter_unittest.cc (69%) delete mode 100644 ios/web/net/crw_cert_verification_controller.h delete mode 100644 ios/web/net/crw_cert_verification_controller.mm delete mode 100644 ios/web/net/crw_cert_verification_controller_unittest.mm diff --git a/ios/web/ios_web.gyp b/ios/web/ios_web.gyp index ff79384af177..1db4a4be1356 100644 --- a/ios/web/ios_web.gyp +++ b/ios/web/ios_web.gyp @@ -130,8 +130,6 @@ 'net/cookie_notification_bridge.mm', 'net/crw_cert_policy_cache.h', 'net/crw_cert_policy_cache.mm', - 'net/crw_cert_verification_controller.h', - 'net/crw_cert_verification_controller.mm', 'net/crw_request_tracker_delegate.h', 'net/crw_url_verifying_protocol_handler.h', 'net/crw_url_verifying_protocol_handler.mm', diff --git a/ios/web/ios_web_unittests.gyp b/ios/web/ios_web_unittests.gyp index f6df39f9cf1f..1a1b9d2dc5cf 100644 --- a/ios/web/ios_web_unittests.gyp +++ b/ios/web/ios_web_unittests.gyp @@ -40,7 +40,6 @@ 'net/clients/crw_js_injection_network_client_unittest.mm', 'net/clients/crw_passkit_network_client_unittest.mm', 'net/crw_cert_policy_cache_unittest.mm', - 'net/crw_cert_verification_controller_unittest.mm', 'net/crw_url_verifying_protocol_handler_unittest.mm', 'net/request_group_util_unittest.mm', 'net/request_tracker_impl_unittest.mm', @@ -86,7 +85,6 @@ 'action_name': 'copy_test_data', 'variables': { 'test_data_files': [ - '../../net/data/ssl/certificates/', 'test/data/chrome.html', 'test/data/testbadpass.pkpass', 'test/data/testfavicon.png', diff --git a/ios/web/net/cert_verifier_block_adapter.cc b/ios/web/net/cert_verifier_block_adapter.cc dissimilarity index 70% index 4c5b8bfbba7c..31901000e955 100644 --- a/ios/web/net/cert_verifier_block_adapter.cc +++ b/ios/web/net/cert_verifier_block_adapter.cc @@ -1,105 +1,95 @@ -// 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. - -#include "ios/web/net/cert_verifier_block_adapter.h" - -#include "base/mac/bind_objc_block.h" -#include "net/base/net_errors.h" -#include "net/cert/crl_set.h" -#include "net/cert/x509_certificate.h" -#include "net/log/net_log.h" - -namespace web { - -namespace { - -// Resource manager which keeps CertVerifyResult, X509Certificate and -// BoundNetLog alive until verification is completed. Also holds unowned pointer -// to |net::CertVerifier::Request|. -struct VerificationContext - : public base::RefCountedThreadSafe { - VerificationContext(scoped_refptr cert, - net::NetLog* net_log) - : request(nullptr), - cert(cert.Pass()), - net_log(net::BoundNetLog::Make( - net_log, - net::NetLog::SOURCE_IOS_WEB_VIEW_CERT_VERIFIER)) {} - // Unowned verification request. - net::CertVerifier::Request* request; - // The result of certificate verification. - net::CertVerifyResult result; - // Certificate being verified. - scoped_refptr cert; - // BoundNetLog required by CertVerifier. - net::BoundNetLog net_log; - - private: - friend class base::RefCountedThreadSafe; - VerificationContext() = delete; - ~VerificationContext() {} -}; -} - -CertVerifierBlockAdapter::CertVerifierBlockAdapter( - net::CertVerifier* cert_verifier, - net::NetLog* net_log) - : cert_verifier_(cert_verifier), net_log_(net_log) { - DCHECK(cert_verifier_); - DCHECK(net_log_); -} - -CertVerifierBlockAdapter::~CertVerifierBlockAdapter() { - DCHECK(thread_checker_.CalledOnValidThread()); -} - -CertVerifierBlockAdapter::Params::Params( - const scoped_refptr& cert, - const std::string& hostname) - : cert(cert), hostname(hostname), flags(0) {} - -CertVerifierBlockAdapter::Params::~Params() { -} - -void CertVerifierBlockAdapter::Verify( - const Params& params, - void (^completion_handler)(net::CertVerifyResult, int)) { - DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(completion_handler); - if (!params.cert || params.hostname.empty()) { - completion_handler(net::CertVerifyResult(), net::ERR_INVALID_ARGUMENT); - return; - } - - scoped_refptr context( - new VerificationContext(params.cert, net_log_)); - net::CompletionCallback callback = base::BindBlock(^(int error) { - // Remove pending request. - auto request_iterator = std::find( - pending_requests_.begin(), pending_requests_.end(), context->request); - DCHECK(pending_requests_.end() != request_iterator); - pending_requests_.erase(request_iterator); - - completion_handler(context->result, error); - }); - scoped_ptr request; - int error = cert_verifier_->Verify(params.cert.get(), params.hostname, - params.ocsp_response, params.flags, - params.crl_set.get(), &(context->result), - callback, &request, context->net_log); - if (error == net::ERR_IO_PENDING) { - // Make sure that |net::CertVerifier::Request| is alive until either - // verification is completed or CertVerifierBlockAdapter is destroyed. - pending_requests_.push_back(request.Pass()); - context->request = pending_requests_.back(); - // Completion handler will be called from |callback| when verification - // request is completed. - return; - } - - // Verification has either failed or result was retrieved from the cache. - completion_handler(context->result, error); -} - -} // namespace web +// 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. + +#include "ios/web/net/cert_verifier_block_adapter.h" + +#include "base/mac/bind_objc_block.h" +#include "net/base/net_errors.h" +#include "net/cert/cert_verify_result.h" +#include "net/cert/crl_set.h" +#include "net/cert/x509_certificate.h" + +namespace net { + +namespace { + +// Resource manager which keeps CertVerifier::Request, CertVerifyResult and +// X509Certificate alive until verification is completed. +struct VerificationContext : public base::RefCounted { + VerificationContext(scoped_refptr cert) : cert(cert) { + result.cert_status = CERT_STATUS_INVALID; + } + // Verification request. Must be alive until verification is completed, + // otherwise it will be cancelled. + scoped_ptr request; + // The result of certificate verification. + CertVerifyResult result; + // Certificate being verificated. + scoped_refptr cert; + + // Copies CertVerifyResult and wraps it into a scoped_ptr. + scoped_ptr scoped_result() { + scoped_ptr scoped_result(new CertVerifyResult()); + scoped_result->CopyFrom(result); + return scoped_result.Pass(); + } + + private: + VerificationContext() = delete; + // Required by base::RefCounted. + friend class base::RefCounted; + ~VerificationContext() {} +}; +} + +CertVerifierBlockAdapter::CertVerifierBlockAdapter() + : CertVerifierBlockAdapter( + scoped_ptr(CertVerifier::CreateDefault())) { +} + +CertVerifierBlockAdapter::CertVerifierBlockAdapter( + scoped_ptr cert_verifier) + : cert_verifier_(cert_verifier.Pass()) { + DCHECK(cert_verifier_); +} + +CertVerifierBlockAdapter::~CertVerifierBlockAdapter() { +} + +CertVerifierBlockAdapter::Params::Params(scoped_refptr cert, + const std::string& hostname) + : cert(cert), + hostname(hostname), + flags(static_cast(0)) { +} + +CertVerifierBlockAdapter::Params::~Params() { +} + +void CertVerifierBlockAdapter::Verify( + const Params& params, + void (^completion_handler)(scoped_ptr, int)) { + DCHECK(completion_handler); + + scoped_refptr context( + new VerificationContext(params.cert)); + CompletionCallback callback = base::BindBlock(^(int) { + completion_handler(context->scoped_result(), 0); + }); + int status = cert_verifier_->Verify(params.cert.get(), params.hostname, + params.ocsp_response, params.flags, + params.crl_set.get(), &(context->result), + callback, &(context->request), net_log_); + + if (status == ERR_IO_PENDING) { + // Completion handler will be called from |callback| when verification + // request is completed. + return; + } + + // Verification has either failed or result was retrieved from the cache. + completion_handler(status ? nullptr : context->scoped_result(), status); +} + +} // net diff --git a/ios/web/net/cert_verifier_block_adapter.h b/ios/web/net/cert_verifier_block_adapter.h index 2646f91797af..ee9829e68c4d 100644 --- a/ios/web/net/cert_verifier_block_adapter.h +++ b/ios/web/net/cert_verifier_block_adapter.h @@ -6,42 +6,34 @@ #define IOS_WEB_NET_CERT_VERIFIER_BLOCK_ADAPTER_H_ #include "base/memory/scoped_ptr.h" -#include "base/memory/scoped_vector.h" -#include "base/threading/thread_checker.h" #include "net/cert/cert_verifier.h" -#include "net/cert/cert_verify_result.h" +#include "net/log/net_log.h" namespace net { + +class CertVerifyResult; class CRLSet; -class NetLog; class X509Certificate; -} // namespace net - -namespace web { -// Provides block-based interface for |net::CertVerifier|. This class must be -// created and used on the same thread where the |net::CertVerifier| was -// created. +// Provides block-based interface for net::CertVerifier. class CertVerifierBlockAdapter { public: - // Constructs adapter with given |CertVerifier| and |NetLog|, both can not be - // null. CertVerifierBlockAdapter does NOT take ownership of |cert_verifier| - // and |net_log|. - CertVerifierBlockAdapter(net::CertVerifier* cert_verifier, - net::NetLog* net_log); + CertVerifierBlockAdapter(); + // Constructs adapter with given |CertVerifier| which can not be null. + CertVerifierBlockAdapter(scoped_ptr cert_verifier); // When the verifier is destroyed, all certificate verification requests are // canceled, and their completion handlers will not be called. ~CertVerifierBlockAdapter(); - // Encapsulates verification params. |cert| and |hostname| are mandatory, the + // Encapsulates verification parms. |cert| and |hostname| are mandatory, the // other params are optional. If either of mandatory arguments is null or // empty then verification |CompletionHandler| will be called with - // ERR_INVALID_ARGUMENT |error|. + // ERR_INVALID_ARGUMENT status. struct Params { // Constructs Params from X509 cert and hostname, which are mandatory for // verification. - Params(const scoped_refptr& cert, + Params(scoped_refptr cert, const std::string& hostname); ~Params(); @@ -54,36 +46,31 @@ class CertVerifierBlockAdapter { // If non-empty, is a stapled OCSP response to use. std::string ocsp_response; - // Bitwise OR of |net::CertVerifier::VerifyFlags|. - int flags; + // Bitwise OR of CertVerifier::VerifyFlags. + CertVerifier::VerifyFlags flags; - // An optional |net::CRLSet| structure which can be used to avoid revocation - // checks over the network. - scoped_refptr crl_set; + // An optional CRLSet structure which can be used to avoid revocation checks + // over the network. + scoped_refptr crl_set; }; - // Type of verification completion block. If cert is successfully validated - // |error| is OK, otherwise |error| is a net error code. - typedef void (^CompletionHandler)(net::CertVerifyResult result, int error); + // Type of verification completion block. On success CertVerifyResult is not + // null and status is OK, otherwise CertVerifyResult is null and status is a + // net error code. + typedef void (^CompletionHandler)(scoped_ptr, int status); // Verifies certificate with given |params|. |completion_handler| must not be - // null and can be called either synchronously (in the same runloop) or - // asynchronously. + // null and call be called either syncronously (in the same runloop) or + // asyncronously. void Verify(const Params& params, CompletionHandler completion_handler); private: - // Pending verification requests. Request must be alive until verification is - // completed, otherwise verification operation will be cancelled. - ScopedVector pending_requests_; - // Underlying unowned CertVerifier. - net::CertVerifier* cert_verifier_; - // Unowned NetLog required by CertVerifier. - net::NetLog* net_log_; - // CertVerifierBlockAdapter should be used on the same thread where it was - // created. - base::ThreadChecker thread_checker_; + // Underlying CertVerifier. + scoped_ptr cert_verifier_; + // Net Log required by CertVerifier. + BoundNetLog net_log_; }; -} // namespace web +} // net #endif // IOS_WEB_NET_CERT_VERIFIER_BLOCK_ADAPTER_H_ diff --git a/ios/web/net/cert_verifier_block_adapter_unittest.cc b/ios/web/net/cert_verifier_block_adapter_unittest.cc dissimilarity index 69% index f05acf68945f..1380045e16e7 100644 --- a/ios/web/net/cert_verifier_block_adapter_unittest.cc +++ b/ios/web/net/cert_verifier_block_adapter_unittest.cc @@ -1,151 +1,206 @@ -// 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. - -#include "ios/web/net/cert_verifier_block_adapter.h" - -#include "base/message_loop/message_loop.h" -#include "base/test/ios/wait_util.h" -#include "ios/web/public/test/test_web_thread_bundle.h" -#include "net/base/net_errors.h" -#include "net/base/test_data_directory.h" -#include "net/cert/cert_verifier.h" -#include "net/cert/cert_verify_result.h" -#include "net/cert/crl_set.h" -#include "net/cert/mock_cert_verifier.h" -#include "net/cert/x509_certificate.h" -#include "net/log/net_log.h" -#include "net/test/cert_test_util.h" -#include "testing/platform_test.h" - -namespace web { - -namespace { -// Test cert filename. -const char kCertFileName[] = "ok_cert.pem"; -// Test hostname for CertVerifier. -const char kHostName[] = "www.example.com"; - -} // namespace - -// Test fixture to test CertVerifierBlockAdapter class. -class CertVerifierBlockAdapterTest : public PlatformTest { - protected: - void SetUp() override { - PlatformTest::SetUp(); - cert_ = - net::ImportCertFromFile(net::GetTestCertsDirectory(), kCertFileName); - ASSERT_TRUE(cert_); - } - - // Performs synchronous verification. - void Verify(CertVerifierBlockAdapter* cert_verifier_adapter, - CertVerifierBlockAdapter::Params params, - net::CertVerifyResult* result, - int* error) { - __block bool verification_completed = false; - cert_verifier_adapter->Verify( - params, ^(net::CertVerifyResult callback_result, int callback_error) { - *result = callback_result; - *error = callback_error; - verification_completed = true; - }); - base::test::ios::WaitUntilCondition(^{ - return verification_completed; - }, base::MessageLoop::current(), base::TimeDelta()); - } - - web::TestWebThreadBundle thread_bundle_; - scoped_refptr cert_; - net::NetLog net_log_; -}; - -// Tests |Verify| with default params and synchronous verification. -TEST_F(CertVerifierBlockAdapterTest, DefaultParamsAndSync) { - // Set up verifier mock. - net::MockCertVerifier verifier; - CertVerifierBlockAdapter test_adapter(&verifier, &net_log_); - const int kExpectedError = net::ERR_CERT_AUTHORITY_INVALID; - net::CertVerifyResult expected_result; - expected_result.cert_status = net::CERT_STATUS_AUTHORITY_INVALID; - expected_result.verified_cert = cert_; - verifier.AddResultForCertAndHost(cert_.get(), kHostName, expected_result, - kExpectedError); - - // Call |Verify|. - net::CertVerifyResult actual_result; - int actual_error = -1; - CertVerifierBlockAdapter::Params params(cert_.get(), kHostName); - Verify(&test_adapter, params, &actual_result, &actual_error); - - // Ensure that Verification results are correct. - EXPECT_EQ(kExpectedError, actual_error); - EXPECT_EQ(expected_result.cert_status, actual_result.cert_status); -} - -// Tests |Verify| with default params and asynchronous verification using real -// net::CertVerifier and ok_cert.pem cert. -TEST_F(CertVerifierBlockAdapterTest, DefaultParamsAndAsync) { - // Call |Verify|. - scoped_ptr verifier(net::CertVerifier::CreateDefault()); - CertVerifierBlockAdapter test_adapter(verifier.get(), &net_log_); - CertVerifierBlockAdapter::Params params(cert_.get(), kHostName); - net::CertVerifyResult actual_result; - int actual_error = -1; - Verify(&test_adapter, params, &actual_result, &actual_error); - - // Ensure that Verification results are correct. - EXPECT_FALSE(actual_result.is_issued_by_known_root); - EXPECT_EQ(net::ERR_CERT_AUTHORITY_INVALID, actual_error); -} - -// Tests |Verify| with invalid cert argument. -TEST_F(CertVerifierBlockAdapterTest, InvalidCert) { - // Call |Verify|. - net::MockCertVerifier verifier; - CertVerifierBlockAdapter test_adapter(&verifier, &net_log_); - net::CertVerifyResult actual_result; - int actual_error = -1; - CertVerifierBlockAdapter::Params params(nullptr, kHostName); - Verify(&test_adapter, params, &actual_result, &actual_error); - - // Ensure that Verification results are correct. - EXPECT_EQ(net::ERR_INVALID_ARGUMENT, actual_error); -} - -// Tests |Verify| with invalid hostname argument. -TEST_F(CertVerifierBlockAdapterTest, InvalidHostname) { - // Call |Verify|. - net::MockCertVerifier verifier; - CertVerifierBlockAdapter test_adapter(&verifier, &net_log_); - net::CertVerifyResult actual_result; - int actual_error = -1; - CertVerifierBlockAdapter::Params params(cert_.get(), std::string()); - Verify(&test_adapter, params, &actual_result, &actual_error); - - // Ensure that Verification results are correct. - EXPECT_EQ(net::ERR_INVALID_ARGUMENT, actual_error); -} - -// Tests |Verify| with synchronous error. -TEST_F(CertVerifierBlockAdapterTest, DefaultParamsAndSyncError) { - // Set up expectation. - net::MockCertVerifier verifier; - CertVerifierBlockAdapter test_adapter(&verifier, &net_log_); - const int kExpectedError = net::ERR_INSUFFICIENT_RESOURCES; - net::CertVerifyResult expected_result; - expected_result.verified_cert = cert_; - verifier.AddResultForCertAndHost(cert_.get(), kHostName, expected_result, - kExpectedError); - - // Call |Verify|. - net::CertVerifyResult actual_result; - int actual_error = -1; - CertVerifierBlockAdapter::Params params(cert_.get(), kHostName); - Verify(&test_adapter, params, &actual_result, &actual_error); - - // Ensure that Verification results are correct. - EXPECT_EQ(kExpectedError, actual_error); -} - -} // namespace web +// 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. + +#include "ios/web/net/cert_verifier_block_adapter.h" + +#include "base/test/ios/wait_util.h" +#include "net/base/net_errors.h" +#include "net/cert/cert_verify_result.h" +#include "net/cert/crl_set.h" +#include "net/cert/x509_certificate.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/platform_test.h" + +namespace net { + +using testing::_; + +namespace { + +// Test hostname for CertVerifier. +const char kHostName[] = "chromium.org"; +// Test OCSP response for CertVerifier. +const char kOcspResponse[] = "ocsp"; + +// Mocks CertVerifier for CertVerifierBlockAdapter testing. +class CertVerifierMock : public CertVerifier { + public: + MOCK_METHOD9(Verify, + int(X509Certificate* cert, + const std::string& hostname, + const std::string& ocsp_response, + int flags, + CRLSet* crl_set, + CertVerifyResult* verify_result, + const CompletionCallback& callback, + scoped_ptr* out_req, + const BoundNetLog& net_log)); +}; + +// Sets CertVerifyResult to emulate CertVerifier behavior. +ACTION_P(SetVerifyResult, result) { + *arg5 = result; +} + +// Calls CompletionCallback to emulate CertVerifier behavior. +ACTION(RunCallback) { + arg6.Run(0); +} + +} // namespace + +// Test fixture to test CertVerifierBlockAdapter class. +class CertVerifierBlockAdapterTest : public PlatformTest { + protected: + void SetUp() override { + PlatformTest::SetUp(); + + cert_ = new X509Certificate("test", "test", base::Time(), base::Time()); + scoped_ptr cert_verifier_mock(new CertVerifierMock()); + cert_verifier_mock_ = cert_verifier_mock.get(); + test_adapter_.reset( + new CertVerifierBlockAdapter(cert_verifier_mock.Pass())); + } + + // Performs synchronous verification. + void Verify(CertVerifierBlockAdapter::Params params, + scoped_ptr* result, + int* status) { + __block bool verification_completed = false; + test_adapter_->Verify(params, + ^(scoped_ptr callback_result, + int callback_status) { + *result = callback_result.Pass(); + *status = callback_status; + verification_completed = true; + }); + base::test::ios::WaitUntilCondition(^{ + return verification_completed; + }); + } + + // Fake certificate created for testing. + scoped_refptr cert_; + // Testable |CertVerifierBlockAdapter| object. + scoped_ptr test_adapter_; + // CertVerifier mock owned by |test_adapter_|. + CertVerifierMock* cert_verifier_mock_; +}; + +// Tests |Verify| with default params and synchronous verification. +TEST_F(CertVerifierBlockAdapterTest, DefaultParamsAndSync) { + // Set up expectation. + net::CertVerifyResult expectedResult; + expectedResult.cert_status = net::CERT_STATUS_AUTHORITY_INVALID; + const int kExpectedStatus = 0; + EXPECT_CALL(*cert_verifier_mock_, + Verify(cert_.get(), kHostName, "", 0, nullptr, _, _, _, _)) + .Times(1) + .WillOnce(testing::DoAll(SetVerifyResult(expectedResult), + testing::Return(kExpectedStatus))); + + // Call |Verify|. + scoped_ptr actualResult; + int actualStatus = -1; + CertVerifierBlockAdapter::Params params(cert_.get(), kHostName); + Verify(params, &actualResult, &actualStatus); + + // Ensure that Verification results are correct. + EXPECT_EQ(kExpectedStatus, actualStatus); + EXPECT_EQ(expectedResult.cert_status, actualResult->cert_status); +} + +// Tests |Verify| with default params and asynchronous verification. +TEST_F(CertVerifierBlockAdapterTest, DefaultParamsAndAsync) { + // Set up expectation. + net::CertVerifyResult expectedResult; + expectedResult.is_issued_by_known_root = true; + const int kExpectedStatus = 0; + EXPECT_CALL(*cert_verifier_mock_, + Verify(cert_.get(), kHostName, "", 0, nullptr, _, _, _, _)) + .Times(1) + .WillOnce(testing::DoAll(SetVerifyResult(expectedResult), RunCallback(), + testing::Return(ERR_IO_PENDING))); + + // Call |Verify|. + scoped_ptr actualResult; + int actualStatus = -1; + CertVerifierBlockAdapter::Params params(cert_.get(), kHostName); + Verify(params, &actualResult, &actualStatus); + + // Ensure that Verification results are correct. + EXPECT_EQ(kExpectedStatus, actualStatus); + EXPECT_EQ(expectedResult.is_issued_by_known_root, + actualResult->is_issued_by_known_root); +} + +// Tests |Verify| with invalid arguments. +TEST_F(CertVerifierBlockAdapterTest, InvalidParamsAndError) { + // Set up expectation. + const int kExpectedStatus = ERR_INVALID_ARGUMENT; + EXPECT_CALL(*cert_verifier_mock_, + Verify(nullptr, "", "", 0, nullptr, _, _, _, _)) + .Times(1) + .WillOnce(testing::Return(kExpectedStatus)); + + // Call |Verify|. + scoped_ptr actualResult; + int actualStatus = -1; + CertVerifierBlockAdapter::Params params(nullptr, ""); + Verify(params, &actualResult, &actualStatus); + + // Ensure that Verification results are correct. + EXPECT_EQ(kExpectedStatus, actualStatus); + EXPECT_FALSE(actualResult); +} + +// Tests |Verify| with error. +TEST_F(CertVerifierBlockAdapterTest, DefaultParamsAndError) { + // Set up expectation. + const int kExpectedStatus = ERR_INSUFFICIENT_RESOURCES; + EXPECT_CALL(*cert_verifier_mock_, + Verify(cert_.get(), kHostName, "", 0, nullptr, _, _, _, _)) + .Times(1) + .WillOnce(testing::Return(kExpectedStatus)); + + // Call |Verify|. + scoped_ptr actualResult; + int actualStatus = -1; + CertVerifierBlockAdapter::Params params(cert_.get(), kHostName); + Verify(params, &actualResult, &actualStatus); + + // Ensure that Verification results are correct. + EXPECT_EQ(kExpectedStatus, actualStatus); + EXPECT_FALSE(actualResult); +} + +// Tests |Verify| with all params and synchronous verification. +TEST_F(CertVerifierBlockAdapterTest, AllParamsAndSync) { + // Set up expectation. + net::CertVerifyResult expectedResult; + expectedResult.verified_cert = cert_; + const int kExpectedStatus = 0; + scoped_refptr crl_set(CRLSet::EmptyCRLSetForTesting()); + EXPECT_CALL(*cert_verifier_mock_, + Verify(cert_.get(), kHostName, kOcspResponse, + CertVerifier::VERIFY_EV_CERT, crl_set.get(), _, _, _, _)) + .Times(1) + .WillOnce(testing::DoAll(SetVerifyResult(expectedResult), + testing::Return(kExpectedStatus))); + + // Call |Verify|. + scoped_ptr actualResult; + int actualStatus = -1; + CertVerifierBlockAdapter::Params params(cert_.get(), kHostName); + params.ocsp_response = kOcspResponse; + params.flags = CertVerifier::VERIFY_EV_CERT; + params.crl_set = crl_set; + Verify(params, &actualResult, &actualStatus); + + // Ensure that Verification results are correct. + EXPECT_EQ(kExpectedStatus, actualStatus); + EXPECT_EQ(expectedResult.verified_cert, actualResult->verified_cert); +} + +} // namespace diff --git a/ios/web/net/crw_cert_verification_controller.h b/ios/web/net/crw_cert_verification_controller.h deleted file mode 100644 index fc63f7361169..000000000000 --- a/ios/web/net/crw_cert_verification_controller.h +++ /dev/null @@ -1,68 +0,0 @@ -// 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. - -#ifndef IOS_WEB_NET_CRW_CERT_VERIFICATION_CONTROLLER_H_ -#define IOS_WEB_NET_CRW_CERT_VERIFICATION_CONTROLLER_H_ - -#import - -#import "base/memory/ref_counted.h" -#include "net/cert/cert_status_flags.h" - -namespace net { -class X509Certificate; -} - -namespace web { - -class BrowserState; - -// Accept policy for valid or invalid SSL cert. -typedef NS_ENUM(NSInteger, CertAcceptPolicy) { - // Cert status can't be determined due to an error. Caller should not proceed - // with the load, but show net error page instead. - CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR = 0, - // Cert is not valid. Caller may present SSL warning and ask used if they - // want to proceed with the load. - CERT_ACCEPT_POLICY_RECOVERABLE_ERROR, - // Cert is valid. Caller should proceed with the load. - CERT_ACCEPT_POLICY_ALLOW, -}; - -// Completion handler called by decidePolicyForCert:host:completionHandler:. -typedef void (^PolicyDecisionHandler)(web::CertAcceptPolicy, net::CertStatus); - -} // namespace web - -// Provides various cert verification API that can be used for blocking requests -// with bad SSL cert, presenting SSL interstitials and determining SSL status -// for Navigation Items. Must be used on UI thread. -@interface CRWCertVerificationController : NSObject - -- (instancetype)init NS_UNAVAILABLE; - -// Initializes CRWCertVerificationController with the given |browserState| which -// cannot be null and must outlive CRWCertVerificationController. -- (instancetype)initWithBrowserState:(web::BrowserState*)browserState - NS_DESIGNATED_INITIALIZER; - -// TODO(eugenebut): add API for: -// - accepting bad SSL cert using CertPolicyCache -// - querying SSL cert status for Navigation Item - -// Decides the policy for the given |cert| for the given |host| and calls -// |completionHandler| on completion. |completionHandler| cannot be null and -// will be called synchronously or asynchronously on UI thread. -- (void)decidePolicyForCert:(const scoped_refptr&)cert - host:(NSString*)host - completionHandler:(web::PolicyDecisionHandler)handler; - -// Cancels all pending verification requests. Completion handlers will not be -// called after |shutDown| call. Must always be called before object's -// deallocation. -- (void)shutDown; - -@end - -#endif // IOS_WEB_NET_CRW_CERT_VERIFICATION_CONTROLLER_H_ diff --git a/ios/web/net/crw_cert_verification_controller.mm b/ios/web/net/crw_cert_verification_controller.mm deleted file mode 100644 index b8dba8fa1068..000000000000 --- a/ios/web/net/crw_cert_verification_controller.mm +++ /dev/null @@ -1,194 +0,0 @@ -// 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" - -namespace { - -// This class takes ownership of block and releases it on UI thread, even if -// |BlockHolder| is destructed on a background thread. -template -class BlockHolder : public base::RefCountedThreadSafe> { - public: - // Takes ownership of |block|, which must not be null. - explicit BlockHolder(T block) : block_([block copy]) { DCHECK(block_); } - - // Calls underlying block with the given variadic arguments. - template - void call(Arguments... Args) { - block_(Args...); - } - - private: - BlockHolder() = delete; - friend class base::RefCountedThreadSafe; - - // 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_; -}; - -} // namespace - -@interface CRWCertVerificationController () { - // Cert verification object which wraps |net::CertVerifier|. Must be created, - // used and destroyed on IO Thread. - scoped_ptr _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&)cert - forHost:(NSString*)host - completionHandler:(void (^)(net::CertVerifyResult, int))completionHandler; - -@end - -@implementation CRWCertVerificationController - -#pragma mark - Superclass - -- (void)dealloc { - DCHECK(!_certVerifier); - [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&)cert - host:(NSString*)host - completionHandler:(web::PolicyDecisionHandler)handler { - 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. - __block scoped_refptr> handlerHolder( - new BlockHolder(handler)); - [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)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 - -- (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(); - 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&)cert - forHost:(NSString*)host - completionHandler:(void (^)(net::CertVerifyResult, int))completionHandler { - DCHECK(completionHandler); - __block scoped_refptr 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 diff --git a/ios/web/net/crw_cert_verification_controller_unittest.mm b/ios/web/net/crw_cert_verification_controller_unittest.mm deleted file mode 100644 index ad3a7f3eb9c6..000000000000 --- a/ios/web/net/crw_cert_verification_controller_unittest.mm +++ /dev/null @@ -1,114 +0,0 @@ -// 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. - -#include "ios/web/net/crw_cert_verification_controller.h" - -#include "base/mac/bind_objc_block.h" -#include "base/message_loop/message_loop.h" -#include "base/test/ios/wait_util.h" -#include "ios/web/public/web_thread.h" -#include "ios/web/test/web_test.h" -#include "net/base/test_data_directory.h" -#include "net/cert/mock_cert_verifier.h" -#include "net/cert/x509_certificate.h" -#include "net/test/cert_test_util.h" -#include "net/url_request/url_request_context.h" -#include "net/url_request/url_request_context_getter.h" - -namespace web { - -namespace { -// Generated cert filename. -const char kCertFileName[] = "ok_cert.pem"; -// Test hostname for cert verification. -NSString* const kHostName = @"www.example.com"; -} // namespace - -// Test fixture to test CRWCertVerificationController class. -class CRWCertVerificationControllerTest : public web::WebTest { - protected: - void SetUp() override { - web::WebTest::SetUp(); - - web::BrowserState* browser_state = GetBrowserState(); - net::URLRequestContextGetter* getter = browser_state->GetRequestContext(); - web::WebThread::PostTask(web::WebThread::IO, FROM_HERE, base::BindBlock(^{ - getter->GetURLRequestContext()->set_cert_verifier(&cert_verifier_); - })); - - controller_.reset([[CRWCertVerificationController alloc] - initWithBrowserState:browser_state]); - cert_ = - net::ImportCertFromFile(net::GetTestCertsDirectory(), kCertFileName); - } - - void TearDown() override { - [controller_ shutDown]; - web::WebTest::TearDown(); - } - - // Synchronously returns result of decidePolicyForCert:host:completionHandler: - // call. - void DecidePolicy(const scoped_refptr& cert, - NSString* host, - web::CertAcceptPolicy* policy, - net::CertStatus* status) { - __block bool completion_handler_called = false; - [controller_ decidePolicyForCert:cert - host:host - completionHandler:^(web::CertAcceptPolicy callback_policy, - net::CertStatus callback_status) { - *policy = callback_policy; - *status = callback_status; - completion_handler_called = true; - }]; - base::test::ios::WaitUntilCondition(^{ - return completion_handler_called; - }, base::MessageLoop::current(), base::TimeDelta()); - } - - scoped_refptr cert_; - net::MockCertVerifier cert_verifier_; - base::scoped_nsobject controller_; -}; - -// Tests cert policy with a valid cert. -TEST_F(CRWCertVerificationControllerTest, ValidCert) { - net::CertVerifyResult verify_result; - verify_result.cert_status = net::CERT_STATUS_NO_REVOCATION_MECHANISM; - verify_result.verified_cert = cert_; - cert_verifier_.AddResultForCertAndHost(cert_.get(), [kHostName UTF8String], - verify_result, net::OK); - web::CertAcceptPolicy policy = CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR; - net::CertStatus status; - DecidePolicy(cert_, kHostName, &policy, &status); - EXPECT_EQ(CERT_ACCEPT_POLICY_ALLOW, policy); - EXPECT_EQ(verify_result.cert_status, status); -} - -// Tests cert policy with an invalid cert. -TEST_F(CRWCertVerificationControllerTest, InvalidCert) { - web::CertAcceptPolicy policy = CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR; - net::CertStatus status; - DecidePolicy(cert_, kHostName, &policy, &status); - EXPECT_EQ(CERT_ACCEPT_POLICY_RECOVERABLE_ERROR, policy); -} - -// Tests cert policy with null cert. -TEST_F(CRWCertVerificationControllerTest, NullCert) { - web::CertAcceptPolicy policy = CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR; - net::CertStatus status; - DecidePolicy(nullptr, kHostName, &policy, &status); - EXPECT_EQ(CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR, policy); -} - -// Tests cert policy with null cert and null host. -TEST_F(CRWCertVerificationControllerTest, NullHost) { - web::CertAcceptPolicy policy = CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR; - net::CertStatus status; - DecidePolicy(cert_, nil, &policy, &status); - EXPECT_EQ(CERT_ACCEPT_POLICY_NON_RECOVERABLE_ERROR, policy); -} - -} // namespace web diff --git a/ios/web/public/test/test_browser_state.cc b/ios/web/public/test/test_browser_state.cc index cdb050a2b123..d001bc3f90dd 100644 --- a/ios/web/public/test/test_browser_state.cc +++ b/ios/web/public/test/test_browser_state.cc @@ -15,9 +15,7 @@ namespace { class TestContextURLRequestContextGetter : public net::URLRequestContextGetter { public: TestContextURLRequestContextGetter() - : null_task_runner_(new base::NullTaskRunner) { - context_.set_net_log(&net_log_); - } + : null_task_runner_(new base::NullTaskRunner) {} net::URLRequestContext* GetURLRequestContext() override { return &context_; } @@ -30,7 +28,6 @@ class TestContextURLRequestContextGetter : public net::URLRequestContextGetter { ~TestContextURLRequestContextGetter() override {} net::TestURLRequestContext context_; - net::NetLog net_log_; scoped_refptr null_task_runner_; }; 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 b17cb9f7d84e..0d1845039df5 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 @@ -19,7 +19,6 @@ #import "ios/web/navigation/crw_session_entry.h" #include "ios/web/navigation/navigation_item_impl.h" #include "ios/web/navigation/web_load_params.h" -#import "ios/web/net/crw_cert_verification_controller.h" #include "ios/web/public/web_client.h" #import "ios/web/public/web_state/js/crw_js_injection_manager.h" #import "ios/web/public/web_state/ui/crw_native_content_provider.h" @@ -136,12 +135,6 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { // CRWWebUIManager object for loading WebUI pages. base::scoped_nsobject _webUIManager; - - // Controller used for certs verification to help with blocking requests with - // bad SSL cert, presenting SSL interstitials and determining SSL status for - // Navigation Items. - base::scoped_nsobject - _certVerificationController; } // Response's MIME type of the last known navigation. @@ -294,14 +287,7 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { #pragma mark CRWWebController public methods - (instancetype)initWithWebState:(scoped_ptr)webState { - DCHECK(webState); - web::BrowserState* browserState = webState->GetBrowserState(); - self = [super initWithWebState:webState.Pass()]; - if (self) { - _certVerificationController.reset([[CRWCertVerificationController alloc] - initWithBrowserState:browserState]); - } - return self; + return [super initWithWebState:webState.Pass()]; } - (BOOL)keyboardDisplayRequiresUserAction { @@ -343,11 +329,6 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { [super setPageDialogOpenPolicy:policy]; } -- (void)close { - [_certVerificationController shutDown]; - [super close]; -} - #pragma mark - #pragma mark Testing-Only Methods @@ -1344,22 +1325,8 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) { completionHandler: (void (^)(NSURLSessionAuthChallengeDisposition disposition, NSURLCredential *credential))completionHandler { - if (![challenge.protectionSpace.authenticationMethod - isEqual:NSURLAuthenticationMethodServerTrust]) { - completionHandler(NSURLSessionAuthChallengePerformDefaultHandling, nil); - return; - } - - SecTrustRef trust = challenge.protectionSpace.serverTrust; - scoped_refptr cert = web::CreateCertFromTrust(trust); - [_certVerificationController - decidePolicyForCert:cert - host:challenge.protectionSpace.host - completionHandler:^(web::CertAcceptPolicy policy, - net::CertStatus status) { - completionHandler(NSURLSessionAuthChallengeRejectProtectionSpace, - nil); - }]; + NOTIMPLEMENTED(); + completionHandler(NSURLSessionAuthChallengeRejectProtectionSpace, nil); } - (void)webViewWebContentProcessDidTerminate:(WKWebView*)webView { -- 2.11.4.GIT