Record when certificates signed with md[2,4,5] are encountered on OS X.

net/base/x509_certificate_mac.cc

Index: net/base/x509_certificate_mac.cc
diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc
index c47d1a51f675e50b61ad09c8cb4aa1890a440fab..351b8f484765aaf7cee1e695ba7ec102cb4a2db3 100644
--- a/net/base/x509_certificate_mac.cc
+++ b/net/base/x509_certificate_mac.cc
@@ -290,6 +290,62 @@ OSStatus CreateTrustPolicies(const std::string& hostname,
   return noErr;
 }
 
+// Saves some information about the certificate chain |cert_chain| in
+// |*verify_result|. The caller MUST initialize |*verify_result| before
+// calling this function.
+void GetCertChainInfo(CFArrayRef cert_chain,
+                      CertVerifyResult* verify_result) {
+  SecCertificateRef verified_cert = NULL;
+  std::vector<SecCertificateRef> verified_chain;
+  for (CFIndex i = 0, count = CFArrayGetCount(cert_chain);
+       i < count; ++i) {

[wtc, 2011/10/25 18:24:44]

Does this not fit on the previous line?

+    SecCertificateRef chain_cert = reinterpret_cast<SecCertificateRef>(
+        const_cast<void*>(CFArrayGetValueAtIndex(cert_chain, i)));
+    if (i == 0) {
+      verified_cert = chain_cert;
+    } else {
+      verified_chain.push_back(chain_cert);
+    }
+
+    CSSMFields fields;
+    OSStatus status = GetCertFields(chain_cert, &fields);
+    if (status)
+      continue;
+    for (size_t field = 0; field < fields.num_of_fields; ++field) {
+      if (!CSSMOIDEqual(&fields.fields[field].FieldOid,

[palmer, 2011/10/25 19:53:50]

"fields fields field field oid" is baffling. Are there better names we could
use?

CSSMFields cssmFields;
...
WhateverTheTypeIs fields = cssmFields.fields;
...

for (size_t i = 0; i < cssmFields.num_of_fields; ++i) {
    if (!CSSMOIDEqual(&fields[i].FieldOid, ...
...
...fields[i].FieldValue.Data)...
et c.

It's also a micro-optimization to hoist all those accesses of cssmFields out of
the loop. (Unless the compiler does it anyway, in which case the code might as
well also be more readable to humans.)

+                        &CSSMOID_X509V1SignatureAlgorithm)) {
+        continue;
+      }
+
+      CSSM_X509_ALGORITHM_IDENTIFIER* signature_algorithm =
+          reinterpret_cast<CSSM_X509_ALGORITHM_IDENTIFIER*>(
+              fields.fields[field].FieldValue.Data);
+      if (!signature_algorithm || (fields.fields[field].FieldValue.Length !=
+          sizeof(CSSM_X509_ALGORITHM_IDENTIFIER))) {
+        break;

[wtc, 2011/10/25 18:24:44]

If we get here, it means the Mac OS X certificate library
malfunctioned, right?  So this is defensive programming?
Would be nice to add a comment to make this clear.

+      }
+      CSSM_OID_PTR alg_oid = &signature_algorithm->algorithm;
+      if (CSSMOIDEqual(alg_oid, &CSSMOID_MD2WithRSA)) {
+        verify_result->has_md2 = true;
+        if (i != 0)
+          verify_result->has_md2_ca = true;
+      } else if (CSSMOIDEqual(alg_oid, &CSSMOID_MD4WithRSA)) {
+        verify_result->has_md4 = true;
+      } else if (CSSMOIDEqual(alg_oid, &CSSMOID_MD5WithRSA)) {

[palmer, 2011/10/25 19:53:50]

As in the other CL, we should keep track of MD4 CAs even though we don't think
we'll ever see any.

+        verify_result->has_md5 = true;
+        if (i != 0)
+          verify_result->has_md5_ca = true;
+      }
+      break;
+    }
+  }
+  if (!verified_cert)
+    return;
+
+  verify_result->verified_cert =
+      X509Certificate::CreateFromHandle(verified_cert, verified_chain);
+}
+
 // 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
@@ -839,22 +895,7 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
     return NetErrorFromOSStatus(status);
   ScopedCFTypeRef<CFArrayRef> scoped_completed_chain(completed_chain);
 
-  SecCertificateRef verified_cert = NULL;
-  std::vector<SecCertificateRef> verified_chain;
-  for (CFIndex i = 0, count = CFArrayGetCount(completed_chain);
-       i < count; ++i) {
-    SecCertificateRef chain_cert = reinterpret_cast<SecCertificateRef>(
-        const_cast<void*>(CFArrayGetValueAtIndex(completed_chain, i)));
-    if (i == 0) {
-      verified_cert = chain_cert;
-    } else {
-      verified_chain.push_back(chain_cert);
-    }
-  }
-  if (verified_cert) {
-    verify_result->verified_cert = CreateFromHandle(verified_cert,
-                                                    verified_chain);
-  }
+  GetCertChainInfo(scoped_completed_chain.get(), verify_result);
 
   // Evaluate the results
   OSStatus cssm_result;