Skip to content
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

Missing hostname in WAF logs #75

Open
TheForcer opened this issue Jun 15, 2023 · 18 comments
Open

Missing hostname in WAF logs #75

TheForcer opened this issue Jun 15, 2023 · 18 comments
Assignees

Comments

@TheForcer
Copy link

Hi everyone,

I am currently trying to implement Coraza into my Caddy setup, but for some reason the hostname of blocked requests does not get logged. As you can see in the log samples below, the hostname is recorded as [hostname ""]

2023/06/15 11:08:02.985 WARN    http.handlers.waf       [client "192.168.178.11"] Coraza: Access denied (phase 1). POST without Content-Length or Transfer-Encoding headers [file "@owasp_crs/REQUEST-920-PROTOCOL-ENFORCEMENT.conf"] [line "1543"] [id "920180"] [rev ""] [msg "POST without Content-Length or Transfer-Encoding headers"] [data "0"] [severity "warning"] [ver "OWASP_CRS/4.0.0-rc1"] [maturity "0"] [accuracy "0"] [tag "application-multi"] [tag "language-multi"] [tag "platform-multi"] [tag "attack-protocol"] [tag "paranoia-level/1"] [tag "OWASP_CRS"] [tag "capec/1000/210/272"] [hostname ""] [uri "/api/add"] [unique_id "iicqKuOYWuBJfVcM"]

2023/06/15 11:08:02.985 ERROR   http.handlers.waf       [client "192.168.178.11"] Coraza: Access denied (phase 2). XSS Attack Detected via libinjection [file "@owasp_crs/REQUEST-941-APPLICATION-ATTACK-XSS.conf"] [line "4432"] [id "941100"] [rev ""] [msg "XSS Attack Detected via libinjection"] [data "Matched Data: XSS data found within ARGS:url: <script>"] [severity "critical"] [ver "OWASP_CRS/4.0.0-rc1"] [maturity "0"] [accuracy "0"] [tag "application-multi"] [tag "language-multi"] [tag "platform-multi"] [tag "attack-xss"] [tag "paranoia-level/1"] [tag "OWASP_CRS"] [tag "capec/1000/152/242"] [hostname ""] [uri "/api/add"] [unique_id "iicqKuOYWuBJfVcM"]

2023/06/15 11:08:02.985 ERROR   http.handlers.waf       [client "192.168.178.11"] Coraza: Access denied (phase 2). XSS Filter - Category 1: Script Tag Vector [file "@owasp_crs/REQUEST-941-APPLICATION-ATTACK-XSS.conf"] [line "4451"] [id "941110"] [rev ""] [msg "XSS Filter - Category 1: Script Tag Vector"] [data "Matched Data: <script> found within ARGS:url: <script>"] [severity "critical"] [ver "OWASP_CRS/4.0.0-rc1"] [maturity "0"] [accuracy "0"] [tag "application-multi"] [tag "language-multi"] [tag "platform-multi"] [tag "attack-xss"] [tag "paranoia-level/1"] [tag "OWASP_CRS"] [tag "capec/1000/152/242"] [hostname ""] [uri "/api/add"] [unique_id "iicqKuOYWuBJfVcM"]

According to #35 this should be fixed, so I am not sure if there is a possible misconfiguration at play also.

I've compiled Caddy 2.6.4 with xcaddy in several ways, with the most recent command seen as below. AFAIK these versions should contain the servername fixes.

xcaddy build --with github.com/corazawaf/coraza-caddy/@v2.0.0-rc.2 --with github.com/corazawaf/coraza/@v3.0.0

My current Caddyfile looks like this.
I am using the current rule files from coraza-coreruleset, haven't made any changes except additionaly allowing HTTP/3 & HTTP/3.0 versions in crs-setup.conf.

{
        debug
        http_port 1180
        https_port 11443
        admin off
        auto_https off
        order coraza_waf first
        servers {
                metrics
        }
}

(tls) {
        tls ./{args.0}_ecc/fullchain.cer ./{args.0}_ecc/{args.0}.key
        header {
                Strict-Transport-Security max-age=63072000;includeSubDomains;preload
                X-Content-Type-Options nosniff
                X-Frame-Options SAMEORIGIN
                X-Robots-Tag none
        }
}

(waf) {
        coraza_waf {
          #load_owasp_crs
          directives `
           Include @coraza.conf-recommended
           Include @crs-setup.conf.example
           Include @owasp_crs/*.conf
           SecRuleEngine On
          `
 }
}

memes.example.com {
        import tls example.com
        import waf
        reverse_proxy 10.0.0.109:5001
}

Would be happy about some hints. Thank you 😀

@jcchavezs
Copy link
Member

Am I right to assume that hostname will come from this line https://github.com/corazawaf/coraza-caddy/blob/main/http.go#L44 @M4tteoP ?

@M4tteoP
Copy link
Member

M4tteoP commented Jun 15, 2023

I think that around the hostname, and server name, we have to clarify a bit the logic and maybe do still some work around them:

Hostname
The hostname field of the error logs has been aligned with corazawaf/coraza#517 to the ModSecurity behavior. It means that we expect that the field is populated with the server IP. The latter is provided by the connector via ProcessConnection (func (tx *Transaction) ProcessConnection(client string, cPort int, server string, sPort int). Currently, in the Caddy connector we are missing this last step, resulting in a hostname always empty.

Server name
corazawaf/coraza#572 is meant to fix the SERVER_NAME variable that was never been populated and therefore was never matching. For that purpose, we are providing SetServerName to the connectors, and we expect that it is populated with the Host header. It is the header provided as-is by the client and the SERVER_NAME variable itself is indeed meant to be used to perform checks on it.

