[owasp-modsecurity compatibility] SecAuditLogParts flags

[tty2]

Description

It is opened as a bug cause current behavior already doesn't match OWASP modsec behavior. It was checked on Apache engine and the behavior is definitely different.
TLDR:
SecAuditLogParts
H - this part should contain messages
K - this part should contain rule

Steps to reproduce

To reproduce the behavior on apache modsec you need to run it and config with this config and this config

Expected result

Test in the PR should pass and H flag should be responsible for messages, K flag for rules.

Actual result

In coraza now it works different way
K flag allows you to see messages (look at tests I've attached below).

See the in the attachments apache modsec behavior.
H flag on (no K flag)

K flag on (no H flag)

both flags on

[M4tteoP]

Thanks for raising this, by any chance, would you also be able to check how modsec v3 on nginx is behaving? It would be great if we could collect all three behaviors and then reason about the "right" behavior and see which engine requires fixes

[tty2]

I'll try to do it soon

[tty2]

So I can admit that nginx behaves similar to apache (H is responsible for messages)

H flag on (no K flag)

both

[tty2]

I'm not sure why K doesn't print out rules on nginx, probably not implemented yet (at least v3 doc says that), but anyways this flag has clear documentation:
K: This part contains a full list of every rule that matched (one per line) in the order they were matched. The rules are fully qualified and will thus show inherited actions and default operators. Supported as of v2.5.0.

[tty2]

And I can see the other inconsistency now, for none-json representation (not sure how to name it) in coraza, we don't print out A and Z sections.

This config

SecRuleEngine DetectionOnly
SecAuditEngine On
# SecAuditLogFormat json
SecAuditLogType serial
SecAuditLogParts ABCHKZ
SecAuditLogRelevantStatus ".*"
# auditlog should not contain messages section without H flag included, but should contain Rule below
SecRule ARGS "@unconditionalMatch" "id:1,phase:1,log,auditlog,msg:'nolog message'"
SecAuditLog ./modsec_audit.log

You can see it with our http-example. But we should probably add it as a separate issue.

[M4tteoP]

And I can see the other inconsistency now, for none-json representation (not sure how to name it) in coraza, we don't print out A and Z sections.

Just linking this conversation #801 (comment) that should be related. We removed A and Z from being modeled in the code, but it does not necessarily mean that we can not still print them in the none-json / plan text representation

[tty2]

So looking at the current implementation I'd say we just need to move this block to the upper case. I just compared coraza messages structure with nginx modsec3 and at first glance it looks a little bit different but matches mostly (I'll add mapping in the next comment).
There are other concerns I have:

• We need to add implement the behavior for K flag, matched rules but for 3d version we don't have an implementation. The only thing we can do is make it like 2d.
  Like here

[tty2]

Message structure comparison

Coraza	Nginx modsec3
actionset	-
message	message
data (struct)	details (struct)
data.file	details.file
data.line	details.lineNumber
data.id	details.ruleId
data.rev	details.rev
data.msg	details.data
data.severity	details.severity
data.ver	details.ver
data.maturity	details.maturity
data.accuracy	details.accuracy
data.tags (list)	details.tags (list)
data.raw	details.match
-	details.reference

Other difference I noticed: for data.raw in coraza we print out the rule (probably because we wanted to use it for rule section.
Example
"raw": "SecRule ARGS \"@unconditionalMatch\" \"id:1,phase:1,log,auditlog,msg:'nolog message'\""

For nginx modsec 3 for details.match it looks like we try to parse rule and guess what the reason
Example
"match": "Matched \"Operator `UnconditionalMatch' with parameter `' against variable `ARGS:i360test' (Value: `88ff0adf94a190b9d1311c8b50fe2891c85af732' )",

Screens
coraza messages

nginx modsec 3 messages

[tty2]

So, I'm ready to make changes for the related PR

• I just need your thoughts about K section. What we do with it now?
• if we ok to keep messages structure and move it to H section as it is?

[tty2]

The PR related to the issue is ready for review.

[M4tteoP]

If I'm fully getting the context, we have three issues. We should be able to isolate and address them individually:

• 1. H and K flags behavior.

  • H: Described as Audit log trailer. It currently returns quite useless information.

  • K: Described as This part contains a full list of every rule that matched (one per line) in the order they were matched. It currently returns the Matched rules.

  • Here I think the main issue and missing piece is that in H modsec is printing also the error logs (Even more, in the apache screenshot I see errors logs of Apache itself, not just the modsec ones). This is something that can be added, and it is not a breaking change in terms of audit logs that Coraza is providing, but just an addition. I opened audit: H should populate also with error logs. #1310 isolating some of the code from your PR and working from it. What do you think? cc @tty2

• 2. log and auditlog actions.

  • log / nolog: are indicating if a match of that rule needs to be logged. By logging it means both error logs and audit logs. So in audit logs we are printing only the rules that are marked as log .
  • auditlog: is a transaction flag, that Marks the transaction for logging in the audit log
  • noauditlog: affects only the current rule, so it requires to not populate the audit logs with that specific rule.
  • TODO: I still have to check your PR around that.

• 3. A and Z sections not printed in native format.

  • Related to [owasp-modsecurity compatibility] SecAuditLogParts flags #1305 (comment).
  • TODO: It should be quite straightforward to tweak the nativeFormatter and always add the two boundaries.

Thank you!

[tty2]

Hey @M4tteoP
Great you've done so dip investigation, now it's even more clear.
As I can see the draft you started to work offers an alternative approach for H flag, right? I mean an alternative for this
If so, could you please add the test for nolog,auditlog behavior? It's is most tricky probably, at least for me from the first glance. Or you think it should be done in a separate PR?