Don't block the UI thread for OCSP/CRLs when viewing a cert on Mac.

When viewing certificates in the system certificate viewer on Mac, the
certificate chain is re-evaluated with the system revocation checking
policies. During this evaluation, the UI thread is blocked, potentially
causing significant delays for slow OCSP responders.

Instead of allowing the default policies, explicitly disable revocation
checking when invoking the viewer. This does not affect revocation checking
from happening in the network stack (if the user has enabled it), only from
the user being able to view exactly which certificate is revoked when a chain
contains revoked certificate.

BUG=79950
TEST=Access a site with a slow OCSP responder (see bug for an example).
Clicking "Certificate Information" should cause the certificate chain to be
displayed immediately, rather than after ~25 seconds.


Review URL: http://codereview.chromium.org/7082031

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@90146 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 128 insertions, 74 deletions

diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h
index f01f9f1..62b51c3 100644
--- a/net/base/x509_certificate.h
+++ b/net/base/x509_certificate.h
@@ -257,15 +257,41 @@ class NET_API X509Certificate
   // Do any of the given issuer names appear in this cert's chain of trust?
   bool IsIssuedBy(const std::vector<CertPrincipal>& valid_issuers);
 
-  // Creates a security policy for SSL client certificates.
-  static OSStatus CreateSSLClientPolicy(SecPolicyRef* outPolicy);
+  // Creates a security policy for certificates used as client certificates
+  // in SSL.
+  // If a policy is successfully created, it will be stored in
+  // |*policy| and ownership transferred to the caller.
+  static OSStatus CreateSSLClientPolicy(SecPolicyRef* policy);
+
+  // Creates a security policy for certificates used by SSL servers.
+  // |hostname| is an optionally-supplied string indicating the name to verify
+  // the server certificate as; if it is empty, no hostname verification will
+  // happen.
+  // If a policy is successfully created, it will be stored in |*policy| and
+  // ownership transferred to the caller.
+  static OSStatus CreateSSLServerPolicy(const std::string& hostname,
+                                        SecPolicyRef* policy);
+
+  // Creates a security policy for basic X.509 validation. If the policy is
+  // successfully created, it will be stored in |*policy| and ownership
+  // transferred to the caller.
+  static OSStatus CreateBasicX509Policy(SecPolicyRef* policy);
+
+  // Creates security policies to control revocation checking (OCSP and CRL).
+  // If |enable_revocation_checking| is false, the policies returned will be
+  // explicitly disabled from accessing the network or the cache. This may be
+  // used to override system settings regarding revocation checking.
+  // If the policies are successfully created, they will be appended to
+  // |policies|.
+  static OSStatus CreateRevocationPolicies(bool enable_revocation_checking,
+                                           CFMutableArrayRef policies);
 
   // Adds all available SSL client identity certs to the given vector.
   // |server_domain| is a hint for which domain the cert is to be sent to
   // (a cert previously specified as the default for that domain will be given
   // precedence and returned first in the output vector.)
