fix: Logs print different messages for each the disruptive actions

[M4tteoP]

Initial report: corazawaf/coraza-proxy-wasm#209

This PR proposes to add different messages for each disruptive action, following the same logic used by ModSecurity v2 ( See here).

Currently, we have two Error log prefixes based on the Disruptive_ field of the matched rules that we are logging:

	if mr.Disruptive_ {
		fmt.Fprintf(log, "Coraza: Access denied (phase %d). ", mr.Rule_.Phase())
	} else {
		log.WriteString("Coraza: Warning. ")
	}

Considering that the Disruptive_ field itself is kind of misleading (actions like deny, allow, and pass are all considered disruptive actions), some logs ends up being misleading.
For example, a log from the 980170 rule, which has a pass action, starts with Access denied:

Coraza: Access denied (phase 5). Anomaly Scores: (Inbound Scores: blocking=3, detection=3, per_pl=3-0-0-0, threshold=5) - (Outbound Scores: blocking=0, detection=0, per_pl=0-0-0-0, threshold=4) - (SQLI=0, XSS=0, RFI=0, LFI=0, RCE=0,  [file "@owasp_crs/RESPONSE-980-CORRELATION.conf"] [line "12628"] [id "980170"] [rev ""] [msg "Anomaly Scores: (Inbound Scores: blocking=3, detection=3, per_pl=3-0-0-0, threshold=5) - (Outbound Scores: blocking=0, detection=0, per_pl=0-0-0-0, threshold=4) - (SQLI=0, XSS=0, RFI=0, LFI=0, RCE=0, "] [data ""] [severity "emergency"] [ver "OWASP_CRS/4.0.0-rc1"] [maturity "0"] [accuracy "0"] [tag "reporting"] [hostname "10.110.32.70"] [uri "/metrics"] [unique_id "KjkRGNlRpxeKJftwrWP"]

[codecov]

Codecov Report

Patch coverage: 92.00% and project coverage change: +0.04% 🎉

Comparison is base (5d3b707) 81.51% compared to head (f848261) 81.56%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #827      +/-   ##
==========================================
+ Coverage   81.51%   81.56%   +0.04%     
==========================================
  Files         160      160              
  Lines        9033     9051      +18     
==========================================
+ Hits         7363     7382      +19     
+ Misses       1418     1417       -1     
  Partials      252      252              

Flag	Coverage Δ
default	76.65% <91.66%> (+0.05%)	⬆️
examples	25.51% <40.00%> (+0.01%)	⬆️
ftw	46.67% <4.16%> (-0.12%)	⬇️
ftw-multiphase	48.78% <4.16%> (-0.12%)	⬇️
tinygo	74.83% <91.66%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
internal/corazawaf/body_buffer.go	74.52% <0.00%> (ø)
internal/corazarules/rule_match.go	65.44% <93.75%> (+4.15%)	⬆️
examples/http-server/main.go	70.27% <100.00%> (ø)
internal/corazawaf/transaction.go	78.61% <100.00%> (+0.08%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jcchavezs]

What is missing here?

[M4tteoP]

What is missing here?

I moved it to ready for review after adding a proper PR description. There is nothing missing from my side

[M4tteoP]

Removing this line should fix corazawaf/coraza-caddy#87 and supersede corazawaf/coraza-caddy#93. Now writing the new line is delegated to the ErrorLog() caller.

[anuraaga]

Maybe for another PR but we should migrate this to an enum instead of string for faster matching

[jcchavezs]

Could you please take care of that before merge, @M4tteoP?

[M4tteoP]

Thanks Rag, I tried to address it. A better way might be propagating the Action enum from the parser mapping into Action functions, but it requires quite a broad refactor. Did you have in mind something like what I just implemented?

[jcchavezs]

Are you happy with the change @anuraaga ?