Mark the Certly.io log as disqualified, as of April 15 2016

net/cert/ct_policy_enforcer_unittest.cc

Index: net/cert/ct_policy_enforcer_unittest.cc
diff --git a/net/cert/ct_policy_enforcer_unittest.cc b/net/cert/ct_policy_enforcer_unittest.cc
index 9692b94fef36ab59c8d028588a635dcf833e7b0f..41819a568c3ee35a570894e93f187e9b6d0758f1 100644
--- a/net/cert/ct_policy_enforcer_unittest.cc
+++ b/net/cert/ct_policy_enforcer_unittest.cc
@@ -92,6 +92,29 @@ class CTPolicyEnforcerTest : public ::testing::Test {
     }
   }
 
+  void AddDisqualifiedLogSCT(
+      ct::SignedCertificateTimestamp::Origin desired_origin,
+      bool timestamp_after_disqualification_date,
+      ct::SCTList* verified_scts) {
+    static const char kCertlyLogID[] =
+        "\xcd\xb5\x17\x9b\x7f\xc1\xc0\x46\xfe\xea\x31\x13\x6a\x3f\x8f\x00\x2e"
+        "\x61\x82\xfa\xf8\x89\x6f\xec\xc8\xb2\xf5\xb5\xab\x60\x49\x00";
+    static_assert(arraysize(kCertlyLogID) - 1 == crypto::kSHA256Length,
+                  "Incorrect log ID length.");
+
+    scoped_refptr<ct::SignedCertificateTimestamp> sct(
+        new ct::SignedCertificateTimestamp());
+    sct->origin = desired_origin;
+    sct->log_id = std::string(kCertlyLogID, crypto::kSHA256Length);
+    if (timestamp_after_disqualification_date)
+      sct->timestamp =

[eroman, 2016/05/05 01:52:15]

is this a multi-line if that requires curlies?

[Ryan Sleevi, 2016/05/05 03:08:43]

Peter Kasting has said no, but I think yes.

+          base::Time::FromUTCExploded({2016, 4, 0, 16, 0, 0, 0, 0});
+    else
+      sct->timestamp = base::Time::FromUTCExploded({2016, 4, 0, 1, 0, 0, 0, 0});

[eroman, 2016/05/05 01:52:15]

Hm, didn't realize we allowed this feature yet.

[Ryan Sleevi, 2016/05/05 03:08:43]

This is not an initializer list (which we do allow now), this is just C89 struct
initialization, which has always been allowed. C99 named member initialization
is not yet blessed (although it should work now that we have MSVC2015 rel 2)

[eroman, 2016/05/05 06:02:55]

To be clear this is not simply C89 struct initialization -- you are relying on a
C++11 feature.

But as you said, we do allow initializer lists now so this is ok. I hadn't
realized we had OK-ed that.

+
+    verified_scts->push_back(sct);
+  }
+
   void FillListWithSCTsOfOrigin(
       ct::SignedCertificateTimestamp::Origin desired_origin,
       size_t num_scts,
@@ -297,6 +320,119 @@ TEST_F(CTPolicyEnforcerTest, DoesNotConformToCTEVPolicyNotEnoughSCTs) {
                 chain_.get(), whitelist.get(), scts, BoundNetLog()));
 }
 
