Revert "Implement HTTPSOCSPTests as browsertests"

This reverts commit ab3d270b.

Reason for revert: Tests are failing in https://ci.chromium.org/p/chromium/builders/ci/Network%20Service%20Linux/7987

Original change's description:
> Implement HTTPSOCSPTests as browsertests
> 
> Right now the most complete set of OCSP tests is implemented in
> url_request_unittest.cc using HTTPSOCSPTest. This CL creates a parallel
> set of browsertests, so we can test that OCSP works with the
> CertVerifierService as well.
> 
> The new tests are in ocsp_browsertest.cc, and existing OCSP
> browsertests were moved from ssl_browsertest.cc into
> ocsp_browsertest.cc to reduce the size of the ssl_browsertest.cc
> file.
> 
> The new tests are nearly identical, structurally, to the original
> tests, but SystemSupportsOCSP() is no longer necessary as it
> was really checking support for EV certificates, and the checks
> for EV certificates are already guarded by
> SystemUsesChromiumEVMetadata().
> 
> Bug: 1015134, 1022561
> Change-Id: I32e9355d276f75de3432c09f8e7309db1525f729
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2274581
> Commit-Queue: Matthew Denton <mpdenton@chromium.org>
> Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
> Reviewed-by: Matt Falkenhagen <falken@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#784338}

TBR=falken@chromium.org,rsleevi@chromium.org,mpdenton@chromium.org

Change-Id: I79bf58c017a324ca32f6b5a4e2dd30d9c2652683
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1015134, 1022561
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2277380Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784348}

chrome/browser/ssl/ssl_browsertest.cc

@@ -398,6 +398,8 @@ class SSLUITestBase : public InProcessBrowserTest,
         https_server_mismatched_(net::EmbeddedTestServer::TYPE_HTTPS),
         https_server_sha1_(net::EmbeddedTestServer::TYPE_HTTPS),
         https_server_common_name_only_(net::EmbeddedTestServer::TYPE_HTTPS),
+        https_server_ocsp_ok_(net::EmbeddedTestServer::TYPE_HTTPS),
+        https_server_ocsp_revoked_(net::EmbeddedTestServer::TYPE_HTTPS),
         wss_server_expired_(net::SpawnedTestServer::TYPE_WSS,
                             SSLOptions(SSLOptions::CERT_EXPIRED),
                             net::GetWebSocketTestDataDirectory()),
@@ -419,6 +421,20 @@ class SSLUITestBase : public InProcessBrowserTest,
     https_server_common_name_only_.SetSSLConfig(
         net::EmbeddedTestServer::CERT_COMMON_NAME_ONLY);
     https_server_common_name_only_.AddDefaultHandlers(GetChromeTestDataDir());
+
+    net::EmbeddedTestServer::ServerCertificateConfig ok_cert_config;
+    ok_cert_config.ocsp_config = net::EmbeddedTestServer::OCSPConfig(
+        {{net::OCSPRevocationStatus::GOOD,
+          net::EmbeddedTestServer::OCSPConfig::SingleResponse::Date::kValid}});
+    https_server_ocsp_ok_.SetSSLConfig(ok_cert_config);
+    https_server_ocsp_ok_.AddDefaultHandlers(GetChromeTestDataDir());
+
+    net::EmbeddedTestServer::ServerCertificateConfig revoked_cert_config;
+    revoked_cert_config.ocsp_config = net::EmbeddedTestServer::OCSPConfig(
+        {{net::OCSPRevocationStatus::REVOKED,
+          net::EmbeddedTestServer::OCSPConfig::SingleResponse::Date::kValid}});
+    https_server_ocsp_revoked_.SetSSLConfig(revoked_cert_config);
+    https_server_ocsp_revoked_.AddDefaultHandlers(GetChromeTestDataDir());
   }
 
   void SetUp() override {
@@ -783,9 +799,16 @@ class SSLUITestBase : public InProcessBrowserTest,
     EXPECT_EQ(app_url, new_tab->GetVisibleURL());
   }
 
+  void set_ssl_config_updated_callback(
+      const base::RepeatingClosure& ssl_config_updated_callback) {
+    ssl_config_updated_callback_ = std::move(ssl_config_updated_callback);
+  }
+
   // network::mojom::SSLConfigClient implementation.
   void OnSSLConfigUpdated(network::mojom::SSLConfigPtr ssl_config) override {
     last_ssl_config_ = *ssl_config;
+    if (ssl_config_updated_callback_)
+      ssl_config_updated_callback_.Run();
   }
 
  protected:
