net: mark cert as revoked if EV revocation check receives revoked response (Win).

net/cert/cert_verify_proc_win.cc

 // Copyright (c) 2012 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/cert_verify_proc_win.h"
 
 #include <string>
 #include <vector>
 
 #include "base/memory/scoped_ptr.h"
@@ skipping 629 matching lines @@
   } else if (crl_set_result == kCRLSetUnknown &&
              (flags & CertVerifier::VERIFY_REV_CHECKING_ENABLED_EV_ONLY) &&
              !rev_checking_enabled &&
              ev_policy_oid != NULL) {
     // We don't have fresh information about this chain from the CRLSet and
     // it's probably an EV certificate. Retry with online revocation checking.
     rev_checking_enabled = true;
     chain_flags &= ~CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY;
     verify_result->cert_status |= CERT_STATUS_REV_CHECKING_ENABLED;
 
+    CertFreeCertificateChain(chain_context);
     if (!CertGetCertificateChain(
              chain_engine,
              cert_list.get(),
              NULL,  // current system time
              cert_list->hCertStore,
              &chain_para,
              chain_flags,
              NULL,  // reserved
              &chain_context)) {
       verify_result->cert_status |= CERT_STATUS_INVALID;
@@ skipping 17 matching lines @@
              NULL,  // reserved
              &chain_context)) {
       verify_result->cert_status |= CERT_STATUS_INVALID;
       return MapSecurityError(GetLastError());
     }
   }
 
   CertVerifyResult temp_verify_result = *verify_result;
   GetCertChainInfo(chain_context, verify_result);
   if (!verify_result->is_issued_by_known_root &&
       (flags & CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS)) {

[wtc, 2013/08/28 23:58:42]

Should we add

    && !rev_checking_enabled

to this conditional expression?

Otherwise we may be redoing the same verification if rev_checking_enabled
was already true.

[agl, 2013/08/29 16:46:59]

Just adding the conditional wouldn't be correct, I believe, because it would
skip the hard fail in the case that we did a soft-fail, online check. But there
might be scope for avoiding a verification here. However, I don't think it's in
scope for this change.

     *verify_result = temp_verify_result;
 
     rev_checking_enabled = true;
     verify_result->cert_status |= CERT_STATUS_REV_CHECKING_ENABLED;
     chain_flags &= ~CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY;
 
     CertFreeCertificateChain(chain_context);
     if (!CertGetCertificateChain(
              chain_engine,
              cert_list.get(),
@@ skipping 83 matching lines @@
     return MapCertStatusToNetError(verify_result->cert_status);
 
   if (ev_policy_oid &&
       CheckEV(chain_context, rev_checking_enabled, ev_policy_oid)) {
     verify_result->cert_status |= CERT_STATUS_IS_EV;
   }
   return OK;
 }
 
 }  // namespace net