From d1fb2f1b569d7268ba4bc5e03295bd06b5d67dd4 Mon Sep 17 00:00:00 2001 From: davidben Date: Fri, 7 Nov 2014 18:51:00 -0800 Subject: [PATCH] Implement OCSP stapling in Windows BoringSSL port. Add tests to ensure we do not talk to the OCSP server if a response is already stapled. BUG=398677 Review URL: https://codereview.chromium.org/707633002 Cr-Commit-Position: refs/heads/master@{#303362} --- net/socket/ssl_client_socket_nss.cc | 4 -- net/socket/ssl_client_socket_openssl.cc | 56 ++++++++++++++++++++--- net/test/spawned_test_server/base_test_server.cc | 6 +++ net/test/spawned_test_server/base_test_server.h | 4 ++ net/tools/testserver/testserver.py | 18 ++++++-- net/url_request/url_request_unittest.cc | 58 +++++++++++++++++++++++- 6 files changed, 130 insertions(+), 16 deletions(-) diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc index 08cf2c55f512..6ed21cece431 100644 --- a/net/socket/ssl_client_socket_nss.cc +++ b/net/socket/ssl_client_socket_nss.cc @@ -218,8 +218,6 @@ bool IsOCSPStaplingSupported() { return GetCacheOCSPResponseFromSideChannelFunction() != NULL; } #else -// TODO(agl): Figure out if we can plumb the OCSP response into Mac's system -// certificate validation functions. bool IsOCSPStaplingSupported() { return false; } @@ -2436,8 +2434,6 @@ void SSLClientSocketNSS::Core::UpdateStapledOCSPResponse() { reinterpret_cast(ocsp_responses->items[0].data), ocsp_responses->items[0].len); - // TODO(agl): figure out how to plumb an OCSP response into the Mac - // system library and update IsOCSPStaplingSupported for Mac. if (IsOCSPStaplingSupported()) { #if defined(OS_WIN) if (nss_handshake_state_.server_cert) { diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc index 9fdfe38ccdd3..3f5a937e525a 100644 --- a/net/socket/ssl_client_socket_openssl.cc +++ b/net/socket/ssl_client_socket_openssl.cc @@ -140,6 +140,19 @@ int LogErrorCallback(const char* str, size_t len, void* context) { return 1; } +bool IsOCSPStaplingSupported() { +#if defined(OS_WIN) + // CERT_OCSP_RESPONSE_PROP_ID is only implemented on Vista+, but it can be + // set on Windows XP without error. There is some overhead from the server + // sending the OCSP response if it supports the extension, for the subset of + // XP clients who will request it but be unable to use it, but this is an + // acceptable trade-off for simplicity of implementation. + return true; +#else + return false; +#endif +} + } // namespace class SSLClientSocketOpenSSL::SSLContext { @@ -829,8 +842,8 @@ int SSLClientSocketOpenSSL::Init() { SSL_enable_ocsp_stapling(ssl_); } - // TODO(davidben): Enable OCSP stapling on platforms which support it and pass - // into the certificate verifier. https://crbug.com/398677 + if (IsOCSPStaplingSupported()) + SSL_enable_ocsp_stapling(ssl_); return OK; } @@ -933,10 +946,16 @@ int SSLClientSocketOpenSSL::DoHandshake() { ssl_config_.channel_id_enabled, crypto::ECPrivateKey::IsSupported()); - uint8_t* ocsp_response; - size_t ocsp_response_len; - SSL_get0_ocsp_response(ssl_, &ocsp_response, &ocsp_response_len); - set_stapled_ocsp_response_received(ocsp_response_len != 0); + // Only record OCSP histograms if OCSP was requested. + if (ssl_config_.signed_cert_timestamps_enabled || + IsOCSPStaplingSupported()) { + uint8_t* ocsp_response; + size_t ocsp_response_len; + SSL_get0_ocsp_response(ssl_, &ocsp_response, &ocsp_response_len); + + set_stapled_ocsp_response_received(ocsp_response_len != 0); + UMA_HISTOGRAM_BOOLEAN("Net.OCSPResponseStapled", ocsp_response_len != 0); + } uint8_t* sct_list; size_t sct_list_len; @@ -1166,6 +1185,31 @@ void SSLClientSocketOpenSSL::UpdateServerCert() { NetLog::TYPE_SSL_CERTIFICATES_RECEIVED, base::Bind(&NetLogX509CertificateCallback, base::Unretained(server_cert_.get()))); + + // TODO(rsleevi): Plumb an OCSP response into the Mac system library and + // update IsOCSPStaplingSupported for Mac. https://crbug.com/430714 + if (IsOCSPStaplingSupported()) { +#if defined(OS_WIN) + uint8_t* ocsp_response_raw; + size_t ocsp_response_len; + SSL_get0_ocsp_response(ssl_, &ocsp_response_raw, &ocsp_response_len); + + CRYPT_DATA_BLOB ocsp_response_blob; + ocsp_response_blob.cbData = ocsp_response_len; + ocsp_response_blob.pbData = ocsp_response_raw; + BOOL ok = CertSetCertificateContextProperty( + server_cert_->os_cert_handle(), + CERT_OCSP_RESPONSE_PROP_ID, + CERT_SET_PROPERTY_IGNORE_PERSIST_ERROR_FLAG, + &ocsp_response_blob); + if (!ok) { + VLOG(1) << "Failed to set OCSP response property: " + << GetLastError(); + } +#else + NOTREACHED(); +#endif + } } } diff --git a/net/test/spawned_test_server/base_test_server.cc b/net/test/spawned_test_server/base_test_server.cc index 0e23c7a3b55a..0c586f223eef 100644 --- a/net/test/spawned_test_server/base_test_server.cc +++ b/net/test/spawned_test_server/base_test_server.cc @@ -101,6 +101,7 @@ BaseTestServer::SSLOptions::SSLOptions() tls_intolerance_type(TLS_INTOLERANCE_ALERT), fallback_scsv_enabled(false), staple_ocsp_response(false), + ocsp_server_unavailable(false), enable_npn(false), disable_session_cache(false) { } @@ -118,6 +119,7 @@ BaseTestServer::SSLOptions::SSLOptions( tls_intolerance_type(TLS_INTOLERANCE_ALERT), fallback_scsv_enabled(false), staple_ocsp_response(false), + ocsp_server_unavailable(false), enable_npn(false), disable_session_cache(false) { } @@ -476,6 +478,10 @@ bool BaseTestServer::GenerateArguments(base::DictionaryValue* arguments) const { } if (ssl_options_.staple_ocsp_response) arguments->Set("staple-ocsp-response", base::Value::CreateNullValue()); + if (ssl_options_.ocsp_server_unavailable) { + arguments->Set("ocsp-server-unavailable", + base::Value::CreateNullValue()); + } if (ssl_options_.enable_npn) arguments->Set("enable-npn", base::Value::CreateNullValue()); if (ssl_options_.disable_session_cache) diff --git a/net/test/spawned_test_server/base_test_server.h b/net/test/spawned_test_server/base_test_server.h index 45ea1cecffa4..73ac855b6734 100644 --- a/net/test/spawned_test_server/base_test_server.h +++ b/net/test/spawned_test_server/base_test_server.h @@ -201,6 +201,10 @@ class BaseTestServer { // Whether to staple the OCSP response. bool staple_ocsp_response; + // Whether to make the OCSP server unavailable. This does not affect the + // stapled OCSP response. + bool ocsp_server_unavailable; + // Whether to enable NPN support. bool enable_npn; diff --git a/net/tools/testserver/testserver.py b/net/tools/testserver/testserver.py index 0dcbd25a6035..93de5cab3970 100755 --- a/net/tools/testserver/testserver.py +++ b/net/tools/testserver/testserver.py @@ -1976,6 +1976,7 @@ class ServerRunner(testserver_base.TestServerRunner): if self.options.server_type == SERVER_HTTP: if self.options.https: pem_cert_and_key = None + ocsp_der = None if self.options.cert_and_key_file: if not os.path.isfile(self.options.cert_and_key_file): raise testserver_base.OptionError( @@ -1988,7 +1989,6 @@ class ServerRunner(testserver_base.TestServerRunner): print ('OCSP server started on %s:%d...' % (host, self.__ocsp_server.server_port)) - ocsp_der = None ocsp_state = None if self.options.ocsp == 'ok': @@ -2012,7 +2012,11 @@ class ServerRunner(testserver_base.TestServerRunner): ocsp_state = ocsp_state, serial = self.options.cert_serial) - self.__ocsp_server.ocsp_response = ocsp_der + if self.options.ocsp_server_unavailable: + # SEQUENCE containing ENUMERATED with value 3 (tryLater). + self.__ocsp_server.ocsp_response = '30030a0103'.decode('hex') + else: + self.__ocsp_server.ocsp_response = ocsp_der for ca_cert in self.options.ssl_client_ca: if not os.path.isfile(ca_cert): @@ -2021,8 +2025,8 @@ class ServerRunner(testserver_base.TestServerRunner): ' exiting...') stapled_ocsp_response = None - if self.__ocsp_server and self.options.staple_ocsp_response: - stapled_ocsp_response = self.__ocsp_server.ocsp_response + if self.options.staple_ocsp_response: + stapled_ocsp_response = ocsp_der server = HTTPSServer((host, port), TestPageHandler, pem_cert_and_key, self.options.ssl_client_auth, @@ -2269,6 +2273,12 @@ class ServerRunner(testserver_base.TestServerRunner): self.option_parser.add_option('--ws-basic-auth', action='store_true', dest='ws_basic_auth', help='Enable basic-auth for WebSocket') + self.option_parser.add_option('--ocsp-server-unavailable', + dest='ocsp_server_unavailable', + default=False, action='store_true', + help='If set, the OCSP server will return ' + 'a tryLater status rather than the actual ' + 'OCSP response.') if __name__ == '__main__': diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc index 3bb7c1544099..2aeecc639e7b 100644 --- a/net/url_request/url_request_unittest.cc +++ b/net/url_request/url_request_unittest.cc @@ -8310,7 +8310,7 @@ static bool SystemUsesChromiumEVMetadata() { } static bool SystemSupportsOCSP() { -#if defined(USE_OPENSSL) +#if defined(USE_OPENSSL_CERTS) // http://crbug.com/117478 - OpenSSL does not support OCSP. return false; #elif defined(OS_WIN) @@ -8323,6 +8323,16 @@ static bool SystemSupportsOCSP() { #endif } +static bool SystemSupportsOCSPStapling() { +#if defined(USE_NSS) + return true; +#elif defined(OS_WIN) + return base::win::GetVersion() >= base::win::VERSION_VISTA; +#else + return false; +#endif +} + TEST_F(HTTPSOCSPTest, Valid) { if (!SystemSupportsOCSP()) { LOG(WARNING) << "Skipping test because system doesn't support OCSP"; @@ -8386,6 +8396,51 @@ TEST_F(HTTPSOCSPTest, Invalid) { EXPECT_TRUE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED); } +TEST_F(HTTPSOCSPTest, ValidStapled) { + if (!SystemSupportsOCSPStapling()) { + LOG(WARNING) + << "Skipping test because system doesn't support OCSP stapling"; + return; + } + + SpawnedTestServer::SSLOptions ssl_options( + SpawnedTestServer::SSLOptions::CERT_AUTO); + ssl_options.ocsp_status = SpawnedTestServer::SSLOptions::OCSP_OK; + ssl_options.staple_ocsp_response = true; + ssl_options.ocsp_server_unavailable = true; + + CertStatus cert_status; + DoConnection(ssl_options, &cert_status); + + EXPECT_EQ(0u, cert_status & CERT_STATUS_ALL_ERRORS); + + EXPECT_EQ(SystemUsesChromiumEVMetadata(), + static_cast(cert_status & CERT_STATUS_IS_EV)); + + EXPECT_TRUE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED); +} + +TEST_F(HTTPSOCSPTest, RevokedStapled) { + if (!SystemSupportsOCSPStapling()) { + LOG(WARNING) + << "Skipping test because system doesn't support OCSP stapling"; + return; + } + + SpawnedTestServer::SSLOptions ssl_options( + SpawnedTestServer::SSLOptions::CERT_AUTO); + ssl_options.ocsp_status = SpawnedTestServer::SSLOptions::OCSP_REVOKED; + ssl_options.staple_ocsp_response = true; + ssl_options.ocsp_server_unavailable = true; + + CertStatus cert_status; + DoConnection(ssl_options, &cert_status); + + EXPECT_EQ(CERT_STATUS_REVOKED, cert_status & CERT_STATUS_ALL_ERRORS); + EXPECT_FALSE(cert_status & CERT_STATUS_IS_EV); + EXPECT_TRUE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED); +} + class HTTPSHardFailTest : public HTTPSOCSPTest { protected: void SetupContext(URLRequestContext* context) override { @@ -8397,7 +8452,6 @@ class HTTPSHardFailTest : public HTTPSOCSPTest { } }; - TEST_F(HTTPSHardFailTest, FailsOnOCSPInvalid) { if (!SystemSupportsOCSP()) { LOG(WARNING) << "Skipping test because system doesn't support OCSP"; -- 2.11.4.GIT