Cleanup X509CertificateMac style nits

net/base/x509_certificate_mac.cc

 // Copyright (c) 2006-2008 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/base/x509_certificate.h"
 
 #include <CommonCrypto/CommonDigest.h>
 #include <Security/Security.h>
 #include <time.h>
 
@@ skipping 204 matching lines @@
          name_type == GNT_URI);
 
   CSSMFields fields;
   OSStatus status = GetCertFields(cert_handle, &fields);
   if (status)
     return;
 
   for (size_t field = 0; field < fields.num_of_fields; ++field) {
     if (CSSMOIDEqual(&fields.fields[field].FieldOid, &oid)) {
       CSSM_X509_EXTENSION_PTR cssm_ext =
-          (CSSM_X509_EXTENSION_PTR)fields.fields[field].FieldValue.Data;
+          reinterpret_cast<CSSM_X509_EXTENSION_PTR>(
+              fields.fields[field].FieldValue.Data);
       CE_GeneralNames* alt_name =
-          (CE_GeneralNames*) cssm_ext->value.parsedValue;
+          reinterpret_cast<CE_GeneralNames*>(cssm_ext->value.parsedValue);
 
       for (size_t name = 0; name < alt_name->numNames; ++name) {
         const CE_GeneralName& name_struct = alt_name->generalName[name];
         // All of the general name types we support are encoded as
         // IA5String. In general, we should be switching off
         // |name_struct.nameType| and doing type-appropriate conversions. See
         // certextensions.h and the comment immediately preceding
         // CE_GeneralNameType for more information.
         if (name_struct.nameType == name_type) {
           const CSSM_DATA& name_data = name_struct.name;
-          std::string value =
-          std::string(reinterpret_cast<std::string::value_type*>
-                      (name_data.Data),
-                      name_data.Length);
+          std::string value = std::string(
+              reinterpret_cast<const char*>(name_data.Data),
+              name_data.Length);
           result->push_back(value);
         }
       }
     }
   }
 }
 
 void GetCertDateForOID(X509Certificate::OSCertHandle cert_handle,
                        CSSM_OID oid, Time* result) {
   *result = Time::Time();
 
   CSSMFields fields;
   OSStatus status = GetCertFields(cert_handle, &fields);
   if (status)
     return;
 
   for (size_t field = 0; field < fields.num_of_fields; ++field) {
     if (CSSMOIDEqual(&fields.fields[field].FieldOid, &oid)) {
-      CSSM_X509_TIME* x509_time =
-          reinterpret_cast<CSSM_X509_TIME *>
-            (fields.fields[field].FieldValue.Data);
-      std::string time_string =
-          std::string(reinterpret_cast<std::string::value_type*>
-                      (x509_time->time.Data),
-                      x509_time->time.Length);
-
-      DCHECK(x509_time->timeType == BER_TAG_UTC_TIME ||
-             x509_time->timeType == BER_TAG_GENERALIZED_TIME);
-
-      struct tm time;
-      const char* parse_string;
-      if (x509_time->timeType == BER_TAG_UTC_TIME)
-        parse_string = "%y%m%d%H%M%SZ";
-      else if (x509_time->timeType == BER_TAG_GENERALIZED_TIME)
-        parse_string = "%y%m%d%H%M%SZ";
-      else {
-        // Those are the only two BER tags for time; if neither are used then
-        // this is a rather broken cert.
+      CSSM_X509_TIME* x509_time = reinterpret_cast<CSSM_X509_TIME*>(
+          fields.fields[field].FieldValue.Data);
+      if (x509_time->timeType != BER_TAG_UTC_TIME &&
+          x509_time->timeType != BER_TAG_GENERALIZED_TIME) {
+        LOG(ERROR) << "Unsupported date/time format "
+                   << x509_time->timeType;
         return;
       }
 
-      strptime(time_string.c_str(), parse_string, &time);
-
-      Time::Exploded exploded;
-      exploded.year         = time.tm_year + 1900;
-      exploded.month        = time.tm_mon + 1;
-      exploded.day_of_week  = time.tm_wday;
-      exploded.day_of_month = time.tm_mday;
-      exploded.hour         = time.tm_hour;
-      exploded.minute       = time.tm_min;
-      exploded.second       = time.tm_sec;
-      exploded.millisecond  = 0;
-
-      *result = Time::FromUTCExploded(exploded);
-      break;
+      base::StringPiece time_string(
+          reinterpret_cast<const char*>(x509_time->time.Data),
+          x509_time->time.Length);
+      CertDateFormat format = x509_time->timeType == BER_TAG_UTC_TIME ?
+          CERT_DATE_FORMAT_UTC_TIME : CERT_DATE_FORMAT_GENERALIZED_TIME;
+      if (!ParseCertificateDate(time_string, format, result))
+        LOG(ERROR) << "Invalid certificate date/time " << time_string;
+      return;
     }
   }
 }
 
 // Creates a SecPolicyRef for the given OID, with optional value.
 OSStatus CreatePolicy(const CSSM_OID* policy_OID,
                       void* option_data,
                       size_t option_length,
                       SecPolicyRef* policy) {
   SecPolicySearchRef search;
@@ skipping 20 matching lines @@
   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
 // result: it shouldn't be used to determine trust, just to traverse the chain.
 // Caller is responsible for releasing the value stored into *out_cert_chain.
 OSStatus CopyCertChain(SecCertificateRef cert_handle,
                        CFArrayRef* out_cert_chain) {
-  DCHECK(cert_handle && out_cert_chain);
+  DCHECK(cert_handle);
+  DCHECK(out_cert_chain);
   // Create an SSL policy ref configured for client cert evaluation.
   SecPolicyRef ssl_policy;
   OSStatus result = X509Certificate::CreateSSLClientPolicy(&ssl_policy);
   if (result)
     return result;
   ScopedCFTypeRef<SecPolicyRef> scoped_ssl_policy(ssl_policy);
 
   // Create a SecTrustRef.
-  ScopedCFTypeRef<CFArrayRef> input_certs(
-      CFArrayCreate(NULL, (const void**)&cert_handle, 1,
-                    &kCFTypeArrayCallBacks));
+  ScopedCFTypeRef<CFArrayRef> input_certs(CFArrayCreate(
+      NULL, const_cast<const void**>(reinterpret_cast<void**>(&cert_handle)),
+      1, &kCFTypeArrayCallBacks));
   SecTrustRef trust_ref = NULL;
   result = SecTrustCreateWithCertificates(input_certs, ssl_policy, &trust_ref);
   if (result)
     return result;
   ScopedCFTypeRef<SecTrustRef> trust(trust_ref);
 
   // Evaluate trust, which creates the cert chain.
   SecTrustResultType status;
   CSSM_TP_APPLE_EVIDENCE_INFO* status_chain;
   result = SecTrustEvaluate(trust, &status);
@@ skipping 34 matching lines @@
 }
 
 // Parses |data| of length |length|, attempting to decode it as the specified
 // |format|. If |data| is in the specified format, any certificates contained
 // within are stored into |output|.
 void AddCertificatesFromBytes(const char* data, size_t length,
                               SecExternalFormat format,
                               X509Certificate::OSCertHandles* output) {
   SecExternalFormat input_format = format;
   ScopedCFTypeRef<CFDataRef> local_data(CFDataCreateWithBytesNoCopy(
-      kCFAllocatorDefault, reinterpret_cast<const UInt8*>(data),
-      length, kCFAllocatorNull));
+      kCFAllocatorDefault, reinterpret_cast<const UInt8*>(data), length,
+      kCFAllocatorNull));
 
   CFArrayRef items = NULL;
   OSStatus status = SecKeychainItemImport(local_data, NULL, &input_format,
                                           NULL, 0, NULL, NULL, &items);
   if (status) {
     DLOG(WARNING) << status << " Unable to import items from data of length "
                   << length;
     return;
   }
 
@@ skipping 30 matching lines @@
 
 }  // namespace
 
 void SetMacTestCertificate(X509Certificate* cert) {
   g_mac_trusted_certificates.Get().SetTestCertificate(cert);
 }
 
 void X509Certificate::Initialize() {
   const CSSM_X509_NAME* name;
   OSStatus status = SecCertificateGetSubject(cert_handle_, &name);
-  if (!status) {
+  if (!status)
     subject_.Parse(name);
-  }
+
   status = SecCertificateGetIssuer(cert_handle_, &name);
-  if (!status) {
+  if (!status)
     issuer_.Parse(name);
-  }
 
   GetCertDateForOID(cert_handle_, CSSMOID_X509V1ValidityNotBefore,
                     &valid_start_);
   GetCertDateForOID(cert_handle_, CSSMOID_X509V1ValidityNotAfter,
                     &valid_expiry_);
 
   fingerprint_ = CalculateFingerprint(cert_handle_);
 }
 
 // static
