corazawaf / coraza-caddy

OWASP Coraza middleware for Caddy. It provides Web Application Firewall capabilities
https://www.coraza.io/
Apache License 2.0
329 stars 41 forks source link

Feature Request: Ability to respond with a custom html file #137

Closed briandoesdev closed 6 months ago

briandoesdev commented 7 months ago

Hey! I've been using Caddy with Coraza for a while and have been really impressed. I've come across a feature I think could be a useful addition: the ability to respond with a custom HTML file. I've been playing around with this the past few days and have a simple example below of the configuration.

As for why: I think a feature like this could provide the ability for Coraza users to brand their responses and provide any instructions to users who trigger a rule on their site(s).

Example usage

{
    order coraza_waf first
}

http://127.0.0.1:8080 {
 coraza_waf {
  directives `
   SecAction "id:1,pass,log"
   SecRule REQUEST_URI "/test5" "id:2, deny, log, phase:1"
   SecRule REQUEST_URI "/test6" "id:4, deny, log, phase:3"
   Include file1.conf 
   Include file2.conf
   Include /some/path/*.conf
  `
  handle_custom_response /path/to/custom_error.html 
 }
 reverse_proxy http://192.168.1.15:8080
}

I actually threw together a basic version of this already. While I'm newer to open source (and a security engineer by trade), I'm excited to refine this and submit a pull request. I'd also appreciate any feedback.

Thanks!

fzipi commented 7 months ago

Hi @briandoesdev! Thanks for chiming in. I think this is a reasonable feature. In Apache + Modsec, I used to do something like:

  1. Have a default error, different than 403, example: SecDefaultAction "phase:2,log,auditlog,pass,status:501"
  2. Show a different error page when 501
    <IfModule lua_module>
    ErrorDocument 501 /security-error
    LuaMapHandler ^/security-error$ /var/www/errors/security-error.lua
    </IfModule>
  3. Then use a Lua script to show the error code, by parsing a template and showing some variables, like the transaction ID. In that process, return 403 to the user. The template could be similar to this:

     r.content_type = "text/html"
     ... (get the filename)
     local f = assert(io.open(filename, "r"))
     local t = f:read("*all")
     f:close()
    
     t = string.gsub(t, 'MOD_SECURITY_UNIQUE_ID', r.subprocess_env['REDIRECT_UNIQUE_ID'])          
     r:puts(t)
     r.status = 403
     return apache2.OK

This looks cumbersome, we probably can do way better by integrating a new directive into coraza that just returns the error page, depending on the error code. I would also create that error page as an html/template and pass all the interesting variables from Coraza so the page can render whatever it is required.

jcchavezs commented 7 months ago

Hi @briandoesdev,

We discussed about the WAF mutating the response in the CRS channel (cc @dune73) as there exist directives to do such thing in seclang but the conclusion was that a WAF should not mutate the response. This means directives should not take care of this.

As for the connector itself, yeah we can support it. My questions would be:

  1. Is this something you need or more like something it would be nice to have.
  2. Is redirect action a reasonable feature to overcome this? Main issue with redirect is that it is on the client to honor it but since here we are talking about html it is very likely the browser will do the redirect.
  3. Do you want to show blocking information on this? If so, what is it?
  4. Does redirect support variables? If you want to show data from matched rules or transaction, can a redirect with query parameters support this? cc @airween @m4tteoP
  5. Shall we make this config portable? E.g. depending on a file isn't desirable as you need to distribute the config and the template file, also if you need to render transaction information, an html file isn't aid.

Cc @jptosso as he proposed this feature aswell.

On Mon, 26 Feb 2024, 12:44 Felipe Zipitría, @.***> wrote:

Hi @briandoesdev https://github.com/briandoesdev! Thanks for chiming in. I think this is a reasonable feature. In Apache + Modsec, I used to do something like:

  1. Have a default error, different than 403, example: SecDefaultAction "phase:2,log,auditlog,pass,status:501"
  2. Show a different error page when 501
ErrorDocument 501 /security-error LuaMapHandler ^/security-error$ /var/www/errors/security-error.lua
  1. Then use a Lua script to show the error code, by parsing a template and showing some variables, like the transaction ID. In that process, return 403 to the user. The template could be similar to this:

    r.content_type = "text/html" ... (get the filename) local f = assert(io.open(filename, "r")) local t = f:read("*all") f:close()

    t = string.gsub(t, 'MOD_SECURITY_UNIQUE_ID', r.subprocess_env['REDIRECT_UNIQUE_ID']) r:puts(t) r.status = 403 return apache2.OK

