Send the time used by CertVerifyProcBuiltin in TrialComparisonCertVerifier error reports.

Bug: 1008994
Change-Id: Icd416d5336844cc83f703fde2cc6d5e617653bf1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1830175
Commit-Queue: Matt Mueller <mattm@chromium.org>
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702066}

chrome/browser/ssl/cert_logger.proto

@@ -100,7 +100,8 @@ message CertLoggerRequest {
   // The certificate chain as a series of PEM-encoded certificates, including
   // intermediates but not necessarily the root.
   required string cert_chain = 2;
-  // The time (in usec since the epoch) when the client generated the report.
+  // The time (in usec since the Windows epoch) when the client generated the
+  // report.
   required int64 time_usec = 3;
   // public_key_hash contains the string forms of the hashes calculated for
   // the chain. (I.e. "sha1/<base64 data>".)
@@ -230,4 +231,12 @@ message TrialVerificationInfo {
   // on reports from macOS.  Contains the union of flags from all the GetTrust
   // calls done during verification.
   repeated MacTrustFlags mac_combined_trust_debug_info = 7;
+
+  // The time (in usec since the Windows epoch) when the trial verifier
+  // attempted to verify the chain.
+  optional int64 trial_verification_time = 8;
+
+  // The GeneralizedTime encoded time when the trial verifier attempted to
+  // verify the chain.
+  optional string trial_der_verification_time = 9;
 }

chrome/browser/ssl/certificate_error_report.cc

@@ -193,6 +193,15 @@ CertificateErrorReport::CertificateErrorReport(
       debug_info->mac_combined_trust_debug_info,
       trial_report->mutable_mac_combined_trust_debug_info());
 #endif
+  if (!debug_info->trial_verification_time.is_null()) {
+    trial_report->set_trial_verification_time(
+        debug_info->trial_verification_time.ToDeltaSinceWindowsEpoch()
+            .InMicroseconds());
+  }
+  if (!debug_info->trial_der_verification_time.empty()) {
+    trial_report->set_trial_der_verification_time(
+        debug_info->trial_der_verification_time);
+  }
 }
 #endif  // BUILDFLAG(TRIAL_COMPARISON_CERT_VERIFIER_SUPPORTED)
 
