Add a CertVerifier flag to enable hard fail for local (non-public) tr…

…ust anchors.

If revocation checking fails when chaining to a locally trusted root,
this will treat the certificate as revoked ('hard fail'). This does not
affect revocation checking when chained to public CAs.

BUG=258642
R=wtc
TBR=robertshield

Review URL: https://chromiumcodereview.appspot.com/18388005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@214312 0039d316-1c4b-4281-b951-d872f2087c98

chrome_frame/test/net/fake_external_tab.cc

@@ -325,6 +325,7 @@ void FilterDisabledTests() {
     "HTTPSRequestTest.SSLSessionCacheShardTest",
     "HTTPSRequestTest.SSLv3Fallback",
     "HTTPSRequestTest.TLSv1Fallback",
+    "HTTPSHardFailTest.*",
     "HTTPSOCSPTest.*",
     "HTTPSEVCRLSetTest.*",
     "HTTPSCRLSetTest.*",

net/cert/cert_verifier.h

@@ -55,6 +55,17 @@ class NET_EXPORT CertVerifier {
     // for certificates which may be EV, and only when VERIFY_EV_CERT is also
     // set.
     VERIFY_REV_CHECKING_ENABLED_EV_ONLY = 1 << 3,
+
+    // If set, this is equivalent to VERIFY_REV_CHECKING_ENABLED, in that it
+    // enables online revocation checking via CRLs or OCSP, but only
+    // for certificates issued by non-public trust anchors. Failure to check
+    // revocation is treated as a hard failure.
+    // Note: If VERIFY_CERT_IO_ENABLE is not also supplied, certificates
+    // that chain to local trust anchors will likely fail - for example, due to
+    // lacking fresh cached revocation issue (Windows) or because OCSP stapling
+    // can only provide information for the leaf, and not for any
+    // intermediates.
+    VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS = 1 << 4,
   };
 
   // When the verifier is destroyed, all certificate verification requests are

net/cert/cert_verify_proc_nss.cc

@@ -67,10 +67,14 @@ typedef scoped_ptr_malloc<
 // and cert_po_certList types, but doesn't release the array itself.
 class ScopedCERTValOutParam {
  public:
-  explicit ScopedCERTValOutParam(CERTValOutParam* cvout)
-      : cvout_(cvout) {}
+  explicit ScopedCERTValOutParam(CERTValOutParam* cvout) : cvout_(cvout) {}
 
   ~ScopedCERTValOutParam() {
+    Clear();
+  }
+
+  // Free the internal resources, but do not release the array itself.
+  void Clear() {
     if (cvout_ == NULL)
       return;
     for (CERTValOutParam *p = cvout_; p->type != cert_po_end; p++) {
@@ -331,13 +335,20 @@ SECOidTag GetFirstCertPolicy(CERTCertificate* cert_handle);
 
 // Call CERT_PKIXVerifyCert for the cert_handle.
 // Verification results are stored in an array of CERTValOutParam.
+// If |hard_fail| is true, and no policy_oids are supplied (eg: EV is NOT being
+// checked), then the failure to obtain valid CRL/OCSP information for all
+// certificates that contain CRL/OCSP URLs will cause the certificate to be
+// treated as if it was revoked. Since failures may be caused by transient
+// network failures or by malicious attackers, in general, hard_fail should be
+// false.
 // If policy_oids is not NULL and num_policy_oids is positive, policies
 // are also checked.
 // additional_trust_anchors is an optional list of certificates that can be
 // trusted as anchors when building a certificate chain.
 // Caller must initialize cvout before calling this function.
 SECStatus PKIXVerifyCert(CERTCertificate* cert_handle,
                          bool check_revocation,
+                         bool hard_fail,
                          bool cert_io_enabled,
                          const SECOidTag* policy_oids,
                          int num_policy_oids,
@@ -387,6 +398,10 @@ SECStatus PKIXVerifyCert(CERTCertificate* cert_handle,
     revocation_method_flags |= CERT_REV_M_REQUIRE_INFO_ON_MISSING_SOURCE;
     revocation_method_independent_flags |=
         CERT_REV_MI_REQUIRE_SOME_FRESH_INFO_AVAILABLE;
+  } else if (check_revocation && hard_fail) {
+    revocation_method_flags |= CERT_REV_M_FAIL_ON_MISSING_FRESH_INFO;
+    revocation_method_independent_flags |=
+        CERT_REV_MI_REQUIRE_SOME_FRESH_INFO_AVAILABLE;
   } else {
     revocation_method_flags |= CERT_REV_M_SKIP_TEST_ON_MISSING_SOURCE;
     revocation_method_independent_flags |=
@@ -692,6 +707,7 @@ bool VerifyEV(CERTCertificate* cert_handle,
   SECStatus status = PKIXVerifyCert(
       cert_handle,
       rev_checking_enabled,
+      true, /* hard fail is implied in EV. */
       flags & CertVerifier::VERIFY_CERT_IO_ENABLED,
       &ev_policy_oid,
       1,
@@ -813,8 +829,22 @@ int CertVerifyProcNSS::VerifyInternal(
         CertificateListToCERTCertList(additional_trust_anchors));
   }
 
-  status = PKIXVerifyCert(cert_handle, check_revocation, cert_io_enabled,
-                          NULL, 0, trust_anchors.get(), cvout);
+  status = PKIXVerifyCert(cert_handle, check_revocation, false,
+                          cert_io_enabled, NULL, 0, trust_anchors.get(),
+                          cvout);
+
+  if (status == SECSuccess &&
+      (flags & CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS) &&
+      !IsKnownRoot(cvout[cvout_trust_anchor_index].value.pointer.cert)) {
+    // TODO(rsleevi): Optimize this by supplying the constructed chain to
+    // libpkix via cvin. Omitting for now, due to lack of coverage in upstream
+    // NSS tests for that feature.
+    scoped_cvout.Clear();
+    verify_result->cert_status |= CERT_STATUS_REV_CHECKING_ENABLED;
+    status = PKIXVerifyCert(cert_handle, true, true,
+                            cert_io_enabled, NULL, 0, trust_anchors.get(),
+                            cvout);
+  }
 
   if (status == SECSuccess) {
     AppendPublicKeyHashes(cvout[cvout_cert_list_index].value.pointer.chain,

net/cert/cert_verify_proc_win.cc

@@ -572,7 +572,7 @@ int CertVerifyProcWin::VerifyInternal(
   // We can set CERT_CHAIN_RETURN_LOWER_QUALITY_CONTEXTS to get more chains.
   DWORD chain_flags = CERT_CHAIN_CACHE_END_CERT |
                       CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT;
-  const bool rev_checking_enabled =
+  bool rev_checking_enabled =
       (flags & CertVerifier::VERIFY_REV_CHECKING_ENABLED) ||
       (ev_policy_oid != NULL &&
        (flags & CertVerifier::VERIFY_REV_CHECKING_ENABLED_EV_ONLY));
@@ -632,9 +632,39 @@ int CertVerifyProcWin::VerifyInternal(
     }
   }
 
+  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)) {
+    *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(),
+             NULL,  // current system time
+             cert_list->hCertStore,
+             &chain_para,
+             chain_flags,
+             NULL,  // reserved
+             &chain_context)) {
+      verify_result->cert_status |= CERT_STATUS_INVALID;
+      return MapSecurityError(GetLastError());
+    }
+    GetCertChainInfo(chain_context, verify_result);
+
+    if (chain_context->TrustStatus.dwErrorStatus &
+        CERT_TRUST_IS_OFFLINE_REVOCATION) {
+      verify_result->cert_status |= CERT_STATUS_REVOKED;
+    }
+  }
+
   ScopedPCCERT_CHAIN_CONTEXT scoped_chain_context(chain_context);
 
-  GetCertChainInfo(chain_context, verify_result);
   verify_result->cert_status |= MapCertChainErrorStatusToCertStatus(
       chain_context->TrustStatus.dwErrorStatus);
 

net/socket/ssl_client_socket_nss.cc

@@ -3400,6 +3400,8 @@ int SSLClientSocketNSS::DoVerifyCert(int result) {
     flags |= CertVerifier::VERIFY_EV_CERT;
   if (ssl_config_.cert_io_enabled)
     flags |= CertVerifier::VERIFY_CERT_IO_ENABLED;
+  if (ssl_config_.rev_checking_required_local_anchors)
+    flags |= CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS;
   verifier_.reset(new SingleRequestCertVerifier(cert_verifier_));
   return verifier_->Verify(
       core_->state().server_cert.get(),

net/socket/ssl_client_socket_openssl.cc

@@ -950,6 +950,8 @@ int SSLClientSocketOpenSSL::DoVerifyCert(int result) {
     flags |= CertVerifier::VERIFY_EV_CERT;
   if (ssl_config_.cert_io_enabled)
     flags |= CertVerifier::VERIFY_CERT_IO_ENABLED;
+  if (ssl_config_.rev_checking_required_local_anchors)
+    flags |= CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS;
   verifier_.reset(new SingleRequestCertVerifier(cert_verifier_));
   return verifier_->Verify(
       server_cert_.get(),

net/ssl/ssl_config_service.cc

@@ -37,6 +37,7 @@ SSLConfig::CertAndStatus::~CertAndStatus() {}
 
 SSLConfig::SSLConfig()
     : rev_checking_enabled(false),
+      rev_checking_required_local_anchors(false),
       version_min(g_default_version_min),
       version_max(g_default_version_max),
       cached_info_enabled(false),
@@ -154,14 +155,16 @@ void SSLConfigService::ProcessConfigUpdate(const SSLConfig& orig_config,
                                            const SSLConfig& new_config) {
   bool config_changed =
       (orig_config.rev_checking_enabled != new_config.rev_checking_enabled) ||
+      (orig_config.rev_checking_required_local_anchors !=
+       new_config.rev_checking_required_local_anchors) ||
       (orig_config.version_min != new_config.version_min) ||
       (orig_config.version_max != new_config.version_max) ||
       (orig_config.disabled_cipher_suites !=
-          new_config.disabled_cipher_suites) ||
+       new_config.disabled_cipher_suites) ||
       (orig_config.channel_id_enabled != new_config.channel_id_enabled) ||
       (orig_config.false_start_enabled != new_config.false_start_enabled) ||
       (orig_config.unrestricted_ssl3_fallback_enabled !=
-          new_config.unrestricted_ssl3_fallback_enabled);
+       new_config.unrestricted_ssl3_fallback_enabled);
 
   if (config_changed)
     NotifySSLConfigChange();

net/ssl/ssl_config_service.h

@@ -56,6 +56,15 @@ struct NET_EXPORT SSLConfig {
   // cached revocation information will be considered.
   bool rev_checking_enabled;
 
+  // rev_checking_required_local_anchors is true if revocation checking is
+  // required to succeed when certificates chain to local trust anchors (that
+  // is, non-public CAs). If revocation information cannot be obtained, such
+  // certificates will be treated as revoked ("hard-fail").
+  // Note: This is distinct from rev_checking_enabled. If true, it is
+  // equivalent to also setting rev_checking_enabled, but only when the
+  // certificate chain chains to a local (non-public) trust anchor.
+  bool rev_checking_required_local_anchors;
+
   // The minimum and maximum protocol versions that are enabled.
   // SSL 3.0 is 0x0300, TLS 1.0 is 0x0301, TLS 1.1 is 0x0302, and so on.
   // (Use the SSL_PROTOCOL_VERSION_xxx enumerators defined above.)
@@ -128,7 +137,7 @@ struct NET_EXPORT SSLConfig {
   // result in additional HTTP requests. (For example: to fetch missing
   // intermediates or to perform OCSP/CRL fetches.) It also implies that online
   // revocation checking is disabled.
-  // NOTE: currently only effective on Linux
+  // NOTE: Only used by NSS.
   bool cert_io_enabled;
 
   // The list of application level protocols supported. If set, this will

net/url_request/url_request_unittest.cc

@@ -5407,16 +5407,21 @@ TEST_F(HTTPSRequestTest, SSLSessionCacheShardTest) {
 
 class TestSSLConfigService : public SSLConfigService {
  public:
-  TestSSLConfigService(bool ev_enabled, bool online_rev_checking)
+  TestSSLConfigService(bool ev_enabled,
+                       bool online_rev_checking,
+                       bool rev_checking_required_local_anchors)
       : ev_enabled_(ev_enabled),
-        online_rev_checking_(online_rev_checking) {
-  }
+        online_rev_checking_(online_rev_checking),
+        rev_checking_required_local_anchors_(
+            rev_checking_required_local_anchors) {}
 
   // SSLConfigService:
   virtual void GetSSLConfig(SSLConfig* config) OVERRIDE {
     *config = SSLConfig();
     config->rev_checking_enabled = online_rev_checking_;
     config->verify_ev_cert = ev_enabled_;
+    config->rev_checking_required_local_anchors =
+        rev_checking_required_local_anchors_;
   }
 
  protected:
@@ -5425,6 +5430,7 @@ class TestSSLConfigService : public SSLConfigService {
  private:
   const bool ev_enabled_;
   const bool online_rev_checking_;
+  const bool rev_checking_required_local_anchors_;
 };
 
 // This the fingerprint of the "Testing CA" certificate used by the testserver.
@@ -5452,7 +5458,7 @@ class HTTPSOCSPTest : public HTTPSRequestTest {
     context_.Init();
 
     scoped_refptr<net::X509Certificate> root_cert =
-      ImportCertFromFile(GetTestCertsDirectory(), "ocsp-test-root.pem");
+        ImportCertFromFile(GetTestCertsDirectory(), "ocsp-test-root.pem");
     CHECK_NE(static_cast<X509Certificate*>(NULL), root_cert);
     test_root_.reset(new ScopedTestRoot(root_cert.get()));
 
@@ -5496,7 +5502,9 @@ class HTTPSOCSPTest : public HTTPSRequestTest {
   virtual void SetupContext(URLRequestContext* context) {
     context->set_ssl_config_service(
         new TestSSLConfigService(true /* check for EV */,
-                                 true /* online revocation checking */));
+                                 true /* online revocation checking */,
+                                 false /* require rev. checking for local
+                                          anchors */));
   }
 
   scoped_ptr<ScopedTestRoot> test_root_;
@@ -5514,6 +5522,21 @@ static CertStatus ExpectedCertStatusForFailedOnlineRevocationCheck() {
 #endif
 }
 
+// SystemSupportsHardFailRevocationChecking returns true iff the current
+// operating system supports revocation checking and can distinguish between
+// situations where a given certificate lacks any revocation information (eg:
+// no CRLDistributionPoints and no OCSP Responder AuthorityInfoAccess) and when
+// revocation information cannot be obtained (eg: the CRL was unreachable).
+// If it does not, then tests which rely on 'hard fail' behaviour should be
+// skipped.
+static bool SystemSupportsHardFailRevocationChecking() {
+#if defined(OS_WIN) || defined(USE_NSS) || defined(OS_IOS)
+  return true;
+#else
+  return false;
+#endif
+}
+
 // SystemUsesChromiumEVMetadata returns true iff the current operating system
 // uses Chromium's EV metadata (i.e. EVRootCAMetadata). If it does not, then
 // several tests are effected because our testing EV certificate won't be
@@ -5608,12 +5631,52 @@ TEST_F(HTTPSOCSPTest, Invalid) {
   EXPECT_TRUE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED);
 }
 
+class HTTPSHardFailTest : public HTTPSOCSPTest {
+ protected:
+  virtual void SetupContext(URLRequestContext* context) OVERRIDE {
+    context->set_ssl_config_service(
+        new TestSSLConfigService(false /* check for EV */,
+                                 false /* online revocation checking */,
+                                 true /* require rev. checking for local
+                                         anchors */));
+  }
+};
+
+
+TEST_F(HTTPSHardFailTest, FailsOnOCSPInvalid) {
+  if (!SystemSupportsOCSP()) {
+    LOG(WARNING) << "Skipping test because system doesn't support OCSP";
+    return;
+  }
+
+  if (!SystemSupportsHardFailRevocationChecking()) {
+    LOG(WARNING) << "Skipping test because system doesn't support hard fail "
+                 << "revocation checking";
+    return;
+  }
+
+  SpawnedTestServer::SSLOptions ssl_options(
+      SpawnedTestServer::SSLOptions::CERT_AUTO);
+  ssl_options.ocsp_status = SpawnedTestServer::SSLOptions::OCSP_INVALID;
+
+  CertStatus cert_status;
+  DoConnection(ssl_options, &cert_status);
+
+  EXPECT_EQ(CERT_STATUS_REVOKED,
+            cert_status & CERT_STATUS_REVOKED);
+
+  // Without a positive OCSP response, we shouldn't show the EV status.
+  EXPECT_TRUE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED);
+}
+
 class HTTPSEVCRLSetTest : public HTTPSOCSPTest {
  protected:
   virtual void SetupContext(URLRequestContext* context) OVERRIDE {
     context->set_ssl_config_service(
         new TestSSLConfigService(true /* check for EV */,
-                                 false /* online revocation checking */));
+                                 false /* online revocation checking */,
+                                 false /* require rev. checking for local
+                                          anchors */));
   }
 };
 
@@ -5737,7 +5800,9 @@ class HTTPSCRLSetTest : public HTTPSOCSPTest {
   virtual void SetupContext(URLRequestContext* context) OVERRIDE {
     context->set_ssl_config_service(
         new TestSSLConfigService(false /* check for EV */,
-                                 false /* online revocation checking */));
+                                 false /* online revocation checking */,
+                                 false /* require rev. checking for local
+                                          anchors */));
   }
 };