Do not treat weak (< 1024 bit RSA) keys as fatal on OS X

A change in system behaviour was introduced in the
OS X Lion 10.7.4/Apple Security Update 2012-002 (
http://support.apple.com/kb/HT5281 ) that caused weak keys to be treated
as fatal errors. Map the newly-returned certificate status to the existing
CERT_STATUS_WEAK_KEY, rather than CERT_STATUS_INVALID, so that users may
override the error as they can on other platforms.

BUG=129126
TEST=net_unittests:CertVerifyProcTest.RejectWeakKeys


Review URL: https://chromiumcodereview.appspot.com/10451059

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@139388 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 33 insertions, 2 deletions

diff --git a/net/base/cert_verify_proc_mac.cc b/net/base/cert_verify_proc_mac.cc
index 959378e..49798b5 100644
--- a/net/base/cert_verify_proc_mac.cc
+++ b/net/base/cert_verify_proc_mac.cc
@@ -111,6 +111,15 @@ CertStatus CertStatusFromOSStatus(OSStatus status) {
     case CSSMERR_APPLETP_IDP_FAIL:
       return CERT_STATUS_INVALID;
 
+    case CSSMERR_CSP_UNSUPPORTED_KEY_SIZE:
+      // Mapping UNSUPPORTED_KEY_SIZE to CERT_STATUS_WEAK_KEY is not strictly
+      // accurate, as the error may have been returned due to a key size
+      // that exceeded the maximum supported. However, within
+      // CertVerifyProcMac::VerifyInternal(), this code should only be
+      // encountered as a certificate status code, and only when the key size
+      // is smaller than the minimum required (1024 bits).
+      return CERT_STATUS_WEAK_KEY;
+
     default: {
       // Failure was due to something Chromium doesn't define a
       // specific status for (such as basic constraints violation, or
@@ -428,6 +437,14 @@ int CertVerifyProcMac::VerifyInternal(X509Certificate* cert,
 
   GetCertChainInfo(scoped_completed_chain.get(), chain_info, verify_result);
 
+  // As of Security Update 2012-002/OS X 10.7.4, when an RSA key < 1024 bits
+  // is encountered, CSSM returns CSSMERR_TP_VERIFY_ACTION_FAILED and adds
+  // CSSMERR_CSP_UNSUPPORTED_KEY_SIZE as a certificate status. Avoid mapping
+  // the CSSMERR_TP_VERIFY_ACTION_FAILED to CERT_STATUS_INVALID if the only
+  // error was due to an unsupported key size.
+  bool policy_failed = false;
+  bool weak_key = false;
+
   // Evaluate the results
   OSStatus cssm_result;
   switch (trust_result) {
@@ -450,7 +467,11 @@ int CertVerifyProcMac::VerifyInternal(X509Certificate* cert,
       status = SecTrustGetCssmResultCode(trust_ref, &cssm_result);
       if (status)
         return NetErrorFromOSStatus(status);
-      verify_result->cert_status |= CertStatusFromOSStatus(cssm_result);
+      if (cssm_result == CSSMERR_TP_VERIFY_ACTION_FAILED) {
+        policy_failed = true;
+      } else {
+        verify_result->cert_status |= CertStatusFromOSStatus(cssm_result);
+      }
       // Walk the chain of error codes in the CSSM_TP_APPLE_EVIDENCE_INFO
       // structure which can catch multiple errors from each certificate.
       for (CFIndex index = 0, chain_count = CFArrayGetCount(completed_chain);
@@ -467,10 +488,18 @@ int CertVerifyProcMac::VerifyInternal(X509Certificate* cert,
         for (uint32 status_code_index = 0;
              status_code_index < chain_info[index].NumStatusCodes;
              ++status_code_index) {
-          verify_result->cert_status |= CertStatusFromOSStatus(
+          CertStatus mapped_status = CertStatusFromOSStatus(
               chain_info[index].StatusCodes[status_code_index]);
+          if (mapped_status == CERT_STATUS_WEAK_KEY)
+            weak_key = true;
+          verify_result->cert_status |= mapped_status;
         }
       }
+      if (policy_failed && !weak_key) {
+        // If CSSMERR_TP_VERIFY_ACTION_FAILED wasn't returned due to a weak
+        // key, map it back to an appropriate error code.
+        verify_result->cert_status |= CertStatusFromOSStatus(cssm_result);
+      }
       if (!IsCertStatusError(verify_result->cert_status)) {
         LOG(ERROR) << "cssm_result=" << cssm_result;
         verify_result->cert_status |= CERT_STATUS_INVALID;
diff --git a/net/base/cert_verify_proc_unittest.cc b/net/base/cert_verify_proc_unittest.cc
index 5799200..dbcf068 100644
--- a/net/base/cert_verify_proc_unittest.cc
+++ b/net/base/cert_verify_proc_unittest.cc
@@ -297,6 +297,8 @@ TEST_F(CertVerifyProcTest, RejectWeakKeys) {
         EXPECT_NE(OK, error);
         EXPECT_EQ(CERT_STATUS_WEAK_KEY,
                   verify_result.cert_status & CERT_STATUS_WEAK_KEY);
+        EXPECT_NE(CERT_STATUS_INVALID,
+                  verify_result.cert_status & CERT_STATUS_INVALID);
       } else {
         EXPECT_EQ(OK, error);
         EXPECT_EQ(0U, verify_result.cert_status & CERT_STATUS_WEAK_KEY);