@@ -244,7 +253,7 @@ void CertificateErrorReport::SetInterstitialInfo(
   interstitial_info->set_user_proceeded(proceed_decision == USER_PROCEEDED);
   interstitial_info->set_overridable(overridable == INTERSTITIAL_OVERRIDABLE);
   interstitial_info->set_interstitial_created_time_usec(
-      interstitial_time.ToInternalValue());
+      interstitial_time.ToDeltaSinceWindowsEpoch().InMicroseconds());
 }
 
 void CertificateErrorReport::AddNetworkTimeInfo(
@@ -347,7 +356,7 @@ CertificateErrorReport::CertificateErrorReport(
     net::CertStatus cert_status)
     : cert_report_(new chrome_browser_ssl::CertLoggerRequest()) {
   base::Time now = base::Time::Now();
-  cert_report_->set_time_usec(now.ToInternalValue());
+  cert_report_->set_time_usec(now.ToDeltaSinceWindowsEpoch().InMicroseconds());
   cert_report_->set_hostname(hostname);
 
   if (!CertificateChainToString(cert, cert_report_->mutable_cert_chain())) {

chrome/browser/ssl/certificate_error_report_unittest.cc

@@ -176,7 +176,7 @@ TEST(ErrorReportTest, SerializedReportAsProtobufWithInterstitialInfo) {
       UnorderedElementsAre(kFirstReportedCertError, kSecondReportedCertError));
 
   EXPECT_EQ(
-      interstitial_time.ToInternalValue(),
+      interstitial_time.ToDeltaSinceWindowsEpoch().InMicroseconds(),
       deserialized_report.interstitial_info().interstitial_created_time_usec());
 }
 
@@ -352,6 +352,9 @@ TEST(ErrorReportTest, TrialDebugInfo) {
       net::TrustStoreMac::TRUST_SETTINGS_DICT_CONTAINS_APPLICATION |
       net::TrustStoreMac::TRUST_SETTINGS_DICT_CONTAINS_RESULT;
 #endif
+  base::Time time = base::Time::Now();
+  debug_info->trial_verification_time = time;
+  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,
@@ -376,6 +379,11 @@ TEST(ErrorReportTest, TrialDebugInfo) {
 #else
   EXPECT_EQ(0, trial_info.mac_combined_trust_debug_info_size());
 #endif
+  ASSERT_TRUE(trial_info.has_trial_verification_time());
+  EXPECT_EQ(time.ToDeltaSinceWindowsEpoch().InMicroseconds(),
+            trial_info.trial_verification_time());
+  ASSERT_TRUE(trial_info.has_trial_der_verification_time());
+  EXPECT_EQ("it's just a string", trial_info.trial_der_verification_time());
 }
 #endif  // BUILDFLAG(TRIAL_COMPARISON_CERT_VERIFIER_SUPPORTED)
 

net/cert/cert_verify_proc_builtin.cc

@@ -48,6 +48,8 @@ constexpr base::TimeDelta kPerAttemptMinVerificationTimeLimit =
 
 DEFINE_CERT_ERROR_ID(kPathLacksEVPolicy, "Path does not have an EV policy");
 
+const void* kResultDebugDataKey = &kResultDebugDataKey;
+
 RevocationPolicy NoRevocationChecking() {
   RevocationPolicy policy;
   policy.check_revocation = false;
@@ -614,6 +616,9 @@ int CertVerifyProcBuiltin::VerifyInternal(
     return ERR_CERT_AUTHORITY_INVALID;
   }
 
+  CertVerifyProcBuiltinResultDebugData::Create(verify_result, verification_time,
+                                               der_verification_time);
+
   // Parse the target certificate.
   scoped_refptr<ParsedCertificate> target =
       ParseCertificateFromBuffer(input_cert->cert_buffer(), &parsing_errors);
@@ -748,6 +753,36 @@ SystemTrustStoreProvider::CreateDefaultForSSL() {
   return std::make_unique<DefaultSystemTrustStoreProvider>();
 }
 
+CertVerifyProcBuiltinResultDebugData::CertVerifyProcBuiltinResultDebugData(
+    base::Time verification_time,
+    const der::GeneralizedTime& der_verification_time)
+    : verification_time_(verification_time),
+      der_verification_time_(der_verification_time) {}
+
+// static
+const CertVerifyProcBuiltinResultDebugData*
+CertVerifyProcBuiltinResultDebugData::Get(
+    const base::SupportsUserData* debug_data) {
+  return static_cast<CertVerifyProcBuiltinResultDebugData*>(
+      debug_data->GetUserData(kResultDebugDataKey));
+}
+
+// static
+void CertVerifyProcBuiltinResultDebugData::Create(
+    base::SupportsUserData* debug_data,
+    base::Time verification_time,
+    const der::GeneralizedTime& der_verification_time) {
+  debug_data->SetUserData(
+      kResultDebugDataKey,
+      std::make_unique<CertVerifyProcBuiltinResultDebugData>(
+          verification_time, der_verification_time));
+}
+
+std::unique_ptr<base::SupportsUserData::Data>
+CertVerifyProcBuiltinResultDebugData::Clone() {
+  return std::make_unique<CertVerifyProcBuiltinResultDebugData>(*this);
+}
+
 scoped_refptr<CertVerifyProc> CreateCertVerifyProcBuiltin(
     scoped_refptr<CertNetFetcher> net_fetcher,
     std::unique_ptr<SystemTrustStoreProvider> system_trust_store_provider) {

net/cert/cert_verify_proc_builtin.h

@@ -8,7 +8,9 @@
 #include <memory>
 
 #include "base/memory/ref_counted.h"
+#include "base/supports_user_data.h"
 #include "net/base/net_export.h"
+#include "net/der/parse_values.h"
 
 namespace net {
 
@@ -35,6 +37,32 @@ class NET_EXPORT SystemTrustStoreProvider {
   virtual std::unique_ptr<SystemTrustStore> CreateSystemTrustStore() = 0;
 };
 
+class NET_EXPORT CertVerifyProcBuiltinResultDebugData
+    : public base::SupportsUserData::Data {
+ public:
+  CertVerifyProcBuiltinResultDebugData(
+      base::Time verification_time,
+      const der::GeneralizedTime& der_verification_time);
+
+  static const CertVerifyProcBuiltinResultDebugData* Get(
+      const base::SupportsUserData* debug_data);
+  static void Create(base::SupportsUserData* debug_data,
+                     base::Time verification_time,
+                     const der::GeneralizedTime& der_verification_time);
+
+  // base::SupportsUserData::Data implementation:
+  std::unique_ptr<Data> Clone() override;
+
+  base::Time verification_time() const { return verification_time_; }
+  const der::GeneralizedTime& der_verification_time() const {
+    return der_verification_time_;
+  }
+
+ private:
+  base::Time verification_time_;
+  der::GeneralizedTime der_verification_time_;
+};
+
 // TODO(crbug.com/649017): This is not how other cert_verify_proc_*.h are
 // implemented -- they expose the type in the header. Use a consistent style
 // here too.

net/cert/cert_verify_proc_builtin_unittest.cc

@@ -13,6 +13,7 @@
 #include "net/cert/ev_root_ca_metadata.h"
 #include "net/cert/internal/system_trust_store.h"
 #include "net/cert_net/cert_net_fetcher_impl.h"
+#include "net/der/encode_values.h"
 #include "net/log/net_log_with_source.h"
 #include "net/test/cert_builder.h"
 #include "net/test/cert_test_util.h"
@@ -370,4 +371,38 @@ TEST_F(CertVerifyProcBuiltinTest, EVRevocationCheckDeadline) {
 }
 #endif  // defined(PLATFORM_USES_CHROMIUM_EV_METADATA)
 
+TEST_F(CertVerifyProcBuiltinTest, DebugData) {
+  std::unique_ptr<CertBuilder> leaf, intermediate, root;
+  CreateChain(&leaf, &intermediate, &root);
+  ASSERT_TRUE(leaf && intermediate && root);
+
+  scoped_refptr<X509Certificate> chain = leaf->GetX509CertificateChain();
+  ASSERT_TRUE(chain.get());
+
+  base::Time time = base::Time::Now();
+
+  CertVerifyResult verify_result;
+  TestCompletionCallback callback;
+  Verify(chain.get(), "www.example.com", /*flags=*/0,
+         /*additional_trust_anchors=*/{root->GetX509Certificate()},
+         &verify_result, callback.callback());
+
+  int error = callback.WaitForResult();
+  EXPECT_THAT(error, IsOk());
+
+  auto* debug_data = CertVerifyProcBuiltinResultDebugData::Get(&verify_result);
+  ASSERT_TRUE(debug_data);
+  // No delayed tasks involved, so the mock time should not have advanced.
+  EXPECT_EQ(time, debug_data->verification_time());
+
+  base::Time der_verification_time_converted_back_to_base_time;
+  EXPECT_TRUE(net::der::GeneralizedTimeToTime(
+      debug_data->der_verification_time(),
+      &der_verification_time_converted_back_to_base_time));
+  // GeneralizedTime only has seconds precision.
+  EXPECT_EQ(
+      0,
+      (time - der_verification_time_converted_back_to_base_time).InSeconds());
+}
+
 }  // namespace net

services/network/public/mojom/trial_comparison_cert_verifier.mojom

@@ -10,6 +10,7 @@
 
 module network.mojom;
 
+import "mojo/public/mojom/base/time.mojom";
 import "services/network/public/mojom/network_param.mojom";
 
 // Receives cert verifier trial configuration updates.
@@ -24,6 +25,11 @@ struct CertVerifierDebugInfo {
   // union of flags from all the GetTrust calls done during verification.
   [EnableIf=is_mac]
   int32 mac_combined_trust_debug_info;
+
+  // The time as seen by CertVerifyProcBuiltin, in raw timestamp and in
+  // exploded & encoded GeneralizedTime string.
+  mojo_base.mojom.Time trial_verification_time;
+  string trial_der_verification_time;
 };
 
 // Sends reports of differences found in the cert verifier trial.

services/network/trial_comparison_cert_verifier_mojo.cc

@@ -8,7 +8,10 @@
 
 #include "build/build_config.h"
 #include "net/cert/cert_verify_proc.h"
+#include "net/cert/cert_verify_proc_builtin.h"
 #include "net/cert/trial_comparison_cert_verifier.h"
+#include "net/der/encode_values.h"
+#include "net/der/parse_values.h"
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
 #include "net/cert/internal/trust_store_mac.h"
@@ -73,6 +76,20 @@ void TrialComparisonCertVerifierMojo::OnSendTrialReport(
         mac_trust_debug_info->combined_trust_debug_info();
   }
 #endif
+  auto* cert_verify_proc_builtin_debug_data =
+      net::CertVerifyProcBuiltinResultDebugData::Get(&trial_result);
+  if (cert_verify_proc_builtin_debug_data) {
+    debug_info->trial_verification_time =
+        cert_verify_proc_builtin_debug_data->verification_time();
+    uint8_t encoded_generalized_time[net::der::kGeneralizedTimeLength];
+    if (net::der::EncodeGeneralizedTime(
+            cert_verify_proc_builtin_debug_data->der_verification_time(),
+            encoded_generalized_time)) {
+      debug_info->trial_der_verification_time = std::string(
+          encoded_generalized_time,
+          encoded_generalized_time + net::der::kGeneralizedTimeLength);
+    }
+  }
 
   report_client_->SendTrialReport(
       hostname, unverified_cert, enable_rev_checking,

services/network/trial_comparison_cert_verifier_mojo_unittest.cc

@@ -7,6 +7,9 @@
 #include "base/test/task_environment.h"
 #include "build/build_config.h"
 #include "net/cert/cert_verify_proc.h"
+#include "net/cert/cert_verify_proc_builtin.h"
+#include "net/der/encode_values.h"
+#include "net/der/parse_values.h"
 #include "net/test/cert_test_util.h"
 #include "net/test/test_data_directory.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -100,6 +103,17 @@ TEST(TrialComparisonCertVerifierMojoTest, SendReportDebugInfo) {
   mac_trust_debug_info->UpdateTrustDebugInfo(kExpectedTrustDebugInfo);
 #endif
 
+  base::Time time = base::Time::Now();
+  net::der::GeneralizedTime der_time;
+  der_time.year = 2019;
+  der_time.month = 9;
+  der_time.day = 27;
+  der_time.hours = 22;
+  der_time.minutes = 11;
+  der_time.seconds = 8;
+  net::CertVerifyProcBuiltinResultDebugData::Create(&trial_result, time,
+                                                    der_time);
+
   network::mojom::TrialComparisonCertVerifierReportClientPtrInfo
       report_client_ptr;
   FakeReportClient report_client(mojo::MakeRequest(&report_client_ptr));
@@ -125,4 +139,7 @@ TEST(TrialComparisonCertVerifierMojoTest, SendReportDebugInfo) {
   EXPECT_EQ(kExpectedTrustDebugInfo,
             report.debug_info->mac_combined_trust_debug_info);
 #endif
+
+  EXPECT_EQ(time, report.debug_info->trial_verification_time);
+  EXPECT_EQ("20190927221108Z", report.debug_info->trial_der_verification_time);
 }