Record when certificates signed with md[2,4,5] are encountered when using OpenSSL

net/base/x509_certificate_openssl.cc

Index: net/base/x509_certificate_openssl.cc
diff --git a/net/base/x509_certificate_openssl.cc b/net/base/x509_certificate_openssl.cc
index 58809112d63733dd61dbb99a941fd2b103030e2e..b25d3835df824b03870560034aabd334550d76f9 100644
--- a/net/base/x509_certificate_openssl.cc
+++ b/net/base/x509_certificate_openssl.cc
@@ -299,6 +299,43 @@ bool GetDERAndCacheIfNeeded(X509Certificate::OSCertHandle cert,
   return true;
 }
 
+void GetCertChainInfo(X509_STORE_CTX* store_ctx,

[joth, 2011/11/01 09:31:45]

Not sure if more stuff is likely to migrate into here, but maybe we could make
the name more explicit about which info it gets? (also, I'm not sure if the verb
here should be Get so much as Set? Or more generically, Process.  c.f.
AppendPublicKeyHashes in the other platform versions of this file)

SetVerifiedCertAndMdFlagFields ?

[Ryan Sleevi, 2011/11/03 01:52:58]

We're currently using this name for [_nss, _win, _mac], so I've left it the same
for now. I don't feel particularly strongly one way or the other re: naming,
other than we have been expanding it over time (such as this CL :p)

+                      CertVerifyResult* verify_result) {
+  STACK_OF(X509)* chain = X509_STORE_CTX_get_chain(store_ctx);
+  X509* verified_cert = NULL;
+  std::vector<X509*> verified_chain;
+  for (int i = 0; i < sk_X509_num(chain); ++i) {
+    X509* cert = sk_X509_value(chain, i);
+    if (i == 0) {
+      verified_cert = cert;
+    } else {
+      verified_chain.push_back(cert);
+    }
+
+    // Only check the algorithm status for certificates that are not in the
+    // trust store.
+    if (i < store_ctx->last_untrusted) {
+      int sig_alg = OBJ_obj2nid(cert->sig_alg->algorithm);
+      if (sig_alg == NID_md2WithRSAEncryption) {
+        verify_result->has_md2 = true;
+        if (i != 0)
+          verify_result->has_md2_ca = true;
+      } else if (sig_alg == NID_md4WithRSAEncryption) {
+        verify_result->has_md4 = true;
+      } else if (sig_alg == NID_md5WithRSAEncryption) {
+        verify_result->has_md5 = true;
+        if (i != 0)
+          verify_result->has_md5_ca = true;
+      }
+    }
+  }
+
+  if (verified_cert) {
+    verify_result->verified_cert =
+        X509Certificate::CreateFromHandle(verified_cert, verified_chain);
+  }
+}
+
 }  // namespace
 
 // static
@@ -480,19 +517,14 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
     verify_result->cert_status |= cert_status;
   }
 
+  GetCertChainInfo(ctx.get(), verify_result);
+
   if (IsCertStatusError(verify_result->cert_status))
     return MapCertStatusToNetError(verify_result->cert_status);
 

[joth, 2011/11/01 09:31:45]

Maybe it would be as well to extract the next loop into AppendPublicKeyHashes()
like on the other platforms? That way it's a bit easier to spot the high-level
steps that occur before vs after the early return for the error-cases.

[Ryan Sleevi, 2011/11/03 01:52:58]

Done.

   STACK_OF(X509)* chain = X509_STORE_CTX_get_chain(ctx.get());
-  X509* verified_cert = NULL;
-  std::vector<X509*> verified_chain;
   for (int i = 0; i < sk_X509_num(chain); ++i) {
     X509* cert = sk_X509_value(chain, i);
-    if (i == 0) {
-      verified_cert = cert;
-    } else {
-      verified_chain.push_back(cert);
-    }
 
     DERCache der_cache;
     if (!GetDERAndCacheIfNeeded(cert, &der_cache))
@@ -510,11 +542,6 @@ int X509Certificate::VerifyInternal(const std::string& hostname,
     verify_result->public_key_hashes.push_back(hash);
   }
 
-  if (verified_cert) {
-    verify_result->verified_cert = CreateFromHandle(verified_cert,
-                                                    verified_chain);
-  }
-
   // Currently we only ues OpenSSL's default root CA paths, so treat all
   // correctly verified certs as being from a known root. TODO(joth): if the
   // motivations described in http://src.chromium.org/viewvc/chrome?view=rev&revision=80778