Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(149)

Unified Diff: net/base/x509_certificate_mac.cc

Issue 6824069: Allow revocation checking to be disabled on Mac, overriding/ignoring system settings (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: With suppressions Created 9 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: net/base/x509_certificate_mac.cc
diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc
index a60b2409fc123e8996cd74d2436e474ccc255879..033ddbf4a6ce64f5c51b7d470512412496dfc654 100644
--- a/net/base/x509_certificate_mac.cc
+++ b/net/base/x509_certificate_mac.cc
@@ -5,6 +5,7 @@
#include "net/base/x509_certificate.h"
#include <CommonCrypto/CommonDigest.h>
+#include <CoreServices/CoreServices.h>
#include <Security/Security.h>
#include <time.h>
@@ -26,6 +27,7 @@
#include "net/base/net_errors.h"
#include "net/base/test_root_certs.h"
#include "net/base/x509_certificate_known_roots_mac.h"
+#include "third_party/apple_apsl/cssmapplePriv.h"
#include "third_party/nss/mozilla/security/nss/lib/certdb/cert.h"
using base::mac::ScopedCFTypeRef;
@@ -286,6 +288,84 @@ OSStatus CreatePolicy(const CSSM_OID* policy_OID,
return noErr;
}
+// Creates a series of SecPolicyRefs to be added to a SecTrustRef used to
+// validate a certificate for an SSL peer. |hostname| contains the name of
+// the SSL peer that the certificate should be verified against. |flags| is
wtc 2011/04/20 19:40:33 Nit: should we change "SSL peer" to "SSL server"?
+// a bitwise-OR of VerifyFlags that can further alter how trust is
+// validated, such as how revocation is checked. If successful, returns
+// noErr, and stores the resultant array of SecPolicyRefs in |policies|.
+OSStatus CreateTrustPolicies(const std::string& hostname, int flags,
+ ScopedCFTypeRef<CFArrayRef>* policies) {
+ // 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);
+ if (status)
+ return status;
+ ScopedCFTypeRef<SecPolicyRef> scoped_ssl_policy(ssl_policy);
+
+ // Manually add OCSP and CRL policies. If neither an OCSP or CRL policy is
+ // specified, the Apple TP module will add whatever the system settings
+ // are, which is not desirable here.
+ //
+ // Note that this causes any locally configured OCSP responder URL to be
+ // ignored.
wtc 2011/04/20 19:40:33 Is the locally configured OCSP responder URL a sys
Ryan Sleevi 2011/04/20 22:46:51 It is a system setting. I documented this because
+ 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;
+
+ 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;
+
+ if (flags & X509Certificate::VERIFY_REV_CHECKING_ENABLED) {
+ // If an OCSP responder is available, use it, and avoid fetching any
+ // CRLs for that certificate if possible, as they may be much larger.
+ tp_ocsp_options.Flags = CSSM_TP_ACTION_OCSP_SUFFICIENT;
+ // Ensure that CRLs can be fetched if a crlDistributionPoint extension
+ // is found. Otherwise, only the local CRL cache will be consulted.
+ tp_crl_options.CrlFlags |= CSSM_TP_ACTION_FETCH_CRL_FROM_NET;
+ } else {
+ // Disable OCSP network fetching, but still permit cached OCSP responses
+ // to be used. This is equivalent to the Windows code's usage of
+ // CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY.
wtc 2011/04/20 19:40:33 Ideally we want to not do any revocation checking
Ryan Sleevi 2011/04/20 22:46:51 Really? This surprises me a little. I thought the
wtc 2011/04/20 23:23:54 Yes. That's what I expect if I uncheck the "Check
+ tp_ocsp_options.Flags = CSSM_TP_ACTION_OCSP_DISABLE_NET;
+ // The default CrlFlags will ensure only cached CRLs are used.
+ }
+
+ SecPolicyRef ocsp_policy;
+ status = CreatePolicy(&CSSMOID_APPLE_TP_REVOCATION_OCSP, &tp_ocsp_options,
+ sizeof(tp_ocsp_options), &ocsp_policy);
+ if (status)
+ return status;
+ ScopedCFTypeRef<SecPolicyRef> scoped_ocsp_policy(ocsp_policy);
+
+ SecPolicyRef crl_policy;
+ status = CreatePolicy(&CSSMOID_APPLE_TP_REVOCATION_CRL, &tp_crl_options,
+ sizeof(tp_crl_options), &crl_policy);
+ if (status)
+ return status;
+ ScopedCFTypeRef<SecPolicyRef> scoped_crl_policy(crl_policy);
+
+ CFTypeRef local_policies[] = { ssl_policy, ocsp_policy, crl_policy };
+ CFArrayRef policy_array = CFArrayCreate(kCFAllocatorDefault, local_policies,
+ arraysize(local_policies),
+ &kCFTypeArrayCallBacks);
wtc 2011/04/20 19:40:33 Just wanted to confirm that CFArrayCreate will ret
Ryan Sleevi 2011/04/20 22:46:51 Yes, per the documentation for the |callBacks| par
+ if (!policy_array)
+ return memFullErr;
+
+ policies->reset(policy_array);
+ return noErr;
+}
+
// Gets the issuer for a given cert, starting with the cert itself and
// including the intermediate and finally root certificates (if any).
// This function calls SecTrust but doesn't actually pay attention to the trust
@@ -734,23 +814,10 @@ int X509Certificate::Verify(const std::string& hostname, int flags,
return ERR_CERT_REVOKED;
}
- // 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);
+ ScopedCFTypeRef<CFArrayRef> trust_policies;
+ OSStatus status = CreateTrustPolicies(hostname, flags, &trust_policies);
if (status)
return NetErrorFromOSStatus(status);
- ScopedCFTypeRef<SecPolicyRef> scoped_ssl_policy(ssl_policy);
// Create and configure a SecTrustRef, which takes our certificate(s)
// and our SSL SecPolicyRef. SecTrustCreateWithCertificates() takes an
@@ -773,7 +840,8 @@ int X509Certificate::Verify(const std::string& hostname, int flags,
base::AutoLock lock(verification_lock_);
SecTrustRef trust_ref = NULL;
- status = SecTrustCreateWithCertificates(cert_array, ssl_policy, &trust_ref);
+ status = SecTrustCreateWithCertificates(cert_array, trust_policies,
+ &trust_ref);
if (status)
return NetErrorFromOSStatus(status);
ScopedCFTypeRef<SecTrustRef> scoped_trust_ref(trust_ref);
@@ -784,34 +852,51 @@ int X509Certificate::Verify(const std::string& hostname, int flags,
return NetErrorFromOSStatus(status);
}
+ CSSM_APPLE_TP_ACTION_DATA tp_action_data;
+ memset(&tp_action_data, 0, sizeof(tp_action_data));
+ tp_action_data.Version = CSSM_APPLE_TP_ACTION_VERSION;
+ // Allow CSSM to download any missing intermediate certificates if an
+ // authorityInfoAccess extension or issuerAltName extension is present.
+ tp_action_data.ActionFlags = CSSM_TP_ACTION_FETCH_CERT_FROM_NET;
wtc 2011/04/20 19:40:33 Good change! I didn't know CSSM can do this. I w
+
if (flags & VERIFY_REV_CHECKING_ENABLED) {
- // When called with VERIFY_REV_CHECKING_ENABLED, we ask SecTrustEvaluate()
- // to apply OCSP and CRL checking, but we're still subject to the global
- // settings, which are configured in the Keychain Access application (in
- // the Certificates tab of the Preferences dialog). If the user has
- // revocation disabled (which is the default), then we will get
- // kSecTrustResultRecoverableTrustFailure back from SecTrustEvaluate()
- // with one of a number of sub error codes indicating that revocation
- // checking did not occur. In that case, we'll set our own result to include
+ // Require a positive result from an OCSP responder or a CRL (or both)
+ // for every certificate in the chain. The Apple TP automatically
+ // excludes the self-signed root from this requirement. If a certificate
+ // is missing both a crlDistributionPoints extension and an
+ // authorityInfoAccess extension with an OCSP responder URL, then we
+ // will get a kSecTrustResultRecoverableTrustFailure back from
+ // SecTrustEvaluate(), with a
+ // CSSMERR_APPLETP_INCOMPLETE_REVOCATION_CHECK error code. In that case,
+ // we'll set our own result to include
+ // CERT_STATUS_NO_REVOCATION_MECHANISM. If one or both extensions are
+ // present, and a check fails (server unavailable, OCSP retry later,
+ // signature mismatch), then we'll set our own result to include
// CERT_STATUS_UNABLE_TO_CHECK_REVOCATION.
- //
- // NOTE: This does not apply to EV certificates, which always get
- // revocation checks regardless of the global settings.
+ tp_action_data.ActionFlags |= CSSM_TP_ACTION_REQUIRE_REV_PER_CERT;
verify_result->cert_status |= CERT_STATUS_REV_CHECKING_ENABLED;
- CSSM_APPLE_TP_ACTION_DATA tp_action_data = { CSSM_APPLE_TP_ACTION_VERSION };
- tp_action_data.ActionFlags = CSSM_TP_ACTION_REQUIRE_REV_PER_CERT;
- CFDataRef action_data_ref =
- CFDataCreate(NULL, reinterpret_cast<UInt8*>(&tp_action_data),
- sizeof(tp_action_data));
- if (!action_data_ref)
- return ERR_OUT_OF_MEMORY;
- ScopedCFTypeRef<CFDataRef> scoped_action_data_ref(action_data_ref);
- status = SecTrustSetParameters(trust_ref, CSSM_TP_ACTION_DEFAULT,
- action_data_ref);
- if (status)
- return NetErrorFromOSStatus(status);
+ } else {
+ // EV requires revocation checking.
+ // Note, under the hood, SecTrustEvaluate() will modify the OCSP options
+ // so as to attempt OCSP fetching if it believes a certificate may chain
wtc 2011/04/20 19:40:33 Nit: OCSP fetching => OCSP checking "fetching" im
+ // to an EV root. However, because network fetches are disabled in
+ // CreateTrustPolicies() when revocation checking is disabled, these
+ // will only go against the local cache.
+ flags &= ~VERIFY_EV_CERT;
}
+ CFDataRef action_data_ref =
+ CFDataCreateWithBytesNoCopy(kCFAllocatorDefault,
+ reinterpret_cast<UInt8*>(&tp_action_data),
+ sizeof(tp_action_data), kCFAllocatorNull);
+ if (!action_data_ref)
+ return ERR_OUT_OF_MEMORY;
+ ScopedCFTypeRef<CFDataRef> scoped_action_data_ref(action_data_ref);
+ status = SecTrustSetParameters(trust_ref, CSSM_TP_ACTION_DEFAULT,
+ action_data_ref);
+ if (status)
+ return NetErrorFromOSStatus(status);
+
// Verify the certificate. A non-zero result from SecTrustGetResult()
// indicates that some fatal error occurred and the chain couldn't be
// processed, not that the chain contains no errors. We need to examine the

Powered by Google App Engine
This is Rietveld 408576698