From 1428a5e3ae3d07961ded1f30987fccc90c6e8015 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 24 May 2017 22:20:08 +0900 Subject: [PATCH] nghttpx: Verify OCSP response At least we should make sure that the OCSP response is targeted to the expected certificate. This is important because we pass the file path to the external script, and if the file is replaced because of renewal, and nghttpx has not reloaded its configuration, the certificate nghttpx has loaded and the one included in the file differ. Verifying the OCSP response detects this, and avoids to send wrong OCSP response. --- src/shrpx_connection_handler.cc | 7 +-- src/shrpx_tls.cc | 79 +++++++++++++++++++++++++++++++++ src/shrpx_tls.h | 5 +++ 3 files changed, 88 insertions(+), 3 deletions(-) diff --git a/src/shrpx_connection_handler.cc b/src/shrpx_connection_handler.cc index ae55701c..07c012f6 100644 --- a/src/shrpx_connection_handler.cc +++ b/src/shrpx_connection_handler.cc @@ -620,8 +620,9 @@ void ConnectionHandler::handle_ocsp_complete() { << " finished successfully"; } + if (tls::verify_ocsp_response(ssl_ctx, ocsp_.resp.data(), + ocsp_.resp.size()) == 0) { #ifndef OPENSSL_IS_BORINGSSL - { #ifdef HAVE_ATOMIC_STD_SHARED_PTR std::atomic_store_explicit( &tls_ctx_data->ocsp_data, @@ -632,10 +633,10 @@ void ConnectionHandler::handle_ocsp_complete() { tls_ctx_data->ocsp_data = std::make_shared>(std::move(ocsp_.resp)); #endif // !HAVE_ATOMIC_STD_SHARED_PTR - } #else // OPENSSL_IS_BORINGSSL - SSL_CTX_set_ocsp_response(ssl_ctx, ocsp_.resp.data(), ocsp_.resp.size()); + SSL_CTX_set_ocsp_response(ssl_ctx, ocsp_.resp.data(), ocsp_.resp.size()); #endif // OPENSSL_IS_BORINGSSL + } ++ocsp_.next; proceed_next_cert_ocsp(); diff --git a/src/shrpx_tls.cc b/src/shrpx_tls.cc index 1b4f8dba..171f2092 100644 --- a/src/shrpx_tls.cc +++ b/src/shrpx_tls.cc @@ -45,6 +45,7 @@ #include #include #include +#include #include @@ -1818,6 +1819,84 @@ int proto_version_from_string(const StringRef &v) { return -1; } +int verify_ocsp_response(SSL_CTX *ssl_ctx, const uint8_t *ocsp_resp, + size_t ocsp_resplen) { +#if !defined(LIBRESSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= 0x10002000L + int rv; + + STACK_OF(X509) * chain_certs; + SSL_CTX_get0_chain_certs(ssl_ctx, &chain_certs); + + auto resp = d2i_OCSP_RESPONSE(nullptr, &ocsp_resp, ocsp_resplen); + if (resp == nullptr) { + LOG(ERROR) << "d2i_OCSP_RESPONSE failed"; + return -1; + } + auto resp_deleter = defer(OCSP_RESPONSE_free, resp); + + ERR_clear_error(); + + auto bs = OCSP_response_get1_basic(resp); + if (bs == nullptr) { + LOG(ERROR) << "OCSP_response_get1_basic failed: " + << ERR_error_string(ERR_get_error(), nullptr); + return -1; + } + auto bs_deleter = defer(OCSP_BASICRESP_free, bs); + + ERR_clear_error(); + + rv = OCSP_basic_verify(bs, chain_certs, nullptr, OCSP_TRUSTOTHER); + + if (rv != 1) { + LOG(ERROR) << "OCSP_basic_verify failed: " + << ERR_error_string(ERR_get_error(), nullptr); + return -1; + } + + auto sresp = OCSP_resp_get0(bs, 0); + if (sresp == nullptr) { + LOG(ERROR) << "OCSP response verification failed: no single response"; + return -1; + } + +#if OPENSSL_1_1_API + auto certid = OCSP_SINGLERESP_get0_id(sresp); +#else // !OPENSSL_1_1_API + auto certid = sresp->certId; +#endif // !OPENSSL_1_1_API + assert(certid != nullptr); + + ASN1_INTEGER *serial; + rv = OCSP_id_get0_info(nullptr, nullptr, nullptr, &serial, + const_cast(certid)); + if (rv != 1) { + LOG(ERROR) << "OCSP_id_get0_info failed"; + return -1; + } + + if (serial == nullptr) { + LOG(ERROR) << "OCSP response does not contain serial number"; + return -1; + } + + auto cert = SSL_CTX_get0_certificate(ssl_ctx); + auto cert_serial = X509_get_serialNumber(cert); + + if (ASN1_INTEGER_cmp(cert_serial, serial)) { + LOG(ERROR) << "OCSP verification serial numbers do not match"; + return -1; + } + + if (LOG_ENABLED(INFO)) { + LOG(INFO) << "OCSP verification succeeded"; + } +#endif // !defined(LIBRESSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= + // 0x10002000L + + return 0; +} + } // namespace tls } // namespace shrpx diff --git a/src/shrpx_tls.h b/src/shrpx_tls.h index 7775d18e..20be9607 100644 --- a/src/shrpx_tls.h +++ b/src/shrpx_tls.h @@ -264,6 +264,11 @@ X509 *load_certificate(const char *filename); // TLS version string. int proto_version_from_string(const StringRef &v); +// Verifies OCSP response |ocsp_resp| of length |ocsp_resplen|. This +// function returns 0 if it succeeds, or -1. +int verify_ocsp_response(SSL_CTX *ssl_ctx, const uint8_t *ocsp_resp, + size_t ocsp_resplen); + } // namespace tls } // namespace shrpx