Chromium Code Reviews| 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 |