Certificate Transparency: Require SCTs for EV certificates.

net/cert/cert_policy_enforcer_unittest.cc

Index: net/cert/cert_policy_enforcer_unittest.cc
diff --git a/net/cert/cert_policy_enforcer_unittest.cc b/net/cert/cert_policy_enforcer_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..68df3faa02cd4e681380c30eb380684525fbb8bc
--- /dev/null
+++ b/net/cert/cert_policy_enforcer_unittest.cc
@@ -0,0 +1,88 @@
+// 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/cert_policy_enforcer.h"
+
+#include <string>
+
+#include "base/memory/scoped_ptr.h"
+#include "net/cert/ct_verify_result.h"
+#include "net/cert/x509_certificate.h"
+#include "net/test/cert_test_util.h"
+#include "net/test/ct_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+class CertPolicyEnforcerTest : public ::testing::Test {
+ public:
+  virtual void SetUp() override {
+    policy_enforcer_.reset(new CertPolicyEnforcer(3));
+    CertPolicyEnforcer::SetEnforceCTEVPolicy(true);

[Ryan Sleevi, 2014/10/22 19:48:36]

This test has global effect that isn't reset.

(As an artifact that our tests run a single test per executable run, you're
being saved, but that's generally a test smell)

[Eran Messeri, 2014/10/24 12:12:36]

Good point - now that it's not a global state, this line is gone.

+
+    std::string der_test_cert(ct::GetDerEncodedX509Cert());
+    chain_ = X509Certificate::CreateFromBytes(der_test_cert.data(),
+                                              der_test_cert.length());
+    ASSERT_TRUE(chain_.get());
+  }
+
+  void FillResultWithSCTsOfOrigin(
+      ct::SignedCertificateTimestamp::Origin desired_origin,
+      int num_scts,
+      ct::CTVerifyResult* result) {
+    for (int i = 0; i < num_scts; ++i) {
+      scoped_refptr<ct::SignedCertificateTimestamp> sct(
+          new ct::SignedCertificateTimestamp());
+      sct->origin = desired_origin;
+      result->verified_scts.push_back(sct);
+    }
+  }
+
+ protected:
+  scoped_ptr<CertPolicyEnforcer> policy_enforcer_;
+  scoped_refptr<X509Certificate> chain_;
+};
+
+TEST_F(CertPolicyEnforcerTest, ConformsToCTEVPolicyWithNonEmbeddedSCTs) {
+  ct::CTVerifyResult result;
+  FillResultWithSCTsOfOrigin(
+      ct::SignedCertificateTimestamp::SCT_FROM_TLS_EXTENSION, 2, &result);
+
+  ASSERT_TRUE(policy_enforcer_->DoesConformToCTEVPolicy(chain_.get(), result));
+}
+
+TEST_F(CertPolicyEnforcerTest, ConformsToCTEVPolicyWithEmbeddedSCTs) {
+  // We know that the chain_ is valid for 10 years - over 121 months - so

[Ryan Sleevi, 2014/10/22 19:48:36]

Don't include pronouns in comments -
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/NH-S6KCkr2M

[Eran Messeri, 2014/10/24 12:12:35]

Done.

+  // requires 5 SCTs.
+  ct::CTVerifyResult result;
+  FillResultWithSCTsOfOrigin(
+      ct::SignedCertificateTimestamp::SCT_EMBEDDED, 5, &result);
+
+  ASSERT_TRUE(policy_enforcer_->DoesConformToCTEVPolicy(chain_.get(), result));
+}
+
+TEST_F(CertPolicyEnforcerTest, ConformsToCTEVPolicyMixedOriginSCTs) {
+  ct::CTVerifyResult result;
+  FillResultWithSCTsOfOrigin(
+      ct::SignedCertificateTimestamp::SCT_FROM_TLS_EXTENSION, 2, &result);
+  result.verified_scts[1]->origin =
+      ct::SignedCertificateTimestamp::SCT_EMBEDDED;
+  ASSERT_TRUE(policy_enforcer_->DoesConformToCTEVPolicy(chain_.get(), result));
+}
+
+TEST_F(CertPolicyEnforcerTest, DoesNotConformToCTEVPolicyNotEnoughSCTs) {
+  // We know that the chain_ is valid for 10 years - over 121 months - so
+  // 5 SCTs are required. However, as there are only two logs, two SCTs
+  // will be required - so provide one to guarantee the test fails.
+  ct::CTVerifyResult result;
+  FillResultWithSCTsOfOrigin(
+      ct::SignedCertificateTimestamp::SCT_EMBEDDED, 1, &result);
+
+  ASSERT_FALSE(policy_enforcer_->DoesConformToCTEVPolicy(chain_.get(), result));
+}
+

[Ryan Sleevi, 2014/10/22 19:48:35]

Seems like there's a lot of stuff in the enforcer that remains untested, such as
the 4/3/2 SCT split, invalid dates, etc. Please consider adding a test for every
branch in the enforcer.

[Eran Messeri, 2014/10/24 12:12:35]

Good point, added tests for all branches.

+}  // namespace
+}  // namespace net