@@ -857,12 +880,15 @@ class SSLUITestBase : public InProcessBrowserTest,
   net::EmbeddedTestServer https_server_mismatched_;
   net::EmbeddedTestServer https_server_sha1_;
   net::EmbeddedTestServer https_server_common_name_only_;
+  net::EmbeddedTestServer https_server_ocsp_ok_;
+  net::EmbeddedTestServer https_server_ocsp_revoked_;
 
   net::SpawnedTestServer wss_server_expired_;
   net::SpawnedTestServer wss_server_mismatched_;
 
   policy::MockConfigurationPolicyProvider policy_provider_;
 
+  base::RepeatingClosure ssl_config_updated_callback_;
   network::mojom::SSLConfig last_ssl_config_;
   mojo::Receiver<network::mojom::SSLConfigClient> receiver_{this};
 
@@ -1590,6 +1616,100 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertGoBackUsingCommand) {
   ssl_test_util::CheckUnauthenticatedState(tab, AuthState::NONE);
 }
 
+// Visits a page with revocation checking enabled and a valid OCSP response.
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSOCSPOk) {
+  // OCSP checking is disabled by default.
+  EXPECT_FALSE(last_ssl_config_.rev_checking_enabled);
+  EXPECT_FALSE(CreateDefaultNetworkContextParams()
+                   ->initial_ssl_config->rev_checking_enabled);
+
+  // Enable, and make sure the default network context params reflect the
+  // change.
+  base::RunLoop run_loop;
+  set_ssl_config_updated_callback(run_loop.QuitClosure());
+  ASSERT_NO_FATAL_FAILURE(
+      EnablePolicy(g_browser_process->local_state(),
+                   policy::key::kEnableOnlineRevocationChecks,
+                   prefs::kCertRevocationCheckingEnabled));
+  run_loop.Run();
+  EXPECT_TRUE(last_ssl_config_.rev_checking_enabled);
+  EXPECT_TRUE(CreateDefaultNetworkContextParams()
+                  ->initial_ssl_config->rev_checking_enabled);
+
+  ASSERT_TRUE(https_server_ocsp_ok_.Start());
+
+  ui_test_utils::NavigateToURL(
+      browser(), https_server_ocsp_ok_.GetURL("/ssl/google.html"));
+
+  ssl_test_util::CheckAuthenticatedState(
+      browser()->tab_strip_model()->GetActiveWebContents(), AuthState::NONE);
+
+  content::NavigationEntry* entry = browser()
+                                        ->tab_strip_model()
+                                        ->GetActiveWebContents()
+                                        ->GetController()
+                                        .GetVisibleEntry();
+  ASSERT_TRUE(entry);
+  EXPECT_TRUE(entry->GetSSL().cert_status &
+              net::CERT_STATUS_REV_CHECKING_ENABLED);
+}
+
+// Visits a page with revocation checking enabled and a revoked OCSP response.
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSOCSPRevoked) {
+  // OCSP checking is disabled by default.
+  EXPECT_FALSE(last_ssl_config_.rev_checking_enabled);
+  EXPECT_FALSE(CreateDefaultNetworkContextParams()
+                   ->initial_ssl_config->rev_checking_enabled);
+
+  // Enable, and make sure the default network context params reflect the
+  // change.
+  base::RunLoop run_loop;
+  set_ssl_config_updated_callback(run_loop.QuitClosure());
+  ASSERT_NO_FATAL_FAILURE(
+      EnablePolicy(g_browser_process->local_state(),
+                   policy::key::kEnableOnlineRevocationChecks,
+                   prefs::kCertRevocationCheckingEnabled));
+  run_loop.Run();
+  EXPECT_TRUE(last_ssl_config_.rev_checking_enabled);
+  EXPECT_TRUE(CreateDefaultNetworkContextParams()
+                  ->initial_ssl_config->rev_checking_enabled);
+
+  ASSERT_TRUE(https_server_ocsp_revoked_.Start());
+
+  ui_test_utils::NavigateToURL(
+      browser(), https_server_ocsp_revoked_.GetURL("/ssl/google.html"));
+
+  ssl_test_util::CheckAuthenticationBrokenState(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      net::CERT_STATUS_REVOKED, AuthState::SHOWING_INTERSTITIAL);
+}
+
+// Visits a page with revocation checking set to the default value (disabled)
+// and a revoked OCSP response.
+IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSOCSPRevokedButNotChecked) {
+  // OCSP checking is disabled by default.
+  EXPECT_FALSE(last_ssl_config_.rev_checking_enabled);
+  EXPECT_FALSE(CreateDefaultNetworkContextParams()
+                   ->initial_ssl_config->rev_checking_enabled);
+
+  ASSERT_TRUE(https_server_ocsp_revoked_.Start());
+
+  ui_test_utils::NavigateToURL(
+      browser(), https_server_ocsp_revoked_.GetURL("/ssl/google.html"));
+
+  ssl_test_util::CheckAuthenticatedState(
+      browser()->tab_strip_model()->GetActiveWebContents(), AuthState::NONE);
+
+  content::NavigationEntry* entry = browser()
+                                        ->tab_strip_model()
+                                        ->GetActiveWebContents()
+                                        ->GetController()
+                                        .GetVisibleEntry();
+  ASSERT_TRUE(entry);
+  EXPECT_FALSE(entry->GetSSL().cert_status &
+               net::CERT_STATUS_REV_CHECKING_ENABLED);
+}
+
 // Visits a page that uses a SHA-1 leaf certificate, which should be rejected
 // by default.
 IN_PROC_BROWSER_TEST_F(SSLUITest, SHA1IsDefaultDisabled) {

chrome/browser/ssl/ssl_browsertest_util.cc

@@ -19,15 +19,10 @@
 #include "mojo/public/cpp/bindings/sync_call_restrictions.h"
 #include "net/base/features.h"
 #include "net/cert/cert_status_flags.h"
-#include "net/cert/ev_root_ca_metadata.h"
 #include "net/net_buildflags.h"
 #include "services/network/public/mojom/network_context.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#if defined(OS_MACOSX)
-#include "base/mac/mac_util.h"
-#endif
-
 namespace ssl_test_util {
 
 namespace AuthState {
@@ -139,7 +134,7 @@ void SecurityStateWebContentsObserver::DidChangeVisibleSecurityState() {
   run_loop_.Quit();
 }
 
-bool UsingBuiltinCertVerifier() {
+static bool UsingBuiltinCertVerifier() {
 #if defined(OS_FUCHSIA) || defined(OS_LINUX) || defined(OS_CHROMEOS)
   return true;
 #elif BUILDFLAG(BUILTIN_CERT_VERIFIER_FEATURE_SUPPORTED)
@@ -149,42 +144,6 @@ bool UsingBuiltinCertVerifier() {
   return false;
 }
 
-bool SystemSupportsHardFailRevocationChecking() {
-  if (UsingBuiltinCertVerifier())
-    return true;
-#if defined(OS_WIN)
-  return true;
-#else
-  return false;
-#endif
-}
-
-bool SystemUsesChromiumEVMetadata() {
-  if (UsingBuiltinCertVerifier())
-    return true;
-#if defined(PLATFORM_USES_CHROMIUM_EV_METADATA)
-  return true;
-#else
-  return false;
-#endif
-}
-
-bool SystemSupportsOCSPStapling() {
-  if (UsingBuiltinCertVerifier())
-    return true;
-#if defined(OS_ANDROID)
-  return false;
-#elif defined(OS_MACOSX)
-  // The SecTrustSetOCSPResponse function exists since macOS 10.9+, but does
-  // not actually do anything until 10.12.
-  if (base::mac::IsAtLeastOS10_12())
-    return true;
-  return false;
-#else
-  return true;
-#endif
-}
-
 bool CertVerifierSupportsCRLSetBlocking() {
   if (UsingBuiltinCertVerifier())
     return true;

chrome/browser/ssl/ssl_browsertest_util.h

@@ -88,28 +88,6 @@ class SecurityStateWebContentsObserver : public content::WebContentsObserver {
   base::RunLoop run_loop_;
 };
 
-// Returns true if Chrome will use its builtin cert verifier rather than the
-// operating system's default.
-bool UsingBuiltinCertVerifier();
-
-// SystemSupportsHardFailRevocationChecking returns true iff the current
-// operating system supports revocation checking and can distinguish between
-// situations where a given certificate lacks any revocation information (eg:
-// no CRLDistributionPoints and no OCSP Responder AuthorityInfoAccess) and when
-// revocation information cannot be obtained (eg: the CRL was unreachable).
-// If it does not, then tests which rely on 'hard fail' behaviour should be
-// skipped.
-bool SystemSupportsHardFailRevocationChecking();
-
-// SystemUsesChromiumEVMetadata returns true iff the current operating system
-// uses Chromium's EV metadata (i.e. EVRootCAMetadata). If it does not, then
-// several tests are effected because our testing EV certificate won't be
-// recognised as EV.
-bool SystemUsesChromiumEVMetadata();
-
-// Returns true iff OCSP stapling is supported on this operating system.
-bool SystemSupportsOCSPStapling();
-
 // Returns |true| if the default CertVerifier used by the NetworkService is
 // expected to support blocking certificates that appear within a CRLSet.
 bool CertVerifierSupportsCRLSetBlocking();

chrome/test/BUILD.gn

@@ -1160,7 +1160,6 @@ if (!is_android) {
       "../browser/ssl/crlset_browsertest.cc",
       "../browser/ssl/known_interception_disclosure_infobar_browsertest.cc",
       "../browser/ssl/known_interception_disclosure_ui_browsertest.cc",
-      "../browser/ssl/ocsp_browsertest.cc",
       "../browser/ssl/security_state_tab_helper_browsertest.cc",
       "../browser/ssl/ssl_browsertest.cc",
       "../browser/ssl/ssl_client_certificate_selector_test.cc",

content/public/test/network_service_test_helper.cc

@@ -20,18 +20,14 @@
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/test_host_resolver.h"
-#include "crypto/sha2.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
-#include "net/base/hash_value.h"
 #include "net/base/ip_address.h"
-#include "net/cert/ev_root_ca_metadata.h"
 #include "net/cert/mock_cert_verifier.h"
 #include "net/cert/test_root_certs.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/http/transport_security_state.h"
 #include "net/http/transport_security_state_test_util.h"
 #include "net/nqe/network_quality_estimator.h"
-#include "net/test/cert_test_util.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 #include "net/test/test_data_directory.h"
@@ -251,19 +247,6 @@ class NetworkServiceTestHelper::NetworkServiceTestImpl
     base::FieldTrialList::FindFullName(field_trial_name);
   }
 
-  void SetEVPolicy(const std::vector<uint8_t>& fingerprint_sha256,
-                   const std::string& policy_oid,
-                   SetEVPolicyCallback callback) override {
-    CHECK_EQ(fingerprint_sha256.size(), crypto::kSHA256Length);
-    net::SHA256HashValue fingerprint_sha256_hash;
-    memcpy(&fingerprint_sha256_hash.data, fingerprint_sha256.data(),
-           crypto::kSHA256Length);
-    ev_test_policy_ = std::make_unique<net::ScopedTestEVPolicy>(
-        net::EVRootCAMetadata::GetInstance(), fingerprint_sha256_hash,
-        policy_oid.data());
-    std::move(callback).Run();
-  }
-
   void BindReceiver(
       mojo::PendingReceiver<network::mojom::NetworkServiceTest> receiver) {
     receivers_.Add(this, std::move(receiver));
@@ -295,7 +278,6 @@ class NetworkServiceTestHelper::NetworkServiceTestImpl
   base::MemoryPressureListener::MemoryPressureLevel
       latest_memory_pressure_level_ =
           base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE;
-  std::unique_ptr<net::ScopedTestEVPolicy> ev_test_policy_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkServiceTestImpl);
 };

services/network/public/mojom/network_service_test.mojom

@@ -112,9 +112,4 @@ interface NetworkServiceTest {
   // Activates the specified field trial. Intended for use verifying that the
   // network service informs the main process when a field trial is activated.
   ActivateFieldTrial(string field_trial_name);
-
-  // Instantiates a net::ScopedTestEVPolicy with the specified fingerprint and
-  // policy oid.
-  [Sync]
-  SetEVPolicy(array<uint8, 32> fingerprint_sha256, string policy_oid) => ();
 };