The discussion/actions we should take are:

  1. fix the ProcessConnection in the Caddy connector providing the server string. It could be the IP, or via some config providing a way to propagate the hostname that the coraza module is intercepting.
  2. Evolve the conversation that we had in missing hostname in logs coraza#517, and take into account the idea of performing a name resolution to provide more meaningful values for the hostname field.

Thanks @TheForcer for raising it!

@jcchavezs
Copy link
Member

jcchavezs commented Jun 15, 2023

Any hint on how to obtain the right server name @mholt?

@mholt
Copy link

mholt commented Jun 15, 2023

Are you talking about the user-given name of the server in the JSON config as keys here? https://caddyserver.com/docs/json/apps/http/servers/

@jptosso
Copy link
Member

jptosso commented Jun 15, 2023

Sometimes users might bind ip address or just a port. We will have to cover all flanks

@M4tteoP
Copy link
Member

M4tteoP commented Jun 19, 2023

FYI, related conversation ModSecurity side: owasp-modsecurity/ModSecurity#2906

@jptosso
Copy link
Member

jptosso commented Jul 14, 2023

Any idea on how to continue? The problem is we are not filling ProcessConnection and that is the variable we are using to display the logs.

@ErazerBrecht
Copy link

I don't want to be this dude but are there any plans?
Afaik it's this line: https://github.com/corazawaf/coraza/blob/dc778126cf458b6a832ced0cffce077f1653147c/internal/corazarules/rule_match.go#L198

And I could create a PR with it being the host header if available.
But it looks like it's being blocked at a 'higher level'.

For me specifically it would be nice to have the hostname because we have a single WAF for multiple domains.
(And is also what we used to have with Apache)

@jcchavezs
Copy link
Member

Please give it a stab @ErazerBrecht

@bdragoiu
Copy link

bdragoiu commented Aug 5, 2024

Hello,

I'd also greatly appreciate this feature! I'm a first time caddy and coraza user and everything fell into place so neatly except this.
I'm currently planning to log to Graylog and the hostname would help with correlation between logs from http.handlers.waf and http.log.access.log*.

@glenn-kusardi
Copy link

I'd also greatly appreciate this feature! I'm a first time caddy and coraza user and everything fell into place so neatly except this. I'm currently planning to log to Graylog and the hostname would help with correlation between logs from http.handlers.waf and http.log.access.log*.

I have exactly the same use case. So I'd like to upvote this issue.

@jptosso
Copy link
Member

jptosso commented Sep 27, 2024

We have to push this solution. Is there any proxy standard that establishes how to get this?

I would say:

  • The first Host header if no proxy is used
  • X-Forwarded-Host if proxy is being used

Questions are

  • What happens if a user sends the X-Forwarded-Host if there is no proxy?
  • What happens if both x-forwarded-host and host comes as part of the request?
  • What happens if we are being proxied and no x-forwarded-host is set?
  • Should we include directives to understand we are being proxied or should we assume it?
  • How do we safely assume it?

@fzipi
Copy link
Member

fzipi commented Sep 28, 2024

I don't think we should trust the Host header. The Host name must be configured by the service and that should be the reasonable case. We should not trust XFF also.

Adding name resolution could add some latency, but I guess it just need to be performed once at initialization probably... so I would say let's do 1) from @M4tteoP above.

@jptosso
Copy link
Member

jptosso commented Sep 28, 2024

We go back to the issue, what if a wildcard was used

@fzipi
Copy link
Member

fzipi commented Sep 30, 2024

Using a wildcard doesn't work when doing name resolution. In the example above there was no wildcard, so I didn't thought it was a problem.

We should agree that adding to logs anything that comes in the Host header is a bad idea. You need to sanitize logs or certainly you will be victim of attacks on logs, which are dangerous.

So, my reasoning here will be:

  • if there is a host configured in the Caddyfile, use it.
  • if the configuration is using a wilcard, we should log "wildcard domain"/"wildcard" or whatever static string we choose. Doing a dynamic reverse DNS on each query is a waste of resources, imposes delays, and even in that case things should be limited to an array of options.

@glenn-kusardi
Copy link

glenn-kusardi commented Sep 30, 2024

Maybe I'm completely misunderstanding the problem. Webservers in general, and Caddy in this case, are already able to log the requested HTTP header "Host", so in my naive idea, Coraza only has to do the same, which is implemented on the Caddy-side?

Wildcard domains should not occur, at least not configured. Also configuration is not relevant to it and no reverse dns should be necessary. It is the Host-header from the HTTP request which should be used for the log and the value must be sanitized for the log in the same way, the webserver does it.

@glenn-kusardi
Copy link

OK, I followed the discussion in the linked pages and I understand it better why there are different approaches and solutions to this problem.

As for me, I'm just trying to get some information from Caddy to the Coraza log, making it possible to link log lines to specific Caddy configuration parts. So maybe it is easier to implement something into Coraza Caddy which makes it possible to define log content in the Caddyfile which is appended to a log line.

@glenn-kusardi
Copy link

glenn-kusardi commented Oct 8, 2024

What I now tested, is to add a tag with the hostname. In @crs-setup.conf this is actually already outlined in an example for SecDefaultAction:
SecDefaultAction "phase:1,log,auditlog,redirect:'http://%{request_headers.host}/',tag:'Host: %{request_headers.host}'"
SecDefaultAction "phase:2,log,auditlog,redirect:'http://%{request_headers.host}/',tag:'Host: %{request_headers.host}'"

But if I uncomment this lines an error message is returned when restarting Caddy, describing that "SecDefaultAction must not contain metadata actions". If that would work, it would be sufficient for my use case.

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

No branches or pull requests

9 participants