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

Unified Diff: net/cert/cert_verify_proc_mac.cc

Issue 1094983002: Account for the OS returning an empty certificate chain. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: test, somewhat tidier code Created 5 years, 8 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.h ('k') | net/cert/cert_verify_proc_unittest.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 388d0fc41a93b81ba84d8565bec4996fbe0b31f7..303f0aa6633d034309d6b126f53f5feb1f91c96c 100644
--- a/net/cert/cert_verify_proc_mac.cc
+++ b/net/cert/cert_verify_proc_mac.cc
@@ -176,15 +176,13 @@ OSStatus CreateTrustPolicies(const std::string& hostname,
}
// 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|.
+// the signature algorithms used into |*verify_result|. |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) {
- *leaf_is_weak = false;
- verify_result->verified_cert = nullptr;
+ CertVerifyResult* verify_result) {
+ DCHECK_LT(0, CFArrayGetCount(cert_chain));
+
verify_result->has_md2 = false;
verify_result->has_md4 = false;
verify_result->has_md5 = false;
@@ -232,16 +230,10 @@ void GetCertChainInfo(CFArrayRef cert_chain,
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) ||
@@ -249,12 +241,12 @@ void GetCertChainInfo(CFArrayRef cert_chain,
CSSMOIDEqual(alg_oid, &CSSMOID_SHA1WithDSA_JDK) ||
CSSMOIDEqual(alg_oid, &CSSMOID_ECDSA_WithSHA1)) {
verify_result->has_sha1 = true;
- if (i == 0)
- *leaf_is_weak = true;
}
}
- if (!verified_cert)
+ if (!verified_cert) {
+ NOTREACHED();
return;
+ }
verify_result->verified_cert =
X509Certificate::CreateFromHandle(verified_cert, verified_chain);
@@ -561,17 +553,19 @@ int CertVerifyProcMac::VerifyInternal(
if (rv != OK)
return rv;
- CertVerifyResult temp_verify_result;
- bool leaf_is_weak = false;
- GetCertChainInfo(temp_chain, temp_chain_info, &temp_verify_result,
- &leaf_is_weak);
-
bool untrusted = (temp_trust_result != kSecTrustResultUnspecified &&
temp_trust_result != kSecTrustResultProceed);
- bool 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);
+ bool weak_chain = false;
+ if (CFArrayGetCount(cert_array) > 0) {
+ CertVerifyResult temp_verify_result;
+ GetCertChainInfo(temp_chain, temp_chain_info, &temp_verify_result);
+ weak_chain = temp_verify_result.has_md2 || temp_verify_result.has_md4 ||
+ temp_verify_result.has_md5 || temp_verify_result.has_sha1;
+ } else {
+ // If the chain is trusted or has recoverable errors, it cannot be empty.
+ DCHECK(untrusted);
+ DCHECK_NE(kSecTrustResultRecoverableTrustFailure, temp_trust_result);
+ }
// Set the result to the current chain if:
// - This is the first verification attempt. This ensures that if
// everything is awful (e.g. it may just be an untrusted cert), that
@@ -583,10 +577,6 @@ int CertVerifyProcMac::VerifyInternal(
// the old chain contained weak algorithms while the current chain only
// contains strong algorithms, then prefer the current chain over the
// old chain.
- //
- // Note: If the leaf certificate itself is weak, then the only
- // consideration is whether or not there is a trusted chain. That's
- // because no amount of path discovery will fix a weak leaf.
Ryan Sleevi 2015/04/23 00:30:50 This is a bug that we _aren't_ exiting this loop a
if (!trust_ref || (!untrusted && (candidate_untrusted ||
(candidate_weak && !weak_chain)))) {
trust_ref = temp_ref;
@@ -609,9 +599,8 @@ int CertVerifyProcMac::VerifyInternal(
if (crl_set && !CheckRevocationWithCRLSet(completed_chain, crl_set))
verify_result->cert_status |= CERT_STATUS_REVOKED;
- bool leaf_is_weak_unused = false;
- GetCertChainInfo(completed_chain, chain_info, verify_result,
- &leaf_is_weak_unused);
+ if (CFArrayGetCount(completed_chain) > 0)
+ GetCertChainInfo(completed_chain, 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
« no previous file with comments | « net/cert/cert_verify_proc.h ('k') | net/cert/cert_verify_proc_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698