Add the context on Envoy requests to the audit logs

[sfc-gh-msathe]

This is not yet fully baked and I am here to get feedback on various choices. The motivation here is to bring a traceability between the Coraza audit logs and the envoy request logs by including the x-request-id header value in the audit logs.

This won't affect any of the existing usage as this inclusion is controlled by the include_request_id_in_audit_logs configuration.

I am looking for feedback over:

1. A broad need for this traceability vs just my use case (in which case a fork might suffice)
2. The configuration include_request_id_in_audit_logs vs something broader like include a list of request headers with given labels.
3. Implementation.

I understand this lacks unit tests and the code could be cleaner but I am waiting for a general agreement over the approach before committing the time.

Thanks all!

[jcchavezs]

@sfc-gh-msathe I really love you brought this up.

We have a couple of issues around this in corazawaf/coraza#711 and #166 where I was specifically targeting specifically request ID.

What I think we could do as discussed in corazawaf/coraza#711 we could:

1. Add the API in coraza (in the experimental package) where we list variables []string{"REQUEST_HEADERS:X-Request-ID"} which will be used to obtain variables from the transaction that can be displayed in the H part (described as Audit log trailer, which contains additional data). If someone truly believes this will break parsers or something we could propose a new audit log part.
2. Update the formatters to output in the audit logs e.g. [var:REQUEST_HEADERS:X-Request-ID=abc123] in modsec formatter or "vars": {"REQUEST_HEADERS:X-Request-ID": "abc123"} in JSON format.

As per coraza-proxy-wasm we could add the slice include_variables_in_audit_logs: ['REQUEST_HEADERS:X-Request-ID'] to achieve the same.

Thoughts @airween @dune73 @jptosso @anuraaga @M4tteoP

[anuraaga]

If this is a request header, shouldn't it be automatically outputted in the headers (B) part of audit logs?

[anuraaga]

Ah I see in corazawaf/coraza#711 there is the issue that all headers are printed which may not be desired. That seems like a big issue with seclang syntax, I would first add a config to seclang to set an allow or deny list for logging headers as without it the B part seems implementation seems incomplete.

While generic selection of variables for logging may have a different use case, for headers it doesn't seem like it should be needed, we already have a part for it we should improve that.

[sfc-gh-msathe]

I could be wrong but it seems like the WASM filter here can't really print all the parts and just a fixed set of params are printed. See this issue I opened: #255

Is this not correct? I didn't find anything in the code that might print anything but these:
https://github.com/corazawaf/coraza-proxy-wasm/blob/main/wasmplugin/plugin.go#L699

The ErrorLog() method is here:
https://github.com/corazawaf/coraza/blob/f68d1c208791d741581a7d413c32777fbd74c611/internal/corazarules/rule_match.go#L254

[sfc-gh-msathe]

Again, I am fairly new to this so I could be wrong