Record stapled OCSP and SCT list in TrialComparisonCertVerifier reports.

Bug: 991247
Change-Id: I598a9130b5d5f15aa24041a6f739abf578842c3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2052612Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741700}

chrome/browser/net/trial_comparison_cert_verifier_controller.cc

@@ -106,6 +106,8 @@ void TrialComparisonCertVerifierController::SendTrialReport(
     bool require_rev_checking_local_anchors,
     bool enable_sha1_local_anchors,
     bool disable_symantec_enforcement,
+    const std::vector<uint8_t>& stapled_ocsp,
+    const std::vector<uint8_t>& sct_list,
     const net::CertVerifyResult& primary_result,
     const net::CertVerifyResult& trial_result,
     network::mojom::CertVerifierDebugInfoPtr debug_info) {
@@ -115,11 +117,13 @@ void TrialComparisonCertVerifierController::SendTrialReport(
     return;
   }
 
-  CertificateErrorReport report(hostname, *unverified_cert, enable_rev_checking,
-                                require_rev_checking_local_anchors,
-                                enable_sha1_local_anchors,
-                                disable_symantec_enforcement, primary_result,
-                                trial_result, std::move(debug_info));
+  CertificateErrorReport report(
+      hostname, *unverified_cert, enable_rev_checking,
+      require_rev_checking_local_anchors, enable_sha1_local_anchors,
+      disable_symantec_enforcement,
+      std::string(stapled_ocsp.begin(), stapled_ocsp.end()),
+      std::string(sct_list.begin(), sct_list.end()), primary_result,
+      trial_result, std::move(debug_info));
 
   report.AddNetworkTimeInfo(g_browser_process->network_time_tracker());
   report.AddChromeChannel(chrome::GetChannel());

chrome/browser/net/trial_comparison_cert_verifier_controller.h

@@ -58,6 +58,8 @@ class TrialComparisonCertVerifierController
       bool require_rev_checking_local_anchors,
       bool enable_sha1_local_anchors,
       bool disable_symantec_enforcement,
+      const std::vector<uint8_t>& stapled_ocsp,
+      const std::vector<uint8_t>& sct_list,
       const net::CertVerifyResult& primary_result,
       const net::CertVerifyResult& trial_result,
       network::mojom::CertVerifierDebugInfoPtr debug_info) override;

chrome/browser/net/trial_comparison_cert_verifier_controller_unittest.cc

@@ -242,8 +242,9 @@ TEST_F(TrialComparisonCertVerifierControllerTest, NothingEnabled) {
 
   // Attempting to send a report should also do nothing.
   report_client()->SendTrialReport(
-      "hostname", leaf_cert_1_, false, false, false, false, ok_result_,
-      ok_result_, network::mojom::CertVerifierDebugInfo::New());
+      "hostname", leaf_cert_1_, false, false, false, false,
+      std::vector<uint8_t>(), std::vector<uint8_t>(), ok_result_, ok_result_,
+      network::mojom::CertVerifierDebugInfo::New());
   // Ensure any in-flight mojo calls get run.
   base::RunLoop().RunUntilIdle();
   // Expect no report since the trial is not allowed.
@@ -264,8 +265,9 @@ TEST_F(TrialComparisonCertVerifierControllerTest,
 
   // Attempting to send a report should do nothing.
   report_client()->SendTrialReport(
-      "hostname", leaf_cert_1_, false, false, false, false, ok_result_,
-      ok_result_, network::mojom::CertVerifierDebugInfo::New());
+      "hostname", leaf_cert_1_, false, false, false, false,
+      std::vector<uint8_t>(), std::vector<uint8_t>(), ok_result_, ok_result_,
+      network::mojom::CertVerifierDebugInfo::New());
 
   // Ensure any in-flight mojo calls get run.
   base::RunLoop().RunUntilIdle();
@@ -300,8 +302,9 @@ TEST_F(TrialComparisonCertVerifierControllerTest,
 
   // Attempting to send a report should do nothing.
   report_client()->SendTrialReport(
-      "hostname", leaf_cert_1_, false, false, false, false, ok_result_,
-      ok_result_, network::mojom::CertVerifierDebugInfo::New());
+      "hostname", leaf_cert_1_, false, false, false, false,
+      std::vector<uint8_t>(), std::vector<uint8_t>(), ok_result_, ok_result_,
+      network::mojom::CertVerifierDebugInfo::New());
 
   // Ensure any in-flight mojo calls get run.
   base::RunLoop().RunUntilIdle();
@@ -334,7 +337,8 @@ TEST_F(TrialComparisonCertVerifierControllerTest, OfficialBuildTrialEnabled) {
 
   // Report should be sent.
   report_client()->SendTrialReport(
-      "127.0.0.1", leaf_cert_1_, false, false, false, false, ok_result_,
+      "127.0.0.1", leaf_cert_1_, false, false, false, false,
+      std::vector<uint8_t>{4, 5, 6}, std::vector<uint8_t>{7, 8, 9}, ok_result_,
       bad_result_, network::mojom::CertVerifierDebugInfo::New());
 
   // Ensure any in-flight mojo calls get run.
@@ -366,6 +370,10 @@ TEST_F(TrialComparisonCertVerifierControllerTest, OfficialBuildTrialEnabled) {
   EXPECT_THAT(report.unverified_cert_chain(), CertChainMatches(leaf_cert_1_));
   EXPECT_THAT(report.cert_chain(), CertChainMatches(cert_chain_1_));
   EXPECT_THAT(trial_info.cert_chain(), CertChainMatches(cert_chain_2_));
+  ASSERT_TRUE(trial_info.has_stapled_ocsp());
+  EXPECT_EQ("\x04\x05\x06", trial_info.stapled_ocsp());
+  ASSERT_TRUE(trial_info.has_sct_list());
+  EXPECT_EQ("\x07\x08\x09", trial_info.sct_list());
 
   // Disable the SBER pref again, which should trigger the OnTrialConfigUpdated
   // callback.
@@ -377,8 +385,9 @@ TEST_F(TrialComparisonCertVerifierControllerTest, OfficialBuildTrialEnabled) {
 
   // Attempting to send a report should do nothing now.
   report_client()->SendTrialReport(
-      "hostname", leaf_cert_1_, false, false, false, false, ok_result_,
-      bad_result_, network::mojom::CertVerifierDebugInfo::New());
+      "hostname", leaf_cert_1_, false, false, false, false,
+      std::vector<uint8_t>(), std::vector<uint8_t>(), ok_result_, bad_result_,
+      network::mojom::CertVerifierDebugInfo::New());
   // Ensure any in-flight mojo calls get run.
   base::RunLoop().RunUntilIdle();
   // Expect no report since the trial is not allowed.
@@ -425,11 +434,13 @@ TEST_F(TrialComparisonCertVerifierControllerTest,
 
   // Report should be sent.
   report_client()->SendTrialReport(
-      "127.0.0.1", leaf_cert_1_, false, false, false, false, ok_result_,
-      bad_result_, network::mojom::CertVerifierDebugInfo::New());
+      "127.0.0.1", leaf_cert_1_, false, false, false, false,
+      std::vector<uint8_t>(), std::vector<uint8_t>(), ok_result_, bad_result_,
+      network::mojom::CertVerifierDebugInfo::New());
   report_client_2->SendTrialReport(
-      "127.0.0.2", leaf_cert_1_, false, false, false, false, ok_result_,
-      bad_result_, network::mojom::CertVerifierDebugInfo::New());
+      "127.0.0.2", leaf_cert_1_, false, false, false, false,
+      std::vector<uint8_t>(), std::vector<uint8_t>(), ok_result_, bad_result_,
+      network::mojom::CertVerifierDebugInfo::New());
 
   // Ensure any in-flight mojo calls get run.
   base::RunLoop().RunUntilIdle();
@@ -462,6 +473,9 @@ TEST_F(TrialComparisonCertVerifierControllerTest,
     EXPECT_THAT(report.unverified_cert_chain(), CertChainMatches(leaf_cert_1_));
     EXPECT_THAT(report.cert_chain(), CertChainMatches(cert_chain_1_));
     EXPECT_THAT(trial_info.cert_chain(), CertChainMatches(cert_chain_2_));
+
+    EXPECT_FALSE(trial_info.has_stapled_ocsp());
+    EXPECT_FALSE(trial_info.has_sct_list());
   }
 
   // Disable the SBER pref again, which should trigger the OnTrialConfigUpdated
@@ -475,11 +489,13 @@ TEST_F(TrialComparisonCertVerifierControllerTest,
 
   // Attempting to send a report should do nothing now.
   report_client()->SendTrialReport(
-      "hostname", leaf_cert_1_, false, false, false, false, ok_result_,
-      bad_result_, network::mojom::CertVerifierDebugInfo::New());
+      "hostname", leaf_cert_1_, false, false, false, false,
+      std::vector<uint8_t>(), std::vector<uint8_t>(), ok_result_, bad_result_,
+      network::mojom::CertVerifierDebugInfo::New());
   report_client_2->SendTrialReport(
-      "hostname2", leaf_cert_1_, false, false, false, false, ok_result_,
-      bad_result_, network::mojom::CertVerifierDebugInfo::New());
+      "hostname2", leaf_cert_1_, false, false, false, false,
+      std::vector<uint8_t>(), std::vector<uint8_t>(), ok_result_, bad_result_,
+      network::mojom::CertVerifierDebugInfo::New());
   // Ensure any in-flight mojo calls get run.
   base::RunLoop().RunUntilIdle();
   // Expect no report since the trial is not allowed.
