Skip to content

Fix logging for Apache 2.4 (again) #2781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

erkia
Copy link

@erkia erkia commented Aug 4, 2022

There was a fix for Apache 2.4 logging in f813365f, which was partly reverted by da995bb6.

Unfortunately, this reversion also removed the APLOG_USE_MODULE macro , which most likely wasn't causing any issues but is needed for Apache log messages to be correctly marked as belonging to mod_security2 module (i.e. %m token in https://httpd.apache.org/docs/2.4/mod/core.html#errorlogformat).

This pull request restores the APLOG_USE_MODULE, also adds it globally so all ap_log_error messages in all files are marked as belonging to mod_security2 module.

@martinhsv martinhsv added the 2.x Related to ModSecurity version 2.x label Nov 11, 2022
@martinhsv
Copy link
Contributor

Thanks for the contribution @erkia .

I expect this would resolve #1135

The only thing I wonder about is whether the name 'security2' is ideal for this purpose. I.e. an apache error.log log line would look like:

... [security2:error] [pid ...

With a quick perusal, I wasn't able to identify any standard that is commonly adhered to.

I.e. Is the official name of the mod after the 'mod_' the common thing to do? Or would 'mod_security2' be more expected? Or perhaps something less based on the official mod name but more like how the engine is normally termed (like 'ModSecurity' -- as was floated as a possibility in #1135 )

Opinions and comments are welcome from the community at large. I'll leave this unmerged for at least a few more days for that purpose.

(One could argue the precise text isn't that critical, as long as we get something meaningful in there.)

@erkia
Copy link
Author

erkia commented Nov 12, 2022

With a quick perusal, I wasn't able to identify any standard that is commonly adhered to.

APLOG_USE_MODULE doesn't allow using any name for logging. It is used to select the module to which the source file belongs to and it is not possible to use anything other than security2 unless the whole module is renamed.

https://github.com/apache/httpd/blob/669bdbb/include/http_config.h#L456

@martinhsv
Copy link
Contributor

Thanks @erkia

@martinhsv martinhsv merged commit c2b47ea into owasp-modsecurity:v2/master Nov 14, 2022
@dune73
Copy link
Member

dune73 commented Nov 14, 2022

Now that's a welcome fix! Thank you @erkia! If I knew it was that easy ...

@marcstern
Copy link

This change breaks custom transformations (unresolved external symbol security2_module)

@martinhsv
Copy link
Contributor

martinhsv commented Nov 15, 2022

Hi @erkia , do you have any thoughts on the issue raised by @marcstern ?

@marcstern: Could you perhaps describe a little bit what you mean by "custom transformations" and how they are structured/implemented?

@marcstern
Copy link

If you try to compile the extensions in the "ext" directory, they contain
#include "modsecurity.h"
If modsecurity.h contains this APLOG_USE_MODULE statement, it tries to link with security2_module and you get that error.

@erkia
Copy link
Author

erkia commented Nov 15, 2022

I was able to compile extensions in "ext", but I get the idea, what could fail. Alternative is to move APLOG_USE_MODULE out of modsecurity.h to each source file which uses ap_log_* functions. I'll make the change.

@erkia
Copy link
Author

erkia commented Nov 15, 2022

Suggested fix in #2832

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Related to ModSecurity version 2.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants