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

net/url_request/url_request_unittest.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 "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <shlobj.h>
 #include <windows.h>
 #endif
@@ skipping 1353 matching lines @@
       if (err_allowed) {
         EXPECT_NE(0, d.bytes_received());
         CheckSSLInfo(r.ssl_info());
       } else {
         EXPECT_EQ(0, d.bytes_received());
       }
     }
   }
 }
 
-class RevCheckedEnabledSSLConfigService : public SSLConfigService {
- public:
-  virtual void GetSSLConfig(SSLConfig* config) {
-    *config = SSLConfig();
-    config->rev_checking_enabled = true;
-    config->verify_ev_cert = true;
-  }
-};
+#if !defined(OS_ANDROID) && !defined(USE_OPENSSL)
+// TODO(jnd): http://crbug.com/117478 - EV verification is not yet supported.
 
 // This the fingerprint of the "Testing CA" certificate used by the testserver.
 // See net/data/ssl/certificates/ocsp-test-root.pem.
 static const SHA1Fingerprint kOCSPTestCertFingerprint =
   { { 0xf1, 0xad, 0xf6, 0xce, 0x42, 0xac, 0xe7, 0xb4, 0xf4, 0x24,
       0xdb, 0x1a, 0xf7, 0xa0, 0x9f, 0x09, 0xa1, 0xea, 0xf1, 0x5c } };
 
 // This is the policy OID contained in the certificates that testserver
 // generates.
 static const char kOCSPTestCertPolicy[] = "1.3.6.1.4.1.11129.2.4.1";
 
 class HTTPSOCSPTest : public HTTPSRequestTest {
  public:
   HTTPSOCSPTest()
       : context_(new TestURLRequestContext(true)),
         ev_test_policy_(EVRootCAMetadata::GetInstance(),
                         kOCSPTestCertFingerprint,
                         kOCSPTestCertPolicy) {
-    context_->set_ssl_config_service(new RevCheckedEnabledSSLConfigService);
+  }
+
+  virtual void SetUp() OVERRIDE {
+    InitContext(context_);
     context_->Init();

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

InitContext(context_) and context_->Init() sound the same to
me :-)

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

Changed to SetupContext.

 
     scoped_refptr<net::X509Certificate> root_cert =
       ImportCertFromFile(GetTestCertsDirectory(), "ocsp-test-root.pem");
     CHECK_NE(static_cast<X509Certificate*>(NULL), root_cert);
     test_root_.reset(new ScopedTestRoot(root_cert));
 
 #if defined(USE_NSS)
     SetURLRequestContextForNSSHttpIO(context_.get());
     EnsureNSSHttpIOInit();
 #endif
@@ skipping 16 matching lines @@
     EXPECT_EQ(1, d.response_started_count());
     *out_cert_status = r.ssl_info().cert_status;
   }
 
   ~HTTPSOCSPTest() {
 #if defined(USE_NSS)
     ShutdownNSSHttpIO();
 #endif
   }
 
- private:
+ protected:
+  class RevCheckedEnabledSSLConfigService : public SSLConfigService {
+   public:
+    virtual void GetSSLConfig(SSLConfig* config) {
+      *config = SSLConfig();
+      config->rev_checking_enabled = true;
+      config->verify_ev_cert = true;
+    }
+  };
+
+  // InitContext configures the URLRequestContext that will be used for making
+  // connetions to testserver. This can be overridden in test subclasses for
+  // different behaviour.
+  virtual void InitContext(URLRequestContext* context) {
+    context->set_ssl_config_service(new RevCheckedEnabledSSLConfigService);
+  }
+
   scoped_ptr<ScopedTestRoot> test_root_;
   scoped_refptr<TestURLRequestContext> context_;
   ScopedTestEVPolicy ev_test_policy_;
 };
 
-#if !defined(OS_ANDROID) && !defined(USE_OPENSSL)
-// TODO(jnd): http://crbug.com/117478 - EV verification is not yet supported.
+static CertStatus ExpectedCertStatusForFailedOnlineRevocationCheck() {
+#if defined(OS_WIN)
+  // Windows can return CERT_STATUS_UNABLE_TO_CHECK_REVOCATION but we don't
+  // have that ability on other platforms.
+  return CERT_STATUS_UNABLE_TO_CHECK_REVOCATION;
+#else
+  return 0;
+#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
+// recognised as EV.
+static bool SystemUsesChromiumEVMetadata() {
+#if defined(OS_MACOSX)
+  // On OS X, we use the system to tell us whether a certificate is EV or not
+  // and the system won't recognise our testing root.
+  return false;
+#else
+  return true;
+#endif
+}
+
 TEST_F(HTTPSOCSPTest, Valid) {
   TestServer::HTTPSOptions https_options(TestServer::HTTPSOptions::CERT_AUTO);
   https_options.ocsp_status = TestServer::HTTPSOptions::OCSP_OK;
 
   CertStatus cert_status;
   DoConnection(https_options, &cert_status);
   EXPECT_EQ(0u, cert_status & CERT_STATUS_ALL_ERRORS);
 
-#if defined(OS_MACOSX)
-  // On OS X, we use the system to tell us whether a certificate is EV or not
-  // and the system won't recognise our testing root.
-  EXPECT_FALSE(cert_status & CERT_STATUS_IS_EV);
-#else
-  EXPECT_TRUE(cert_status & CERT_STATUS_IS_EV);
-#endif
+  EXPECT_EQ(SystemUsesChromiumEVMetadata(),
+            static_cast<bool>(cert_status & CERT_STATUS_IS_EV));

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

nit: !!(cert_status & CERT_STATUS_IS_EV) ?

The static_cast<bool>() just feels like a rarer pattern in Chromium code vs the
double negation, but I'm hardly particular.

 
   EXPECT_TRUE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED);
 }
 
 TEST_F(HTTPSOCSPTest, Revoked) {
   TestServer::HTTPSOptions https_options(
       TestServer::HTTPSOptions::CERT_AUTO);
   https_options.ocsp_status = TestServer::HTTPSOptions::OCSP_REVOKED;
 
   CertStatus cert_status;
   DoConnection(https_options, &cert_status);
 #if !defined(OS_MACOSX)
   // Doesn't pass on OS X yet for reasons that need to be investigated.
   EXPECT_EQ(CERT_STATUS_REVOKED, cert_status & CERT_STATUS_ALL_ERRORS);
 #endif
   EXPECT_FALSE(cert_status & CERT_STATUS_IS_EV);
   EXPECT_TRUE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED);
 }
 
 TEST_F(HTTPSOCSPTest, Invalid) {
   TestServer::HTTPSOptions https_options(
       TestServer::HTTPSOptions::CERT_AUTO);
   https_options.ocsp_status = TestServer::HTTPSOptions::OCSP_INVALID;
 
   CertStatus cert_status;
   DoConnection(https_options, &cert_status);
 
-#if defined(OS_WIN)
-  // Windows can return CERT_STATUS_UNABLE_TO_CHECK_REVOCATION but we don't
-  // have that ability on other platforms.
-  EXPECT_EQ(CERT_STATUS_UNABLE_TO_CHECK_REVOCATION,
+  EXPECT_EQ(ExpectedCertStatusForFailedOnlineRevocationCheck(),
             cert_status & CERT_STATUS_ALL_ERRORS);
-#else
-  EXPECT_EQ(0u, cert_status & CERT_STATUS_ALL_ERRORS);
-#endif
 
   // Without a positive OCSP response, we shouldn't show the EV status.
   EXPECT_FALSE(cert_status & CERT_STATUS_IS_EV);
   EXPECT_TRUE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED);
 }
+
+class HTTPSEVCRLSetTest : public HTTPSOCSPTest {
+ protected:
+  class RevCheckedDisabledSSLConfigService : public SSLConfigService {
+   public:
+    virtual void GetSSLConfig(SSLConfig* config) {
+      *config = SSLConfig();
+      config->rev_checking_enabled = false;
+      config->verify_ev_cert = true;
+    }
+  };
+
+  virtual void InitContext(URLRequestContext* context) OVERRIDE {
+    context->set_ssl_config_service(new RevCheckedDisabledSSLConfigService);
+  }
+};
+
+TEST_F(HTTPSEVCRLSetTest, MissingCRLSetAndInvalidOCSP) {
+  TestServer::HTTPSOptions https_options(
+      TestServer::HTTPSOptions::CERT_AUTO);
+  https_options.ocsp_status = TestServer::HTTPSOptions::OCSP_INVALID;
+  SSLConfigService::SetCRLSet(scoped_refptr<CRLSet>());
+
+  CertStatus cert_status;
+  DoConnection(https_options, &cert_status);
+
+  EXPECT_EQ(ExpectedCertStatusForFailedOnlineRevocationCheck(),
+            cert_status & CERT_STATUS_ALL_ERRORS);
+
+  EXPECT_FALSE(cert_status & CERT_STATUS_IS_EV);
+  EXPECT_TRUE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED);
+}
+
+TEST_F(HTTPSEVCRLSetTest, MissingCRLSetAndGoodOCSP) {
+  TestServer::HTTPSOptions https_options(
+      TestServer::HTTPSOptions::CERT_AUTO);
+  https_options.ocsp_status = TestServer::HTTPSOptions::OCSP_OK;
+  SSLConfigService::SetCRLSet(scoped_refptr<CRLSet>());
+
+  CertStatus cert_status;
+  DoConnection(https_options, &cert_status);
+
+  EXPECT_EQ(0u, cert_status & CERT_STATUS_ALL_ERRORS);
+
+  EXPECT_EQ(SystemUsesChromiumEVMetadata(),
+            static_cast<bool>(cert_status & CERT_STATUS_IS_EV));
+
+  EXPECT_TRUE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED);
+}
+
+TEST_F(HTTPSEVCRLSetTest, ExpiredCRLSet) {
+  TestServer::HTTPSOptions https_options(
+      TestServer::HTTPSOptions::CERT_AUTO);
+  https_options.ocsp_status = TestServer::HTTPSOptions::OCSP_INVALID;
+  SSLConfigService::SetCRLSet(
+      scoped_refptr<CRLSet>(CRLSet::ExpiredCRLSetForTesting()));
+
+  CertStatus cert_status;
+  DoConnection(https_options, &cert_status);
+
+  EXPECT_EQ(ExpectedCertStatusForFailedOnlineRevocationCheck(),
+            cert_status & CERT_STATUS_ALL_ERRORS);
+
+  EXPECT_FALSE(cert_status & CERT_STATUS_IS_EV);
+  EXPECT_TRUE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED);
+}
+
+TEST_F(HTTPSEVCRLSetTest, FreshCRLSet) {
+  TestServer::HTTPSOptions https_options(
+      TestServer::HTTPSOptions::CERT_AUTO);
+  https_options.ocsp_status = TestServer::HTTPSOptions::OCSP_INVALID;
+  SSLConfigService::SetCRLSet(
+      scoped_refptr<CRLSet>(CRLSet::EmptyCRLSetForTesting()));
+
+  CertStatus cert_status;
+  DoConnection(https_options, &cert_status);
+
+  // With a valid, fresh CRLSet the bad OCSP response shouldn't matter because
+  // we wont check it.
+  EXPECT_EQ(0u, cert_status & CERT_STATUS_ALL_ERRORS);
+
+  EXPECT_EQ(SystemUsesChromiumEVMetadata(),
+            static_cast<bool>(cert_status & CERT_STATUS_IS_EV));
+
+  EXPECT_FALSE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED);
+}
+
+class HTTPSCRLSetTest : public HTTPSOCSPTest {
+ protected:
+  class RevCheckedDisabledSSLConfigService : public SSLConfigService {

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

nit: Just create a single SSLConfigService that takes the flags as parameters?

I totally don't feel strongly about that, just weird to see three
SSLConfigService-derived classes to flip two variables total.

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

Done. Makes more sense now that there are three of them.

+   public:
+    virtual void GetSSLConfig(SSLConfig* config) {
+      *config = SSLConfig();
+      config->rev_checking_enabled = false;
+      config->verify_ev_cert = false;
+    }
+  };
+
+  virtual void InitContext(URLRequestContext* context) OVERRIDE {
+    context->set_ssl_config_service(new RevCheckedDisabledSSLConfigService);
+  }
+};
+
+TEST_F(HTTPSCRLSetTest, ExpiredCRLSet) {
+  TestServer::HTTPSOptions https_options(
+      TestServer::HTTPSOptions::CERT_AUTO);
+  https_options.ocsp_status = TestServer::HTTPSOptions::OCSP_INVALID;
+  SSLConfigService::SetCRLSet(
+      scoped_refptr<CRLSet>(CRLSet::ExpiredCRLSetForTesting()));
+
+  CertStatus cert_status;
+  DoConnection(https_options, &cert_status);
+
+  // If we're not trying EV verification then, even if the CRLSet has expired,
+  // we don't fall back to online revocation checks.
+  EXPECT_EQ(0u, cert_status & CERT_STATUS_ALL_ERRORS);
+  EXPECT_FALSE(cert_status & CERT_STATUS_IS_EV);
+  EXPECT_FALSE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED);
+}
+
 #endif // !OS_ANDROID && !USE_OPENSSL
 
 // This tests that a load of www.google.com with a certificate error sets
 // the |certificate_errors_are_fatal| flag correctly. This flag will cause
 // the interstitial to be fatal.
 TEST_F(HTTPSRequestTest, HTTPSPreloadedHSTSTest) {
   TestServer::HTTPSOptions https_options(
       TestServer::HTTPSOptions::CERT_MISMATCHED_NAME);
   TestServer test_server(https_options,
                          FilePath(FILE_PATH_LITERAL("net/data/ssl")));
@@ skipping 2820 matching lines @@
   req.SetExtraRequestHeaders(headers);
   req.Start();
   MessageLoop::current()->Run();
   // If the net tests are being run with ChromeFrame then we need to allow for
   // the 'chromeframe' suffix which is added to the user agent before the
   // closing parentheses.
   EXPECT_TRUE(StartsWithASCII(d.data_received(), "Lynx (textmode", true));
 }
 
 }  // namespace net