Update SSL error handling code to account for Subject CN deprecation

chrome/browser/ssl/ssl_error_handler_unittest.cc

 // Copyright 2014 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 "chrome/browser/ssl/ssl_error_handler.h"
 
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
@@ skipping 183 matching lines @@
 // A class to test name mismatch errors. Creates an error handler with a name
 // mismatch error.
 class SSLErrorHandlerNameMismatchTest : public ChromeRenderViewHostTestHarness {
  public:
   SSLErrorHandlerNameMismatchTest() : field_trial_list_(nullptr) {}
 
   void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
     SSLErrorHandler::ResetConfigForTesting();
     SSLErrorHandler::SetInterstitialDelayForTesting(base::TimeDelta());
+    ssl_info_.cert = net::ImportCertFromFile(
+        net::GetTestCertsDirectory(), "subjectAltName_www_example_com.pem");
+    ssl_info_.cert_status = net::CERT_STATUS_COMMON_NAME_INVALID;
+    ssl_info_.public_key_hashes.push_back(
+        net::HashValue(kCertPublicKeyHashValue));
+
+    delegate_ =
+        new TestSSLErrorHandlerDelegate(profile(), web_contents(), ssl_info_);
+    error_handler_.reset(new TestSSLErrorHandler(
+        std::unique_ptr<SSLErrorHandler::Delegate>(delegate_), web_contents(),
+        profile(), net::MapCertStatusToNetError(ssl_info_.cert_status),
+        ssl_info_,
+        GURL(),  // request_url
+        base::Callback<void(content::CertificateRequestResultType)>()));
+  }
+
+  void TearDown() override {
+    EXPECT_FALSE(error_handler()->IsTimerRunningForTesting());
+    error_handler_.reset(nullptr);
+    SSLErrorHandler::ResetConfigForTesting();
+    ChromeRenderViewHostTestHarness::TearDown();
+  }
+
+  TestSSLErrorHandler* error_handler() { return error_handler_.get(); }
+  TestSSLErrorHandlerDelegate* delegate() { return delegate_; }
+
+  const net::SSLInfo& ssl_info() { return ssl_info_; }
+
+ private:
+  net::SSLInfo ssl_info_;
+  std::unique_ptr<TestSSLErrorHandler> error_handler_;
+  TestSSLErrorHandlerDelegate* delegate_;
+  base::FieldTrialList field_trial_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(SSLErrorHandlerNameMismatchTest);
+};
+
+// A class to test name mismatch errors, where the certificate lacks a
+// SubjectAltName. Creates an error handler with a name mismatch error.
+class SSLErrorHandlerNameMismatchNoSANTest

[estark, 2017/04/03 02:01:42]

This test fixture is the same as the one above except for the certificate,
right? If so, perhaps you could stick with one test fixture, and pull out
SetUp() lines 242-265 into a SetUpErrorHandler() method that takes a certificate
as an argument.

[elawrence, 2017/04/04 15:52:28]

I didn't know how to do that in a straightforward way because I don't control
the GoogleTest calls to SetUp(). So instead I've created a simple derived
fixture, a pattern I've seen elsewhere. Reasonable?

[estark, 2017/04/04 17:22:07]

Oh, I was thinking you could just call SetUpErrorHandler() in the test itself,
but I like what you did better.

+    : public ChromeRenderViewHostTestHarness {
+ public:
+  SSLErrorHandlerNameMismatchNoSANTest() : field_trial_list_(nullptr) {}
+
+  void SetUp() override {
+    ChromeRenderViewHostTestHarness::SetUp();
+    SSLErrorHandler::ResetConfigForTesting();
+    SSLErrorHandler::SetInterstitialDelayForTesting(base::TimeDelta());
     ssl_info_.cert =
         net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem");
     ssl_info_.cert_status = net::CERT_STATUS_COMMON_NAME_INVALID;
     ssl_info_.public_key_hashes.push_back(
         net::HashValue(kCertPublicKeyHashValue));
 
     delegate_ =
         new TestSSLErrorHandlerDelegate(profile(), web_contents(), ssl_info_);
     error_handler_.reset(new TestSSLErrorHandler(
         std::unique_ptr<SSLErrorHandler::Delegate>(delegate_), web_contents(),
@@ skipping 14 matching lines @@
   TestSSLErrorHandlerDelegate* delegate() { return delegate_; }
 
   const net::SSLInfo& ssl_info() { return ssl_info_; }
 
  private:
   net::SSLInfo ssl_info_;
   std::unique_ptr<TestSSLErrorHandler> error_handler_;
   TestSSLErrorHandlerDelegate* delegate_;
   base::FieldTrialList field_trial_list_;
 
-  DISALLOW_COPY_AND_ASSIGN(SSLErrorHandlerNameMismatchTest);
+  DISALLOW_COPY_AND_ASSIGN(SSLErrorHandlerNameMismatchNoSANTest);
 };
 
 // A class to test the captive portal certificate list feature. Creates an error
 // handler with a name mismatch error by default. The error handler can be
 // recreated by calling ResetErrorHandler() with an appropriate cert status.
 class SSLErrorHandlerCaptivePortalCertListTest
     : public ChromeRenderViewHostTestHarness {
  public:
   SSLErrorHandlerCaptivePortalCertListTest() : field_trial_list_(nullptr) {}
 
@@ skipping 430 matching lines @@
 
   histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(), 3);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::HANDLE_ALL, 1);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::WWW_MISMATCH_FOUND, 1);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::WWW_MISMATCH_URL_AVAILABLE, 1);
 }
 
+// No suggestions should be requested if certificate lacks a SubjectAltName.
+TEST_F(SSLErrorHandlerNameMismatchNoSANTest,
+       SSLCommonNameMismatchHandlingRequiresSubjectAltName) {
+  base::HistogramTester histograms;
+  EXPECT_FALSE(error_handler()->IsTimerRunningForTesting());
+  delegate()->set_suggested_url_exists();
+  error_handler()->StartHandlingError();
+
+  EXPECT_FALSE(delegate()->suggested_url_checked());
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(delegate()->ssl_interstitial_shown());
+  EXPECT_FALSE(delegate()->redirected_to_suggested_url());
+
+  histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(), 2);
+  histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+                               SSLErrorHandler::HANDLE_ALL, 1);
+  histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+                               SSLErrorHandler::WWW_MISMATCH_FOUND, 0);
+  histograms.ExpectBucketCount(
+      SSLErrorHandler::GetHistogramNameForTesting(),
+      SSLErrorHandler::SHOW_SSL_INTERSTITIAL_OVERRIDABLE, 1);
+}
+
 TEST_F(SSLErrorHandlerNameMismatchTest,
        ShouldShowSSLInterstitialOnInvalidUrlCheckResult) {
   base::HistogramTester histograms;
   delegate()->set_suggested_url_exists();
   error_handler()->StartHandlingError();
 
   EXPECT_TRUE(error_handler()->IsTimerRunningForTesting());
   EXPECT_TRUE(delegate()->suggested_url_checked());
   EXPECT_FALSE(delegate()->ssl_interstitial_shown());
   EXPECT_FALSE(delegate()->redirected_to_suggested_url());
@@ skipping 255 matching lines @@
 
   histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(), 2);
   histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
                                SSLErrorHandler::HANDLE_ALL, 1);
   histograms.ExpectBucketCount(
       SSLErrorHandler::GetHistogramNameForTesting(),
       SSLErrorHandler::SHOW_SSL_INTERSTITIAL_OVERRIDABLE, 1);
 }
 
 #endif  // BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)