From 1f4b9513359be80ada998cd2ba5bc806dd78cc6b Mon Sep 17 00:00:00 2001 From: Armin Abfalterer Date: Thu, 10 Jan 2019 16:13:27 +0100 Subject: [PATCH 1/4] allow per-module log level configuration --- apache2/modsecurity.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h index 11313b9b48..195572c2b5 100644 --- a/apache2/modsecurity.h +++ b/apache2/modsecurity.h @@ -22,6 +22,10 @@ #include #include +#ifdef APLOG_USE_MODULE +APLOG_USE_MODULE(security2); +#endif + typedef struct rule_exception rule_exception; typedef struct rule_exception hash_method; typedef struct modsec_rec modsec_rec; From b9e83171757597c23daed55598e84e005dee5c49 Mon Sep 17 00:00:00 2001 From: Armin Abfalterer Date: Fri, 11 Jan 2019 09:14:33 +0100 Subject: [PATCH 2/4] allow custom log format to be applied for messages being logged at startup --- apache2/mod_security2.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/apache2/mod_security2.c b/apache2/mod_security2.c index 7bb215e2ed..28ce146e23 100644 --- a/apache2/mod_security2.c +++ b/apache2/mod_security2.c @@ -105,39 +105,39 @@ 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; - 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!"); } pcre_vrs = apr_psprintf(mp,"%d.%d ", PCRE_MAJOR, PCRE_MINOR); - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: PCRE compiled version=\"%s\"; " "loaded version=\"%s\"", pcre_vrs, pcre_version()); if (strstr(pcre_version(),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); } @@ -762,7 +762,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) { @@ -776,7 +776,7 @@ static int hook_post_config(apr_pool_t *mp, apr_pool_t *mp_log, apr_pool_t *mp_t msc_status_engine_call(); } else { - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: Status engine is currently disabled, enable " \ "it by set SecStatusEngine to On."); } @@ -796,14 +796,14 @@ static int hook_post_config(apr_pool_t *mp, apr_pool_t *mp_log, apr_pool_t *mp_t { if (remote_rules_server->amount_of_rules == 1) { - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: Loaded %d rule from: '%s'.", remote_rules_server->amount_of_rules, remote_rules_server->uri); } else { - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: Loaded %d rules from: '%s'.", remote_rules_server->amount_of_rules, remote_rules_server->uri); @@ -812,7 +812,7 @@ static int hook_post_config(apr_pool_t *mp, apr_pool_t *mp_log, apr_pool_t *mp_t #endif if (remote_rules_fail_message != NULL) { - ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, "ModSecurity: " \ + ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s, "ModSecurity: " \ "Problems loading external resources: %s", remote_rules_fail_message); } From 79a6d3f6462fbea7517373a0fe031473d8b199d8 Mon Sep 17 00:00:00 2001 From: Armin Abfalterer Date: Tue, 12 Mar 2019 16:29:43 +0100 Subject: [PATCH 3/4] use uid if user name is not available --- apache2/msc_logging.c | 10 +++++++--- apache2/persist_dbm.c | 29 +++++++++++++++++++---------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/apache2/msc_logging.c b/apache2/msc_logging.c index d50f709e97..d1a867c35d 100644 --- a/apache2/msc_logging.c +++ b/apache2/msc_logging.c @@ -234,16 +234,20 @@ static char *construct_auditlog_filename(apr_pool_t *mp, const char *uniqueid) { * This is required for mpm-itk & mod_ruid2, though should be harmless for other implementations * It also changes the return statement. */ - char *username; + char *userinfo; + apr_status_t rc; apr_uid_t uid; apr_gid_t gid; apr_uid_current(&uid, &gid, mp); - apr_uid_name_get(&username, uid, mp); + rc = apr_uid_name_get(&userinfo, uid, mp); + if (rc != APR_SUCCESS) { + userinfo = apr_psprintf(mp, "%u", uid); + } apr_time_exp_lt(&t, apr_time_now()); apr_strftime(tstr, &len, 299, "/%Y%m%d/%Y%m%d-%H%M/%Y%m%d-%H%M%S", &t); - return apr_psprintf(mp, "/%s%s-%s", username, tstr, uniqueid); + return apr_psprintf(mp, "/%s%s-%s", userinfo, tstr, uniqueid); } /** diff --git a/apache2/persist_dbm.c b/apache2/persist_dbm.c index efbbf6ebd9..e4f8036f6f 100644 --- a/apache2/persist_dbm.c +++ b/apache2/persist_dbm.c @@ -104,11 +104,14 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec /** * This is required for mpm-itk & mod_ruid2, though should be harmless for other implementations */ - char *username; + char *userinfo; apr_uid_t uid; apr_gid_t gid; apr_uid_current(&uid, &gid, msr->mp); - apr_uid_name_get(&username, uid, msr->mp); + rc = apr_uid_name_get(&userinfo, uid, msr->mp); + if (rc != APR_SUCCESS) { + userinfo = apr_psprintf(msr->mp, "%u", uid); + } if (msr->txcfg->data_dir == NULL) { msr_log(msr, 1, "collection_retrieve_ex: Unable to retrieve collection (name \"%s\", key \"%s\"). Use " @@ -117,7 +120,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec goto cleanup; } - dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", username, "-", col_name, NULL); + dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", userinfo, "-", col_name, NULL); if (msr->txcfg->debuglog_level >= 9) { msr_log(msr, 9, "collection_retrieve_ex: collection_retrieve_ex: Retrieving collection (name \"%s\", filename \"%s\")",log_escape(msr->mp, col_name), @@ -385,11 +388,14 @@ int collection_store(modsec_rec *msr, apr_table_t *col) { /** * This is required for mpm-itk & mod_ruid2, though should be harmless for other implementations */ - char *username; + char *userinfo; apr_uid_t uid; apr_gid_t gid; apr_uid_current(&uid, &gid, msr->mp); - apr_uid_name_get(&username, uid, msr->mp); + rc = apr_uid_name_get(&userinfo, uid, msr->mp); + if (rc != APR_SUCCESS) { + userinfo = apr_psprintf(msr->mp, "%u", uid); + } var_name = (msc_string *)apr_table_get(col, "__name"); if (var_name == NULL) { @@ -409,7 +415,7 @@ int collection_store(modsec_rec *msr, apr_table_t *col) { } // ENH: lowercase the var name in the filename - dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", username, "-", var_name->value, NULL); + dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", userinfo, "-", var_name->value, NULL); if (msr->txcfg->debuglog_level >= 9) { msr_log(msr, 9, "collection_store: Retrieving collection (name \"%s\", filename \"%s\")",log_escape(msr->mp, var_name->value), @@ -675,11 +681,14 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) { /** * This is required for mpm-itk & mod_ruid2, though should be harmless for other implementations */ - char *username; + char *userinfo; apr_uid_t uid; apr_gid_t gid; apr_uid_current(&uid, &gid, msr->mp); - apr_uid_name_get(&username, uid, msr->mp); + rc = apr_uid_name_get(&userinfo, uid, msr->mp); + if (rc != APR_SUCCESS) { + userinfo = apr_psprintf(msr->mp, "%u", uid); + } if (msr->txcfg->data_dir == NULL) { /* The user has been warned about this problem enough times already by now. @@ -690,9 +699,9 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) { } if(strstr(col_name,"USER") || strstr(col_name,"SESSION") || strstr(col_name, "RESOURCE")) - dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", username, "-", msr->txcfg->webappid, "_", col_name, NULL); + dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", userinfo, "-", msr->txcfg->webappid, "_", col_name, NULL); else - dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", username, "-", col_name, NULL); + dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", userinfo, "-", col_name, NULL); if (msr->txcfg->debuglog_level >= 9) { msr_log(msr, 9, "collections_remove_stale: Retrieving collection (name \"%s\", filename \"%s\")",log_escape(msr->mp, col_name), From 5ae0e329867a701736fc647e67e07c8e42a964b9 Mon Sep 17 00:00:00 2001 From: Armin Abfalterer Date: Wed, 13 Mar 2019 13:05:23 +0100 Subject: [PATCH 4/4] Revert change that is part of another issue and should not be merged with pull request #1997 This reverts commit 79a6d3f6462fbea7517373a0fe031473d8b199d8. --- apache2/msc_logging.c | 10 +++------- apache2/persist_dbm.c | 29 ++++++++++------------------- 2 files changed, 13 insertions(+), 26 deletions(-) diff --git a/apache2/msc_logging.c b/apache2/msc_logging.c index d1a867c35d..d50f709e97 100644 --- a/apache2/msc_logging.c +++ b/apache2/msc_logging.c @@ -234,20 +234,16 @@ static char *construct_auditlog_filename(apr_pool_t *mp, const char *uniqueid) { * This is required for mpm-itk & mod_ruid2, though should be harmless for other implementations * It also changes the return statement. */ - char *userinfo; - apr_status_t rc; + char *username; apr_uid_t uid; apr_gid_t gid; apr_uid_current(&uid, &gid, mp); - rc = apr_uid_name_get(&userinfo, uid, mp); - if (rc != APR_SUCCESS) { - userinfo = apr_psprintf(mp, "%u", uid); - } + apr_uid_name_get(&username, uid, mp); apr_time_exp_lt(&t, apr_time_now()); apr_strftime(tstr, &len, 299, "/%Y%m%d/%Y%m%d-%H%M/%Y%m%d-%H%M%S", &t); - return apr_psprintf(mp, "/%s%s-%s", userinfo, tstr, uniqueid); + return apr_psprintf(mp, "/%s%s-%s", username, tstr, uniqueid); } /** diff --git a/apache2/persist_dbm.c b/apache2/persist_dbm.c index e4f8036f6f..efbbf6ebd9 100644 --- a/apache2/persist_dbm.c +++ b/apache2/persist_dbm.c @@ -104,14 +104,11 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec /** * This is required for mpm-itk & mod_ruid2, though should be harmless for other implementations */ - char *userinfo; + char *username; apr_uid_t uid; apr_gid_t gid; apr_uid_current(&uid, &gid, msr->mp); - rc = apr_uid_name_get(&userinfo, uid, msr->mp); - if (rc != APR_SUCCESS) { - userinfo = apr_psprintf(msr->mp, "%u", uid); - } + apr_uid_name_get(&username, uid, msr->mp); if (msr->txcfg->data_dir == NULL) { msr_log(msr, 1, "collection_retrieve_ex: Unable to retrieve collection (name \"%s\", key \"%s\"). Use " @@ -120,7 +117,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec goto cleanup; } - dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", userinfo, "-", col_name, NULL); + dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", username, "-", col_name, NULL); if (msr->txcfg->debuglog_level >= 9) { msr_log(msr, 9, "collection_retrieve_ex: collection_retrieve_ex: Retrieving collection (name \"%s\", filename \"%s\")",log_escape(msr->mp, col_name), @@ -388,14 +385,11 @@ int collection_store(modsec_rec *msr, apr_table_t *col) { /** * This is required for mpm-itk & mod_ruid2, though should be harmless for other implementations */ - char *userinfo; + char *username; apr_uid_t uid; apr_gid_t gid; apr_uid_current(&uid, &gid, msr->mp); - rc = apr_uid_name_get(&userinfo, uid, msr->mp); - if (rc != APR_SUCCESS) { - userinfo = apr_psprintf(msr->mp, "%u", uid); - } + apr_uid_name_get(&username, uid, msr->mp); var_name = (msc_string *)apr_table_get(col, "__name"); if (var_name == NULL) { @@ -415,7 +409,7 @@ int collection_store(modsec_rec *msr, apr_table_t *col) { } // ENH: lowercase the var name in the filename - dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", userinfo, "-", var_name->value, NULL); + dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", username, "-", var_name->value, NULL); if (msr->txcfg->debuglog_level >= 9) { msr_log(msr, 9, "collection_store: Retrieving collection (name \"%s\", filename \"%s\")",log_escape(msr->mp, var_name->value), @@ -681,14 +675,11 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) { /** * This is required for mpm-itk & mod_ruid2, though should be harmless for other implementations */ - char *userinfo; + char *username; apr_uid_t uid; apr_gid_t gid; apr_uid_current(&uid, &gid, msr->mp); - rc = apr_uid_name_get(&userinfo, uid, msr->mp); - if (rc != APR_SUCCESS) { - userinfo = apr_psprintf(msr->mp, "%u", uid); - } + apr_uid_name_get(&username, uid, msr->mp); if (msr->txcfg->data_dir == NULL) { /* The user has been warned about this problem enough times already by now. @@ -699,9 +690,9 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) { } if(strstr(col_name,"USER") || strstr(col_name,"SESSION") || strstr(col_name, "RESOURCE")) - dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", userinfo, "-", msr->txcfg->webappid, "_", col_name, NULL); + dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", username, "-", msr->txcfg->webappid, "_", col_name, NULL); else - dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", userinfo, "-", col_name, NULL); + dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", username, "-", col_name, NULL); if (msr->txcfg->debuglog_level >= 9) { msr_log(msr, 9, "collections_remove_stale: Retrieving collection (name \"%s\", filename \"%s\")",log_escape(msr->mp, col_name),