Consider the signature algorithms of incomplete chains on Windows

net/base/x509_certificate_win.cc

 // Copyright (c) 2011 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/base/x509_certificate.h"
 
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/pickle.h"
 #include "base/sha1.h"
@@ skipping 288 matching lines @@
   if (chain_context->cChain == 0)
     return;
 
   PCERT_SIMPLE_CHAIN first_chain = chain_context->rgpChain[0];
   int num_elements = first_chain->cElement;
   PCERT_CHAIN_ELEMENT* element = first_chain->rgpElement;
 
   PCCERT_CONTEXT verified_cert = NULL;
   std::vector<PCCERT_CONTEXT> verified_chain;
 
+  bool has_root_ca = num_elements > 1 &&
+      !(chain_context.TrustStatus.dwErrorStatus &
+          CERT_TRUST_IS_PARTIAL_CHAIN);
+
   // Each chain starts with the end entity certificate (i = 0) and ends with
-  // the root CA certificate (i = num_elements - 1).  Do not inspect the
-  // signature algorithm of the root CA certificate because the signature on
-  // the trust anchor is not important.
-  for (int i = 0; i < num_elements - 1; ++i) {
+  // either the root CA certificate (i = num_elements - 1) or the last
+  // available intermediate. If a root CA certificate is present, do not

[wtc, 2011/10/25 01:36:46]

Move "(i = num_elements - 1)" after "the last available intermediate" because it
applies to both possibilities,
not just the root CA certificate.

Or you can simply remove "(i = num_elements - 1)".  This
may be a good idea because we may change num_elements
below,.

+  // inspect the signature algorithm of the root CA certificate because the
+  // signature on the trust anchor is not important
+  if (has_root_ca) {
+    // If a full chain was constructed, regardless of whether it was trusted,
+    // don't inspect the root's signature algorithm.
+    num_elements -= 1;
+  }
+
+  for (int i = 0; i < num_elements; ++i) {
     PCCERT_CONTEXT cert = element[i]->pCertContext;
     if (i == 0) {
       verified_cert = cert;
     } else {
       verified_chain.push_back(cert);
     }
 
     const char* algorithm = cert->pCertInfo->SignatureAlgorithm.pszObjId;
     if (strcmp(algorithm, szOID_RSA_MD5RSA) == 0) {
       // md5WithRSAEncryption: 1.2.840.113549.1.1.4
       verify_result->has_md5 = true;
       if (i != 0)
         verify_result->has_md5_ca = true;
     } else if (strcmp(algorithm, szOID_RSA_MD2RSA) == 0) {
       // md2WithRSAEncryption: 1.2.840.113549.1.1.2
       verify_result->has_md2 = true;
       if (i != 0)
         verify_result->has_md2_ca = true;
     } else if (strcmp(algorithm, szOID_RSA_MD4RSA) == 0) {
       // md4WithRSAEncryption: 1.2.840.113549.1.1.3
       verify_result->has_md4 = true;
     }
   }
 
   if (verified_cert) {
     // Add the root certificate, if present, as it was not added above.
-    if (num_elements > 1)
+    if (has_root_ca)
       verified_chain.push_back(element[num_elements - 1]->pCertContext);

[wtc, 2011/10/25 01:36:46]

BUG: the array index should be num_elements because we
decremented num_elements above in this case (lines 318-322).

     verify_result->verified_cert =
           X509Certificate::CreateFromHandle(verified_cert, verified_chain);
   }
 }
 
 // Decodes the cert's certificatePolicies extension into a CERT_POLICIES_INFO
 // structure and stores it in *output.
 void GetCertPoliciesInfo(PCCERT_CONTEXT cert,
                          scoped_ptr_malloc<CERT_POLICIES_INFO>* output) {
   PCERT_EXTENSION extension = CertFindExtension(szOID_CERT_POLICIES,
@@ skipping 699 matching lines @@
   if (!CertSerializeCertificateStoreElement(cert_handle, 0, &buffer[0],
                                             &length)) {
     return false;
   }
 
   return pickle->WriteData(reinterpret_cast<const char*>(&buffer[0]),
                            length);
 }
 
 }  // namespace net