Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(162)

Unified Diff: net/cert/cert_verify_proc_mac.cc

Issue 2627523002: Refactor the assignment of CertVerifyResult::has_md2, etc. (Closed)
Patch Set: fix PrintTo() Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/cert/cert_verify_proc_ios.cc ('k') | net/cert/cert_verify_proc_nss.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/cert/cert_verify_proc_mac.cc
diff --git a/net/cert/cert_verify_proc_mac.cc b/net/cert/cert_verify_proc_mac.cc
index 2290db04b69ecb48c8e983293daba26a76522552..46d1cc864b8c7c7b3e15cf38660202dca53ea019 100644
--- a/net/cert/cert_verify_proc_mac.cc
+++ b/net/cert/cert_verify_proc_mac.cc
@@ -177,23 +177,12 @@ OSStatus CreateTrustPolicies(int flags, ScopedCFTypeRef<CFArrayRef>* policies) {
return noErr;
}
-// Stores the constructed certificate chain |cert_chain| and information about
-// the signature algorithms used into |*verify_result|. If the leaf cert in
-// |cert_chain| contains a weak (MD2, MD4, MD5, SHA-1) signature, stores that
-// in |*leaf_is_weak|. |cert_chain| must not be empty.
-void GetCertChainInfo(CFArrayRef cert_chain,
- CSSM_TP_APPLE_EVIDENCE_INFO* chain_info,
- CertVerifyResult* verify_result,
- bool* leaf_is_weak) {
+// Stores the constructed certificate chain |cert_chain| into
+// |*verify_result|. |cert_chain| must not be empty.
+void CopyCertChainToVerifyResult(CFArrayRef cert_chain,
+ CertVerifyResult* verify_result) {
DCHECK_LT(0, CFArrayGetCount(cert_chain));
- *leaf_is_weak = false;
- verify_result->has_md2 = false;
- verify_result->has_md4 = false;
- verify_result->has_md5 = false;
- verify_result->has_sha1 = false;
- verify_result->has_sha1_leaf = false;
-
SecCertificateRef verified_cert = NULL;
std::vector<SecCertificateRef> verified_chain;
for (CFIndex i = 0, count = CFArrayGetCount(cert_chain); i < count; ++i) {
@@ -204,6 +193,29 @@ void GetCertChainInfo(CFArrayRef cert_chain,
} else {
verified_chain.push_back(chain_cert);
}
+ }
+ if (!verified_cert) {
+ NOTREACHED();
+ return;
+ }
+
+ verify_result->verified_cert =
+ X509Certificate::CreateFromHandle(verified_cert, verified_chain);
+}
+
+// Returns true if the intermediates (excluding trusted certificates) use a
+// weak hashing algorithm, but the target does not use a weak hash.
+bool IsWeakChainBasedOnHashingAlgorithms(
+ CFArrayRef cert_chain,
+ CSSM_TP_APPLE_EVIDENCE_INFO* chain_info) {
+ DCHECK_LT(0, CFArrayGetCount(cert_chain));
+
+ bool intermediates_contain_weak_hash = false;
+ bool leaf_uses_weak_hash = false;
+
+ for (CFIndex i = 0, count = CFArrayGetCount(cert_chain); i < count; ++i) {
+ SecCertificateRef chain_cert = reinterpret_cast<SecCertificateRef>(
+ const_cast<void*>(CFArrayGetValueAtIndex(cert_chain, i)));
if ((chain_info[i].StatusBits & CSSM_CERT_STATUS_IS_IN_ANCHORS) ||
(chain_info[i].StatusBits & CSSM_CERT_STATUS_IS_ROOT)) {
@@ -215,29 +227,26 @@ void GetCertChainInfo(CFArrayRef cert_chain,
continue;
}
- bool is_leaf = i == 0;
X509Certificate::SignatureHashAlgorithm hash_algorithm =
- FillCertVerifyResultWeakSignature(chain_cert, is_leaf, verify_result);
- if (is_leaf) {
- switch (hash_algorithm) {
- case X509Certificate::kSignatureHashAlgorithmMd2:
- case X509Certificate::kSignatureHashAlgorithmMd4:
- case X509Certificate::kSignatureHashAlgorithmMd5:
- case X509Certificate::kSignatureHashAlgorithmSha1:
- *leaf_is_weak = true;
- break;
- case X509Certificate::kSignatureHashAlgorithmOther:
- break;
- }
+ X509Certificate::GetSignatureHashAlgorithm(chain_cert);
+
+ switch (hash_algorithm) {
+ case X509Certificate::kSignatureHashAlgorithmMd2:
+ case X509Certificate::kSignatureHashAlgorithmMd4:
+ case X509Certificate::kSignatureHashAlgorithmMd5:
+ case X509Certificate::kSignatureHashAlgorithmSha1:
+ if (i == 0) {
+ leaf_uses_weak_hash = true;
+ } else {
+ intermediates_contain_weak_hash = true;
+ }
+ break;
+ case X509Certificate::kSignatureHashAlgorithmOther:
+ break;
}
}
- if (!verified_cert) {
- NOTREACHED();
- return;
- }
- verify_result->verified_cert =
- X509Certificate::CreateFromHandle(verified_cert, verified_chain);
+ return !leaf_uses_weak_hash && intermediates_contain_weak_hash;
}
using ExtensionsMap = std::map<net::der::Input, net::ParsedExtension>;
@@ -814,14 +823,8 @@ int VerifyWithGivenFlags(X509Certificate* cert,
DCHECK(untrusted);
DCHECK_NE(kSecTrustResultRecoverableTrustFailure, temp_trust_result);
} else {
- CertVerifyResult temp_verify_result;
- bool leaf_is_weak = false;
- GetCertChainInfo(temp_chain, temp_chain_info, &temp_verify_result,
- &leaf_is_weak);
weak_chain =
- !leaf_is_weak &&
- (temp_verify_result.has_md2 || temp_verify_result.has_md4 ||
- temp_verify_result.has_md5 || temp_verify_result.has_sha1);
+ IsWeakChainBasedOnHashingAlgorithms(temp_chain, temp_chain_info);
}
// Set the result to the current chain if:
// - This is the first verification attempt. This ensures that if
@@ -866,9 +869,7 @@ int VerifyWithGivenFlags(X509Certificate* cert,
verify_result->cert_status |= CERT_STATUS_REVOKED;
if (CFArrayGetCount(completed_chain) > 0) {
- bool leaf_is_weak_unused = false;
- GetCertChainInfo(completed_chain, chain_info, verify_result,
- &leaf_is_weak_unused);
+ CopyCertChainToVerifyResult(completed_chain, verify_result);
}
// As of Security Update 2012-002/OS X 10.7.4, when an RSA key < 1024 bits
« no previous file with comments | « net/cert/cert_verify_proc_ios.cc ('k') | net/cert/cert_verify_proc_nss.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698