From e72f5e3b910d2e53680598e10789475b43b3afb7 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Fri, 7 Apr 2023 11:12:19 -0600 Subject: [PATCH 01/12] Use FetchSM for OCSP HTTP requests --- iocore/net/Makefile.am | 2 + iocore/net/OCSPStapling.cc | 240 ++++++++++++++++++++++++++-------- src/traffic_server/FetchSM.cc | 16 ++- 3 files changed, 199 insertions(+), 59 deletions(-) diff --git a/iocore/net/Makefile.am b/iocore/net/Makefile.am index 2e9e800f1d8..f393c525aa2 100644 --- a/iocore/net/Makefile.am +++ b/iocore/net/Makefile.am @@ -30,6 +30,8 @@ AM_CPPFLAGS += \ -I$(abs_top_srcdir)/mgmt \ -I$(abs_top_srcdir)/mgmt/utils \ -I$(abs_top_srcdir)/proxy/http \ + -I$(abs_top_srcdir)/proxy/http/remap \ + -I$(abs_top_srcdir)/src/traffic_server \ $(TS_INCLUDES) \ @OPENSSL_INCLUDES@ \ @BORINGOCSP_INCLUDES@ \ diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index 58df7c77b68..114d6093635 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -32,10 +32,12 @@ #include #endif +#include "tscore/ink_memory.h" #include "P_Net.h" #include "P_SSLConfig.h" #include "P_SSLUtils.h" #include "SSLStats.h" +#include "FetchSM.h" // Maximum OCSP stapling response size. // This should be the response for a single certificate and will typically include the responder certificate chain, @@ -57,6 +59,142 @@ struct certinfo { time_t expire_time; }; +extern ClassAllocator FetchSMAllocator; + +class HTTPRequest : public Continuation +{ +public: + static constexpr int MAX_RESP_LEN = 100 * 1024; + + HTTPRequest() + { + mutex = new_ProxyMutex(); + SET_HANDLER(&HTTPRequest::event_handler); + } + + ~HTTPRequest() + { + this->_fsm->ext_destroy(); + OPENSSL_free(this->_req_body); + } + + int + event_handler(int event, Event *e) + { + if (event == TS_EVENT_IMMEDIATE) { + this->fetch(); + } else { + auto fsm = reinterpret_cast(e); + auto ctx = reinterpret_cast(fsm->ext_get_user_data()); + if (event == TS_FETCH_EVENT_EXT_BODY_DONE) { + ctx->set_done(); + } else if (event == TS_EVENT_ERROR) { + ctx->set_error(); + } + } + + return 0; + } + + void + set_request_line(bool use_post, const char *uri) + { + struct sockaddr_in sin; + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_addr.s_addr = inet_addr("127.0.0.1"); + sin.sin_port = 65535; + + this->_fsm = FetchSMAllocator.alloc(); + this->_fsm->ext_set_user_data(this); + if (use_post) { + this->_fsm->ext_init(this, "POST", uri, "HTTP/1.1", reinterpret_cast(&sin), 0); + } else { + this->_fsm->ext_init(this, "GET", uri, "HTTP/1.1", reinterpret_cast(&sin), 0); + } + } + + int + set_body(const char *content_type, const ASN1_ITEM *it, const ASN1_VALUE *req) + { + this->_req_body = nullptr; + + if (req != nullptr) { + // const_cast is needed for OpenSSL 1.1.1 + this->_req_body_len = ASN1_item_i2d(const_cast(req), &this->_req_body, it); + if (this->_req_body_len == -1) { + return 0; + } + } + this->add_header("Content-Type", content_type); + char req_body_len_str[10]; + int req_body_len_str_len; + req_body_len_str_len = ink_fast_itoa(this->_req_body_len, req_body_len_str, sizeof(req_body_len_str)); + this->add_header("Content-Length", 14, req_body_len_str, req_body_len_str_len); + + return 1; + } + + void + add_header(const char *name, int name_len, const char *value, int value_len) + { + this->_fsm->ext_add_header(name, name_len, value, value_len); + } + + void + add_header(const char *name, const char *value) + { + this->add_header(name, strlen(name), value, strlen(value)); + } + + void + fetch() + { + SCOPED_MUTEX_LOCK(lock, mutex, this_ethread()); + this->_fsm->ext_launch(); + this->_fsm->ext_write_data(this->_req_body, this->_req_body_len); + } + + void + set_done() + { + this->_result = 1; + } + + void + set_error() + { + this->_result = -1; + } + + bool + is_done() + { + return this->_result != 0; + } + + bool + is_success() + { + return this->_result == 1; + } + + unsigned char * + get_response_body(int *len) + { + SCOPED_MUTEX_LOCK(lock, mutex, this_ethread()); + char *buf = static_cast(malloc(MAX_RESP_LEN)); + *len = this->_fsm->ext_read_data(buf, MAX_RESP_LEN); + return reinterpret_cast(buf); + } + +private: + FetchSM *_fsm = nullptr; + unsigned char *_req_body = nullptr; + int _req_body_len = 0; + int _result = 0; +}; + /* * In the case of multiple certificates associated with a SSL_CTX, we must store a map * of cached responses @@ -373,84 +511,73 @@ stapling_check_response(certinfo *cinf, OCSP_RESPONSE *rsp) } static OCSP_RESPONSE * -query_responder(BIO *b, const char *host, const char *path, const char *user_agent, OCSP_REQUEST *req, int req_timeout) +query_responder(const char *uri, const char *user_agent, OCSP_REQUEST *req, int req_timeout) { ink_hrtime start, end; OCSP_RESPONSE *resp = nullptr; - OCSP_REQ_CTX *ctx; - int rv; start = Thread::get_hrtime(); end = ink_hrtime_add(start, ink_hrtime_from_sec(req_timeout)); - ctx = OCSP_sendreq_new(b, path, nullptr, -1); - OCSP_REQ_CTX_add1_header(ctx, "Host", host); - if (user_agent != nullptr) { - OCSP_REQ_CTX_add1_header(ctx, "User-Agent", user_agent); - } - OCSP_REQ_CTX_set1_req(ctx, req); + HTTPRequest httpreq; + bool use_post = true; - do { - rv = OCSP_sendreq_nbio(&resp, ctx); - ink_hrtime_sleep(HRTIME_MSECONDS(1)); - } while ((rv == -1) && BIO_should_retry(b) && (Thread::get_hrtime() < end)); + httpreq.set_request_line(use_post, uri); - OCSP_REQ_CTX_free(ctx); + // Host header + HdrHeap *heap = new_HdrHeap(); + heap->init(); + URL url; + url.create(heap); + url.parse(uri, strlen(uri)); + int host_len; + const char *host = url.host_get(&host_len); + httpreq.add_header("Host", 4, host, host_len); - if (rv == 1) { - return resp; + // User-Agent header + if (user_agent != nullptr) { + httpreq.add_header("User-Agent", user_agent); } - Error("failed to connect to OCSP server; host=%s path=%s", host, path); + // Content-Type, Content-Length, Request Body + if (use_post) { + httpreq.set_body("application/ocsp-request", ASN1_ITEM_rptr(OCSP_REQUEST), (const ASN1_VALUE *)req); + } - return nullptr; -} + // Send request + eventProcessor.schedule_imm(&httpreq, ET_NET); -static OCSP_RESPONSE * -process_responder(OCSP_REQUEST *req, const char *host, const char *path, const char *port, const char *user_agent, int req_timeout) -{ - BIO *cbio = nullptr; - OCSP_RESPONSE *resp = nullptr; - cbio = BIO_new_connect(host); - if (!cbio) { - goto end; - } - if (port) { - BIO_set_conn_port(cbio, port); - } + // Wait until the request completes + do { + ink_hrtime_sleep(HRTIME_MSECONDS(1)); + } while (!httpreq.is_done() && (Thread::get_hrtime() < end)); - BIO_set_nbio(cbio, 1); - if (BIO_do_connect(cbio) <= 0 && !BIO_should_retry(cbio)) { - Debug("ssl_ocsp", "process_responder: failed to connect to OCSP server; host=%s port=%s path=%s", host, port, path); - goto end; - } - resp = query_responder(cbio, host, path, user_agent, req, req_timeout); + if (httpreq.is_success()) { + // Parse the response + int len; + const unsigned char *p = httpreq.get_response_body(&len); + resp = reinterpret_cast(ASN1_item_d2i(nullptr, &p, len, ASN1_ITEM_rptr(OCSP_RESPONSE))); -end: - if (cbio) { - BIO_free_all(cbio); + if (resp) { + return resp; + } } - return resp; + + Error("failed to get a response from OCSP server; uri=%s", uri); + return nullptr; } static bool stapling_refresh_response(certinfo *cinf, OCSP_RESPONSE **prsp) { - bool rv = true; - OCSP_REQUEST *req = nullptr; - OCSP_CERTID *id = nullptr; - char *host = nullptr, *port = nullptr, *path = nullptr; - int ssl_flag = 0; + bool rv = true; + OCSP_REQUEST *req = nullptr; + OCSP_CERTID *id = nullptr; int response_status = 0; *prsp = nullptr; - if (!OCSP_parse_url(cinf->uri, &host, &port, &path, &ssl_flag)) { - Debug("ssl_ocsp", "stapling_refresh_response: OCSP_parse_url failed; uri=%s", cinf->uri); - goto err; - } - - Debug("ssl_ocsp", "stapling_refresh_response: querying responder; host=%s port=%s path=%s", host, port, path); + Debug("ssl_ocsp", "stapling_refresh_response: querying responder; uri=%s", cinf->uri); req = OCSP_REQUEST_new(); if (!req) { @@ -464,7 +591,7 @@ stapling_refresh_response(certinfo *cinf, OCSP_RESPONSE **prsp) goto err; } - *prsp = process_responder(req, host, path, port, cinf->user_agent, SSLConfigParams::ssl_ocsp_request_timeout); + *prsp = query_responder(cinf->uri, cinf->user_agent, req, SSLConfigParams::ssl_ocsp_request_timeout); if (*prsp == nullptr) { goto done; } @@ -474,8 +601,7 @@ stapling_refresh_response(certinfo *cinf, OCSP_RESPONSE **prsp) Debug("ssl_ocsp", "stapling_refresh_response: query response received"); stapling_check_response(cinf, *prsp); } else { - Error("stapling_refresh_response: responder response error; host=%s port=%s path=%s response_status=%d", host, port, path, - response_status); + Error("stapling_refresh_response: responder response error; uri=%s response_status=%d", cinf->uri, response_status); } if (!stapling_cache_response(*prsp, cinf)) { @@ -496,9 +622,6 @@ stapling_refresh_response(certinfo *cinf, OCSP_RESPONSE **prsp) if (*prsp) { OCSP_RESPONSE_free(*prsp); } - OPENSSL_free(host); - OPENSSL_free(path); - OPENSSL_free(port); return rv; } @@ -611,6 +734,7 @@ ssl_callback_ocsp_stapling(SSL *ssl, void *) ink_mutex_release(&cinf->stapling_mutex); SSL_set_tlsext_status_ocsp_resp(ssl, p, cinf->resp_derlen); Debug("ssl_ocsp", "ssl_callback_ocsp_stapling: successfully got certificate status for %s", cinf->certname); + Debug("ssl_ocsp", "is_prefetched:%d uri:%s", cinf->is_prefetched, cinf->uri); return SSL_TLSEXT_ERR_OK; } } diff --git a/src/traffic_server/FetchSM.cc b/src/traffic_server/FetchSM.cc index 38af0d0dd05..896e3349e65 100644 --- a/src/traffic_server/FetchSM.cc +++ b/src/traffic_server/FetchSM.cc @@ -53,7 +53,9 @@ FetchSM::cleanUp() client_response_hdr.destroy(); ats_free(client_response); cont_mutex.clear(); - http_vc->do_io_close(); + if (http_vc) { + http_vc->do_io_close(); + } FetchSMAllocator.free(this); } @@ -66,6 +68,15 @@ FetchSM::httpConnect() Debug(DEBUG_TAG, "[%s] calling httpconnect write pi=%p tag=%s id=%" PRId64, __FUNCTION__, pi, tag, id); http_vc = reinterpret_cast(TSHttpConnectWithPluginId(&_addr.sa, tag, id)); + if (http_vc == nullptr) { + Debug(DEBUG_TAG, "Not ready to use"); + if (fetch_flags & TS_FETCH_FLAGS_STREAM) { + return InvokePluginExt(TS_EVENT_ERROR); + } + InvokePlugin(callback_events.failure_event_id, nullptr); + cleanUp(); + return; + } /* * TS-2906: We need a way to unset internal request when using FetchSM, the use case for this @@ -649,6 +660,9 @@ FetchSM::ext_launch() void FetchSM::ext_write_data(const void *data, size_t len) { + if (write_vio == nullptr) { + return; + } if (fetch_flags & TS_FETCH_FLAGS_NEWLOCK) { MUTEX_TAKE_LOCK(mutex, this_ethread()); } From 09c997f24cdc0d2bc13661553b5c73eb22a511c8 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Fri, 7 Apr 2023 15:20:42 -0600 Subject: [PATCH 02/12] Don't use Hdrheap --- iocore/net/OCSPStapling.cc | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index 114d6093635..7c3c7051538 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -525,13 +525,12 @@ query_responder(const char *uri, const char *user_agent, OCSP_REQUEST *req, int httpreq.set_request_line(use_post, uri); // Host header - HdrHeap *heap = new_HdrHeap(); - heap->init(); - URL url; - url.create(heap); - url.parse(uri, strlen(uri)); - int host_len; - const char *host = url.host_get(&host_len); + const char *host = strstr(uri, "://") + 3; + const char *slash = strchr(host, '/'); + if (slash == nullptr) { + slash = host + strlen(host); + } + int host_len = slash - host; httpreq.add_header("Host", 4, host, host_len); // User-Agent header From 8eee4a93b198a5327cc5b0b2612a139b14ea4d75 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Fri, 7 Apr 2023 17:20:37 -0600 Subject: [PATCH 03/12] Check OCSP availability by a function used by ATS --- build/crypto.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/crypto.m4 b/build/crypto.m4 index ce91ea60e88..87666d07fd8 100644 --- a/build/crypto.m4 +++ b/build/crypto.m4 @@ -282,7 +282,7 @@ AC_DEFUN([TS_CHECK_CRYPTO_OCSP], [ AC_CHECK_HEADERS(openssl/ocsp.h, [ocsp_have_headers=1], [enable_tls_ocsp=no]) if test "$ocsp_have_headers" == "1"; then - AC_CHECK_FUNCS(OCSP_sendreq_new OCSP_REQ_CTX_add1_header OCSP_REQ_CTX_set1_req, [enable_tls_ocsp=yes], [enable_tls_ocsp=no]) + AC_CHECK_FUNCS(OCSP_response_status, [enable_tls_ocsp=yes], [enable_tls_ocsp=no]) LIBS=$_ocsp_saved_LIBS fi From 421f67592c848ed1fac08773a6865133a4c2cd37 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Fri, 7 Apr 2023 17:32:24 -0600 Subject: [PATCH 04/12] Fix a link issue --- src/traffic_quic/traffic_quic.cc | 37 ++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/traffic_quic/traffic_quic.cc b/src/traffic_quic/traffic_quic.cc index 5e1212b340c..7bd59e12297 100644 --- a/src/traffic_quic/traffic_quic.cc +++ b/src/traffic_quic/traffic_quic.cc @@ -347,3 +347,40 @@ PreWarmManager::reconfigure() } PreWarmManager prewarmManager; + +#include "../src/traffic_server/FetchSM.h" +ClassAllocator FetchSMAllocator("unusedFetchSMAllocator"); +void +FetchSM::ext_launch() +{ +} +void +FetchSM::ext_destroy() +{ +} +ssize_t +FetchSM::ext_read_data(char *, unsigned long) +{ + return 0; +} +void +FetchSM::ext_add_header(char const *, int, char const *, int) +{ +} +void +FetchSM::ext_write_data(void const *, unsigned long) +{ +} +void * +FetchSM::ext_get_user_data() +{ + return nullptr; +} +void +FetchSM::ext_set_user_data(void *) +{ +} +void +FetchSM::ext_init(Continuation *, char const *, char const *, char const *, sockaddr const *, int) +{ +} From 8e1637a335a77634995b93d49e6eb4a9b77d6cee Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Fri, 7 Apr 2023 18:09:22 -0600 Subject: [PATCH 05/12] Fix a couple of more link issues --- iocore/cache/test/stub.cc | 37 ++++++++++++++++++++++++++++++++++ iocore/net/libinknet_stub.cc | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/iocore/cache/test/stub.cc b/iocore/cache/test/stub.cc index cad02ab6dcd..d315b2a02af 100644 --- a/iocore/cache/test/stub.cc +++ b/iocore/cache/test/stub.cc @@ -284,3 +284,40 @@ INKContInternal::free() INKVConnInternal::INKVConnInternal() : INKContInternal() {} INKVConnInternal::INKVConnInternal(TSEventFunc funcp, TSMutex mutexp) : INKContInternal(funcp, mutexp) {} + +#include "../src/traffic_server/FetchSM.h" +ClassAllocator FetchSMAllocator("unusedFetchSMAllocator"); +void +FetchSM::ext_launch() +{ +} +void +FetchSM::ext_destroy() +{ +} +ssize_t +FetchSM::ext_read_data(char *, unsigned long) +{ + return 0; +} +void +FetchSM::ext_add_header(char const *, int, char const *, int) +{ +} +void +FetchSM::ext_write_data(void const *, unsigned long) +{ +} +void * +FetchSM::ext_get_user_data() +{ + return nullptr; +} +void +FetchSM::ext_set_user_data(void *) +{ +} +void +FetchSM::ext_init(Continuation *, char const *, char const *, char const *, sockaddr const *, int) +{ +} diff --git a/iocore/net/libinknet_stub.cc b/iocore/net/libinknet_stub.cc index b7a28788dc3..d08ae170151 100644 --- a/iocore/net/libinknet_stub.cc +++ b/iocore/net/libinknet_stub.cc @@ -155,3 +155,42 @@ PreWarmManager::reconfigure() } PreWarmManager prewarmManager; + +#include "../src/traffic_server/FetchSM.h" +ClassAllocator FetchSMAllocator("unusedFetchSMAllocator"); +void +FetchSM::ext_launch() +{ +} +void +FetchSM::ext_destroy() +{ +} +ssize_t +FetchSM::ext_read_data(char *, unsigned long) +{ + return 0; +} +void +FetchSM::ext_add_header(char const *, int, char const *, int) +{ +} +void +FetchSM::ext_write_data(void const *, unsigned long) +{ +} +void * +FetchSM::ext_get_user_data() +{ + return nullptr; +} +void +FetchSM::ext_set_user_data(void *) +{ +} +void +FetchSM::ext_init(Continuation *, char const *, char const *, char const *, sockaddr const *, int) +{ +} + +ChunkedHandler::ChunkedHandler() {} From c946a806ce26504916ce82422db221836e170947 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Fri, 7 Apr 2023 18:46:00 -0600 Subject: [PATCH 06/12] Fix a null pointer dereference --- src/traffic_server/FetchSM.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/traffic_server/FetchSM.cc b/src/traffic_server/FetchSM.cc index 896e3349e65..38a053a3910 100644 --- a/src/traffic_server/FetchSM.cc +++ b/src/traffic_server/FetchSM.cc @@ -70,10 +70,12 @@ FetchSM::httpConnect() http_vc = reinterpret_cast(TSHttpConnectWithPluginId(&_addr.sa, tag, id)); if (http_vc == nullptr) { Debug(DEBUG_TAG, "Not ready to use"); - if (fetch_flags & TS_FETCH_FLAGS_STREAM) { - return InvokePluginExt(TS_EVENT_ERROR); + if (contp) { + if (fetch_flags & TS_FETCH_FLAGS_STREAM) { + return InvokePluginExt(TS_EVENT_ERROR); + } + InvokePlugin(callback_events.failure_event_id, nullptr); } - InvokePlugin(callback_events.failure_event_id, nullptr); cleanUp(); return; } From 270513c6ce343a25538eeb0d747780885dfa2abb Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Wed, 12 Apr 2023 13:52:23 -0600 Subject: [PATCH 07/12] Add error handling --- iocore/net/OCSPStapling.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index 7c3c7051538..77ef9984735 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -540,7 +540,10 @@ query_responder(const char *uri, const char *user_agent, OCSP_REQUEST *req, int // Content-Type, Content-Length, Request Body if (use_post) { - httpreq.set_body("application/ocsp-request", ASN1_ITEM_rptr(OCSP_REQUEST), (const ASN1_VALUE *)req); + if (httpreq.set_body("application/ocsp-request", ASN1_ITEM_rptr(OCSP_REQUEST), (const ASN1_VALUE *)req) != 1) { + Error("failed to make a request for OCSP server; uri=%s", uri); + return nullptr; + } } // Send request From df2034a67a446db8876b75b8bbd1ce6a16e0ee76 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Wed, 12 Apr 2023 13:52:50 -0600 Subject: [PATCH 08/12] Rename a variable --- iocore/net/OCSPStapling.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index 77ef9984735..6b7b4947886 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -85,11 +85,11 @@ class HTTPRequest : public Continuation this->fetch(); } else { auto fsm = reinterpret_cast(e); - auto ctx = reinterpret_cast(fsm->ext_get_user_data()); + auto req = reinterpret_cast(fsm->ext_get_user_data()); if (event == TS_FETCH_EVENT_EXT_BODY_DONE) { - ctx->set_done(); + req->set_done(); } else if (event == TS_EVENT_ERROR) { - ctx->set_error(); + req->set_error(); } } From bbdccbeffdb09f94d051e0f46764c6a3f5959317 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Thu, 13 Apr 2023 16:42:01 -0600 Subject: [PATCH 09/12] Add anonymous namespace --- iocore/net/OCSPStapling.cc | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index 6b7b4947886..4096c09d27d 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -44,6 +44,11 @@ // so 10K should be more than enough. #define MAX_STAPLING_DER 10240 +extern ClassAllocator FetchSMAllocator; + +namespace +{ + // Cached info stored in SSL_CTX ex_info struct certinfo { unsigned char idx[20]; // Index in session cache SHA1 hash of certificate @@ -59,8 +64,6 @@ struct certinfo { time_t expire_time; }; -extern ClassAllocator FetchSMAllocator; - class HTTPRequest : public Continuation { public: @@ -195,6 +198,8 @@ class HTTPRequest : public Continuation int _result = 0; }; +} // End of namespace + /* * In the case of multiple certificates associated with a SSL_CTX, we must store a map * of cached responses From 48add1a57ba88e86f7f15fc074cadd6fe9f226ba Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Thu, 13 Apr 2023 17:11:46 -0600 Subject: [PATCH 10/12] Print more error log --- iocore/net/OCSPStapling.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index 4096c09d27d..f5b24c8776e 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -562,12 +562,21 @@ query_responder(const char *uri, const char *user_agent, OCSP_REQUEST *req, int if (httpreq.is_success()) { // Parse the response int len; - const unsigned char *p = httpreq.get_response_body(&len); - resp = reinterpret_cast(ASN1_item_d2i(nullptr, &p, len, ASN1_ITEM_rptr(OCSP_RESPONSE))); + const unsigned char *res = httpreq.get_response_body(&len); + const unsigned char *p = res; + resp = reinterpret_cast(ASN1_item_d2i(nullptr, &p, len, ASN1_ITEM_rptr(OCSP_RESPONSE))); if (resp) { return resp; } + + if (len < 5) { + Error("failed to parse a response from OCSP server; uri=%s len=%d", uri, len); + } else { + Error("failed to parse a response from OCSP server; uri=%s len=%d data=%02x%02x%02x%02x%02x...", uri, len, res[0], res[1], + res[2], res[3], res[4]); + } + return nullptr; } Error("failed to get a response from OCSP server; uri=%s", uri); From 726ec3585a47c65e5df83a31690ec3ee93d4e055 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Thu, 13 Apr 2023 20:29:00 -0600 Subject: [PATCH 11/12] Fix memory leak --- iocore/net/OCSPStapling.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index f5b24c8776e..22892314dac 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -567,6 +567,7 @@ query_responder(const char *uri, const char *user_agent, OCSP_REQUEST *req, int resp = reinterpret_cast(ASN1_item_d2i(nullptr, &p, len, ASN1_ITEM_rptr(OCSP_RESPONSE))); if (resp) { + free(res); return resp; } @@ -576,6 +577,7 @@ query_responder(const char *uri, const char *user_agent, OCSP_REQUEST *req, int Error("failed to parse a response from OCSP server; uri=%s len=%d data=%02x%02x%02x%02x%02x...", uri, len, res[0], res[1], res[2], res[3], res[4]); } + free(res); return nullptr; } From f0fccf57d6be7a3d32e87396c956376966d585d8 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Fri, 14 Apr 2023 08:49:18 -0600 Subject: [PATCH 12/12] Use ats_malloc --- iocore/net/OCSPStapling.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc index 22892314dac..f0d5e73c37f 100644 --- a/iocore/net/OCSPStapling.cc +++ b/iocore/net/OCSPStapling.cc @@ -186,7 +186,7 @@ class HTTPRequest : public Continuation get_response_body(int *len) { SCOPED_MUTEX_LOCK(lock, mutex, this_ethread()); - char *buf = static_cast(malloc(MAX_RESP_LEN)); + char *buf = static_cast(ats_malloc(MAX_RESP_LEN)); *len = this->_fsm->ext_read_data(buf, MAX_RESP_LEN); return reinterpret_cast(buf); } @@ -562,12 +562,12 @@ query_responder(const char *uri, const char *user_agent, OCSP_REQUEST *req, int if (httpreq.is_success()) { // Parse the response int len; - const unsigned char *res = httpreq.get_response_body(&len); - const unsigned char *p = res; - resp = reinterpret_cast(ASN1_item_d2i(nullptr, &p, len, ASN1_ITEM_rptr(OCSP_RESPONSE))); + unsigned char *res = httpreq.get_response_body(&len); + const unsigned char *p = res; + resp = reinterpret_cast(ASN1_item_d2i(nullptr, &p, len, ASN1_ITEM_rptr(OCSP_RESPONSE))); if (resp) { - free(res); + ats_free(res); return resp; } @@ -577,7 +577,7 @@ query_responder(const char *uri, const char *user_agent, OCSP_REQUEST *req, int Error("failed to parse a response from OCSP server; uri=%s len=%d data=%02x%02x%02x%02x%02x...", uri, len, res[0], res[1], res[2], res[3], res[4]); } - free(res); + ats_free(res); return nullptr; }