@@ skipping 180 matching lines @@
           verify_result->cert_status |= CERT_STATUS_DATE_INVALID;
         for (uint32 status_code_index = 0;
              status_code_index < chain_info[index].NumStatusCodes;
              ++status_code_index) {
           got_certificate_error = true;
           int cert_status = CertStatusFromOSStatus(
               chain_info[index].StatusCodes[status_code_index]);
           if (cert_status == CERT_STATUS_COMMON_NAME_INVALID) {
             std::vector<std::string> names;
             GetDNSNames(&names);
-            if (OverrideHostnameMismatch(hostname, &names)) {
+            if (OverrideHostnameMismatch(hostname, &names))
               cert_status = 0;
-            }
           }
           verify_result->cert_status |= cert_status;
         }
       }
       // Be paranoid and ensure that we recorded at least one certificate
       // status on receiving kSecTrustResultRecoverableTrustFailure. The
       // call to SecTrustGetCssmResultCode() should pick up when the chain
       // is not trusted and the loop through CSSM_TP_APPLE_EVIDENCE_INFO
       // should pick up everything else, but let's be safe.
       if (!verify_result->cert_status && !got_certificate_error) {
         verify_result->cert_status |= CERT_STATUS_INVALID;
         NOTREACHED();
       }
       break;
 
     default:
       status = SecTrustGetCssmResultCode(trust_ref, &cssm_result);
       if (status)
         return NetErrorFromOSStatus(status);
       verify_result->cert_status |= CertStatusFromOSStatus(cssm_result);
