[refactor] Extract the CertVerifyResult assignment of has_md2, has_md4,

net/cert/cert_verify_proc_mac.cc

 // Copyright (c) 2012 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/cert/cert_verify_proc_mac.h"
 
 #include <CommonCrypto/CommonDigest.h>
 #include <CoreServices/CoreServices.h>
 #include <Security/Security.h>
 
@@ skipping 197 matching lines @@
     if ((chain_info[i].StatusBits & CSSM_CERT_STATUS_IS_IN_ANCHORS) ||
         (chain_info[i].StatusBits & CSSM_CERT_STATUS_IS_ROOT)) {
       // The current certificate is either in the user's trusted store or is
       // a root (self-signed) certificate. Ignore the signature algorithm for
       // these certificates, as it is meaningless for security. We allow
       // self-signed certificates (i == 0 & IS_ROOT), since we accept that
       // any security assertions by such a cert are inherently meaningless.
       continue;
     }
 
-    x509_util::CSSMCachedCertificate cached_cert;
-    OSStatus status = cached_cert.Init(chain_cert);
-    if (status)
-      continue;
-    x509_util::CSSMFieldValue signature_field;
-    status = cached_cert.GetField(&CSSMOID_X509V1SignatureAlgorithm,
-                                  &signature_field);
-    if (status || !signature_field.field())
-      continue;
-    // Match the behaviour of OS X system tools and defensively check that
-    // sizes are appropriate. This would indicate a critical failure of the
-    // OS X certificate library, but based on history, it is best to play it
-    // safe.
-    const CSSM_X509_ALGORITHM_IDENTIFIER* sig_algorithm =
-        signature_field.GetAs<CSSM_X509_ALGORITHM_IDENTIFIER>();
-    if (!sig_algorithm)
-      continue;
-
-    const CSSM_OID* alg_oid = &sig_algorithm->algorithm;
-    if (CSSMOIDEqual(alg_oid, &CSSMOID_MD2WithRSA)) {
-      verify_result->has_md2 = true;
-      if (i == 0)
-        *leaf_is_weak = true;
-    } else if (CSSMOIDEqual(alg_oid, &CSSMOID_MD4WithRSA)) {
-      verify_result->has_md4 = true;
-      if (i == 0)
-        *leaf_is_weak = true;
-    } else if (CSSMOIDEqual(alg_oid, &CSSMOID_MD5WithRSA)) {
-      verify_result->has_md5 = true;
-      if (i == 0)
-        *leaf_is_weak = true;
-    } else if (CSSMOIDEqual(alg_oid, &CSSMOID_SHA1WithRSA) ||
-               CSSMOIDEqual(alg_oid, &CSSMOID_SHA1WithRSA_OIW) ||
-               CSSMOIDEqual(alg_oid, &CSSMOID_SHA1WithDSA) ||
-               CSSMOIDEqual(alg_oid, &CSSMOID_SHA1WithDSA_CMS) ||
-               CSSMOIDEqual(alg_oid, &CSSMOID_SHA1WithDSA_JDK) ||
-               CSSMOIDEqual(alg_oid, &CSSMOID_ECDSA_WithSHA1)) {
-      verify_result->has_sha1 = true;
-      if (i == 0) {
-        verify_result->has_sha1_leaf = true;
-        *leaf_is_weak = true;
-      }
-    }
+    bool is_leaf = i == 0;
+    auto hash_is_weak =

[Ryan Sleevi, 2017/01/05 22:48:24]

I would definitely argue this use of "auto" is not appropriate. You definitely
don't save anything over 'bool', and lose in readability.

[eroman, 2017/01/05 23:21:53]

Agreed auto is wrong here (left over from earlier iteration).

[eroman, 2017/01/05 23:36:30]

Done.

+        FillCertVerifyResultWeakSignature(chain_cert, is_leaf, verify_result);
+    if (is_leaf && hash_is_weak)
+      *leaf_is_weak = hash_is_weak;
   }
   if (!verified_cert) {
     NOTREACHED();
     return;
   }
 
   verify_result->verified_cert =
       X509Certificate::CreateFromHandle(verified_cert, verified_chain);
 }
 
@@ skipping 831 matching lines @@
     // EV cert and it was covered by CRLSets or revocation checking passed.
     verify_result->cert_status |= CERT_STATUS_IS_EV;
   }
 
   return OK;
 }
 
 }  // namespace net
 
 #pragma clang diagnostic pop  // "-Wdeprecated-declarations"