Introduce the ability to require CT for specific hosts

net/quic/crypto/proof_verifier_chromium_test.cc

Index: net/quic/crypto/proof_verifier_chromium_test.cc
diff --git a/net/quic/crypto/proof_verifier_chromium_test.cc b/net/quic/crypto/proof_verifier_chromium_test.cc
index 80994f72eaafc7c9ee161778d3302bd9227068dc..75f37397577873926778bdf072f0ef1c93ade8d7 100644
--- a/net/quic/crypto/proof_verifier_chromium_test.cc
+++ b/net/quic/crypto/proof_verifier_chromium_test.cc
@@ -19,8 +19,12 @@
 #include "net/test/cert_test_util.h"
 #include "net/test/ct_test_util.h"
 #include "net/test/test_data_directory.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ::testing::_;
+using ::testing::Return;
+
 namespace net {
 namespace test {
 
@@ -61,24 +65,24 @@ class FailsTestCTPolicyEnforcer : public CTPolicyEnforcer {
   }
 };
 
-// CTPolicyEnforcer that can simulate whether or not a given certificate
-// conforms to the CT/EV policy.
+// A mock CTPolicyEnforcer that returns a custom verification result.
 class MockCTPolicyEnforcer : public CTPolicyEnforcer {
  public:
-  explicit MockCTPolicyEnforcer(bool is_ev) : is_ev_(is_ev) {}
-  ~MockCTPolicyEnforcer() override {}
-
-  ct::EVPolicyCompliance DoesConformToCTEVPolicy(
-      X509Certificate* cert,
-      const ct::EVCertsWhitelist* ev_whitelist,
-      const ct::SCTList& verified_scts,
-      const BoundNetLog& net_log) override {
-    return is_ev_ ? ct::EVPolicyCompliance::EV_POLICY_COMPLIES_VIA_SCTS
-                  : ct::EVPolicyCompliance::EV_POLICY_NOT_ENOUGH_SCTS;
-  }
+  MOCK_METHOD3(DoesConformToCertPolicy,
+               ct::CertPolicyCompliance(X509Certificate* cert,
+                                        const ct::SCTList&,
+                                        const BoundNetLog&));
+  MOCK_METHOD4(DoesConformToCTEVPolicy,
+               ct::EVPolicyCompliance(X509Certificate* cert,
+                                      const ct::EVCertsWhitelist*,
+                                      const ct::SCTList&,
+                                      const BoundNetLog&));
+};
 
- private:
-  bool is_ev_;
+class MockRequireCTDelegate : public TransportSecurityState::RequireCTDelegate {
+ public:
+  MOCK_METHOD1(IsCTRequiredForHost,
+               CTRequirementLevel(const std::string& host));
 };
 
 class DummyProofVerifierCallback : public ProofVerifierCallback {
@@ -103,11 +107,17 @@ const char kLogDescription[] = "somelog";
 class ProofVerifierChromiumTest : public ::testing::Test {
  public:
   ProofVerifierChromiumTest()
-      : ct_policy_enforcer_(false /*is_ev*/),
-        verify_context_(new ProofVerifyContextChromium(0 /*cert_verify_flags*/,
+      : verify_context_(new ProofVerifyContextChromium(0 /*cert_verify_flags*/,
                                                        BoundNetLog())) {}
 
   void SetUp() override {
+    EXPECT_CALL(ct_policy_enforcer_, DoesConformToCertPolicy(_, _, _))
+        .WillRepeatedly(
+            Return(ct::CertPolicyCompliance::CERT_POLICY_NOT_ENOUGH_SCTS));
+    EXPECT_CALL(ct_policy_enforcer_, DoesConformToCTEVPolicy(_, _, _, _))
+        .WillRepeatedly(
+            Return(ct::EVPolicyCompliance::EV_POLICY_DOES_NOT_APPLY));
+
     scoped_refptr<const CTLogVerifier> log(CTLogVerifier::Create(
         ct::GetTestPublicKey(), kLogDescription, "https://test.example.com"));
     ASSERT_TRUE(log);
@@ -297,9 +307,11 @@ TEST_F(ProofVerifierChromiumTest, PreservesEVIfAllowed) {
   MockCertVerifier dummy_verifier;
   dummy_verifier.AddResultForCert(test_cert.get(), dummy_result, OK);
 
-  MockCTPolicyEnforcer policy_enforcer(true /*is_ev*/);
+  EXPECT_CALL(ct_policy_enforcer_, DoesConformToCTEVPolicy(_, _, _, _))
+      .WillRepeatedly(
+          Return(ct::EVPolicyCompliance::EV_POLICY_COMPLIES_VIA_SCTS));

[davidben, 2016/06/23 19:51:43]

Just to confirm, clobbering the EXPECT_CALL in SetUp like this is how GMock is
intended to work? (Other option I can see is to use a different
MockCTPolicyEnforcer as the old version did.)

[Ryan Sleevi, 2016/06/23 21:38:31]

Yup.
https://github.com/google/googletest/blob/master/googlemock/docs/ForDummies.m...
covers this in the intro to GMock 

 
-  ProofVerifierChromium proof_verifier(&dummy_verifier, &policy_enforcer,
+  ProofVerifierChromium proof_verifier(&dummy_verifier, &ct_policy_enforcer_,
                                        &transport_security_state_,
                                        ct_verifier_.get());
 
@@ -331,9 +343,7 @@ TEST_F(ProofVerifierChromiumTest, StripsEVIfNotAllowed) {
   MockCertVerifier dummy_verifier;
   dummy_verifier.AddResultForCert(test_cert.get(), dummy_result, OK);
 
-  MockCTPolicyEnforcer policy_enforcer(false /*is_ev*/);
-
-  ProofVerifierChromium proof_verifier(&dummy_verifier, &policy_enforcer,
+  ProofVerifierChromium proof_verifier(&dummy_verifier, &ct_policy_enforcer_,
                                        &transport_security_state_,
                                        ct_verifier_.get());
 
@@ -409,14 +419,12 @@ TEST_F(ProofVerifierChromiumTest, PKPEnforced) {
   dummy_verifier.AddResultForCert(test_cert.get(), dummy_result, OK);
 
   HashValueVector pin_hashes = MakeHashValueVector(0x02);
-  TransportSecurityState transport_security_state;
-  transport_security_state.AddHPKP(
+  transport_security_state_.AddHPKP(
       kTestHostname, base::Time::Now() + base::TimeDelta::FromSeconds(10000),
       true, pin_hashes, GURL());
 
-  MockCTPolicyEnforcer policy_enforcer(true /*is_ev*/);
-  ProofVerifierChromium proof_verifier(&dummy_verifier, &policy_enforcer,
-                                       &transport_security_state,
+  ProofVerifierChromium proof_verifier(&dummy_verifier, &ct_policy_enforcer_,
+                                       &transport_security_state_,
                                        ct_verifier_.get());
 
   std::unique_ptr<DummyProofVerifierCallback> callback(
@@ -452,14 +460,12 @@ TEST_F(ProofVerifierChromiumTest, PKPBypassFlagSet) {
   dummy_verifier.AddResultForCert(test_cert.get(), dummy_result, OK);
 
   HashValueVector expected_hashes = MakeHashValueVector(0x02);
-  TransportSecurityState transport_security_state_fail;
-  transport_security_state_fail.AddHPKP(
+  transport_security_state_.AddHPKP(
       kTestHostname, base::Time::Now() + base::TimeDelta::FromSeconds(10000),
       true, expected_hashes, GURL());
 
-  MockCTPolicyEnforcer policy_enforcer(true /*is_ev*/);
-  ProofVerifierChromium proof_verifier(&dummy_verifier, &policy_enforcer,
-                                       &transport_security_state_fail,
+  ProofVerifierChromium proof_verifier(&dummy_verifier, &ct_policy_enforcer_,
+                                       &transport_security_state_,
                                        ct_verifier_.get());
 
   std::unique_ptr<DummyProofVerifierCallback> callback(
@@ -476,5 +482,106 @@ TEST_F(ProofVerifierChromiumTest, PKPBypassFlagSet) {
   EXPECT_TRUE(verify_details->pkp_bypassed);
 }
 
+// Test that when CT is required (in this case, by the delegate), the
+// absence of CT information is a socket error.
+TEST_F(ProofVerifierChromiumTest, CTIsRequired) {
+  scoped_refptr<X509Certificate> test_cert = GetTestServerCertificate();
+  ASSERT_TRUE(test_cert);
+
+  CertVerifyResult dummy_result;
+  dummy_result.verified_cert = test_cert;
+  dummy_result.is_issued_by_known_root = true;
+  dummy_result.public_key_hashes = MakeHashValueVector(0x01);
+  dummy_result.cert_status = 0;
+
+  MockCertVerifier dummy_verifier;
+  dummy_verifier.AddResultForCert(test_cert.get(), dummy_result, OK);
+
+  // Set up CT.
+  MockRequireCTDelegate require_ct_delegate;
+  transport_security_state_.SetRequireCTDelegate(&require_ct_delegate);
+  EXPECT_CALL(require_ct_delegate, IsCTRequiredForHost(_))
+      .WillRepeatedly(Return(TransportSecurityState::RequireCTDelegate::
+                                 CTRequirementLevel::NOT_REQUIRED));

[davidben, 2016/06/23 19:51:43]

Does this line do anything?

[Ryan Sleevi, 2016/06/23 21:38:31]

Objectively, no. It just means that if the hostname should ever change from
kTestHostname, the test will fail. That is, it guards against any implicit or
default mode by making explicit what happens for any hostname other than
kTestHostname (the line 506 expectation).

So it's really documentation more than anything - and guaranteeing the
preconditions.

+  EXPECT_CALL(require_ct_delegate, IsCTRequiredForHost(kTestHostname))
+      .WillRepeatedly(Return(TransportSecurityState::RequireCTDelegate::
+                                 CTRequirementLevel::REQUIRED));
+  EXPECT_CALL(ct_policy_enforcer_, DoesConformToCertPolicy(_, _, _))
+      .WillRepeatedly(
+          Return(ct::CertPolicyCompliance::CERT_POLICY_NOT_ENOUGH_SCTS));
+
+  ProofVerifierChromium proof_verifier(&dummy_verifier, &ct_policy_enforcer_,
+                                       &transport_security_state_,
+                                       ct_verifier_.get());
+
+  std::unique_ptr<DummyProofVerifierCallback> callback(
+      new DummyProofVerifierCallback);
+  QuicAsyncStatus status = proof_verifier.VerifyProof(
+      kTestHostname, kTestPort, kTestConfig, QUIC_VERSION_25, "", certs_, "",
+      GetTestSignature(), verify_context_.get(), &error_details_, &details_,
+      callback.get());
+  ASSERT_EQ(QUIC_FAILURE, status);
+
+  ASSERT_TRUE(details_.get());
+  ProofVerifyDetailsChromium* verify_details =
+      static_cast<ProofVerifyDetailsChromium*>(details_.get());
+  EXPECT_TRUE(verify_details->cert_verify_result.cert_status &
+              CERT_STATUS_CERTIFICATE_TRANSPARENCY_REQUIRED);
+}
+
+// Test that CT is considered even when HPKP fails.
+TEST_F(ProofVerifierChromiumTest, PKPAndCTBothTested) {
+  scoped_refptr<X509Certificate> test_cert = GetTestServerCertificate();
+  ASSERT_TRUE(test_cert);
+
+  CertVerifyResult dummy_result;
+  dummy_result.verified_cert = test_cert;
+  dummy_result.is_issued_by_known_root = true;
+  dummy_result.public_key_hashes = MakeHashValueVector(0x01);
+  dummy_result.cert_status = 0;
+
+  MockCertVerifier dummy_verifier;
+  dummy_verifier.AddResultForCert(test_cert.get(), dummy_result, OK);
+
+  // Set up HPKP.
+  HashValueVector pin_hashes = MakeHashValueVector(0x02);
+  transport_security_state_.AddHPKP(
+      kTestHostname, base::Time::Now() + base::TimeDelta::FromSeconds(10000),
+      true, pin_hashes, GURL());
+
+  // Set up CT.
+  MockRequireCTDelegate require_ct_delegate;
+  transport_security_state_.SetRequireCTDelegate(&require_ct_delegate);
+  EXPECT_CALL(require_ct_delegate, IsCTRequiredForHost(_))
+      .WillRepeatedly(Return(TransportSecurityState::RequireCTDelegate::
+                                 CTRequirementLevel::NOT_REQUIRED));
+  EXPECT_CALL(require_ct_delegate, IsCTRequiredForHost(kTestHostname))
+      .WillRepeatedly(Return(TransportSecurityState::RequireCTDelegate::
+                                 CTRequirementLevel::REQUIRED));
+  EXPECT_CALL(ct_policy_enforcer_, DoesConformToCertPolicy(_, _, _))
+      .WillRepeatedly(
+          Return(ct::CertPolicyCompliance::CERT_POLICY_NOT_ENOUGH_SCTS));
+
+  ProofVerifierChromium proof_verifier(&dummy_verifier, &ct_policy_enforcer_,
+                                       &transport_security_state_,
+                                       ct_verifier_.get());
+
+  std::unique_ptr<DummyProofVerifierCallback> callback(
+      new DummyProofVerifierCallback);
+  QuicAsyncStatus status = proof_verifier.VerifyProof(
+      kTestHostname, kTestPort, kTestConfig, QUIC_VERSION_25, "", certs_, "",
+      GetTestSignature(), verify_context_.get(), &error_details_, &details_,
+      callback.get());
+  ASSERT_EQ(QUIC_FAILURE, status);
+
+  ASSERT_TRUE(details_.get());
+  ProofVerifyDetailsChromium* verify_details =
+      static_cast<ProofVerifyDetailsChromium*>(details_.get());
+  EXPECT_TRUE(verify_details->cert_verify_result.cert_status &
+              CERT_STATUS_PINNED_KEY_MISSING);
+  EXPECT_TRUE(verify_details->cert_verify_result.cert_status &
+              CERT_STATUS_CERTIFICATE_TRANSPARENCY_REQUIRED);
+}
+
 }  // namespace test
 }  // namespace net