This looks cumbersome, we probably can do way better by integrating a new directive into coraza that just returns the error page, depending on the error code. I would also create that error page as an html/template and pass all the interesting variables from Coraza so the page can render whatever it is required.

— Reply to this email directly, view it on GitHub https://github.com/corazawaf/coraza-caddy/issues/137#issuecomment-1963951896, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYASZT5J4SOBIUQZ3KOTYVRYSRAVCNFSM6AAAAABDZQM7UOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRTHE2TCOBZGY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

briandoesdev commented 7 months ago

@fzipi @jcchavezs Thanks for taking the time to respond! I appreciate it.

As for the questions you had:

  1. Is this something you need or more like something it would be nice to have.

Definetly a nice to have. By no means is this required. I only thought of this as a way to provide some form of feedback for users to show them they are receiving a block based on their request.

  1. Is redirect action a reasonable feature to overcome this? Main issue with redirect is that it is on the client to honor it but since here we are talking about html it is very likely the browser will do the redirect.

I didn't think of using a redirect, but that could be reasonable to me.

  1. Do you want to show blocking information on this? If so, what is it?

I did think of this one though. My initial thoughts were to only show the transaction ID, that way if a user were to reach out I could focus on that specific log event. I don't have this implemented in the quick demo I made, but did debate on adding it before making this feature request.

  1. Shall we make this config portable? E.g. depending on a file isn't desirable as you need to distribute the config and the template file, also if you need to render transaction information, an html file isn't aid.

This is another one I did think about. I actually initially wanted to have a sub-directive (?) like file_server kind of similar to what you can do with reverse-proxy to intercept error codes from the origin server:

example.com {
    reverse_proxy localhost:8080 {
        @error status 500 503
        handle_response @error {
            root    * /path/to/error/pages
            rewrite * /{rp.status_code}.html
            file_server
        }
    }
}

But instead of status codes, you could intercept based on rule ID's. To me, this would've been my preferred implementation. What I have now is very basic comparatively.


I was unaware this had been proposed before, I'm sure there is a valid reason it hasn't been implemented. If it's not something that you all want then that's fine with me of course. I appreciate you even entertaining my request. If it's more of a needing more contributors thing, then I'd be happy to give it a go if there's a good solution for everyone.

Thanks again!

jcchavezs commented 7 months ago

Thanks a lot for the feedback @briandoesdev. Sustainability is a key concept in this project so we aim to only include features that users will actually use (otherwise we end up maintaining code that nobody uses) and hence all this questions. In general I follow a rule I learnt in the pass called "rule of three" where we implement features required by three users from different companies otherwise it is not worth to do so. In this case I feel like there are different aspects of this solution that has to be designed and hence I'd rather explore easier alternatives first e.g. redirect which delegates this responsibility outside the WAF.

All that said, it is so cool to have people like you coming by with a good eye for the details, definitively you are more than welcome to contribute to this project as provide as much feedback on your own experience as possible.

briandoesdev commented 7 months ago

I appreciate the feedback on this. I did some messing around with Caddy this afternoon, and I discovered the handle_errors directive.

:8080 {
    coraza_waf {
        load_owasp_crs
        directives `
        Include @coraza.conf-recommended
        Include @crs-setup.conf.example
        Include @owasp_crs/*.conf
        SecRuleEngine On
        SecDebugLog /dev/stdout
        SecDebugLogLevel 9
        SecRule REQUEST_URI "@streq /admin" "id:101,phase:1,t:lowercase,deny,status:403"
        SecRule REQUEST_BODY "@rx maliciouspayload" "id:102,phase:2,t:lowercase,deny,status:403"
        SecRule RESPONSE_HEADERS::status "@rx 406" "id:103,phase:3,t:lowercase,deny,status:403"
        SecResponseBodyAccess On
        SecResponseBodyMimeType application/json
        SecRule RESPONSE_BODY "@contains responsebodycode" "id:104,phase:4,t:lowercase,deny,status:403"
        `
    }

    handle_errors {
        @block_codes `{err.status_code} in [403]`
        handle @block_codes {
            root    * /home/briandoesdev/projects/cloudwaf-projects/caddy-plugins/coraza-caddy/example
            rewrite * /{err.status_code}.html
            file_server
        }
    }

    respond "ok"
}

This does the same thing as my proposed addition; it just takes the burden off the WAF, as @jcchavezs suggests. Like I said before, I appreciate you guys taking the time to provide your thoughts. It looks like, for my use case, this will work.

jcchavezs commented 6 months ago

Awesome, I will happily close this issue.

Thanks for taking the time to go through the process, it will definitively help further questions.