Optimize CT & OCSP handling code

net/socket/ssl_client_socket_unittest.cc

 // Copyright (c) 2012 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/socket/ssl_client_socket.h"
 
 #include <errno.h>
 #include <string.h>
 
 #include <utility>
@@ skipping 12 matching lines @@
 #include "base/trace_event/trace_event_argument.h"
 #include "base/values.h"
 #include "net/base/address_list.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
 #include "net/cert/asn1_util.h"
 #include "net/cert/ct_policy_enforcer.h"
 #include "net/cert/ct_policy_status.h"
 #include "net/cert/ct_verifier.h"
+#include "net/cert/do_nothing_ct_verifier.h"
 #include "net/cert/mock_cert_verifier.h"
 #include "net/cert/signed_certificate_timestamp_and_status.h"
 #include "net/cert/test_root_certs.h"
 #include "net/der/input.h"
 #include "net/der/parser.h"
 #include "net/der/tag.h"
 #include "net/dns/host_resolver.h"
 #include "net/http/transport_security_state.h"
 #include "net/log/net_log_event_type.h"
 #include "net/log/net_log_source.h"
@@ skipping 656 matching lines @@
   int GetChannelIDCount() override { return 0; }
   void SetForceKeepSessionState() override {}
   bool IsEphemeral() override { return true; }
 };
 
 // A mock CTVerifier that records every call to Verify but doesn't verify
 // anything.
 class MockCTVerifier : public CTVerifier {
  public:
   MOCK_METHOD5(Verify,
-               int(X509Certificate*,
-                   const std::string&,
-                   const std::string&,
-                   SignedCertificateTimestampAndStatusList*,
-                   const NetLogWithSource&));
+               void(X509Certificate*,
+                    base::StringPiece,
+                    base::StringPiece,
+                    SignedCertificateTimestampAndStatusList*,
+                    const NetLogWithSource&));
   MOCK_METHOD1(SetObserver, void(CTVerifier::Observer*));
 };
 
 // A mock CTPolicyEnforcer that returns a custom verification result.
 class MockCTPolicyEnforcer : public CTPolicyEnforcer {
  public:
   MOCK_METHOD3(DoesConformToCertPolicy,
                ct::CertPolicyCompliance(X509Certificate* cert,
                                         const ct::SCTList&,
                                         const NetLogWithSource&));
   MOCK_METHOD4(DoesConformToCTEVPolicy,
                ct::EVPolicyCompliance(X509Certificate* cert,
                                       const ct::EVCertsWhitelist*,
                                       const ct::SCTList&,
                                       const NetLogWithSource&));
 };
 
 class MockRequireCTDelegate : public TransportSecurityState::RequireCTDelegate {
  public:
   MOCK_METHOD1(IsCTRequiredForHost,
                CTRequirementLevel(const std::string& host));
 };
 
 class SSLClientSocketTest : public PlatformTest {
  public:
   SSLClientSocketTest()
       : socket_factory_(ClientSocketFactory::GetDefaultFactory()),
         cert_verifier_(new MockCertVerifier),
         transport_security_state_(new TransportSecurityState),
-        ct_verifier_(new MockCTVerifier),
+        ct_verifier_(new DoNothingCTVerifier),
         ct_policy_enforcer_(new MockCTPolicyEnforcer) {
     cert_verifier_->set_default_result(OK);
     context_.cert_verifier = cert_verifier_.get();
     context_.transport_security_state = transport_security_state_.get();
     context_.cert_transparency_verifier = ct_verifier_.get();
     context_.ct_policy_enforcer = ct_policy_enforcer_.get();
 
-    EXPECT_CALL(*ct_verifier_, Verify(_, _, _, _, _))
-        .WillRepeatedly(Return(OK));
     EXPECT_CALL(*ct_policy_enforcer_, DoesConformToCertPolicy(_, _, _))
         .WillRepeatedly(
             Return(ct::CertPolicyCompliance::CERT_POLICY_COMPLIES_VIA_SCTS));
     EXPECT_CALL(*ct_policy_enforcer_, DoesConformToCTEVPolicy(_, _, _, _))
         .WillRepeatedly(
             Return(ct::EVPolicyCompliance::EV_POLICY_COMPLIES_VIA_SCTS));
   }
 
  protected:
   // The address of the spawned test server, after calling StartTestServer().
@@ skipping 77 matching lines @@
     // Get the MockCertVerifier to verify it as an EV cert.
     CertVerifyResult verify_result;
     verify_result.cert_status = status;
     verify_result.verified_cert = server_cert;
     cert_verifier_->AddResultForCert(server_cert.get(), verify_result, OK);
   }
 
   ClientSocketFactory* socket_factory_;
   std::unique_ptr<MockCertVerifier> cert_verifier_;
   std::unique_ptr<TransportSecurityState> transport_security_state_;
