[SCT-Auditing] Update SCT auditing with newer report proto

This CL updates Chrome to use a newer variant of the report proto. It
does not change any logic in Chrome otherwise.

In particular, the updated proto supports submitting SCTs for multiple
certificates at once. This CL does not change Chrome logic to aggregate
across reports. As a result, every report has exactly one certificate
report in it.

Bug: b/175493719
Change-Id: I3586998f04846509a589b7f09fa65fe949a8e4ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2588811Reviewed-by: Chris Thompson <cthomp@chromium.org>
Reviewed-by: Carlos IL <carlosil@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843405}

chrome/browser/ssl/sct_reporting_service_browsertest.cc

@@ -183,8 +183,8 @@ class SCTReportingServiceBrowserTest : public CertVerifierBrowserTest {
 
   size_t requests_seen() { return requests_seen_; }
 
-  sct_auditing::TLSConnectionReport GetLastSeenReport() {
-    sct_auditing::TLSConnectionReport auditing_report;
+  sct_auditing::SCTClientReport GetLastSeenReport() {
+    sct_auditing::SCTClientReport auditing_report;
     if (last_seen_request_.has_content)
       auditing_report.ParseFromString(last_seen_request_.content);
     return auditing_report;
@@ -202,8 +202,11 @@ class SCTReportingServiceBrowserTest : public CertVerifierBrowserTest {
         https_server()->GetURL("flush-and-check-zero-reports.test", "/"));
     WaitForRequests(1);
     return (1u == requests_seen() &&
-            "flush-and-check-zero-reports.test" ==
-                GetLastSeenReport().context().origin().hostname());
+            "flush-and-check-zero-reports.test" == GetLastSeenReport()
+                                                       .certificate_report(0)
+                                                       .context()
+                                                       .origin()
+                                                       .hostname());
   }
 
  private:
@@ -258,7 +261,9 @@ IN_PROC_BROWSER_TEST_F(SCTReportingServiceBrowserTest,
 
   // Check that one report was sent and contains the expected details.
   EXPECT_EQ(1u, requests_seen());
-  EXPECT_EQ("a.test", GetLastSeenReport().context().origin().hostname());
+  EXPECT_EQ(
+      "a.test",
+      GetLastSeenReport().certificate_report(0).context().origin().hostname());
 }
 
 // Tests that disabling Safe Browsing entirely should cause reports to not get
@@ -312,7 +317,9 @@ IN_PROC_BROWSER_TEST_F(SCTReportingServiceBrowserTest,
 
   // Check that one report was sent.
   EXPECT_EQ(1u, requests_seen());
-  EXPECT_EQ("a.test", GetLastSeenReport().context().origin().hostname());
+  EXPECT_EQ(
+      "a.test",
+      GetLastSeenReport().certificate_report(0).context().origin().hostname());
 
   // Disable Extended Reporting which should clear the underlying cache.
   SetExtendedReportingEnabled(false);
@@ -324,7 +331,9 @@ IN_PROC_BROWSER_TEST_F(SCTReportingServiceBrowserTest,
                                https_server()->GetURL("a.test", "/"));
   WaitForRequests(2);
   EXPECT_EQ(2u, requests_seen());
-  EXPECT_EQ("a.test", GetLastSeenReport().context().origin().hostname());
+  EXPECT_EQ(
+      "a.test",
+      GetLastSeenReport().certificate_report(0).context().origin().hostname());
 }
 
 // Tests that reports are still sent for opted-in profiles after the network
@@ -377,7 +386,9 @@ IN_PROC_BROWSER_TEST_F(SCTReportingServiceBrowserTest,
 
   // Check that one report was enqueued.
   EXPECT_EQ(1u, requests_seen());
-  EXPECT_EQ("a.test", GetLastSeenReport().context().origin().hostname());
+  EXPECT_EQ(
+      "a.test",
+      GetLastSeenReport().certificate_report(0).context().origin().hostname());
 }
 
 // Tests that invalid SCTs don't get reported when the overall result is
@@ -421,7 +432,7 @@ IN_PROC_BROWSER_TEST_F(SCTReportingServiceBrowserTest,
   EXPECT_EQ(1u, requests_seen());
 
   auto report = GetLastSeenReport();
-  EXPECT_EQ(3, report.included_scts_size());
+  EXPECT_EQ(3, report.certificate_report(0).included_sct_size());
 }
 
 // Tests that invalid SCTs don't get included when the overall result is
@@ -461,7 +472,7 @@ IN_PROC_BROWSER_TEST_F(SCTReportingServiceBrowserTest,
   EXPECT_EQ(1u, requests_seen());
 
   auto report = GetLastSeenReport();
-  EXPECT_EQ(1, report.included_scts_size());
+  EXPECT_EQ(1, report.certificate_report(0).included_sct_size());
 }
 
 IN_PROC_BROWSER_TEST_F(SCTReportingServiceBrowserTest, NoValidSCTsNoReport) {

services/network/BUILD.gn

@@ -267,7 +267,10 @@ component("network_service") {
       "sct_auditing_cache.cc",
       "sct_auditing_cache.h",
     ]
-    deps += [ "//components/certificate_transparency" ]
+    deps += [
+      "//components/certificate_transparency",
+      "//components/version_info",
+    ]
   }
 
   if (is_linux || is_chromeos) {

services/network/DEPS

@@ -9,6 +9,7 @@ include_rules = [
   # store for networking-related data (Like which servers support QUIC), rather
   # than to store user preferences.
   "+components/prefs",
+  "+components/version_info",
   "+components/web_package",
   "+crypto",
   "+ipc",

services/network/public/proto/sct_audit_report.proto

@@ -13,16 +13,26 @@ option optimize_for = LITE_RUNTIME;
 
 package sct_auditing;
 
+// SCTClientReport represents a single report from a client, containing reports
+// from multiple independent connections, each of which contain multiple SCTs.
+message SCTClientReport {
+  // The simplified user agent of the submitter (e.g.  Chrome/xy.0.abcd.e).
+  // Supplements API keys to provide per-version granularity.
+  string user_agent = 1;
+
+  repeated TLSConnectionReport certificate_report = 2;
+}
+
 // TLSConnectionReport is the primary per-handshake report for SCT auditing.
 message TLSConnectionReport {
   TLSConnectionContext context = 1;
 
-  // SCTs may appear in any order. See SCTWithSourceAndVerifyStatus for details.
-  repeated SCTWithSourceAndVerifyStatus included_scts = 2;
+  // SCTs may appear in any order. See SCTWithVerifyStatus for details.
+  repeated SCTWithVerifyStatus included_sct = 2;
 }
 
-// TLSConnectionContext contains details about the connection that are common
-// to all SCTs observed in that connection.
+// TLSConnectionContext contains details about the connection that are common to
+// all SCTs observed in that connection.
 message TLSConnectionContext {
   // Time when the UA observed the certificate in seconds since the Unix epoch.
   int64 time_seen = 1;
@@ -41,9 +51,9 @@ message TLSConnectionContext {
   repeated bytes certificate_chain = 3;
 }
 
-// SCTWithSourceAndVerifyStatus contains the raw SCT, where it was found in the
-// certificate, and its validation status according to the UA.
-message SCTWithSourceAndVerifyStatus {
+// SCTWithVerifyStatus contains the serialized SCT along with the validation
+// status according to the UA.
+message SCTWithVerifyStatus {
   // Keep sync'd with SctVerifyStatus in Chrome's net/cert/sct_status_flags.h.
   enum SctVerifyStatus {
     // Default to unspecified status.  See go/unspecified-enum.
@@ -67,15 +77,7 @@ message SCTWithSourceAndVerifyStatus {
   }
   SctVerifyStatus status = 1;
 
-  // The source is the manner in which the client received the SCT (embedded in
-  // the certificate, delivered via the TLS handshake, or delivered via OCSP).
-  enum Source {
-    SOURCE_UNSPECIFIED = 0;
-    EMBEDDED = 1;
-    TLS_EXTENSION = 2;
-    OCSP_RESPONSE = 3;
-  }
-  Source source = 2;
-
-  bytes sct = 3;
+  // SignedCertificateTimestamp struct serialized via
+  // net::ct::EncodeSignedCertificateTimestamp().
+  bytes serialized_sct = 2;
 }

services/network/sct_auditing_cache.cc

@@ -9,6 +9,7 @@
 #include "base/metrics/field_trial_params.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/rand_util.h"
+#include "components/version_info/version_info.h"
 #include "crypto/secure_hash.h"
 #include "crypto/sha2.h"
 #include "net/base/elements_upload_data_stream.h"
@@ -43,24 +44,6 @@ namespace {
 
 constexpr int kSendSCTReportTimeoutSeconds = 30;
 
-sct_auditing::SCTWithSourceAndVerifyStatus::Source MapSCTOriginToSource(
-    net::ct::SignedCertificateTimestamp::Origin origin) {
-  switch (origin) {
-    case net::ct::SignedCertificateTimestamp::Origin::SCT_EMBEDDED:
-      return sct_auditing::SCTWithSourceAndVerifyStatus::Source::
-          SCTWithSourceAndVerifyStatus_Source_EMBEDDED;
-    case net::ct::SignedCertificateTimestamp::Origin::SCT_FROM_TLS_EXTENSION:
-      return sct_auditing::SCTWithSourceAndVerifyStatus::Source::
-          SCTWithSourceAndVerifyStatus_Source_TLS_EXTENSION;
-    case net::ct::SignedCertificateTimestamp::Origin::SCT_FROM_OCSP_RESPONSE:
-      return sct_auditing::SCTWithSourceAndVerifyStatus::Source::
-          SCTWithSourceAndVerifyStatus_Source_OCSP_RESPONSE;
-    default:
-      return sct_auditing::SCTWithSourceAndVerifyStatus::Source::
-          SCTWithSourceAndVerifyStatus_Source_SOURCE_UNSPECIFIED;
-  }
-}
-
 // Records the high-water mark of the cache size (in number of reports).
 void RecordSCTAuditingCacheHighWaterMarkMetrics(size_t hwm) {
   base::UmaHistogramCounts1000("Security.SCTAuditing.OptIn.CacheHWM", hwm);
@@ -153,7 +136,8 @@ void SCTAuditingCache::MaybeEnqueueReport(
   if (!enabled_ || !context->is_sct_auditing_enabled())
     return;
 
-  auto report = std::make_unique<sct_auditing::TLSConnectionReport>();
+  auto report = std::make_unique<sct_auditing::SCTClientReport>();
+  auto* tls_report = report->add_certificate_report();
 
   // Encode the SCTs in the report and generate the cache key. The hash of the
   // SCTs is used as the cache key to deduplicate reports with the same SCTs.
@@ -168,22 +152,21 @@ void SCTAuditingCache::MaybeEnqueueReport(
     if (sct.status != net::ct::SCT_STATUS_OK)
       continue;
 
-    auto* sct_source_and_status = report->add_included_scts();
+    auto* sct_source_and_status = tls_report->add_included_sct();
     // TODO(crbug.com/1082860): Update the proto to remove the status entirely
     // since only valid SCTs are reported now.
     sct_source_and_status->set_status(
-        sct_auditing::SCTWithSourceAndVerifyStatus::SctVerifyStatus::
-            SCTWithSourceAndVerifyStatus_SctVerifyStatus_OK);
+        sct_auditing::SCTWithVerifyStatus::SctVerifyStatus::
+            SCTWithVerifyStatus_SctVerifyStatus_OK);
 
-    sct_source_and_status->set_source(MapSCTOriginToSource(sct.sct->origin));
     net::ct::EncodeSignedCertificateTimestamp(
-        sct.sct, sct_source_and_status->mutable_sct());
+        sct.sct, sct_source_and_status->mutable_serialized_sct());
 
-    SHA256_Update(&ctx, sct_source_and_status->sct().data(),
-                  sct_source_and_status->sct().size());
+    SHA256_Update(&ctx, sct_source_and_status->serialized_sct().data(),
+                  sct_source_and_status->serialized_sct().size());
   }
   // Don't handle reports if there were no valid SCTs.
-  if (report->included_scts().empty())
+  if (tls_report->included_sct().empty())
     return;
 
   net::SHA256HashValue cache_key;
@@ -198,6 +181,8 @@ void SCTAuditingCache::MaybeEnqueueReport(
   }
   RecordSCTAuditingReportDeduplicatedMetrics(false);
 
+  report->set_user_agent(version_info::GetProductNameAndVersionForUserAgent());
+
   // Set the `cache_key` with an null report. If we don't choose to sample these
   // SCTs, then we don't need to store a report as we won't reference it again
   // (and can save on memory usage). If we do choose to sample these SCTs, we
@@ -210,7 +195,7 @@ void SCTAuditingCache::MaybeEnqueueReport(
   }
   RecordSCTAuditingReportSampledMetrics(true);
 
-  auto* connection_context = report->mutable_context();
+  auto* connection_context = tls_report->mutable_context();
   base::TimeDelta time_since_unix_epoch =
       base::Time::Now() - base::Time::UnixEpoch();
   connection_context->set_time_seen(time_since_unix_epoch.InSeconds());
@@ -241,7 +226,7 @@ void SCTAuditingCache::MaybeEnqueueReport(
   SendReport(cache_key);
 }
 
-sct_auditing::TLSConnectionReport* SCTAuditingCache::GetPendingReport(
+sct_auditing::SCTClientReport* SCTAuditingCache::GetPendingReport(
     const net::SHA256HashValue& cache_key) {
   auto it = cache_.Get(cache_key);
   if (it == cache_.end())

services/network/sct_auditing_cache.h

@@ -61,7 +61,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) SCTAuditingCache {
       const net::SignedCertificateTimestampAndStatusList&
           signed_certificate_timestamps);
 
-  sct_auditing::TLSConnectionReport* GetPendingReport(
+  sct_auditing::SCTClientReport* GetPendingReport(
       const net::SHA256HashValue& cache_key);
 
   // Sends the report associated with `cache_key` to `report_uri` (which is
@@ -84,7 +84,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) SCTAuditingCache {
   }
 
   base::MRUCache<net::SHA256HashValue,
-                 std::unique_ptr<sct_auditing::TLSConnectionReport>>*
+                 std::unique_ptr<sct_auditing::SCTClientReport>>*
   GetCacheForTesting() {
     return &cache_;
   }
@@ -95,7 +95,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) SCTAuditingCache {
                         int http_response_code);
 
   base::MRUCache<net::SHA256HashValue,
-                 std::unique_ptr<sct_auditing::TLSConnectionReport>>
+                 std::unique_ptr<sct_auditing::SCTClientReport>>
       cache_;
 
   bool enabled_ = false;

services/network/sct_auditing_cache_unittest.cc

@@ -213,7 +213,9 @@ TEST_F(SCTAuditingCacheTest, EvictLRUAfterCacheFull) {
                              chain_.get(), sct_list);
     ASSERT_EQ(2u, cache.GetCacheForTesting()->size());
     for (const auto& entry : *cache.GetCacheForTesting()) {
-      ASSERT_NE("example1.com", entry.second->context().origin().hostname());
+      ASSERT_NE(
+          "example1.com",
+          entry.second->certificate_report(0).context().origin().hostname());
     }
   }
 }
@@ -287,7 +289,9 @@ TEST_F(SCTAuditingCacheTest, DeduplicationUpdatesLastSeenTime) {
 
   EXPECT_EQ(2u, cache.GetCacheForTesting()->size());
   for (const auto& entry : *cache.GetCacheForTesting()) {
-    ASSERT_NE("example2.com", entry.second->context().origin().hostname());
+    ASSERT_NE(
+        "example2.com",
+        entry.second->certificate_report(0).context().origin().hostname());
   }
 }
 
@@ -531,9 +535,10 @@ TEST_F(SCTAuditingCacheTest, ReportsOnlyIncludesValidSCTs) {
 
   // No invalid SCTs should be in any reports in the cache.
   for (const auto& entry : *cache.GetCacheForTesting()) {
-    for (auto& sct_and_status : entry.second->included_scts()) {
+    for (auto& sct_and_status :
+         entry.second->certificate_report(0).included_sct()) {
       // Decode the SCT and check that only the valid SCT was included.
-      base::StringPiece encoded_sct(sct_and_status.sct());
+      base::StringPiece encoded_sct(sct_and_status.serialized_sct());
       scoped_refptr<net::ct::SignedCertificateTimestamp> decoded_sct;
       ASSERT_TRUE(net::ct::DecodeSignedCertificateTimestamp(&encoded_sct,
                                                             &decoded_sct));