Add Expect CT policy that gets checked on all certs

net/socket/ssl_client_socket_nss.cc

Index: net/socket/ssl_client_socket_nss.cc
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index e1a8335055799cda13761e3b1d0fd88015e9b100..1416c77d60b31283a2da58399150c000c9389eba 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -3125,20 +3125,25 @@ void SSLClientSocketNSS::VerifyCT() {
   // TODO(ekasper): wipe stapled_ocsp_response and sct_list_from_tls_extension
   // from the state after verification is complete, to conserve memory.
 
-  if (policy_enforcer_ &&
-      (server_cert_verify_result_.cert_status & CERT_STATUS_IS_EV)) {
+  if (policy_enforcer_) {
     scoped_refptr<ct::EVCertsWhitelist> ev_whitelist =
         SSLConfigService::GetEVCertsWhitelist();
-    if (!policy_enforcer_->DoesConformToCTEVPolicy(
-            server_cert_verify_result_.verified_cert.get(), ev_whitelist.get(),
-            ct_verify_result_, net_log_)) {
+    if (!policy_enforcer_->DoesConformToCertPolicy(
+            server_cert_verify_result_.verified_cert.get(),
+            ct_verify_result_)) {
+      server_cert_verify_result_.cert_status |=
+          CERT_STATUS_CT_COMPLIANCE_FAILED;
+    }
+    if ((server_cert_verify_result_.cert_status & CERT_STATUS_IS_EV) &&
+        !policy_enforcer_->DoesConformToEVPolicy(
+            server_cert_verify_result_.verified_cert.get(),
+            server_cert_verify_result_.cert_status, ev_whitelist.get(),
+            net_log_)) {

[Ryan Sleevi, 2016/01/22 23:49:41]

The interface between these two methods feels wrong. PolicyEnforcer (previously)
was ignorant of the CertStatus behaviour and machinations - PolicyEnforcer's
responsibilities were solely about returning results about the CT status.

The new API introduces a dependency on the CertStatus that we likely don't want
to do.

One possible solution is to move this to an else-if on 3136-3137, another would
be to make the EVPolicy separately re-evaluate the certificate using the
ct_verify_result_ (which may be more robust anyways), although that would mean
double-validation in the case of EV.

[estark, 2016/01/23 01:38:41]

If we're okay with double-validating for EV, that sounds good to me (and maybe
even potentially useful in the future, if we ever, say, wanted to apply a more
strict SCT check to EV certs?).

       // TODO(eranm): Log via the BoundNetLog, see crbug.com/437766
       VLOG(1) << "EV certificate for "
               << server_cert_verify_result_.verified_cert->subject()
                      .GetDisplayName()
               << " does not conform to CT policy, removing EV status.";
-      server_cert_verify_result_.cert_status |=
-          CERT_STATUS_CT_COMPLIANCE_FAILED;
       server_cert_verify_result_.cert_status &= ~CERT_STATUS_IS_EV;
     }
   }