From f32be70793ffc2374875d0646f0ee715b06af00f Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Mon, 22 Jul 2024 16:24:56 +0200 Subject: [PATCH 1/4] Use standard httpd logging format in error log --- apache2/mod_security2.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/apache2/mod_security2.c b/apache2/mod_security2.c index 0ee72865fc..1850191eb7 100644 --- a/apache2/mod_security2.c +++ b/apache2/mod_security2.c @@ -103,17 +103,17 @@ static int server_limit, thread_limit; * * \param mp Pointer to memory pool */ -static void version(apr_pool_t *mp) { +static void version(apr_pool_t *mp, server_rec* s) { char *pcre_vrs = NULL; const char *pcre_loaded_vrs = NULL; char pcre2_loaded_vrs_buffer[80] ={0}; - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: APR compiled version=\"%s\"; " "loaded version=\"%s\"", APR_VERSION_STRING, apr_version_string()); if (strstr(apr_version_string(), APR_VERSION_STRING) == NULL) { - ap_log_error(APLOG_MARK, APLOG_WARNING, 0, NULL, "ModSecurity: Loaded APR do not match with compiled!"); + ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, "ModSecurity: Loaded APR do not match with compiled!"); } #ifdef WITH_PCRE2 @@ -134,21 +134,21 @@ static void version(apr_pool_t *mp) { "loaded version=\"%s\"", pcre_vrs, pcre_loaded_vrs); if (strstr(pcre_loaded_vrs,pcre_vrs) == NULL) { - ap_log_error(APLOG_MARK, APLOG_WARNING, 0, NULL, "ModSecurity: Loaded PCRE do not match with compiled!"); + ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, "ModSecurity: Loaded PCRE do not match with compiled!"); } /* Lua version function was removed in current 5.1. Need to check in future versions if it's back */ #if defined(WITH_LUA) - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: LUA compiled version=\"%s\"", LUA_VERSION); #endif /* WITH_LUA */ #ifdef WITH_YAJL - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: YAJL compiled version=\"%d.%d.%d\"", YAJL_MAJOR, YAJL_MINOR, YAJL_MICRO); #endif /* WITH_YAJL */ - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: LIBXML compiled version=\"%s\"", LIBXML_DOTTED_VERSION); } @@ -778,7 +778,7 @@ static int hook_post_config(apr_pool_t *mp, apr_pool_t *mp_log, apr_pool_t *mp_t ap_log_error(APLOG_MARK, APLOG_NOTICE | APLOG_NOERRNO, 0, s, "%s configured.", MODSEC_MODULE_NAME_FULL); - version(mp); + version(mp, s); /* If we've changed the server signature make note of the original. */ if (new_server_signature != NULL) { From 0be1f1566a6a98e48822be0d6bea1a5dfc28ecab Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Wed, 31 Jul 2024 09:38:20 +0200 Subject: [PATCH 2/4] Remove redundant entry [client %s] is added by the standard httpd log function => remove it --- apache2/apache2_util.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/apache2/apache2_util.c b/apache2/apache2_util.c index 20ea940ee5..f42436790f 100644 --- a/apache2/apache2_util.c +++ b/apache2/apache2_util.c @@ -286,19 +286,16 @@ static void internal_log_ex(request_rec *r, directory_config *dcfg, modsec_rec * #if AP_SERVER_MAJORVERSION_NUMBER > 1 && AP_SERVER_MINORVERSION_NUMBER > 2 ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, - "[client %s] ModSecurity: %s%s [uri \"%s\"]%s", r->useragent_ip ? r->useragent_ip : r->connection->client_ip, str1, - hostname, log_escape(msr->mp, r->uri), unique_id); + "ModSecurity: %s%s [uri \"%s\"]%s", str1, hostname, log_escape(msr->mp, r->uri), unique_id); #else ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r->server, - "[client %s] ModSecurity: %s%s [uri \"%s\"]%s", msr->remote_addr ? msr->remote_addr : r->connection->remote_ip, str1, - hostname, log_escape(msr->mp, r->uri), unique_id); + "ModSecurity: %s%s [uri \"%s\"]%s", str1, hostname, log_escape(msr->mp, r->uri), unique_id); #endif /* Add this message to the list. */ if (msr != NULL) { /* Force relevency if this is an alert */ msr->is_relevant++; - *(const char **)apr_array_push(msr->alerts) = apr_pstrdup(msr->mp, str1); } } From 686a74173ff1a7d3fbca777194643980bd278760 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Wed, 7 Aug 2024 17:01:20 +0200 Subject: [PATCH 3/4] # Send some requests & check log format --- .github/workflows/ci.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0c32ce189a..d4925d441d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,4 +49,14 @@ jobs: run: | sudo systemctl restart apache2.service sudo cat /var/log/apache2/error.log - + - name: Check error.log + run: | + # Send requests & check log format + # Valid request + curl -s http://127.0.01/ > /dev/null || echo $? + # Invalid request + curl -s http://127.0.01/%2e%2f > /dev/null || echo $? + # Check log format + grep -F ModSecurity < /var/log/apache2/error.log | grep -vP "^\[[^\]]+\] \[security2:[a-z]+\] \[pid [0-9]+:tid [0-9]+\] (?:\[client [0-9.:]+\] )?ModSecurity" || exit 0 + # grep -v succeeded => found some lines with invalid format + exit 1 From d704af657ca8ea10400fdb60424ef57c42719245 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Thu, 8 Aug 2024 16:16:14 +0200 Subject: [PATCH 4/4] Define _FORTIFY_SOURCE=3 & _GLIBCXX_ASSERTIONS that add glibc/libstdc++ assertions. See https://www.gnu.org/software/libc/manual/html_node/Source-Fortification.html & https://gcc.gnu.org/wiki/LibstdcxxDebugMode _GLIBCXX_ASSERTIONS is probably useless as we have pure C here, but let's define it in case some checks are included (or will be in a future version). As we handle some requests here, that may help to trap a problem. --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index c75335c14d..69228a6b93 100644 --- a/configure.ac +++ b/configure.ac @@ -309,10 +309,10 @@ fi AC_ARG_ENABLE(assertions, AS_HELP_STRING([--enable-assertions], - [Turn on assertions checks (undefine NDEBUG)]), + [Turn on assertions checks (undefine NDEBUG, define _GLIBCXX_ASSERTIONS & _FORTIFY_SOURCE)]), [ if test "${enableval}" = "yes"; then - assertions='-UNDEBUG' + assertions='-UNDEBUG -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS' else assertions='-DNDEBUG' fi