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 |