-  // If valid_issuers is non-empty, only certs that were transitively issued by
-  // one of the given names will be included in the list.
+  // If valid_issuers is non-empty, only certs that were transitively issued
+  // by one of the given names will be included in the list.
   static bool GetSSLClientCertificates(
       const std::string& server_domain,
       const std::vector<CertPrincipal>& valid_issuers,
diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc
index 6c41c50..8f4ba4e 100644
--- a/net/base/x509_certificate_mac.cc
+++ b/net/base/x509_certificate_mac.cc
@@ -319,73 +319,22 @@ OSStatus CreateTrustPolicies(const std::string& hostname, int flags,
   // Create an SSL SecPolicyRef, and configure it to perform hostname
   // validation. The hostname check does 99% of what we want, with the
   // exception of dotted IPv4 addreses, which we handle ourselves below.
-  CSSM_APPLE_TP_SSL_OPTIONS tp_ssl_options = {
-    CSSM_APPLE_TP_SSL_OPTS_VERSION,
-    hostname.size(),
-    hostname.data(),
-    0
-  };
   SecPolicyRef ssl_policy;
-  OSStatus status = CreatePolicy(&CSSMOID_APPLE_TP_SSL, &tp_ssl_options,
-                                 sizeof(tp_ssl_options), &ssl_policy);
+  OSStatus status = X509Certificate::CreateSSLServerPolicy(hostname,
+                                                           &ssl_policy);
   if (status)
     return status;
   CFArrayAppendValue(local_policies, ssl_policy);
   CFRelease(ssl_policy);
 
-  // Manually add revocation policies. In order to actually disable revocation
-  // checking, the SecTrustRef must have at least one revocation policy
-  // associated with it. If none are present, the Apple TP will add policies
-  // according to the system preferences, which will enable revocation
-  // checking even if the caller explicitly disabled it. An OCSP policy is
-  // used, rather than a CRL policy, because the Apple TP will force an OCSP
-  // policy to be present and enabled if it believes the certificate may chain
-  // to an EV root. By explicitly disabling network and OCSP cache access,
-  // then even if the Apple TP enables OCSP checking, no revocation checking
-  // will actually succeed.
-  CSSM_APPLE_TP_OCSP_OPTIONS tp_ocsp_options;
-  memset(&tp_ocsp_options, 0, sizeof(tp_ocsp_options));
-  tp_ocsp_options.Version = CSSM_APPLE_TP_OCSP_OPTS_VERSION;
-
-  if (flags & X509Certificate::VERIFY_REV_CHECKING_ENABLED) {
-    // The default for the OCSP policy is to fetch responses via the network,
-    // unlike the CRL policy default. The policy is further modified to
-    // prefer OCSP over CRLs, if both are specified on the certificate. This
-    // is because an OCSP response is both sufficient and typically
-    // significantly smaller than the CRL counterpart.
-    tp_ocsp_options.Flags = CSSM_TP_ACTION_OCSP_SUFFICIENT;
-  } else {
-    // Effectively disable OCSP checking by making it impossible to get an
-    // OCSP response. Even if the Apple TP forces OCSP, no checking will
-    // be able to succeed. If this happens, the Apple TP will report an error
-    // that OCSP was unavailable, but this will be handled and suppressed in
-    // X509Certificate::Verify().
-    tp_ocsp_options.Flags = CSSM_TP_ACTION_OCSP_DISABLE_NET |
-                            CSSM_TP_ACTION_OCSP_CACHE_READ_DISABLE;
-  }
-
-  SecPolicyRef ocsp_policy;
-  status = CreatePolicy(&CSSMOID_APPLE_TP_REVOCATION_OCSP, &tp_ocsp_options,
-                        sizeof(tp_ocsp_options), &ocsp_policy);
+  // Explicitly add revocation policies, in order to override system
+  // revocation checking policies and instead respect the application-level
+  // revocation preference.
+  status = X509Certificate::CreateRevocationPolicies(
+      (flags & X509Certificate::VERIFY_REV_CHECKING_ENABLED),
+      local_policies);
   if (status)
     return status;
-  CFArrayAppendValue(local_policies, ocsp_policy);
-  CFRelease(ocsp_policy);
-
-  if (flags & X509Certificate::VERIFY_REV_CHECKING_ENABLED) {
-    CSSM_APPLE_TP_CRL_OPTIONS tp_crl_options;
-    memset(&tp_crl_options, 0, sizeof(tp_crl_options));
-    tp_crl_options.Version = CSSM_APPLE_TP_CRL_OPTS_VERSION;
-    tp_crl_options.CrlFlags = CSSM_TP_ACTION_FETCH_CRL_FROM_NET;
-
-    SecPolicyRef crl_policy;
-    status = CreatePolicy(&CSSMOID_APPLE_TP_REVOCATION_CRL, &tp_crl_options,
-                          sizeof(tp_crl_options), &crl_policy);
-    if (status)
-      return status;
-    CFArrayAppendValue(local_policies, crl_policy);
-    CFRelease(crl_policy);
-  }
 
   policies->reset(local_policies.release());
   return noErr;
@@ -1211,19 +1160,98 @@ bool X509Certificate::IsIssuedBy(
 }
 
 // static
-OSStatus X509Certificate::CreateSSLClientPolicy(SecPolicyRef* out_policy) {
-  CSSM_APPLE_TP_SSL_OPTIONS tp_ssl_options = {
-    CSSM_APPLE_TP_SSL_OPTS_VERSION,
-    0,
-    NULL,
-    CSSM_APPLE_TP_SSL_CLIENT
-  };
-  return CreatePolicy(&CSSMOID_APPLE_TP_SSL,
-                      &tp_ssl_options,
-                      sizeof(tp_ssl_options),
-                      out_policy);
+OSStatus X509Certificate::CreateSSLClientPolicy(SecPolicyRef* policy) {
+  CSSM_APPLE_TP_SSL_OPTIONS tp_ssl_options;
+  memset(&tp_ssl_options, 0, sizeof(tp_ssl_options));
+  tp_ssl_options.Version = CSSM_APPLE_TP_SSL_OPTS_VERSION;
+  tp_ssl_options.Flags |= CSSM_APPLE_TP_SSL_CLIENT;
+
+  return CreatePolicy(&CSSMOID_APPLE_TP_SSL, &tp_ssl_options,
+                      sizeof(tp_ssl_options), policy);
+}
+
+// static
+OSStatus X509Certificate::CreateSSLServerPolicy(const std::string& hostname,
+                                                SecPolicyRef* policy) {
+  CSSM_APPLE_TP_SSL_OPTIONS tp_ssl_options;
+  memset(&tp_ssl_options, 0, sizeof(tp_ssl_options));
+  tp_ssl_options.Version = CSSM_APPLE_TP_SSL_OPTS_VERSION;
+  if (!hostname.empty()) {
+    tp_ssl_options.ServerName = hostname.data();
+    tp_ssl_options.ServerNameLen = hostname.size();
+  }
+
+  return CreatePolicy(&CSSMOID_APPLE_TP_SSL, &tp_ssl_options,
+                      sizeof(tp_ssl_options), policy);
+}
+
+// static
+OSStatus X509Certificate::CreateBasicX509Policy(SecPolicyRef* policy) {
+  return CreatePolicy(&CSSMOID_APPLE_X509_BASIC, NULL, 0, policy);
+}
+
+// static
+OSStatus X509Certificate::CreateRevocationPolicies(
+    bool enable_revocation_checking,
+    CFMutableArrayRef policies) {
+  // In order to actually disable revocation checking, the SecTrustRef must
+  // have at least one revocation policy associated with it. If none are
+  // present, the Apple TP will add policies according to the system
+  // preferences, which will enable revocation checking even if the caller
+  // explicitly disabled it. An OCSP policy is used, rather than a CRL policy,
+  // because the Apple TP will force an OCSP policy to be present and enabled
+  // if it believes the certificate may chain to an EV root. By explicitly
+  // disabling network and OCSP cache access, then even if the Apple TP
+  // enables OCSP checking, no revocation checking will actually succeed.
+  CSSM_APPLE_TP_OCSP_OPTIONS tp_ocsp_options;
+  memset(&tp_ocsp_options, 0, sizeof(tp_ocsp_options));
+  tp_ocsp_options.Version = CSSM_APPLE_TP_OCSP_OPTS_VERSION;
+
+  if (enable_revocation_checking) {
+    // The default for the OCSP policy is to fetch responses via the network,
+    // unlike the CRL policy default. The policy is further modified to
+    // prefer OCSP over CRLs, if both are specified on the certificate. This
+    // is because an OCSP response is both sufficient and typically
+    // significantly smaller than the CRL counterpart.
+    tp_ocsp_options.Flags = CSSM_TP_ACTION_OCSP_SUFFICIENT;
+  } else {
+    // Effectively disable OCSP checking by making it impossible to get an
+    // OCSP response. Even if the Apple TP forces OCSP, no checking will
+    // be able to succeed. If this happens, the Apple TP will report an error
+    // that OCSP was unavailable, but this will be handled and suppressed in
+    // X509Certificate::Verify().
+    tp_ocsp_options.Flags = CSSM_TP_ACTION_OCSP_DISABLE_NET |
+                            CSSM_TP_ACTION_OCSP_CACHE_READ_DISABLE;
+  }
+
+  SecPolicyRef ocsp_policy;
+  OSStatus status = CreatePolicy(&CSSMOID_APPLE_TP_REVOCATION_OCSP,
+                                 &tp_ocsp_options, sizeof(tp_ocsp_options),
+                                 &ocsp_policy);
+  if (status)
+    return status;
+  CFArrayAppendValue(policies, ocsp_policy);
+  CFRelease(ocsp_policy);
+
+  if (enable_revocation_checking) {
+    CSSM_APPLE_TP_CRL_OPTIONS tp_crl_options;
+    memset(&tp_crl_options, 0, sizeof(tp_crl_options));
+    tp_crl_options.Version = CSSM_APPLE_TP_CRL_OPTS_VERSION;
+    tp_crl_options.CrlFlags = CSSM_TP_ACTION_FETCH_CRL_FROM_NET;
+
+    SecPolicyRef crl_policy;
+    status = CreatePolicy(&CSSMOID_APPLE_TP_REVOCATION_CRL, &tp_crl_options,
+                          sizeof(tp_crl_options), &crl_policy);
+    if (status)
+      return status;
+    CFArrayAppendValue(policies, crl_policy);
+    CFRelease(crl_policy);
+  }
+
+  return status;
 }
 
+
 // static
 bool X509Certificate::GetSSLClientCertificates(
     const std::string& server_domain,