Mac EV verification using Chrome methods rather than OS methods.

net/cert/x509_util_mac.cc

Index: net/cert/x509_util_mac.cc
diff --git a/net/cert/x509_util_mac.cc b/net/cert/x509_util_mac.cc
index 46ae8fa7a77a875fbc9769330990a484c5769b9a..9170ef0ad53a47020ded984e6ac3ee9d110fcd7c 100644
--- a/net/cert/x509_util_mac.cc
+++ b/net/cert/x509_util_mac.cc
@@ -77,49 +77,73 @@ OSStatus CreateBasicX509Policy(SecPolicyRef* policy) {
 }
 
 OSStatus CreateRevocationPolicies(bool enable_revocation_checking,
-                                  bool enable_ev_checking,
                                   CFMutableArrayRef policies) {
-  OSStatus status = noErr;
-
-  // In order to bypass the system revocation checking settings, the
-  // SecTrustRef must have at least one revocation policy associated with it.
-  // Since it is not known prior to verification whether the Apple TP will
-  // consider a certificate as an EV candidate, the default policy used is a
-  // CRL policy, since it does not communicate over the network.
-  // If the TP believes the leaf is an EV cert, it will explicitly add an
-  // OCSP policy to perform the online checking, and if it doesn't believe
-  // that the leaf is EV, then the default CRL policy will effectively no-op.
-  // This behaviour is used to implement EV-only revocation checking.
-  if (enable_ev_checking || 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;
-    // Only allow network CRL fetches if the caller explicitly requests
-    // online revocation checking. Note that, as of OS X 10.7.2, the system
-    // will set force this flag on according to system policies, so
-    // online revocation checks cannot be completely disabled.
-    // Starting with OS X 10.12, if a CRL policy is added without the
-    // FETCH_CRL_FROM_NET flag, AIA fetching is disabled.
-    if (enable_revocation_checking || base::mac::IsAtLeastOS10_12())
+  if (base::mac::IsAtLeastOS10_12()) {
+// SecPolicyCreateRevocation is only on 10.9 or newer. This pragma stops
+// clang from complaining about it.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunguarded-availability"
+    // On Sierra we can't disable network revocation checking without also

[Ryan Sleevi, 2016/11/08 00:11:21]

nit: On Sierra, it's not possible to disable network revocation checking without
also breaking AIA.

(Reworded w/o the we)

[mattm, 2016/11/08 23:06:25]

Done.

+    // breaking AIA. If revocation checking isn't explicitly enabled, just don't
+    // add a revocation policy.
+    if (!enable_revocation_checking)
+      return noErr;
+
+    // If revocation checking is requested, enable checking and require positive
+    // results. Note that this will fail if there are certs with no
+    // CRLDistributionPoints or OCSP AIA urls, which differs from the behavior
+    // of |enable_revocation_checking| on pre-10.12. There does not appear to be
+    // a way around this, but it shouldn't matter much in practice since
+    // revocation checking is generally used with EV certs, where it is expected
+    // that all certs include revocation mechanisms.
+    SecPolicyRef revocation_policy =
+        SecPolicyCreateRevocation(kSecRevocationUseAnyAvailableMethod |
+                                  kSecRevocationRequirePositiveResponse);
+
+    if (!revocation_policy)
+      return errSecNoPolicyModule;
+    CFArrayAppendValue(policies, revocation_policy);
+    CFRelease(revocation_policy);
+    return noErr;
+#pragma clang diagnostic pop
+  } else {

[Ryan Sleevi, 2016/11/08 00:11:21]

Don't } else after a return

if (base::mac::IsAtLeastOS10_12()) {
  ...
  return noErr;
}

if ...

[mattm, 2016/11/08 23:06:25]

Done.

+    OSStatus status = noErr;
+
+    // In order to bypass the system revocation checking settings, the
+    // SecTrustRef must have at least one revocation policy associated with it.
+    // Since it is not known prior to verification whether the Apple TP will
+    // consider a certificate as an EV candidate, the default policy used is a
+    // CRL policy, since it does not communicate over the network.
+    // If the TP believes the leaf is an EV cert, it will explicitly add an
+    // OCSP policy to perform the online checking, and if it doesn't believe
+    // that the leaf is EV, then the default CRL policy will effectively no-op.
+    // This behaviour is used to implement EV-only revocation checking.
+    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;
+      // Only allow network CRL fetches if the caller explicitly requests
+      // online revocation checking. Note that, as of OS X 10.7.2, the system
+      // will set force this flag on according to system policies, so
+      // online revocation checks cannot be completely disabled.
+      // Starting with OS X 10.12, if a CRL policy is added without the
+      // FETCH_CRL_FROM_NET flag, AIA fetching is disabled.
       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);
-  }
+      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);
+    }
 
-  // If revocation checking is explicitly enabled, then add an OCSP policy
-  // and allow network access. If both revocation checking and EV checking
-  // are disabled, then the added OCSP policy will be prevented from
-  // accessing the network. This is done because the TP will force an OCSP
-  // policy to be present when it believes the certificate is EV. If network
-  // fetching was not explicitly disabled, then it would be as if
-  // enable_ev_checking was always set to true.
-  if (enable_revocation_checking || !enable_ev_checking) {
+    // If revocation checking is explicitly enabled, then add an OCSP policy
+    // and allow network access. If both revocation checking is
+    // disabled, then the added OCSP policy will be prevented from
+    // accessing the network. This is done because the TP will force an OCSP
+    // policy to be present when it believes the certificate is EV.
     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;
@@ -148,9 +172,9 @@ OSStatus CreateRevocationPolicies(bool enable_revocation_checking,
       return status;
     CFArrayAppendValue(policies, ocsp_policy);
     CFRelease(ocsp_policy);
-  }
 
-  return status;
+    return status;
+  }
 }
 
 CSSMFieldValue::CSSMFieldValue()