From 293cd214c79c8104c87d52188987956ca3ce1b36 Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Wed, 7 Aug 2024 13:54:58 -0700 Subject: [PATCH] Removed usage of pthreads and replaced with std C++ features - Replaced pthread_mutex_t in modsecurity::operators::Pm with std::mutex - Replaced pthread's thread usage in reading_logs_via_rule_message example with std::thread. - Simplified and modernized C++ code. - Removed unnecessary includes of pthread.h --- build/win32/CMakeLists.txt | 4 +- build/win32/conanfile.txt | 1 - examples/multiprocess_c/Makefile.am | 1 - .../reading_logs_via_rule_message/Makefile.am | 1 - .../reading_logs_via_rule_message.h | 67 ++++++------------- examples/reading_logs_with_offset/Makefile.am | 1 - examples/using_bodies_in_chunks/Makefile.am | 2 - .../backend/in_memory-per_process.cc | 2 - src/collection/backend/lmdb.cc | 2 - src/collection/backend/lmdb.h | 2 - src/operators/pm.cc | 11 +-- src/operators/pm.h | 3 +- 12 files changed, 27 insertions(+), 70 deletions(-) diff --git a/build/win32/CMakeLists.txt b/build/win32/CMakeLists.txt index 83b020a7..1e6303fd 100644 --- a/build/win32/CMakeLists.txt +++ b/build/win32/CMakeLists.txt @@ -110,7 +110,6 @@ include(${CMAKE_CURRENT_LIST_DIR}/ConfigureChecks.cmake) configure_file(config.h.cmake ${BASE_DIR}/src/config.h) find_package(PCRE2 REQUIRED) -find_package(PThreads4W REQUIRED) find_package(Poco REQUIRED) find_package(dirent REQUIRED) # used only by tests (check dirent::dirent refernces) @@ -139,7 +138,7 @@ add_library(libModSecurity SHARED ${libModSecuritySources}) target_compile_definitions(libModSecurity PRIVATE WITH_PCRE2) target_include_directories(libModSecurity PRIVATE ${BASE_DIR} ${BASE_DIR}/headers ${BASE_DIR}/others ${MBEDTLS_DIR}/include) -target_link_libraries(libModSecurity PRIVATE pcre2::pcre2 pthreads4w::pthreads4w libinjection mbedcrypto Poco::Poco Iphlpapi.lib) +target_link_libraries(libModSecurity PRIVATE pcre2::pcre2 libinjection mbedcrypto Poco::Poco Iphlpapi.lib) macro(add_package_dependency project compile_definition link_library flag) if(${flag}) @@ -255,7 +254,6 @@ setExampleTargetProperties(using_bodies_in_chunks) # reading_logs_via_rule_message add_executable(reading_logs_via_rule_message ${BASE_DIR}/examples/reading_logs_via_rule_message/simple_request.cc) setExampleTargetProperties(reading_logs_via_rule_message) -target_link_libraries(reading_logs_via_rule_message PRIVATE libModSecurity pthreads4w::pthreads4w) # reading_logs_with_offset add_executable(reading_logs_with_offset ${BASE_DIR}/examples/reading_logs_with_offset/read.cc) diff --git a/build/win32/conanfile.txt b/build/win32/conanfile.txt index b1a69821..b8f9721d 100644 --- a/build/win32/conanfile.txt +++ b/build/win32/conanfile.txt @@ -1,7 +1,6 @@ [requires] yajl/2.1.0 pcre2/10.42 -pthreads4w/3.0.0 libxml2/2.12.6 lua/5.4.6 libcurl/8.6.0 diff --git a/examples/multiprocess_c/Makefile.am b/examples/multiprocess_c/Makefile.am index 85c26487..726d1d90 100644 --- a/examples/multiprocess_c/Makefile.am +++ b/examples/multiprocess_c/Makefile.am @@ -15,7 +15,6 @@ multi_LDFLAGS = \ -L$(top_builddir)/src/.libs/ \ $(GEOIP_LDFLAGS) \ -lmodsecurity \ - -lpthread \ -lm \ -lstdc++ \ $(LUA_LDFLAGS) \ diff --git a/examples/reading_logs_via_rule_message/Makefile.am b/examples/reading_logs_via_rule_message/Makefile.am index 210edef3..55d3a93f 100644 --- a/examples/reading_logs_via_rule_message/Makefile.am +++ b/examples/reading_logs_via_rule_message/Makefile.am @@ -21,7 +21,6 @@ simple_request_LDFLAGS = \ -L$(top_builddir)/src/.libs/ \ $(GEOIP_LDFLAGS) \ -lmodsecurity \ - -lpthread \ -lm \ -lstdc++ \ $(LMDB_LDFLAGS) \ diff --git a/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h b/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h index 6b280048..33148cd7 100644 --- a/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h +++ b/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h @@ -13,14 +13,19 @@ * */ +#ifndef EXAMPLES_READING_LOGS_VIA_RULE_MESSAGE_READING_LOGS_VIA_RULE_MESSAGE_H_ +#define EXAMPLES_READING_LOGS_VIA_RULE_MESSAGE_READING_LOGS_VIA_RULE_MESSAGE_H_ + #include #include #include +#include #include -#include + +#include "modsecurity/rule_message.h" -#define NUM_THREADS 100 +constexpr auto NUM_THREADS = 100; char request_header[] = "" \ @@ -62,40 +67,21 @@ char response_body[] = "" \ char ip[] = "200.249.12.31"; -#include "modsecurity/rule_message.h" -#ifndef EXAMPLES_READING_LOGS_VIA_RULE_MESSAGE_READING_LOGS_VIA_RULE_MESSAGE_H_ -#define EXAMPLES_READING_LOGS_VIA_RULE_MESSAGE_READING_LOGS_VIA_RULE_MESSAGE_H_ +static void process_request(modsecurity::ModSecurity *modsec, modsecurity::RulesSet *rules) { + for (auto z = 0; z < 10000; z++) { + auto modsecTransaction = std::make_unique(modsec, rules, nullptr); - -struct data_ms { - modsecurity::ModSecurity *modsec; - modsecurity::RulesSet *rules; -}; - -#if defined _MSC_VER -#pragma warning(push) -#pragma warning(disable:4716) // avoid error C4716: 'process_request': must return a value, as MSVC C++ compiler doesn't support [[noreturn]] -#pragma warning(disable:4715) // avoid warning c4715: 'process_request' : not all control paths return a value -#endif - -[[noreturn]] static void *process_request(void *data) { - struct data_ms *a = (struct data_ms *)data; - modsecurity::ModSecurity *modsec = a->modsec; - modsecurity::RulesSet *rules = a->rules; - int z = 0; - - for (z = 0; z < 10000; z++) { - modsecurity::Transaction *modsecTransaction = \ - new modsecurity::Transaction(modsec, rules, NULL); modsecTransaction->processConnection(ip, 12345, "127.0.0.1", 80); modsecTransaction->processURI(request_uri, "GET", "1.1"); std::this_thread::sleep_for(std::chrono::microseconds(10)); + modsecTransaction->addRequestHeader("Host", "net.tutsplus.com"); modsecTransaction->processRequestHeaders(); modsecTransaction->processRequestBody(); + modsecTransaction->addResponseHeader("HTTP/1.1", "200 OK"); modsecTransaction->processResponseHeaders(200, "HTTP 1.2"); @@ -103,18 +89,11 @@ struct data_ms { (const unsigned char*)response_body, strlen((const char*)response_body)); modsecTransaction->processResponseBody(); + modsecTransaction->processLogging(); - - delete modsecTransaction; } - - pthread_exit(nullptr); } -#if defined _MSC_VER -#pragma warning(pop) -#endif - class ReadingLogsViaRuleMessage { public: ReadingLogsViaRuleMessage(char *request_header, @@ -134,11 +113,6 @@ class ReadingLogsViaRuleMessage { { } int process() const { - pthread_t threads[NUM_THREADS]; - int i; - struct data_ms dms; - void *status; - auto modsec = std::make_unique(); modsec->setConnectorInformation("ModSecurity-test v0.0.1-alpha" \ " (ModSecurity test)"); @@ -152,18 +126,19 @@ class ReadingLogsViaRuleMessage { return -1; } - dms.modsec = modsec.get(); - dms.rules = rules.get(); + std::array threads; - for (i = 0; i < NUM_THREADS; i++) { - pthread_create(&threads[i], NULL, process_request, - reinterpret_cast(&dms)); + for (auto i = 0; i != threads.size(); ++i) { + threads[i] = std::thread( + [&modsec, &rules]() { + process_request(modsec.get(), rules.get()); + }); } std::this_thread::sleep_for(std::chrono::microseconds(10000)); - for (i=0; i < NUM_THREADS; i++) { - pthread_join(threads[i], &status); + for (auto i = 0; i != threads.size(); ++i) { + threads[i].join(); std::cout << "Main: completed thread id :" << i << std::endl; } diff --git a/examples/reading_logs_with_offset/Makefile.am b/examples/reading_logs_with_offset/Makefile.am index f845c583..3ecda10c 100644 --- a/examples/reading_logs_with_offset/Makefile.am +++ b/examples/reading_logs_with_offset/Makefile.am @@ -21,7 +21,6 @@ read_LDFLAGS = \ -L$(top_builddir)/src/.libs/ \ $(GEOIP_LDFLAGS) \ -lmodsecurity \ - -lpthread \ -lm \ -lstdc++ \ $(LMDB_LDFLAGS) \ diff --git a/examples/using_bodies_in_chunks/Makefile.am b/examples/using_bodies_in_chunks/Makefile.am index aa28fe07..5d645379 100644 --- a/examples/using_bodies_in_chunks/Makefile.am +++ b/examples/using_bodies_in_chunks/Makefile.am @@ -21,12 +21,10 @@ simple_request_LDFLAGS = \ -L$(top_builddir)/src/.libs/ \ $(GEOIP_LDFLAGS) \ -lmodsecurity \ - -lpthread \ -lm \ -lstdc++ \ $(MAXMIND_LDFLAGS) \ $(LMDB_LDFLAGS) \ - -lpthread \ $(LUA_LDFLAGS) \ $(SSDEEP_LDFLAGS) \ $(YAJL_LDFLAGS) diff --git a/src/collection/backend/in_memory-per_process.cc b/src/collection/backend/in_memory-per_process.cc index 57e87072..b16ee843 100644 --- a/src/collection/backend/in_memory-per_process.cc +++ b/src/collection/backend/in_memory-per_process.cc @@ -25,8 +25,6 @@ #include #endif -#include - #include "modsecurity/variable_value.h" #include "src/utils/regex.h" #include "src/utils/string.h" diff --git a/src/collection/backend/lmdb.cc b/src/collection/backend/lmdb.cc index 85c68e3c..4a77f5fb 100644 --- a/src/collection/backend/lmdb.cc +++ b/src/collection/backend/lmdb.cc @@ -27,8 +27,6 @@ #include #include -#include - #include "modsecurity/variable_value.h" #include "src/utils/regex.h" #include "src/variables/variable.h" diff --git a/src/collection/backend/lmdb.h b/src/collection/backend/lmdb.h index 15c4fa9f..49633e37 100644 --- a/src/collection/backend/lmdb.h +++ b/src/collection/backend/lmdb.h @@ -27,12 +27,10 @@ #ifdef WITH_LMDB #include -#include #endif // WITH_LMDB #include #include #include -#include #include "modsecurity/variable_value.h" #include "modsecurity/collection/collection.h" diff --git a/src/operators/pm.cc b/src/operators/pm.cc index ebf31c40..d92fc163 100644 --- a/src/operators/pm.cc +++ b/src/operators/pm.cc @@ -39,9 +39,6 @@ Pm::~Pm() { free(m_p); m_p = NULL; -#ifdef MODSEC_MUTEX_ON_PM - pthread_mutex_destroy(&m_lock); -#endif } @@ -89,11 +86,12 @@ bool Pm::evaluate(Transaction *transaction, RuleWithActions *rule, pt.ptr = NULL; const char *match = NULL; #ifdef MODSEC_MUTEX_ON_PM - pthread_mutex_lock(&m_lock); + { + const std::lock_guard lock(m_mutex); #endif rc = acmp_process_quick(&pt, &match, input.c_str(), input.length()); #ifdef MODSEC_MUTEX_ON_PM - pthread_mutex_unlock(&m_lock); + } #endif if (rc >= 0 && transaction) { @@ -118,9 +116,6 @@ bool Pm::init(const std::string &file, std::string *error) { std::istringstream *iss; const char *err = NULL; -#ifdef MODSEC_MUTEX_ON_PM - pthread_mutex_init(&m_lock, NULL); -#endif char *content = parse_pm_content(m_param.c_str(), m_param.length(), &err); if (content == NULL) { iss = new std::istringstream(m_param); diff --git a/src/operators/pm.h b/src/operators/pm.h index b090ec5d..ed5649af 100644 --- a/src/operators/pm.h +++ b/src/operators/pm.h @@ -20,6 +20,7 @@ #include #include #include +#include #include "src/operators/operator.h" #include "src/utils/acmp.h" @@ -56,7 +57,7 @@ class Pm : public Operator { #ifdef MODSEC_MUTEX_ON_PM private: - pthread_mutex_t m_lock; + std::mutex m_mutex; #endif };