ErrorLogFormat compatibility

[marcstern]

ModSecurity v2/master in httpd doesn't use the right log format in error log when using custom error logs.

Example:
ErrorLogFormat "<%{cu}t> <%-m:%l> <pid %P:tid %T> %7F: %E: <client\ %a> %M%

Example of a standard entry:
<2023-04-03 14:57:25.021425> <ssl:trace1> <pid 34660:tid 524> ssl_engine_init.c(931): Configuring client authentication

ModSecurity entries:

1. <2023-04-03 14:29:20.863810> <-:notice> <pid 20236:tid 520> ModSecurity for Apache/2.9.6.2 (Approach Dec 15 2022) configured.
2. [Mon Apr 03 14:29:20.864542 2023] [:notice] [pid 20236:tid 520] ModSecurity: APR compiled version="1.7.0"; loaded version="1.7.0"
3. <2023-04-03 14:33:43.770363> <-:error> <pid 20236:tid 1576> <client 192.168.59.9:52105> [client 192.168.59.9] ModSecurity: Access denied with code 4000 (phase 2). Unconditional match in SecAction. [file "C:/apache/regression-tests/conf/common/security/final.conf"] [line "2"] [id "2002401"] [msg "..."] [hostname "test.test.com"] [uri "/"] [unique_id "ZCrHp7B-6ZPVXKfnOXKp_AAAADE"]

Line 1: the module name not fulfilled
Line 2: everything is hard-coded to default values
Line 3: the module name not fulfilled, [client] is duplicated

[client] duplicated: this is easy to fix: in internal_log_ex(), we have
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);
removing "[client %s] " and "r->useragent_ip ? r->useragent_ip : r->connection->client_ip, " solves the problem

Module name not fulfilled: this looks easy
In mod_security2.c, we have:
module AP_MODULE_DECLARE_DATA security2_module = {...

It should be

module AP_MODULE_DECLARE_DATA security2_module;
AP_DECLARE_MODULE(security2) = {

This fills the module name on line 2, but not on line 3. Any idea why?

Line 2: Does mod_security2 initiliaze before the directive ErrorLogFormat is parsed?
It's in httpd core, so it's weird.
Any idea how to take ErrorLogFormat into account?

[airween]

For module's name: it's interesting that you do not have the fulfilled module name, it's already in v2, and it works on my side:

[Mon Apr 03 16:47:14.452006 2023] [security2:notice] [pid 2844] ModSecurity for Apache/2.9.7 (http://www.modsecurity.org/) configured.

(I do not want to replace my ErrorLogFormat format)

For duplicate client field: I'm not sure what was the original reason for the hardcoded second [client] field, but I assume if someone uses a custom ErrorLogFormat syntax, and remove the [client %a], then the ModSecurity's own message still contains it, because that's part of it. Personally it doesn't bother me.

For formatting: I do not see the issue there, your 3rd line formatted as you described in your ErrorLogFormat. As I know there is no possibility to format the module's message part... Consider the module process the ErrorLogFormat - and then? What's your expectation? You mean you want to see the fields between < and > markers instead of [ and ]?

[marcstern]

it's already in v2

My fault, I wasn't on the latest branch

For duplicate client field, I guess everybody will agree it's a bug (and it's breaking parsers when exporting logs to a SIEM, Elastic, ...)

As I know there is no possibility to format the module's message part

Obviously, but I wonder why the message runs before the ErrorLogFormat is taken into account. It's the only module with this behaviour. ErrorLogFormat being in core, it should be processed before mod_security2. The custom format was only given as example, but the timestamp format may be important for parsers when exporting logs to a SIEM, Elastic, ...

[marcstern]

Obviously, but I wonder why the message runs before the ErrorLogFormat is taken into account. It's the only module with this behaviour. ErrorLogFormat being in core, it should be processed before mod_security2. The custom format was only given as example, but the timestamp format may be important for parsers when exporting logs to a SIEM, Elastic, ...

Problem is solved with PR #1997