Index: net/cert/cert_verify_proc.cc |
diff --git a/net/cert/cert_verify_proc.cc b/net/cert/cert_verify_proc.cc |
index 175deb6329c4a99f79bd11f29a7da595653e5aef..9376780829ba130c354941e75b01d1d08e6211f5 100644 |
--- a/net/cert/cert_verify_proc.cc |
+++ b/net/cert/cert_verify_proc.cc |
@@ -25,6 +25,7 @@ |
#include "net/cert/cert_verify_result.h" |
#include "net/cert/crl_set.h" |
#include "net/cert/internal/parse_ocsp.h" |
+#include "net/cert/internal/signature_algorithm.h" |
#include "net/cert/ocsp_revocation_status.h" |
#include "net/cert/x509_certificate.h" |
#include "net/der/encode_values.h" |
@@ -372,41 +373,97 @@ bool AreSHA1IntermediatesAllowed() { |
// Sets the "has_*" boolean members in |verify_result| that correspond with |
// the the presence of |hash| somewhere in the certificate chain (excluding the |
// trust anchor). |
-void MapHashAlgorithmToBool(X509Certificate::SignatureHashAlgorithm hash, |
- CertVerifyResult* verify_result) { |
+void MapAlgorithmToBool(DigestAlgorithm hash, CertVerifyResult* verify_result) { |
switch (hash) { |
- case X509Certificate::kSignatureHashAlgorithmMd2: |
+ case DigestAlgorithm::Md2: |
verify_result->has_md2 = true; |
break; |
- case X509Certificate::kSignatureHashAlgorithmMd4: |
+ case DigestAlgorithm::Md4: |
verify_result->has_md4 = true; |
break; |
- case X509Certificate::kSignatureHashAlgorithmMd5: |
+ case DigestAlgorithm::Md5: |
verify_result->has_md5 = true; |
break; |
- case X509Certificate::kSignatureHashAlgorithmSha1: |
+ case DigestAlgorithm::Sha1: |
verify_result->has_sha1 = true; |
break; |
- case X509Certificate::kSignatureHashAlgorithmOther: |
+ case DigestAlgorithm::Sha256: |
+ case DigestAlgorithm::Sha384: |
+ case DigestAlgorithm::Sha512: |
break; |
} |
} |
-// Sets to true the |verify_result->has_*| boolean members for the hash |
-// algorithms present in the certificate chain. |
+// Inspects the signature algorithms in a single certificate |cert|. |
// |
-// This considers the hash algorithms in all certificates except trusted |
-// certificates. |
+// * Sets |verify_result->has_md2| to true if the certificate uses MD2. |
+// * Sets |verify_result->has_md4| to true if the certificate uses MD4. |
+// * Sets |verify_result->has_md5| to true if the certificate uses MD5. |
+// * Sets |verify_result->has_sha1| to true if the certificate uses SHA1. |
// |
-// In the case of a successful verification the trust anchor is the final |
-// certificate in the chain (either the final intermediate, or the leaf |
-// certificate). |
+// * Sets |*has_mismatched_signature_algorithms| to true if |
mattm
2017/03/04 02:34:26
note that the has_mismatched_signature_algorithms
eroman
2017/03/07 23:43:00
N/A -- I removed these out-parameters and combined
|
+// Certificate.signatureAlgorithm is not consistent with |
+// TBSCertificate.algorithm |
// |
-// Whereas if verification was uncessful, the chain may be partial, and the |
-// final certificate may not be a trust anchor. This heuristic is used |
-// in both successful and failed verifications, despite this ambiguity (failure |
-// to tag one of the signature algorithms should only affect the final error). |
-void ComputeSignatureHashAlgorithms(CertVerifyResult* verify_result) { |
+// * Sets |*has_unknown_signature_algorithms| to true if one or both of the |
+// signature algorithms could not be parsed or matched against a known |
+// whitelist. |
mattm
2017/03/04 02:34:26
Maybe just me, but I was a bit confused by the com
eroman
2017/03/07 23:43:00
N/A -- replaced this with a hopefully clearer com
|
+void InspectSignatureAlgorithmForCert(X509Certificate::OSCertHandle cert, |
+ CertVerifyResult* verify_result, |
+ bool* has_mismatched_signature_algorithms, |
+ bool* has_unknown_signature_algorithms) { |
+ std::string cert_der; |
+ base::StringPiece cert_algorithm_sequence; |
+ base::StringPiece tbs_algorithm_sequence; |
+ |
+ // Extract the AlgorithmIdentifier SEQUENCEs |
+ if (!X509Certificate::GetDEREncoded(cert, &cert_der) || |
+ !asn1::ExtractSignatureAlgorithmsFromDERCert( |
+ cert_der, &cert_algorithm_sequence, &tbs_algorithm_sequence)) { |
+ *has_unknown_signature_algorithms = true; |
+ return; |
+ } |
+ |
+ if (!SignatureAlgorithm::IsEquivalent(der::Input(cert_algorithm_sequence), |
+ der::Input(tbs_algorithm_sequence))) { |
+ *has_mismatched_signature_algorithms = true; |
+ } |
+ |
+ auto algorithm = |
+ SignatureAlgorithm::Create(der::Input(cert_algorithm_sequence), nullptr); |
+ |
+ if (!algorithm) { |
+ // Couldn't parse the signature algorithm (either malformed, or an unknown |
+ // OID). |
+ *has_unknown_signature_algorithms = true; |
+ return; |
mattm
2017/03/04 02:34:26
I guess it doesn't matter with the current usage,
eroman
2017/03/07 23:43:00
N/A -- I no longer distinguish between the two, an
|
+ } |
+ |
+ MapAlgorithmToBool(algorithm->digest(), verify_result); |
+ |
+ // Check the MGF1 digest for RSASSA-PSS if applicable. |
+ if (algorithm->has_params()) { |
+ switch (algorithm->algorithm()) { |
+ case SignatureAlgorithmId::Ecdsa: |
+ case SignatureAlgorithmId::RsaPkcs1: |
+ NOTREACHED(); |
mattm
2017/03/04 02:34:26
Is this reachable with malformed input data? Or do
eroman
2017/03/07 23:43:00
I restructured this code so there is no longer a N
|
+ break; |
+ case SignatureAlgorithmId::RsaPss: |
+ MapAlgorithmToBool(algorithm->ParamsForRsaPss()->mgf1_hash(), |
+ verify_result); |
+ break; |
+ } |
+ } |
+} |
+ |
+// InspectSignatureAlgorithms() calls InspectSignatureAlgorithmForCert() for |
+// each certificate in the chain. |
+void InspectSignatureAlgorithms(CertVerifyResult* verify_result, |
+ bool* has_mismatched_signature_algorithms, |
+ bool* has_unknown_signature_algorithms) { |
+ *has_mismatched_signature_algorithms = false; |
+ *has_unknown_signature_algorithms = false; |
+ |
const X509Certificate::OSCertHandles& intermediates = |
verify_result->verified_cert->GetIntermediateCertificates(); |
@@ -418,17 +475,18 @@ void ComputeSignatureHashAlgorithms(CertVerifyResult* verify_result) { |
DCHECK(!verify_result->has_sha1); |
// Fill in hash algorithms for the leaf certificate. |
- MapHashAlgorithmToBool(X509Certificate::GetSignatureHashAlgorithm( |
- verify_result->verified_cert->os_cert_handle()), |
- verify_result); |
+ InspectSignatureAlgorithmForCert( |
+ verify_result->verified_cert->os_cert_handle(), verify_result, |
+ has_mismatched_signature_algorithms, has_unknown_signature_algorithms); |
verify_result->has_sha1_leaf = verify_result->has_sha1; |
// Fill in hash algorithms for the intermediate cerificates, excluding the |
- // final one (which is the trust anchor). |
+ // final one (which is presumably the trust anchor; may be incorrect for |
+ // partial chains). |
for (size_t i = 0; i + 1 < intermediates.size(); ++i) { |
- MapHashAlgorithmToBool( |
- X509Certificate::GetSignatureHashAlgorithm(intermediates[i]), |
- verify_result); |
+ InspectSignatureAlgorithmForCert(intermediates[i], verify_result, |
+ has_mismatched_signature_algorithms, |
+ has_unknown_signature_algorithms); |
} |
} |
@@ -483,7 +541,16 @@ int CertVerifyProc::Verify(X509Certificate* cert, |
int rv = VerifyInternal(cert, hostname, ocsp_response, flags, crl_set, |
additional_trust_anchors, verify_result); |
- ComputeSignatureHashAlgorithms(verify_result); |
+ // Look through all the signature algorithms in the chain. |
+ bool has_mismatched_signature_algorithms = false; |
+ bool has_unknown_signature_algorithms = false; |
+ InspectSignatureAlgorithms(verify_result, |
+ &has_mismatched_signature_algorithms, |
+ &has_unknown_signature_algorithms); |
+ if (has_mismatched_signature_algorithms || has_unknown_signature_algorithms) { |
+ verify_result->cert_status |= CERT_STATUS_INVALID; |
+ rv = MapCertStatusToNetError(verify_result->cert_status); |
+ } |
if (!cert->VerifyNameMatch(hostname, |
&verify_result->common_name_fallback_used)) { |