net: fallback to online revocation checks for EV status when CRLSet has expired.

net/base/x509_certificate.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/base/x509_certificate.h"
 
 #include <stdlib.h>
 
 #include <algorithm>
 #include <map>
 #include <string>
 #include <vector>
 
 #include "base/base64.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/metrics/histogram.h"
 #include "base/pickle.h"
 #include "base/sha1.h"
 #include "base/string_piece.h"
 #include "base/string_util.h"
 #include "base/synchronization/lock.h"
 #include "base/time.h"
 #include "googleurl/src/url_canon_ip.h"
 #include "net/base/cert_status_flags.h"
 #include "net/base/cert_verify_result.h"
+#include "net/base/crl_set.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "net/base/pem_tokenizer.h"
 
 namespace net {
 
 namespace {
 
 // Indicates the order to use when trying to decode binary data, which is
 // based on (speculation) as to what will be most common -> least common
@@ skipping 560 matching lines @@
                             CRLSet* crl_set,
                             CertVerifyResult* verify_result) const {
   verify_result->Reset();
   verify_result->verified_cert = const_cast<X509Certificate*>(this);
 
   if (IsBlacklisted()) {
     verify_result->cert_status |= CERT_STATUS_REVOKED;
     return ERR_CERT_REVOKED;
   }
 
+  // If we were asked to attempt EV verification and we are missing a CRLSet,
+  // or if the CRLSet has expired, then we enable online revocation checks. If
+  // the online check fails, we wont show the EV status.

[wtc, 2012/03/16 00:33:10]

Nit: wont => won't

It may be a good idea to document that we could optimize and
only enable online checks if the cert is likely to be an EV
cert, but since it is uncommon to have an expired CRLSet,
it is fine to not optimize.

[agl, 2012/03/20 20:02:19]

Done
Done.

+  if ((flags & VERIFY_EV_CERT) && (!crl_set || crl_set->IsExpired()))

[Ryan Sleevi, 2012/03/16 00:50:52]

Comment nit: In past reviews, I've been dinged for using first person like "we",
since it's very easy to use passive voice here, and it's not really
we-the-people so much as this-is-the-code.

Hardly a big deal, but...

// If EV verification is requested and no CRLSet is present,
// or the supplied CRLSet has expired, enable online
// revocation checks. If the revocation checks fail, do not
// mark the certificate as EV.

[agl, 2012/03/20 20:02:19]

Have de-we'ed the comment.

+    flags |= VERIFY_REV_CHECKING_ENABLED;
+
   int rv = VerifyInternal(hostname, flags, crl_set, verify_result);
 
   // This check is done after VerifyInternal so that VerifyInternal can fill in
   // the list of public key hashes.
   if (IsPublicKeyBlacklisted(verify_result->public_key_hashes)) {
     verify_result->cert_status |= CERT_STATUS_REVOKED;
     rv = MapCertStatusToNetError(verify_result->cert_status);
   }
 
   // Check for weak keys in the entire verified chain.
@@ skipping 244 matching lines @@
 bool X509Certificate::IsSHA1HashInSortedArray(const SHA1Fingerprint& hash,
                                               const uint8* array,
                                               size_t array_byte_len) {
   DCHECK_EQ(0u, array_byte_len % base::kSHA1Length);
   const size_t arraylen = array_byte_len / base::kSHA1Length;
   return NULL != bsearch(hash.data, array, arraylen, base::kSHA1Length,
                          CompareSHA1Hashes);
 }
 
 }  // namespace net