Do not send Expect-CT reports on old builds

When processing an Expect-CT header, we send a report if it's received over a
non-compliant connection. An old build shouldn't be considered non-compliant in
this check: we're only interested in notifying site owners about server-side
misconfigurations that cause non-compliance.

Bug: 786563
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Id9cf7941325fee1b58f15dc6b1a033ae1bf5471d
Reviewed-on: https://chromium-review.googlesource.com/777883
Commit-Queue: Emily Stark <estark@chromium.org>
Reviewed-by: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517707}

net/http/transport_security_state.cc

@@ -1467,15 +1467,16 @@ void TransportSecurityState::ProcessExpectCTHeader(
   if (value == "preload") {
     if (!expect_ct_reporter_)
       return;
-    if (!IsBuildTimely())
-      return;
     if (!ssl_info.is_issued_by_known_root)
       return;
     if (!ssl_info.ct_compliance_details_available)
       return;
     if (ssl_info.ct_cert_policy_compliance ==
-        ct::CertPolicyCompliance::CERT_POLICY_COMPLIES_VIA_SCTS)
+            ct::CertPolicyCompliance::CERT_POLICY_COMPLIES_VIA_SCTS ||
+        ssl_info.ct_cert_policy_compliance ==
+            ct::CertPolicyCompliance::CERT_POLICY_BUILD_NOT_TIMELY) {
       return;
+    }
     ExpectCTState state;
     if (GetStaticExpectCTState(host_port_pair.host(), &state)) {
       MaybeNotifyExpectCTFailed(host_port_pair, state.report_uri, base::Time(),
@@ -1506,7 +1507,6 @@ void TransportSecurityState::ProcessExpectCTHeader(
     return;
   if (ssl_info.ct_cert_policy_compliance !=
       ct::CertPolicyCompliance::CERT_POLICY_COMPLIES_VIA_SCTS) {
-    ExpectCTState state;
     // If an Expect-CT header is observed over a non-compliant connection, the
     // site owner should be notified about the misconfiguration. If the site was
     // already opted in to Expect-CT, this report would have been sent at
@@ -1514,6 +1514,13 @@ void TransportSecurityState::ProcessExpectCTHeader(
     // however, the lack of CT compliance would not have been evaluated/reported
     // at connection setup time, so it needs to be reported here while
     // processing the header.
+    if (ssl_info.ct_cert_policy_compliance ==
+        ct::CertPolicyCompliance::CERT_POLICY_BUILD_NOT_TIMELY) {
+      // Only send reports for truly non-compliant connections, not those for
+      // which compliance wasn't checked due to an out-of-date build.
+      return;
+    }
+    ExpectCTState state;
     if (expect_ct_reporter_ && !report_uri.is_empty() &&
         !GetDynamicExpectCTState(host_port_pair.host(), &state)) {
       MaybeNotifyExpectCTFailed(host_port_pair, report_uri, base::Time(),

net/http/transport_security_state_unittest.cc

@@ -1565,6 +1565,76 @@ TEST_F(TransportSecurityStateTest, ExpectCTCompliantCert) {
   EXPECT_EQ(1u, reporter.num_failures());
 }
 
+// Tests that the Expect CT reporter is not notified for preloaded Expect-CT
+// when the build is not timely.
+TEST_F(TransportSecurityStateTest, PreloadedExpectCTBuildNotTimely) {
+  HostPortPair host_port(kExpectCTStaticHostname, 443);
+  SSLInfo ssl_info;
+  ssl_info.ct_compliance_details_available = true;
+  ssl_info.ct_cert_policy_compliance =
+      ct::CertPolicyCompliance::CERT_POLICY_BUILD_NOT_TIMELY;
+  ssl_info.is_issued_by_known_root = true;
+  scoped_refptr<X509Certificate> cert1 =
+      ImportCertFromFile(GetTestCertsDirectory(), "ok_cert.pem");
+  ASSERT_TRUE(cert1);
+  scoped_refptr<X509Certificate> cert2 =
+      ImportCertFromFile(GetTestCertsDirectory(), "expired_cert.pem");
+  ASSERT_TRUE(cert2);
+  ssl_info.unverified_cert = cert1;
+  ssl_info.cert = cert2;
+
+  TransportSecurityState state;
+  TransportSecurityStateTest::EnableStaticExpectCT(&state);
+  MockExpectCTReporter reporter;
+  state.SetExpectCTReporter(&reporter);
+  state.ProcessExpectCTHeader("preload", host_port, ssl_info);
+  EXPECT_EQ(0u, reporter.num_failures());
+
+  // Sanity-check that the reporter is notified if the build is timely and the
+  // connection is not compliant.
+  ssl_info.ct_cert_policy_compliance =
+      ct::CertPolicyCompliance::CERT_POLICY_NOT_DIVERSE_SCTS;
+  state.ProcessExpectCTHeader("preload", host_port, ssl_info);
+  EXPECT_EQ(1u, reporter.num_failures());
+}
+
+// Tests that the Expect CT reporter is not notified for dynamic Expect-CT when
+// the build is not timely.
+TEST_F(TransportSecurityStateTest, DynamicExpectCTBuildNotTimely) {
+  HostPortPair host_port("example.test", 443);
+  SSLInfo ssl_info;
+  ssl_info.ct_compliance_details_available = true;
+  ssl_info.ct_cert_policy_compliance =
+      ct::CertPolicyCompliance::CERT_POLICY_BUILD_NOT_TIMELY;
+  ssl_info.is_issued_by_known_root = true;
+  scoped_refptr<X509Certificate> cert1 =
+      ImportCertFromFile(GetTestCertsDirectory(), "ok_cert.pem");
+  ASSERT_TRUE(cert1);
+  scoped_refptr<X509Certificate> cert2 =
+      ImportCertFromFile(GetTestCertsDirectory(), "expired_cert.pem");
+  ASSERT_TRUE(cert2);
+  ssl_info.unverified_cert = cert1;
+  ssl_info.cert = cert2;
+
+  TransportSecurityState state;
+  MockExpectCTReporter reporter;
+  state.SetExpectCTReporter(&reporter);
+  const char kHeader[] = "max-age=10, report-uri=http://report.test";
+  state.ProcessExpectCTHeader(kHeader, host_port, ssl_info);
+
+  // No report should have been sent and the state should not have been saved.
+  EXPECT_EQ(0u, reporter.num_failures());
+  TransportSecurityState::ExpectCTState expect_ct_state;
+  EXPECT_FALSE(state.GetDynamicExpectCTState("example.test", &expect_ct_state));
+
+  // Sanity-check that the reporter is notified if the build is timely and the
+  // connection is not compliant.
+  ssl_info.ct_cert_policy_compliance =
+      ct::CertPolicyCompliance::CERT_POLICY_NOT_DIVERSE_SCTS;
+  state.ProcessExpectCTHeader(kHeader, host_port, ssl_info);
+  EXPECT_EQ(1u, reporter.num_failures());
+}
+
 // Tests that the Expect CT reporter is not notified for a site that
 // isn't preloaded.
 TEST_F(TransportSecurityStateTest, ExpectCTNotPreloaded) {