Check TBSCertificate.algorithm and Certificate.signatureAlgorithm for

net/cert/internal/verify_certificate_chain.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/cert/internal/verify_certificate_chain.h"
 
 #include <memory>
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
@@ skipping 117 matching lines @@
   }
 
   if (cert.tbs().validity_not_after < time) {
     errors->AddError(kValidityFailedNotAfter);
     return false;
   }
 
   return true;
 }
 
-// Returns true if |signature_algorithm_tlv| is a valid algorithm encoding for
-// RSA with SHA1.
-WARN_UNUSED_RESULT bool IsRsaWithSha1SignatureAlgorithm(
-    const der::Input& signature_algorithm_tlv) {
-  std::unique_ptr<SignatureAlgorithm> algorithm =
-      SignatureAlgorithm::Create(signature_algorithm_tlv, nullptr);
-
-  return algorithm &&
-         algorithm->algorithm() == SignatureAlgorithmId::RsaPkcs1 &&
-         algorithm->digest() == DigestAlgorithm::Sha1;
-}
-
 // Returns true if |cert| has internally consistent signature algorithms.
 //
 // X.509 certificates contain two different signature algorithms:
 //  (1) The signatureAlgorithm field of Certificate
 //  (2) The signature field of TBSCertificate
 //
 // According to RFC 5280 section 4.1.1.2 and 4.1.2.3 these two fields must be
 // equal:
 //
 //     This field MUST contain the same algorithm identifier as the
@@ skipping 10 matching lines @@
     const ParsedCertificate& cert,
     CertErrors* errors) {
   const der::Input& alg1_tlv = cert.signature_algorithm_tlv();
   const der::Input& alg2_tlv = cert.tbs().signature_algorithm_tlv;
 
   // Ensure that the two DER-encoded signature algorithms are byte-for-byte
   // equal.
   if (alg1_tlv == alg2_tlv)
     return true;
 
-  // But make a compatibility concession for RSA with SHA1.
-  if (IsRsaWithSha1SignatureAlgorithm(alg1_tlv) &&
-      IsRsaWithSha1SignatureAlgorithm(alg2_tlv)) {
+  // But make a compatibility concession if alternate encodings are used
+  // TODO(eroman): Turn this warning into an error.
+  if (!SignatureAlgorithm::IsEquivalent(alg1_tlv, alg2_tlv)) {

[mattm, 2017/03/04 02:34:27]

is the ! here correct? Seems like that's backwards.

[eroman, 2017/03/07 23:43:00]

Thanks for spotting that bug!
Ugh. I will follow-up with a unit-test that covers this.

     errors->AddWarning(
         kSignatureAlgorithmsDifferentEncoding,
         CreateCertErrorParams2Der("Certificate.algorithm", alg1_tlv,
                                   "TBSCertificate.signature", alg2_tlv));
     return true;
   }
 
   errors->AddError(
       kSignatureAlgorithmMismatch,
       CreateCertErrorParams2Der("Certificate.algorithm", alg1_tlv,
@@ skipping 424 matching lines @@
 
   // TODO(eroman): RFC 5280 forbids duplicate certificates per section 6.1:
   //
   //    A certificate MUST NOT appear more than once in a prospective
   //    certification path.
 
   return true;
 }
 
 }  // namespace net