From bec33810e95ac9ae4d05c74a7fd8cedb61224ca2 Mon Sep 17 00:00:00 2001 From: Tomas Korbar Date: Wed, 9 Oct 2024 14:39:14 +0200 Subject: [PATCH] Move log opening to appropriate execution phase When piped logs are opened during parsing of configuration it results in unexpected situations in apache httpd and can cause hang of process which is trying to log into auditlog. Code should work as before, with the exception of one additional condition evaluation when primary audit log is not set and secondary audit log path to piped executable is now not relative to server root. --- apache2/apache2_config.c | 58 ---------------------------------------- apache2/mod_security2.c | 1 + apache2/msc_logging.c | 52 +++++++++++++++++++++++++++++++++++ apache2/msc_logging.h | 3 +++ 4 files changed, 56 insertions(+), 58 deletions(-) diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index da10b4bfe6..8c2ebe3ba4 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -1239,35 +1239,6 @@ static const char *cmd_audit_log(cmd_parms *cmd, void *_dcfg, const char *p1) directory_config *dcfg = _dcfg; dcfg->auditlog_name = (char *)p1; - - if (dcfg->auditlog_name[0] == '|') { - const char *pipe_name = dcfg->auditlog_name + 1; - piped_log *pipe_log; - - pipe_log = ap_open_piped_log(cmd->pool, pipe_name); - if (pipe_log == NULL) { - return apr_psprintf(cmd->pool, "ModSecurity: Failed to open the audit log pipe: %s", - pipe_name); - } - dcfg->auditlog_fd = ap_piped_log_write_fd(pipe_log); - } - else { - const char *file_name = ap_server_root_relative(cmd->pool, dcfg->auditlog_name); - apr_status_t rc; - - if (dcfg->auditlog_fileperms == NOT_SET) { - dcfg->auditlog_fileperms = CREATEMODE; - } - rc = apr_file_open(&dcfg->auditlog_fd, file_name, - APR_WRITE | APR_APPEND | APR_CREATE | APR_BINARY, - dcfg->auditlog_fileperms, cmd->pool); - - if (rc != APR_SUCCESS) { - return apr_psprintf(cmd->pool, "ModSecurity: Failed to open the audit log file: %s", - file_name); - } - } - return NULL; } @@ -1283,35 +1254,6 @@ static const char *cmd_audit_log2(cmd_parms *cmd, void *_dcfg, const char *p1) } dcfg->auditlog2_name = (char *)p1; - - if (dcfg->auditlog2_name[0] == '|') { - const char *pipe_name = ap_server_root_relative(cmd->pool, dcfg->auditlog2_name + 1); - piped_log *pipe_log; - - pipe_log = ap_open_piped_log(cmd->pool, pipe_name); - if (pipe_log == NULL) { - return apr_psprintf(cmd->pool, "ModSecurity: Failed to open the secondary audit log pipe: %s", - pipe_name); - } - dcfg->auditlog2_fd = ap_piped_log_write_fd(pipe_log); - } - else { - const char *file_name = ap_server_root_relative(cmd->pool, dcfg->auditlog2_name); - apr_status_t rc; - - if (dcfg->auditlog_fileperms == NOT_SET) { - dcfg->auditlog_fileperms = CREATEMODE; - } - rc = apr_file_open(&dcfg->auditlog2_fd, file_name, - APR_WRITE | APR_APPEND | APR_CREATE | APR_BINARY, - dcfg->auditlog_fileperms, cmd->pool); - - if (rc != APR_SUCCESS) { - return apr_psprintf(cmd->pool, "ModSecurity: Failed to open the secondary audit log file: %s", - file_name); - } - } - return NULL; } diff --git a/apache2/mod_security2.c b/apache2/mod_security2.c index 1850191eb7..e257978b8f 100644 --- a/apache2/mod_security2.c +++ b/apache2/mod_security2.c @@ -1735,6 +1735,7 @@ static void register_hooks(apr_pool_t *mp) { /* Logging */ ap_hook_error_log(hook_error_log, NULL, NULL, APR_HOOK_MIDDLE); + ap_hook_open_logs(modsec_open_logs, NULL, NULL, APR_HOOK_MIDDLE); ap_hook_log_transaction(hook_log_transaction, NULL, transaction_afterme_list, APR_HOOK_MIDDLE); /* Filter hooks */ diff --git a/apache2/msc_logging.c b/apache2/msc_logging.c index 39588b10fa..b07732adb7 100644 --- a/apache2/msc_logging.c +++ b/apache2/msc_logging.c @@ -2316,3 +2316,55 @@ void sec_audit_logger(modsec_rec *msr) { } #endif } + +static int open_audit_log(char *auditlog_name, unsigned char primary, apr_file_t **auditlog_fd, + apr_fileperms_t *auditlog_fileperms, apr_pool_t *p) { + if (auditlog_name == NOT_SET_P) { + return OK; + } + if (auditlog_name[0] == '|') { + const char *pipe_name = auditlog_name + 1; + piped_log *pipe_log; + + pipe_log = ap_open_piped_log(p, pipe_name); + if (pipe_log == NULL) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, + "ModSecurity: Failed to open the %saudit log pipe: %s", + primary ? "" : "secondary ", pipe_name); + return primary ? DONE : OK; + } + *auditlog_fd = ap_piped_log_write_fd(pipe_log); + } + else { + const char *file_name = ap_server_root_relative(p, auditlog_name); + apr_status_t rc; + + if (*auditlog_fileperms == NOT_SET) { + *auditlog_fileperms = CREATEMODE; + } + rc = apr_file_open(auditlog_fd, file_name, + APR_WRITE | APR_APPEND | APR_CREATE | APR_BINARY, + *auditlog_fileperms, p); + + if (rc != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, + "ModSecurity: Failed to open the %saudit log file: %s", + primary ? "" : "secondary ", file_name); + return primary ? DONE : OK; + } + } + + return OK; +} + +int modsec_open_logs(apr_pool_t *pconf, apr_pool_t *p, apr_pool_t *ptemp, server_rec *s_main) { + directory_config *dcfg = ap_get_module_config(s_main->lookup_defaults, &security2_module); + + int primary_log_rc = open_audit_log(dcfg->auditlog_name, 1, + &dcfg->auditlog_fd, &dcfg->auditlog_fileperms, p); + if (primary_log_rc != OK) { + return primary_log_rc; + } + return open_audit_log(dcfg->auditlog2_name, 0, + &dcfg->auditlog2_fd, &dcfg->auditlog_fileperms, p); +} diff --git a/apache2/msc_logging.h b/apache2/msc_logging.h index 5378ddc659..d2e17fe237 100644 --- a/apache2/msc_logging.h +++ b/apache2/msc_logging.h @@ -43,6 +43,7 @@ #define AUDITLOG_PART_ENDMARKER 'Z' #include "modsecurity.h" +#include "httpd.h" #include "apr_pools.h" int DSOLOCAL is_valid_parts_specification(char *p); @@ -51,4 +52,6 @@ char DSOLOCAL *construct_log_vcombinedus_limited(modsec_rec *msr, int _limit, in void DSOLOCAL sec_audit_logger(modsec_rec *msr); +int modsec_open_logs(apr_pool_t *pconf, apr_pool_t *p, apr_pool_t *ptemp, server_rec *s_main); + #endif