Make it a fatal SSL error when encountering certs signed with md[2,4], and interstitial md5

net/base/x509_certificate.cc

Index: net/base/x509_certificate.cc
diff --git a/net/base/x509_certificate.cc b/net/base/x509_certificate.cc
index e1fca4e77597f664ec7b0da2fd384c314e638b39..966b61654ad0a208872620848e1a5195be1b9b91 100644
--- a/net/base/x509_certificate.cc
+++ b/net/base/x509_certificate.cc
@@ -590,6 +590,18 @@ int X509Certificate::Verify(const std::string& hostname,
     rv = MapCertStatusToNetError(verify_result->cert_status);
   }
 
+  // Treat certificates signed using broken signature algorithms as invalid.
+  if (verify_result->has_md2 || verify_result->has_md4) {
+    verify_result->cert_status |= CERT_STATUS_INVALID;
+    rv = MapCertStatusToNetError(verify_result->cert_status);
+  }
+
+  // Flag certificates using weak signature algorithms.
+  if (verify_result->has_md5) {
+    verify_result->cert_status |= CERT_STATUS_WEAK_SIGNATURE_ALGORITHM;
+    rv = MapCertStatusToNetError(verify_result->cert_status);

[joth, 2011/11/03 09:09:38]

I just noticed all three of these MapCertStatusToNetError may overwrite a more
serious error in |rv| with a less critical one. I can't see a pretty way to
factor it out, but possibly we want to guard them:

  if (rv == OK) rv = MapCertStatusToNetError(verify_result->cert_status);

(internally MapCertStatusToNetError attempts to report the most serious error in
rv, we can do that in the general case here as rv may already contain a platform
specific error of unknown severity)

[palmer, 2011/11/03 17:47:04]

As I understand it, only CERT_STATUS_WEAK_SIGNATURE_ALGORITHM is potentially
less severe than what might already be set. CERT_STATUS_INVALID and
CERT_STATUS_AUTHORITY_INVALID are already fully fatal, right?

[joth, 2011/11/03 20:01:27]

The case I was thinking of was VerifyInternal may return something
super-duper-severe, but then any of there overwrite it here.
In practice though I agree this would only be a risk for the non-fatal
WEAK_SIGNATURE_ALGORITHM case .

[Ryan Sleevi, 2011/11/03 23:22:45]

Good catch. You're absolutely correct that this could potentially mask errors
(primarily, library errors, from looking at the implementations).

I've updated it to reflect your suggestion - to only replace |rv| when the
existing rv == OK, at least for the MD5 case.

This does mean that there is the potential for system library errors to be
replaced with a more generic error (ERR_CERT_INVALID) when using MD2/MD4 . These
will still result in non-user-overridable errors though.

I'm not thrilled with this, but I'm less thrilled about the idea of pushing the
'severity' of errors (user-overridable vs fatal) further into net/ from content/
&& chrome/.

+  }
+
   return rv;
 }