-      if (!verify_result->cert_status) {
+      if (!verify_result->cert_status)
         verify_result->cert_status |= CERT_STATUS_INVALID;
-      }
       break;
   }
 
   // TODO(wtc): Suppress CERT_STATUS_NO_REVOCATION_MECHANISM for now to be
   // compatible with Windows, which in turn implements this behavior to be
   // compatible with WinHTTP, which doesn't report this error (bug 3004).
   verify_result->cert_status &= ~CERT_STATUS_NO_REVOCATION_MECHANISM;
 
   if (IsCertStatusError(verify_result->cert_status))
     return MapCertStatusToNetError(verify_result->cert_status);
@@ skipping 113 matching lines @@
 SHA1Fingerprint X509Certificate::CalculateFingerprint(
     OSCertHandle cert) {
   SHA1Fingerprint sha1;
   memset(sha1.data, 0, sizeof(sha1.data));
 
   CSSM_DATA cert_data;
   OSStatus status = SecCertificateGetData(cert, &cert_data);
   if (status)
     return sha1;
 
-  DCHECK(NULL != cert_data.Data);
-  DCHECK(0 != cert_data.Length);
+  DCHECK(cert_data.Data);
+  DCHECK_NE(cert_data.Length, 0U);
 
   CC_SHA1(cert_data.Data, cert_data.Length, sha1.data);
 
   return sha1;
 }
 
 bool X509Certificate::SupportsSSLClientAuth() const {
   CSSMFields fields;
   if (GetCertFields(cert_handle_, &fields) != noErr)
     return false;
@@ skipping 30 matching lines @@
     return false;
   return true;
 }
 
 bool X509Certificate::IsIssuedBy(
     const std::vector<CertPrincipal>& valid_issuers) {
   // Get the cert's issuer chain.
   CFArrayRef cert_chain = NULL;
   OSStatus result;
   result = CopyCertChain(os_cert_handle(), &cert_chain);
-  if (result != noErr)
+  if (result)
     return false;
   ScopedCFTypeRef<CFArrayRef> scoped_cert_chain(cert_chain);
 
   // Check all the certs in the chain for a match.
   int n = CFArrayGetCount(cert_chain);
   for (int i = 0; i < n; ++i) {
     SecCertificateRef cert_handle = reinterpret_cast<SecCertificateRef>(
         const_cast<void*>(CFArrayGetValueAtIndex(cert_chain, i)));
     scoped_refptr<X509Certificate> cert(X509Certificate::CreateFromHandle(
         cert_handle,
@@ skipping 15 matching lines @@
     NULL,
     CSSM_APPLE_TP_SSL_CLIENT
   };
   return CreatePolicy(&CSSMOID_APPLE_TP_SSL,
                       &tp_ssl_options,
                       sizeof(tp_ssl_options),
                       out_policy);
 }
 
 // static
-bool X509Certificate::GetSSLClientCertificates (
+bool X509Certificate::GetSSLClientCertificates(
     const std::string& server_domain,
     const std::vector<CertPrincipal>& valid_issuers,
-    std::vector<scoped_refptr<X509Certificate> >* certs) {
+    CertificateList* certs) {
   ScopedCFTypeRef<SecIdentityRef> preferred_identity;
   if (!server_domain.empty()) {
     // See if there's an identity preference for this domain:
     ScopedCFTypeRef<CFStringRef> domain_str(
         base::SysUTF8ToCFStringRef("https://" + server_domain));
     SecIdentityRef identity = NULL;
-    if (SecIdentityCopyPreference(domain_str,
-                                  0,
-                                  NULL, // validIssuers argument is ignored :(
-                                  &identity) == noErr)
+    // While SecIdentityCopyPreferences appears to take a list of CA issuers
+    // to restrict the identity search to, within Security.framework the
+    // argument is ignored and filtering unimplemented. See
+    // SecIdentity.cpp in libsecurity_keychain, specifically
+    // _SecIdentityCopyPreferenceMatchingName().
+    if (SecIdentityCopyPreference(domain_str, 0, NULL, &identity) == noErr)
       preferred_identity.reset(identity);
   }
 
   // Now enumerate the identities in the available keychains.
   SecIdentitySearchRef search = nil;
   OSStatus err = SecIdentitySearchCreate(NULL, CSSM_KEYUSE_SIGN, &search);
   ScopedCFTypeRef<SecIdentitySearchRef> scoped_search(search);
   while (!err) {
     SecIdentityRef identity = NULL;
     err = SecIdentitySearchCopyNext(search, &identity);
@@ skipping 57 matching lines @@
   if (result) {
     LOG(ERROR) << "SecIdentityCreateWithCertificate error " << result;
     return NULL;
   }
   ScopedCFTypeRef<CFMutableArrayRef> chain(
       CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks));
   CFArrayAppendValue(chain, identity);
 
   CFArrayRef cert_chain = NULL;
   result = CopyCertChain(cert_handle_, &cert_chain);
-  if (result)
-    goto exit;
+  if (result) {
+    LOG(ERROR) << "CreateIdentityCertificateChain error " << result;
+    return chain.release();
+  }
 
   // Append the intermediate certs from SecTrust to the result array:
   if (cert_chain) {
     int chain_count = CFArrayGetCount(cert_chain);
     if (chain_count > 1) {
       CFArrayAppendArray(chain,
                          cert_chain,
                          CFRangeMake(1, chain_count - 1));
     }
     CFRelease(cert_chain);
   }
-exit:
-  if (result)
-    LOG(ERROR) << "CreateIdentityCertificateChain error " << result;
+
   return chain.release();
 }
 
 }  // namespace net