-  std::unique_ptr<MockCTVerifier> ct_verifier_;
+  std::unique_ptr<DoNothingCTVerifier> ct_verifier_;
   std::unique_ptr<MockCTPolicyEnforcer> ct_policy_enforcer_;
   SSLClientSocketContext context_;
   std::unique_ptr<SSLClientSocket> sock_;
   TestNetLog log_;
 
  private:
   std::unique_ptr<SpawnedTestServer> spawned_test_server_;
   TestCompletionCallback callback_;
   AddressList addr_;
 };
@@ skipping 1435 matching lines @@
   ssl_options.client_cert_types.push_back(CLIENT_CERT_ECDSA_SIGN);
   scoped_refptr<SSLCertRequestInfo> request_info = GetCertRequest(ssl_options);
   ASSERT_TRUE(request_info.get());
   ASSERT_EQ(2u, request_info->cert_key_types.size());
   EXPECT_EQ(CLIENT_CERT_RSA_SIGN, request_info->cert_key_types[0]);
   EXPECT_EQ(CLIENT_CERT_ECDSA_SIGN, request_info->cert_key_types[1]);
 }
 
 TEST_F(SSLClientSocketTest, ConnectSignedCertTimestampsEnabledTLSExtension) {
   // Encoding of SCT List containing 'test'.
-  std::string sct_ext("\x00\x06\x00\x04test", 8);
+  base::StringPiece sct_ext("\x00\x06\x00\x04test", 8);
 
   SpawnedTestServer::SSLOptions ssl_options;
-  ssl_options.signed_cert_timestamps_tls_ext = sct_ext;
+  ssl_options.signed_cert_timestamps_tls_ext = sct_ext.as_string();
   ASSERT_TRUE(StartTestServer(ssl_options));
 
   SSLConfig ssl_config;
   ssl_config.signed_cert_timestamps_enabled = true;
 
   MockCTVerifier ct_verifier;
   SetCTVerifier(&ct_verifier);
 
   // Check that the SCT list is extracted as expected.
-  EXPECT_CALL(ct_verifier, Verify(_, "", sct_ext, _, _))
-      .WillRepeatedly(Return(ERR_CT_NO_SCTS_VERIFIED_OK));
+  EXPECT_CALL(ct_verifier, Verify(_, _, sct_ext, _, _));

[eroman, 2016/12/27 22:00:41]

Shouldn't this also clear the sct list?

I am not clear based on some of the test implementations whether CTVerifier's
contract is an assign model, or an append model.

 * DoNothingCTVerifier assumes an assign model
 * This callsite and some other tests imply the contract is an append model
 * The documentation (which refers to defunct argument) describes it as "fill",
and doesn't mention any constraint that the list is already empty.

Either the documentation needs to guarantee that the input list is empty (and
hence implementations need only append)

[Ryan Sleevi, 2016/12/27 22:17:19]

It's supposed to be exclusively an assign-on-success model.
I'm not sure why this line implies an append model to you. Could you explain
further?
Incomplete comment, but I'm not sure why you feel that one contract or the other
is implied from this file.

 
   int rv;
   ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
   EXPECT_THAT(rv, IsOk());
 
   EXPECT_TRUE(sock_->signed_cert_timestamps_received_);
 }
 
