Replace SCTReport struct with proto

This replaces the SCTReport struct used to store SCT audit reports in
the SCTAuditingCache with the TLSConnectionReport proto we will use
for sending reports to the server. Creating the proto at the time of
adding the report to the cache avoids having to later translate the
SCTReport into the final proto form, and lets us remove the SCTReport
struct entirely.

Bug: 1082860
Change-Id: Ib5848744fb79d33ce50f0909531ff983dc55af54
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2333302Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Christopher Thompson <cthomp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805943}

content/browser/web_package/signed_exchange_handler_unittest.cc

@@ -31,6 +31,7 @@
 #include "net/cert/ct_policy_enforcer.h"
 #include "net/cert/ct_verifier.h"
 #include "net/cert/mock_cert_verifier.h"
+#include "net/cert/sct_auditing_delegate.h"
 #include "net/cert/signed_certificate_timestamp_and_status.h"
 #include "net/filter/mock_source_stream.h"
 #include "net/http/transport_security_state.h"

services/network/BUILD.gn

@@ -387,6 +387,7 @@ source_set("tests") {
     "//services/network/public/cpp:buildflags",
     "//services/network/public/cpp/cert_verifier:cert_verifier_creation",
     "//services/network/public/mojom",
+    "//services/network/public/proto",
     "//services/network/trust_tokens",
     "//services/network/trust_tokens:tests",
     "//services/service_manager/public/cpp",

services/network/network_context.cc

@@ -122,6 +122,7 @@
 #include "net/cert/ct_log_verifier.h"
 #include "net/cert/multi_log_ct_verifier.h"
 #include "services/network/expect_ct_reporter.h"
+#include "services/network/sct_auditing_cache.h"
 #endif  // BUILDFLAG(IS_CT_SUPPORTED)
 
 #if !BUILDFLAG(DISABLE_FTP_SUPPORT)

services/network/network_context.h

@@ -54,7 +54,6 @@
 #include "services/network/public/mojom/udp_socket.mojom.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
 #include "services/network/public/mojom/websocket.mojom.h"
-#include "services/network/sct_auditing_cache.h"
 #include "services/network/socket_factory.h"
 #include "services/network/url_request_context_owner.h"
 

services/network/network_service.cc

@@ -62,7 +62,6 @@
 #include "services/network/public/cpp/initiator_lock_compatibility.h"
 #include "services/network/public/cpp/load_info_util.h"
 #include "services/network/public/cpp/network_switches.h"
-#include "services/network/sct_auditing_cache.h"
 #include "services/network/url_loader.h"
 
 #if defined(OS_ANDROID) && defined(ARCH_CPU_ARMEL)
@@ -79,6 +78,10 @@
 #include "net/android/http_auth_negotiate_android.h"
 #endif
 
+#if BUILDFLAG(IS_CT_SUPPORTED)
+#include "services/network/sct_auditing_cache.h"
+#endif
+
 namespace network {
 
 namespace {

services/network/public/proto/BUILD.gn

@@ -2,12 +2,23 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//services/network/public/cpp/features.gni")
 import("//third_party/protobuf/proto_library.gni")
 
 group("proto") {
   public_deps = [ ":tls_deprecation_config_proto" ]
+
+  if (is_ct_supported) {
+    public_deps += [ ":sct_audit_report_proto" ]
+  }
 }
 
 proto_library("tls_deprecation_config_proto") {
   sources = [ "tls_deprecation_config.proto" ]
 }
+
+if (is_ct_supported) {
+  proto_library("sct_audit_report_proto") {
+    sources = [ "sct_audit_report.proto" ]
+  }
+}

services/network/public/proto/sct_audit_report.proto

+// Copyright 2020 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.
+
+// Protocol for sending SCT auditing reports.
+//
+// If any changes are made to this file, the server-side proto definitions must
+// be updated as well.
+
+syntax = "proto3";
+
+option optimize_for = LITE_RUNTIME;
+
+package sct_auditing;
+
+// 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;
+}
+
+// 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;
+
+  // The origin that the UA connected to.
+  message Origin {
+    string hostname = 1;
+    int32 port = 2;
+    // Implicitly, scheme is HTTPS/WSS.
+  }
+  Origin origin = 2;
+
+  // The certificate chain as constructed by the UA.  Each entry is a
+  // DER-encoded X.509 certificate as described in RFC7468.  Order matches the
+  // order validated, with the first cert representing the end-entity (leaf).
+  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 {
+  // Keep sync'd with SctVerifyStatus in Chrome's net/cert/sct_status_flags.h.
+  enum SctVerifyStatus {
+    // Default to unspecified status.  See go/unspecified-enum.
+    NONE = 0;
+
+    // The SCT is from an unknown log, so we cannot verify its signature.
+    LOG_UNKNOWN = 1;
+
+    // Obsolete in net/cert/sct_status_flags.h. Included to remain consistent.
+    reserved 2;
+    reserved "INVALID";
+
+    // The SCT is from a known log, and the signature is valid.
+    OK = 3;
+
+    // The SCT is from a known log, but the signature is invalid.
+    INVALID_SIGNATURE = 4;
+
+    // The SCT is from a known log, but the timestamp is in the future.
+    INVALID_TIMESTAMP = 5;
+  }
+  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;
+}

services/network/sct_auditing_cache.cc

@@ -18,15 +18,54 @@
 #include "services/network/network_context.h"
 #include "services/network/network_service.h"
 #include "services/network/public/cpp/features.h"
+#include "services/network/public/proto/sct_audit_report.pb.h"
 #include "third_party/boringssl/src/include/openssl/pool.h"
 #include "third_party/boringssl/src/include/openssl/sha.h"
 
 namespace network {
 
-SCTAuditReport::SCTAuditReport() = default;
-SCTAuditReport::~SCTAuditReport() = default;
-SCTAuditReport::SCTAuditReport(const SCTAuditReport& other) = default;
-SCTAuditReport::SCTAuditReport(SCTAuditReport&& other) = default;
+namespace {
+
+sct_auditing::SCTWithSourceAndVerifyStatus::SctVerifyStatus
+MapSCTVerifyStatusToProtoStatus(net::ct::SCTVerifyStatus status) {
+  switch (status) {
+    case net::ct::SCTVerifyStatus::SCT_STATUS_NONE:
+      return sct_auditing::SCTWithSourceAndVerifyStatus::SctVerifyStatus::
+          SCTWithSourceAndVerifyStatus_SctVerifyStatus_NONE;
+    case net::ct::SCTVerifyStatus::SCT_STATUS_LOG_UNKNOWN:
+      return sct_auditing::SCTWithSourceAndVerifyStatus::SctVerifyStatus::
+          SCTWithSourceAndVerifyStatus_SctVerifyStatus_LOG_UNKNOWN;
+    case net::ct::SCTVerifyStatus::SCT_STATUS_OK:
+      return sct_auditing::SCTWithSourceAndVerifyStatus::SctVerifyStatus::
+          SCTWithSourceAndVerifyStatus_SctVerifyStatus_OK;
+    case net::ct::SCTVerifyStatus::SCT_STATUS_INVALID_SIGNATURE:
+      return sct_auditing::SCTWithSourceAndVerifyStatus::SctVerifyStatus::
+          SCTWithSourceAndVerifyStatus_SctVerifyStatus_INVALID_SIGNATURE;
+    case net::ct::SCTVerifyStatus::SCT_STATUS_INVALID_TIMESTAMP:
+      return sct_auditing::SCTWithSourceAndVerifyStatus::SctVerifyStatus::
+          SCTWithSourceAndVerifyStatus_SctVerifyStatus_INVALID_TIMESTAMP;
+  }
+}
+
+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;
+  }
+}
+
+}  // namespace
 
 SCTAuditingCache::SCTAuditingCache(size_t cache_size) : cache_(cache_size) {}
 SCTAuditingCache::~SCTAuditingCache() = default;
@@ -62,24 +101,40 @@ void SCTAuditingCache::MaybeEnqueueReport(
     return;
 
   // Insert SCTs into cache.
-  // TODO(crbug.com/1082860): Construct the proto object directly and store that
-  // in the cache instead of this intermediate form, once the proto is added.
-  auto report = std::make_unique<SCTAuditReport>();
-  report->time_seen = base::Time::Now();
-  report->host_port_pair = host_port_pair;
-
-  // GetPEMEncodedChain() can fail, but we still want to enqueue the report for
-  // the SCTs (in that case, |report->certificate_chain| is not guaranteed to
-  // be valid).
-  report->certificate_chain = std::vector<std::string>();
-  validated_certificate_chain->GetPEMEncodedChain(&report->certificate_chain);
+  auto report = std::make_unique<sct_auditing::TLSConnectionReport>();
+  auto* connection_context = 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());
+  auto* origin = connection_context->mutable_origin();
+  origin->set_hostname(host_port_pair.host());
+  origin->set_port(host_port_pair.port());
+
+  // Convert the certificate chain to a PEM encoded vector, and then initialize
+  // the proto's |certificate_chain| repeated field using the data in the
+  // vector. Note that GetPEMEncodedChain() can fail, but we still want to
+  // enqueue the report for the SCTs (in that case, |certificate_chain| is not
+  // guaranteed to be valid).
+  std::vector<std::string> certificate_chain;
+  validated_certificate_chain->GetPEMEncodedChain(&certificate_chain);
+  *connection_context->mutable_certificate_chain() = {certificate_chain.begin(),
+                                                      certificate_chain.end()};
 
   for (const auto& sct : signed_certificate_timestamps) {
-    report->sct_list.push_back(sct);
+    auto* sct_source_and_status = report->add_included_scts();
+    sct_source_and_status->set_status(
+        MapSCTVerifyStatusToProtoStatus(sct.status));
+    sct_source_and_status->set_source(MapSCTOriginToSource(sct.sct->origin));
+    std::string serialized_sct;
+    net::ct::EncodeSignedCertificateTimestamp(sct.sct, &serialized_sct);
+    sct_source_and_status->set_sct(serialized_sct);
   }
 
   cache_.Put(cache_key, std::move(report));
 
+  // TODO(1082860): We should optimize memory usage by only storing an empty
+  // report for items we don't sample.
   double sampling_rate = features::kSCTAuditingSamplingRate.Get();
   if (base::RandDouble() > sampling_rate)
     return;
@@ -87,7 +142,7 @@ void SCTAuditingCache::MaybeEnqueueReport(
   context->client()->OnSCTReportReady(net::HashValue(cache_key).ToString());
 }
 
-SCTAuditReport* SCTAuditingCache::GetPendingReport(
+sct_auditing::TLSConnectionReport* SCTAuditingCache::GetPendingReport(
     const net::SHA256HashValue& cache_key) {
   NOTIMPLEMENTED();
   return nullptr;

services/network/sct_auditing_cache.h

@@ -15,6 +15,7 @@
 #include "net/base/host_port_pair.h"
 #include "net/cert/sct_auditing_delegate.h"
 #include "net/cert/signed_certificate_timestamp_and_status.h"
+#include "services/network/public/proto/sct_audit_report.pb.h"
 
 namespace net {
 class X509Certificate;
@@ -23,21 +24,6 @@ class X509Certificate;
 namespace network {
 class NetworkContext;
 
-struct SCTAuditReport {
-  SCTAuditReport();
-  ~SCTAuditReport();
-
-  SCTAuditReport(const SCTAuditReport& other);
-  SCTAuditReport& operator=(const SCTAuditReport& other) = default;
-  SCTAuditReport(SCTAuditReport&& other);
-  SCTAuditReport& operator=(SCTAuditReport&& other) = default;
-
-  base::Time time_seen;
-  net::HostPortPair host_port_pair;
-  std::vector<std::string> certificate_chain;
-  std::vector<net::SignedCertificateTimestampAndStatus> sct_list;
-};
-
 // SCTAuditingCache tracks SCTs seen during CT verification. The cache supports
 // a configurable sample rate to reduce load, and deduplicates SCTs seen more
 // than once. The cache evicts least-recently-used entries after it reaches its
@@ -69,17 +55,21 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) SCTAuditingCache {
       const net::SignedCertificateTimestampAndStatusList&
           signed_certificate_timestamps);
 
-  SCTAuditReport* GetPendingReport(const net::SHA256HashValue& cache_key);
+  sct_auditing::TLSConnectionReport* GetPendingReport(
+      const net::SHA256HashValue& cache_key);
 
   void ClearCache();
 
-  base::MRUCache<net::SHA256HashValue, std::unique_ptr<SCTAuditReport>>*
+  base::MRUCache<net::SHA256HashValue,
+                 std::unique_ptr<sct_auditing::TLSConnectionReport>>*
   GetCacheForTesting() {
     return &cache_;
   }
 
  private:
-  base::MRUCache<net::SHA256HashValue, std::unique_ptr<SCTAuditReport>> cache_;
+  base::MRUCache<net::SHA256HashValue,
+                 std::unique_ptr<sct_auditing::TLSConnectionReport>>
+      cache_;
 };
 
 }  // namespace network

services/network/sct_auditing_cache_unittest.cc

@@ -18,6 +18,7 @@
 #include "services/network/network_context.h"
 #include "services/network/network_service.h"
 #include "services/network/public/cpp/features.h"
+#include "services/network/public/proto/sct_audit_report.pb.h"
 #include "services/network/test/test_network_context_client.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
@@ -189,7 +190,7 @@ 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->host_port_pair.host());
+      ASSERT_NE("example1.com", entry.second->context().origin().hostname());
     }
   }
 }
@@ -265,7 +266,7 @@ TEST_F(SCTAuditingCacheTest, DeduplicationUpdatesLastSeenTime) {
 
   EXPECT_EQ(2u, cache.GetCacheForTesting()->size());
   for (const auto& entry : *cache.GetCacheForTesting()) {
-    ASSERT_NE("example2.com", entry.second->host_port_pair.host());
+    ASSERT_NE("example2.com", entry.second->context().origin().hostname());
   }
 }