+TEST_F(CTPolicyEnforcerTest, DoesNotConformToCTEVPolicyNotEnoughFreshSCTs) {
+  ct::SCTList scts;
+
+  // The results should be the same before and after disqualification,
+  // regardless of the delivery method.
+
+  // SCT from before disqualification.
+  scts.clear();
+  FillListWithSCTsOfOrigin(
+      ct::SignedCertificateTimestamp::SCT_FROM_TLS_EXTENSION, 1, &scts);
+  AddDisqualifiedLogSCT(ct::SignedCertificateTimestamp::SCT_FROM_TLS_EXTENSION,
+                        false, &scts);
+  EXPECT_EQ(ct::CertPolicyCompliance::CERT_POLICY_NOT_DIVERSE_SCTS,
+            policy_enforcer_->DoesConformToCertPolicy(chain_.get(), scts,
+                                                      BoundNetLog()));
+  EXPECT_EQ(ct::EVPolicyCompliance::EV_POLICY_NOT_DIVERSE_SCTS,
+            policy_enforcer_->DoesConformToCTEVPolicy(chain_.get(), nullptr,
+                                                      scts, BoundNetLog()));
+
+  // SCT from after disqualification.
+  scts.clear();
+  FillListWithSCTsOfOrigin(
+      ct::SignedCertificateTimestamp::SCT_FROM_TLS_EXTENSION, 1, &scts);
+  AddDisqualifiedLogSCT(ct::SignedCertificateTimestamp::SCT_FROM_TLS_EXTENSION,
+                        true, &scts);
+  EXPECT_EQ(ct::CertPolicyCompliance::CERT_POLICY_NOT_DIVERSE_SCTS,
+            policy_enforcer_->DoesConformToCertPolicy(chain_.get(), scts,
+                                                      BoundNetLog()));
+  EXPECT_EQ(ct::EVPolicyCompliance::EV_POLICY_NOT_DIVERSE_SCTS,
+            policy_enforcer_->DoesConformToCTEVPolicy(chain_.get(), nullptr,
+                                                      scts, BoundNetLog()));
+
+  // Embedded SCT from before disqualification.
+  scts.clear();
+  FillListWithSCTsOfOrigin(
+      ct::SignedCertificateTimestamp::SCT_FROM_TLS_EXTENSION, 1, &scts);
+  AddDisqualifiedLogSCT(ct::SignedCertificateTimestamp::SCT_EMBEDDED, false,
+                        &scts);
+  EXPECT_EQ(ct::CertPolicyCompliance::CERT_POLICY_NOT_DIVERSE_SCTS,
+            policy_enforcer_->DoesConformToCertPolicy(chain_.get(), scts,
+                                                      BoundNetLog()));
+  EXPECT_EQ(ct::EVPolicyCompliance::EV_POLICY_NOT_DIVERSE_SCTS,
+            policy_enforcer_->DoesConformToCTEVPolicy(chain_.get(), nullptr,
+                                                      scts, BoundNetLog()));
+
+  // Embedded SCT from after disqualification.
+  scts.clear();
+  FillListWithSCTsOfOrigin(
+      ct::SignedCertificateTimestamp::SCT_FROM_TLS_EXTENSION, 1, &scts);
+  AddDisqualifiedLogSCT(ct::SignedCertificateTimestamp::SCT_EMBEDDED, true,
+                        &scts);
+  EXPECT_EQ(ct::CertPolicyCompliance::CERT_POLICY_NOT_DIVERSE_SCTS,
+            policy_enforcer_->DoesConformToCertPolicy(chain_.get(), scts,
+                                                      BoundNetLog()));
+  EXPECT_EQ(ct::EVPolicyCompliance::EV_POLICY_NOT_DIVERSE_SCTS,
+            policy_enforcer_->DoesConformToCTEVPolicy(chain_.get(), nullptr,
+                                                      scts, BoundNetLog()));
+}
+
+TEST_F(CTPolicyEnforcerTest,
+       ConformsWithDisqualifiedLogBeforeDisqualificationDate) {
+  ct::SCTList scts;
+  FillListWithSCTsOfOrigin(ct::SignedCertificateTimestamp::SCT_EMBEDDED, 4,
+                           &scts);
+  AddDisqualifiedLogSCT(ct::SignedCertificateTimestamp::SCT_EMBEDDED, false,
+                        &scts);
+
+  // This chain_ is valid for 10 years - over 121 months - so requires 5 SCTs.

[eroman, 2016/05/05 01:52:15]

Looks like this is the style used throughout file, but "This chain_" is funky. I
would suggest either:

 This chain is

or

  |chain_| is

[Ryan Sleevi, 2016/05/05 03:08:43]

I agree, but have left it as an exercise for future cleanup.

+  EXPECT_EQ(ct::CertPolicyCompliance::CERT_POLICY_COMPLIES_VIA_SCTS,
+            policy_enforcer_->DoesConformToCertPolicy(chain_.get(), scts,
+                                                      BoundNetLog()));
+  EXPECT_EQ(ct::EVPolicyCompliance::EV_POLICY_COMPLIES_VIA_SCTS,
+            policy_enforcer_->DoesConformToCTEVPolicy(chain_.get(), nullptr,
+                                                      scts, BoundNetLog()));
+}
+
+TEST_F(CTPolicyEnforcerTest,
+       DoesNotConformWithDisqualifiedLogAfterDisqualificationDate) {
+  ct::SCTList scts;
+  FillListWithSCTsOfOrigin(ct::SignedCertificateTimestamp::SCT_EMBEDDED, 4,
+                           &scts);
+  AddDisqualifiedLogSCT(ct::SignedCertificateTimestamp::SCT_EMBEDDED, true,
+                        &scts);
+
+  // This chain_ is valid for 10 years - over 121 months - so requires 5 SCTs.
+  EXPECT_EQ(ct::CertPolicyCompliance::CERT_POLICY_NOT_ENOUGH_SCTS,
+            policy_enforcer_->DoesConformToCertPolicy(chain_.get(), scts,
+                                                      BoundNetLog()));
+  EXPECT_EQ(ct::EVPolicyCompliance::EV_POLICY_NOT_ENOUGH_SCTS,
+            policy_enforcer_->DoesConformToCTEVPolicy(chain_.get(), nullptr,
+                                                      scts, BoundNetLog()));
+}
+
+TEST_F(CTPolicyEnforcerTest,
+       DoesNotConformWithIssuanceDateAfterDisqualificationDate) {
+  ct::SCTList scts;
+  AddDisqualifiedLogSCT(ct::SignedCertificateTimestamp::SCT_EMBEDDED, true,
+                        &scts);
+  FillListWithSCTsOfOrigin(ct::SignedCertificateTimestamp::SCT_EMBEDDED, 4,
+                           &scts);
+  // Make sure all SCTs are after the disqualification date.
+  for (size_t i = 1; i < scts.size(); ++i)
+    scts[i]->timestamp = scts[0]->timestamp;
+
+  // This chain_ is valid for 10 years - over 121 months - so requires 5 SCTs.
+  EXPECT_EQ(ct::CertPolicyCompliance::CERT_POLICY_NOT_ENOUGH_SCTS,
+            policy_enforcer_->DoesConformToCertPolicy(chain_.get(), scts,
+                                                      BoundNetLog()));
+  EXPECT_EQ(ct::EVPolicyCompliance::EV_POLICY_NOT_ENOUGH_SCTS,
+            policy_enforcer_->DoesConformToCTEVPolicy(chain_.get(), nullptr,
+                                                      scts, BoundNetLog()));
+}
+
 TEST_F(CTPolicyEnforcerTest,
        DoesNotConformToCTEVPolicyNotEnoughUniqueEmbeddedLogs) {
   ct::SCTList scts;