Missing "hostname" key in logs

[mac-chaffee]

Description

As discovered here: jcmoraisjr/haproxy-ingress#964 (comment)

Logs appear to be missing the hostname key (newlines inserted for visibility):

Loading 1 applications
time="2022-11-21T19:21:07Z" level=info msg="spoe: listening on [::]:9000"
{"level":"error","ts":1669058760.188159,"msg":"[client \"10.100.0.172\"] Coraza: Warning. OS File Access Attempt 
[file \"/etc/coraza-spoa/rules/REQUEST-930-APPLICATION-ATTACK-LFI.conf\"] [line \"0\"] [id \"930120\"] [rev \"\"] 
[msg \"OS File Access Attempt\"] [data \"Matched Data: etc/passwd found within ARGS:p: /etc/passwd\"] [severity \"critical\"] 
[ver \"OWASP_CRS/4.0.0-rc1\"] [maturity \"0\"] [accuracy \"0\"] [tag \"application-multi\"] [tag \"language-multi\"] 
[tag \"platform-multi\"] [tag \"attack-lfi\"] [tag \"paranoia-level/1\"] [tag \"OWASP_CRS\"] [tag \"capec/1000/255/153/126\"] 
[tag \"PCI/6.5.4\"] 
[hostname \"\"]   <---
[uri \"/?p=/etc/passwd\"] [unique_id \"260e3d15-dd04-41b7-a634-45d17f0a8390\"]\n"}

If I'm reading the code correctly, that log value is always set to an empty string:

coraza/internal/corazarules/rule_match.go

Lines 123 to 139 in 9d003ed

	resolvedIP := ""
	msg := matchData.Message()
	data := matchData.Data()
	if len(msg) > 200 {
	msg = msg[:200]
	}
	if len(data) > 200 {
	data = data[:200]
	}
	log.WriteString(fmt.Sprintf("[file %q] [line %q] [id %q] [rev %q] [msg %q] [data %q] [severity %q] [ver %q] [maturity %q] [accuracy %q]",
	mr.Rule_.File(), strconv.Itoa(mr.Rule_.Line()), strconv.Itoa(mr.Rule_.ID()), mr.Rule_.Revision(), msg, data, mr.Rule_.Severity().String(), mr.Rule_.Version(),
	strconv.Itoa(mr.Rule_.Maturity()), strconv.Itoa(mr.Rule_.Accuracy())))
	for _, t := range mr.Rule_.Tags() {
	log.WriteString(fmt.Sprintf(" [tag %q]", t))
	}
	log.WriteString(fmt.Sprintf(" [hostname %q] [uri %q] [unique_id %q]",
	resolvedIP, mr.URI_, mr.TransactionID_))

When multiple sites are fronted by a single server running Coraza, you need the hostname field to tell which site triggered the rule.

Since Coraza scans the headers anyway, it might be possible to just grab the Host header to populate that hostname field. HTTP says that header is required anyway.

[fuomag9]

Related issue corazawaf/coraza-caddy#35

[M4tteoP]

Current situation and Coraza/Modsec comparison:

ModSec v3:

• Populates m_variableServerName looking for the host header (see here).
• Logs side: the hostname field is populated with the server IP (see here).

Currently Coraza:

• tx.variables.serverName is never populated, therefore SERVER_NAME is always empty.
• Logs: the hostname field is empty.

Possible Fixes

SERVER_NAME:

1. Populate it with host header Coraza side (just like ModSec)
2. Just like other connection details (IP, ports), let's delegate its retrievement to the connectors adding one field to ProcessConnection. If empty string, let's populate it with the IP address. (Accoring to ModSec doc: SERVER_NAME: contains hostname or IP address.

Logs:

• Current PR adds the server IP to the hostname field for logging.
• Once tx.variables.serverName is populated we may sanitize (at least check allowed characters?) and two different fields for address and hostname.

I'm happy to work on it, I just want to agree on the implementation. At first sight, I would go for adding one field to ProcessConnection, could some connectors be aware of the hostname they are listening at and not have to look at the headers?

[github-actions]

This issue is stale because it has been open for 30 days with no activity.

[jcchavezs]

@fuomag9 @mac-chaffee we just medged the fixing PR. Could you please test it?

[M4tteoP]

The merged PR aligns the hostname field for logging to the ModSec behaviour. Still, I think that it is a partial fix mainly because of the never populated SERVER_NAME field. I'm gonna open a PR that proposed to delegate it to ProcessConnection (Option 2 of my above comment)
update: I went for option 1 see details inside the PR.