@@ -511,8 +527,9 @@ TEST_F(TrialComparisonCertVerifierControllerTest,
   // In uma_only mode, the network service will generate a report, but the
   // trial controller will not send it to the reporting service.
   report_client()->SendTrialReport(
-      "127.0.0.1", leaf_cert_1_, false, false, false, false, ok_result_,
-      bad_result_, network::mojom::CertVerifierDebugInfo::New());
+      "127.0.0.1", leaf_cert_1_, false, false, false, false,
+      std::vector<uint8_t>(), std::vector<uint8_t>(), ok_result_, bad_result_,
+      network::mojom::CertVerifierDebugInfo::New());
 
   // Ensure any in-flight mojo calls get run.
   base::RunLoop().RunUntilIdle();
@@ -540,8 +557,9 @@ TEST_F(TrialComparisonCertVerifierControllerTest,
 
   // Attempting to send a report should also do nothing.
   report_client()->SendTrialReport(
-      "hostname", leaf_cert_1_, false, false, false, false, ok_result_,
-      ok_result_, network::mojom::CertVerifierDebugInfo::New());
+      "hostname", leaf_cert_1_, false, false, false, false,
+      std::vector<uint8_t>(), std::vector<uint8_t>(), ok_result_, ok_result_,
+      network::mojom::CertVerifierDebugInfo::New());
   // Ensure any in-flight mojo calls get run.
   base::RunLoop().RunUntilIdle();
   // Expect no report since the trial is not allowed.

components/security_interstitials/content/cert_logger.proto

@@ -275,4 +275,11 @@ message TrialVerificationInfo {
 
   // Debug information from CertVerifyProcMac (if it was the primary verifier).
   optional MacPlatformDebugInfo mac_platform_debug_info = 10;
+
+  // DER encoded stapled OCSP response, if present.
+  optional bytes stapled_ocsp = 11;
+
+  // DER encoded SignedCertificateTimestampList from the TLS extension, if
+  // present.
+  optional bytes sct_list = 12;
 }

components/security_interstitials/content/certificate_error_report.cc

@@ -183,6 +183,8 @@ CertificateErrorReport::CertificateErrorReport(
     bool require_rev_checking_local_anchors,
     bool enable_sha1_local_anchors,
     bool disable_symantec_enforcement,
+    const std::string& stapled_ocsp,
+    const std::string& sct_list,
     const net::CertVerifyResult& primary_result,
     const net::CertVerifyResult& trial_result,
     network::mojom::CertVerifierDebugInfoPtr debug_info)
@@ -209,6 +211,12 @@ CertificateErrorReport::CertificateErrorReport(
       enable_rev_checking, require_rev_checking_local_anchors,
       enable_sha1_local_anchors, disable_symantec_enforcement,
       trial_report->mutable_verify_flags());
+
+  if (!stapled_ocsp.empty())
+    trial_report->set_stapled_ocsp(stapled_ocsp);
+  if (!sct_list.empty())
+    trial_report->set_sct_list(sct_list);
+
 #if defined(OS_MACOSX)
   AddMacPlatformDebugInfoToReport(debug_info->mac_platform_debug_info,
                                   trial_report);

components/security_interstitials/content/certificate_error_report.h

@@ -77,6 +77,8 @@ class CertificateErrorReport {
                          bool require_rev_checking_local_anchors,
                          bool enable_sha1_local_anchors,
                          bool disable_symantec_enforcement,
+                         const std::string& stapled_ocsp,
+                         const std::string& sct_list,
                          const net::CertVerifyResult& primary_result,
                          const net::CertVerifyResult& trial_result,
                          network::mojom::CertVerifierDebugInfoPtr debug_info);

components/security_interstitials/content/certificate_error_report_unittest.cc

@@ -374,8 +374,8 @@ TEST(ErrorReportTest, TrialDebugInfo) {
   debug_info->trial_der_verification_time = "it's just a string";
 
   CertificateErrorReport report("example.com", *unverified_cert, false, false,
-                                false, false, primary_result, trial_result,
-                                std::move(debug_info));
+                                false, false, "ocsp", "sct", primary_result,
+                                trial_result, std::move(debug_info));
   std::string serialized_report;
   ASSERT_TRUE(report.Serialize(&serialized_report));
   chrome_browser_ssl::CertLoggerRequest parsed;