-// Test that when an EV certificate is received, but no CT verifier
-// or certificate policy enforcer are defined, then the EV status
-// of the certificate is maintained.
-TEST_F(SSLClientSocketTest, EVCertStatusMaintainedNoCTVerifier) {
-  SpawnedTestServer::SSLOptions ssl_options;
-  ASSERT_TRUE(StartTestServer(ssl_options));
-
-  SSLConfig ssl_config;
-  AddServerCertStatusToSSLConfig(CERT_STATUS_IS_EV, &ssl_config);
-
-  // No verifier to skip CT and policy checks.
-  int rv;
-  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
-  EXPECT_THAT(rv, IsOk());
-
-  SSLInfo result;
-  ASSERT_TRUE(sock_->GetSSLInfo(&result));
-
-  EXPECT_TRUE(result.cert_status & CERT_STATUS_IS_EV);
-}
-
 // Test that when a CT verifier and a CTPolicyEnforcer are defined, and
 // the EV certificate used conforms to the CT/EV policy, its EV status
 // is maintained.
 TEST_F(SSLClientSocketTest, EVCertStatusMaintainedForCompliantCert) {
   SpawnedTestServer::SSLOptions ssl_options;
   ASSERT_TRUE(StartTestServer(ssl_options));
 
   SSLConfig ssl_config;
   AddServerCertStatusToSSLConfig(CERT_STATUS_IS_EV, &ssl_config);
 
-  // To activate the CT/EV policy enforcement non-null CTVerifier and
-  // CTPolicyEnforcer are needed.
-  MockCTVerifier ct_verifier;
-  SetCTVerifier(&ct_verifier);
-  EXPECT_CALL(ct_verifier, Verify(_, "", "", _, _)).WillRepeatedly(Return(OK));
-
   // Emulate compliance of the certificate to the policy.
   MockCTPolicyEnforcer policy_enforcer;
   SetCTPolicyEnforcer(&policy_enforcer);
   EXPECT_CALL(policy_enforcer, DoesConformToCertPolicy(_, _, _))
       .WillRepeatedly(
           Return(ct::CertPolicyCompliance::CERT_POLICY_COMPLIES_VIA_SCTS));
   EXPECT_CALL(policy_enforcer, DoesConformToCTEVPolicy(_, _, _, _))
       .WillRepeatedly(
           Return(ct::EVPolicyCompliance::EV_POLICY_COMPLIES_VIA_SCTS));
 
@@ skipping 10 matching lines @@
 // Test that when a CT verifier and a CTPolicyEnforcer are defined, but
 // the EV certificate used does not conform to the CT/EV policy, its EV status
 // is removed.
 TEST_F(SSLClientSocketTest, EVCertStatusRemovedForNonCompliantCert) {
   SpawnedTestServer::SSLOptions ssl_options;
   ASSERT_TRUE(StartTestServer(ssl_options));
 
   SSLConfig ssl_config;
   AddServerCertStatusToSSLConfig(CERT_STATUS_IS_EV, &ssl_config);
 
-  // To activate the CT/EV policy enforcement non-null CTVerifier and
-  // CTPolicyEnforcer are needed.
-  MockCTVerifier ct_verifier;
-  SetCTVerifier(&ct_verifier);
-  EXPECT_CALL(ct_verifier, Verify(_, "", "", _, _)).WillRepeatedly(Return(OK));
-
   // Emulate non-compliance of the certificate to the policy.
   MockCTPolicyEnforcer policy_enforcer;
   SetCTPolicyEnforcer(&policy_enforcer);
   EXPECT_CALL(policy_enforcer, DoesConformToCertPolicy(_, _, _))
       .WillRepeatedly(
           Return(ct::CertPolicyCompliance::CERT_POLICY_NOT_ENOUGH_SCTS));
   EXPECT_CALL(policy_enforcer, DoesConformToCTEVPolicy(_, _, _, _))
       .WillRepeatedly(
           Return(ct::EVPolicyCompliance::EV_POLICY_NOT_ENOUGH_SCTS));
 
   int rv;
   ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
   EXPECT_THAT(rv, IsOk());
 
   SSLInfo result;
   ASSERT_TRUE(sock_->GetSSLInfo(&result));
 
   EXPECT_FALSE(result.cert_status & CERT_STATUS_IS_EV);
   EXPECT_TRUE(result.cert_status & CERT_STATUS_CT_COMPLIANCE_FAILED);
 }
 
-namespace {
-
-bool IsValidOCSPResponse(const base::StringPiece& input) {
-  der::Parser parser((der::Input(input)));
-  der::Parser sequence;
-  return parser.ReadSequence(&sequence) && !parser.HasMore() &&
-         sequence.SkipTag(der::kEnumerated) &&
-         sequence.SkipTag(der::kTagContextSpecific | der::kTagConstructed |
-                          0) &&
-         !sequence.HasMore();
-}
-
-}  // namespace
-
 // Test that enabling Signed Certificate Timestamps enables OCSP stapling.
 TEST_F(SSLClientSocketTest, ConnectSignedCertTimestampsEnabledOCSP) {
   SpawnedTestServer::SSLOptions ssl_options;
   ssl_options.staple_ocsp_response = true;
   // The test server currently only knows how to generate OCSP responses
   // for a freshly minted certificate.
   ssl_options.server_certificate = SpawnedTestServer::SSLOptions::CERT_AUTO;
 
   ASSERT_TRUE(StartTestServer(ssl_options));
 
   SSLConfig ssl_config;
   // Enabling Signed Cert Timestamps ensures we request OCSP stapling for
   // Certificate Transparency verification regardless of whether the platform
   // is able to process the OCSP status itself.
   ssl_config.signed_cert_timestamps_enabled = true;
 
-  MockCTVerifier ct_verifier;
-  SetCTVerifier(&ct_verifier);
-
-  // Check that the OCSP response is extracted and well-formed. It should be the
-  // DER encoding of an OCSPResponse (RFC 2560), so check that it consists of a
-  // SEQUENCE of an ENUMERATED type and an element tagged with [0] EXPLICIT. In
-  // particular, it should not include the overall two-byte length prefix from
-  // TLS.
-  EXPECT_CALL(ct_verifier,
-              Verify(_, Truly(IsValidOCSPResponse), "", _, _)).WillRepeatedly(
-                  Return(ERR_CT_NO_SCTS_VERIFIED_OK));
-
   int rv;
   ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
   EXPECT_THAT(rv, IsOk());
 
   EXPECT_TRUE(sock_->stapled_ocsp_response_received_);
 }
 
 TEST_F(SSLClientSocketTest, ConnectSignedCertTimestampsDisabled) {
   SpawnedTestServer::SSLOptions ssl_options;
   ssl_options.signed_cert_timestamps_tls_ext = "test";
@@ skipping 1257 matching lines @@
     if (dump_name.find("parent2/ssl_socket") != std::string::npos) {
       did_dump[1] = true;
       // The read buffer is not released, so |buffer_size| can't be 0.
       ASSERT_NE("0", buffer_size);
     }
   }
   EXPECT_THAT(did_dump, testing::ElementsAre(true, true));
 }
 
 }  // namespace net