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

net/cert/ct_policy_enforcer_unittest.cc

 // Copyright 2014 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.
 
 #include "net/cert/ct_policy_enforcer.h"
 
 #include <memory>
 #include <string>
 
 #include "base/time/time.h"
@@ skipping 74 matching lines @@
         sct->timestamp =
             base::Time::FromUTCExploded({2015, 8, 0, 15, 0, 0, 0, 0});
       else
         sct->timestamp =
             base::Time::FromUTCExploded({2015, 6, 0, 15, 0, 0, 0, 0});
 
       verified_scts->push_back(sct);
     }
   }
 
+  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,
       ct::SCTList* verified_scts) {
     std::vector<std::string> desired_log_ids;
     desired_log_ids.push_back(google_log_id_);
     FillListWithSCTsOfOrigin(desired_origin, num_scts, desired_log_ids, true,
                              verified_scts);
   }
 
@@ skipping 185 matching lines @@
           chain_.get(), non_including_whitelist.get(), scts, BoundNetLog()));
 
   // ... but should be OK if whitelisted.
   scoped_refptr<ct::EVCertsWhitelist> whitelist(
       new DummyEVCertsWhitelist(true, true));
   EXPECT_EQ(ct::EVPolicyCompliance::EV_POLICY_COMPLIES_VIA_WHITELIST,
             policy_enforcer_->DoesConformToCTEVPolicy(
                 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;
   std::vector<std::string> desired_logs;
 
   // One Google Log.
   desired_logs.clear();
   desired_logs.push_back(google_log_id_);
   FillListWithSCTsOfOrigin(ct::SignedCertificateTimestamp::SCT_EMBEDDED,
                            desired_logs.size(), desired_logs, true, &scts);
@@ skipping 123 matching lines @@
   FillListWithSCTsOfOrigin(ct::SignedCertificateTimestamp::SCT_EMBEDDED, 2,
                            &scts);
   EXPECT_EQ(ct::EVPolicyCompliance::EV_POLICY_NOT_ENOUGH_SCTS,
             policy_enforcer_->DoesConformToCTEVPolicy(chain_.get(), nullptr,
                                                       scts, BoundNetLog()));
 }
 
 }  // namespace
 
 }  // namespace net