@@ -384,6 +384,10 @@ TEST(ErrorReportTest, TrialDebugInfo) {
   ASSERT_TRUE(parsed.features_info().has_trial_verification_info());
   const chrome_browser_ssl::TrialVerificationInfo& trial_info =
       parsed.features_info().trial_verification_info();
+  ASSERT_TRUE(trial_info.has_stapled_ocsp());
+  EXPECT_EQ("ocsp", trial_info.stapled_ocsp());
+  ASSERT_TRUE(trial_info.has_sct_list());
+  EXPECT_EQ("sct", trial_info.sct_list());
 
 #if defined(OS_MACOSX)
   ASSERT_TRUE(trial_info.has_mac_platform_debug_info());

net/cert/trial_comparison_cert_verifier.cc

@@ -362,7 +362,8 @@ void TrialComparisonCertVerifier::Job::Finish(
         params_.hostname(), params_.certificate(), config_.enable_rev_checking,
         config_.require_rev_checking_local_anchors,
         config_.enable_sha1_local_anchors, config_.disable_symantec_enforcement,
-        primary_result_, trial_result_);
+        params_.ocsp_response(), params_.sct_list(), primary_result_,
+        trial_result_);
   }
 
   if (weak_this) {

net/cert/trial_comparison_cert_verifier.h

@@ -52,6 +52,8 @@ class NET_EXPORT TrialComparisonCertVerifier : public CertVerifier {
       bool require_rev_checking_local_anchors,
       bool enable_sha1_local_anchors,
       bool disable_symantec_enforcement,
+      const std::string& stapled_ocsp,
+      const std::string& sct_list,
       const net::CertVerifyResult& primary_result,
       const net::CertVerifyResult& trial_result)>;
 

net/cert/trial_comparison_cert_verifier_unittest.cc

@@ -229,6 +229,8 @@ struct TrialReportInfo {
                   bool require_rev_checking_local_anchors,
                   bool enable_sha1_local_anchors,
                   bool disable_symantec_enforcement,
+                  const std::string& stapled_ocsp,
+                  const std::string& sct_list,
                   const CertVerifyResult& primary_result,
                   const CertVerifyResult& trial_result)
       : hostname(hostname),
@@ -237,6 +239,8 @@ struct TrialReportInfo {
         require_rev_checking_local_anchors(require_rev_checking_local_anchors),
         enable_sha1_local_anchors(enable_sha1_local_anchors),
         disable_symantec_enforcement(disable_symantec_enforcement),
+        stapled_ocsp(stapled_ocsp),
+        sct_list(sct_list),
         primary_result(primary_result),
         trial_result(trial_result) {}
 
@@ -246,6 +250,8 @@ struct TrialReportInfo {
   bool require_rev_checking_local_anchors;
   bool enable_sha1_local_anchors;
   bool disable_symantec_enforcement;
+  std::string stapled_ocsp;
+  std::string sct_list;
   CertVerifyResult primary_result;
   CertVerifyResult trial_result;
 };
@@ -257,12 +263,15 @@ void RecordTrialReport(std::vector<TrialReportInfo>* reports,
                        bool require_rev_checking_local_anchors,
                        bool enable_sha1_local_anchors,
                        bool disable_symantec_enforcement,
+                       const std::string& stapled_ocsp,
+                       const std::string& sct_list,
                        const CertVerifyResult& primary_result,
                        const CertVerifyResult& trial_result) {
-  TrialReportInfo report(
-      hostname, unverified_cert, enable_rev_checking,
-      require_rev_checking_local_anchors, enable_sha1_local_anchors,
-      disable_symantec_enforcement, primary_result, trial_result);
+  TrialReportInfo report(hostname, unverified_cert, enable_rev_checking,
+                         require_rev_checking_local_anchors,
+                         enable_sha1_local_anchors,
+                         disable_symantec_enforcement, stapled_ocsp, sct_list,
+                         primary_result, trial_result);
   reports->push_back(report);
 }
 
@@ -727,8 +736,7 @@ TEST_F(TrialComparisonCertVerifierTest, PrimaryVerifierOkSecondaryError) {
   verifier.set_trial_allowed(true);
 
   CertVerifier::RequestParams params(leaf_cert_1_, "127.0.0.1", /*flags=*/0,
-                                     /*ocsp_response=*/std::string(),
-                                     /*sct_list=*/std::string());
+                                     "ocsp", "sct");
   CertVerifyResult result;
   TestCompletionCallback callback;
   std::unique_ptr<CertVerifier::Request> request;
@@ -755,6 +763,8 @@ TEST_F(TrialComparisonCertVerifierTest, PrimaryVerifierOkSecondaryError) {
   EXPECT_TRUE(report.trial_result.verified_cert->EqualsIncludingChain(
       cert_chain_1_.get()));
   EXPECT_TRUE(report.unverified_cert->EqualsIncludingChain(leaf_cert_1_.get()));
+  EXPECT_EQ("ocsp", report.stapled_ocsp);
+  EXPECT_EQ("sct", report.sct_list);
 
   EXPECT_EQ(1, verify_proc1->num_verifications());
   EXPECT_EQ(1, verify_proc2->num_verifications());
@@ -1377,6 +1387,7 @@ TEST_F(TrialComparisonCertVerifierTest, DeletedDuringTrialReport) {
               const scoped_refptr<X509Certificate>& unverified_cert,
               bool enable_rev_checking, bool require_rev_checking_local_anchors,
               bool enable_sha1_local_anchors, bool disable_symantec_enforcement,
+              const std::string& stapled_ocsp, const std::string& sct_list,
               const net::CertVerifyResult& primary_result,
               const net::CertVerifyResult& trial_result) {
             // During processing of a report, delete the underlying verifier.

services/network/public/mojom/trial_comparison_cert_verifier.mojom

@@ -63,7 +63,8 @@ interface TrialComparisonCertVerifierReportClient {
   SendTrialReport(
       string hostname, X509Certificate cert, bool enable_rev_checking,
       bool require_rev_checking_local_anchors, bool enable_sha1_local_anchors,
-      bool disable_symantec_enforcement, CertVerifyResult primary_result,
+      bool disable_symantec_enforcement, array<uint8> stapled_ocsp,
+      array<uint8> sct_list, CertVerifyResult primary_result,
       CertVerifyResult trial_result, CertVerifierDebugInfo debug_info);
 };
 

services/network/trial_comparison_cert_verifier_mojo.cc

@@ -68,6 +68,8 @@ void TrialComparisonCertVerifierMojo::OnSendTrialReport(
     bool require_rev_checking_local_anchors,
     bool enable_sha1_local_anchors,
     bool disable_symantec_enforcement,
+    const std::string& stapled_ocsp,
+    const std::string& sct_list,
     const net::CertVerifyResult& primary_result,
     const net::CertVerifyResult& trial_result) {
   network::mojom::CertVerifierDebugInfoPtr debug_info =
@@ -117,8 +119,10 @@ void TrialComparisonCertVerifierMojo::OnSendTrialReport(
   report_client_->SendTrialReport(
       hostname, unverified_cert, enable_rev_checking,
       require_rev_checking_local_anchors, enable_sha1_local_anchors,
-      disable_symantec_enforcement, primary_result, trial_result,
-      std::move(debug_info));
+      disable_symantec_enforcement,
+      std::vector<uint8_t>(stapled_ocsp.begin(), stapled_ocsp.end()),
+      std::vector<uint8_t>(sct_list.begin(), sct_list.end()), primary_result,
+      trial_result, std::move(debug_info));
 }
 
 }  // namespace network

services/network/trial_comparison_cert_verifier_mojo.h

@@ -72,6 +72,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) TrialComparisonCertVerifierMojo
       bool require_rev_checking_local_anchors,
       bool enable_sha1_local_anchors,
       bool disable_symantec_enforcement,
+      const std::string& stapled_ocsp,
+      const std::string& sct_list,
       const net::CertVerifyResult& primary_result,
       const net::CertVerifyResult& trial_result);
 

services/network/trial_comparison_cert_verifier_mojo_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "services/network/trial_comparison_cert_verifier_mojo.h"
 
+#include "base/containers/span.h"
 #include "base/test/task_environment.h"
 #include "build/build_config.h"
 #include "net/cert/cert_verify_proc.h"
@@ -26,6 +27,8 @@ struct ReceivedReport {
   bool require_rev_checking_local_anchors;
   bool enable_sha1_local_anchors;
   bool disable_symantec_enforcement;
+  std::vector<uint8_t> stapled_ocsp;
+  std::vector<uint8_t> sct_list;
   net::CertVerifyResult primary_result;
   net::CertVerifyResult trial_result;
   network::mojom::CertVerifierDebugInfoPtr debug_info;
@@ -48,6 +51,8 @@ class FakeReportClient
       bool require_rev_checking_local_anchors,
       bool enable_sha1_local_anchors,
       bool disable_symantec_enforcement,
+      const std::vector<uint8_t>& stapled_ocsp,
+      const std::vector<uint8_t>& sct_list,
       const net::CertVerifyResult& primary_result,
       const net::CertVerifyResult& trial_result,
       network::mojom::CertVerifierDebugInfoPtr debug_info) override {
@@ -59,6 +64,8 @@ class FakeReportClient
         require_rev_checking_local_anchors;
     report.enable_sha1_local_anchors = enable_sha1_local_anchors;
     report.disable_symantec_enforcement = disable_symantec_enforcement;
+    report.stapled_ocsp = stapled_ocsp;
+    report.sct_list = sct_list;
     report.primary_result = primary_result;
     report.trial_result = trial_result;
     report.debug_info = std::move(debug_info);
@@ -143,7 +150,8 @@ TEST(TrialComparisonCertVerifierMojoTest, SendReportDebugInfo) {
       true, {}, std::move(report_client_remote), nullptr, nullptr);
 
   tccvm.OnSendTrialReport("example.com", unverified_cert, false, false, false,
-                          false, primary_result, trial_result);
+                          false, "stapled ocsp", "sct list", primary_result,
+                          trial_result);
 
   report_client.WaitForReport();
 
@@ -155,6 +163,10 @@ TEST(TrialComparisonCertVerifierMojoTest, SendReportDebugInfo) {
       chain1->EqualsIncludingChain(report.primary_result.verified_cert.get()));
   EXPECT_TRUE(
       chain2->EqualsIncludingChain(report.trial_result.verified_cert.get()));
+  EXPECT_EQ("stapled ocsp", std::string(report.stapled_ocsp.begin(),
+                                        report.stapled_ocsp.end()));
+  EXPECT_EQ("sct list",
+            std::string(report.sct_list.begin(), report.sct_list.end()));
 
   ASSERT_TRUE(report.debug_info);
 #if defined(OS_MACOSX) && !defined(OS_IOS)