This is an automated email from the ASF dual-hosted git repository.
maskit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 4cf91f8acd Use FetchSM for OCSP HTTP requests (#9591)
4cf91f8acd is described below
commit 4cf91f8acd7290e960edefc6b5f87d8a50d2271f
Author: Masakazu Kitajo <[email protected]>
AuthorDate: Tue Apr 18 08:04:17 2023 +0900
Use FetchSM for OCSP HTTP requests (#9591)
* Use FetchSM for OCSP HTTP requests
* Don't use Hdrheap
* Check OCSP availability by a function used by ATS
* Fix a link issue
* Fix a couple of more link issues
* Fix a null pointer dereference
* Add error handling
* Rename a variable
* Add anonymous namespace
* Print more error log
* Fix memory leak
* Use ats_malloc
---
build/crypto.m4 | 2 +-
iocore/cache/test/stub.cc | 37 ++++++
iocore/net/Makefile.am | 2 +
iocore/net/OCSPStapling.cc | 258 ++++++++++++++++++++++++++++++---------
iocore/net/libinknet_stub.cc | 39 ++++++
src/traffic_quic/traffic_quic.cc | 37 ++++++
src/traffic_server/FetchSM.cc | 18 ++-
7 files changed, 333 insertions(+), 60 deletions(-)
diff --git a/build/crypto.m4 b/build/crypto.m4
index ce91ea60e8..87666d07fd 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
diff --git a/iocore/cache/test/stub.cc b/iocore/cache/test/stub.cc
index cad02ab6dc..d315b2a02a 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<FetchSM> 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/Makefile.am b/iocore/net/Makefile.am
index 2e9e800f1d..f393c525aa 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 58df7c77b6..f0d5e73c37 100644
--- a/iocore/net/OCSPStapling.cc
+++ b/iocore/net/OCSPStapling.cc
@@ -32,16 +32,23 @@
#include <openssl/ocsp.h>
#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,
// so 10K should be more than enough.
#define MAX_STAPLING_DER 10240
+extern ClassAllocator<FetchSM> FetchSMAllocator;
+
+namespace
+{
+
// Cached info stored in SSL_CTX ex_info
struct certinfo {
unsigned char idx[20]; // Index in session cache SHA1 hash of certificate
@@ -57,6 +64,142 @@ struct certinfo {
time_t expire_time;
};
+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<FetchSM *>(e);
+ auto req = reinterpret_cast<HTTPRequest *>(fsm->ext_get_user_data());
+ if (event == TS_FETCH_EVENT_EXT_BODY_DONE) {
+ req->set_done();
+ } else if (event == TS_EVENT_ERROR) {
+ req->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<sockaddr *>(&sin), 0);
+ } else {
+ this->_fsm->ext_init(this, "GET", uri, "HTTP/1.1",
reinterpret_cast<sockaddr *>(&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<ASN1_VALUE *>(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<char *>(ats_malloc(MAX_RESP_LEN));
+ *len = this->_fsm->ext_read_data(buf, MAX_RESP_LEN);
+ return reinterpret_cast<unsigned char *>(buf);
+ }
+
+private:
+ FetchSM *_fsm = nullptr;
+ unsigned char *_req_body = nullptr;
+ int _req_body_len = 0;
+ 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
@@ -373,84 +516,86 @@ 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
+ 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);
- 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) {
+ 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;
+ }
+ }
- 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));
+
+ if (httpreq.is_success()) {
+ // Parse the response
+ int len;
+ unsigned char *res = httpreq.get_response_body(&len);
+ const unsigned char *p = res;
+ resp = reinterpret_cast<OCSP_RESPONSE
*>(ASN1_item_d2i(nullptr, &p, len, ASN1_ITEM_rptr(OCSP_RESPONSE)));
+
+ if (resp) {
+ ats_free(res);
+ return resp;
+ }
- 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;
+ 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]);
+ }
+ ats_free(res);
+ return nullptr;
}
- resp = query_responder(cbio, host, path, user_agent, req, req_timeout);
-end:
- if (cbio) {
- BIO_free_all(cbio);
- }
- 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 +609,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 +619,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 +640,6 @@ done:
if (*prsp) {
OCSP_RESPONSE_free(*prsp);
}
- OPENSSL_free(host);
- OPENSSL_free(path);
- OPENSSL_free(port);
return rv;
}
@@ -611,6 +752,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/iocore/net/libinknet_stub.cc b/iocore/net/libinknet_stub.cc
index b7a28788dc..d08ae17015 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<FetchSM> 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() {}
diff --git a/src/traffic_quic/traffic_quic.cc b/src/traffic_quic/traffic_quic.cc
index 5e1212b340..7bd59e1229 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<FetchSM> 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/src/traffic_server/FetchSM.cc b/src/traffic_server/FetchSM.cc
index 38af0d0dd0..38a053a391 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,17 @@ FetchSM::httpConnect()
Debug(DEBUG_TAG, "[%s] calling httpconnect write pi=%p tag=%s id=%" PRId64,
__FUNCTION__, pi, tag, id);
http_vc = reinterpret_cast<PluginVC *>(TSHttpConnectWithPluginId(&_addr.sa,
tag, id));
+ if (http_vc == nullptr) {
+ Debug(DEBUG_TAG, "Not ready to use");
+ if (contp) {
+ 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 +662,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());
}
