corazawaf / coraza

OWASP Coraza WAF is a golang modsecurity compatible web application firewall library
https://www.coraza.io
Apache License 2.0
2.27k stars 225 forks source link

exposing more methods in the `types.Transaction` interface ? #799

Open buixor opened 1 year ago

buixor commented 1 year ago

Hello,

As per the discussion below and our slack chat, I open an issue to track this topic,

What we want to do

We are looking to integrate Coraza within the crowdsec project. Concretely, it means that the existing web bouncers can forward their requests to crowdsec, which will evaluate their requests with Coraza. We want to evaluate rules both in-band and out-of-band.

However, (hence the issue) we want to avoid exposing the user(s) to SecLang/SecRule, as the users can rely on the existing hub mechanism of crowdsec to get, update etc. their rules. One of the points we'd like to address is to allow the user to disable/modify some rules at runtime (here runtime means before the requests start to be evaluated, but once the engine has already been loaded and the transaction possibly created) based on some conditions, using the existing expr language that exists in crowdsec:

...
modify_waf_rule:
#disable some rules if people are coming  from our office range(s)
  - disable_id: [13,14,15]
    condition: evt.Meta.SourceRange == "1.2.3.0/24"

However, it seems that the types.Transaction interface only exposes a small subset of the methods of the underlying internal object. Would it be possible to expose more methods in the types.Transaction interface, RemoveRuleByID would be very useful for example. This would allow us (within crowdsec) to let the user manipulate the rules without touching coraza's configuration or the rules themselves.

Please let me know if you believe there are better ways to achieve this. While I do understand that this is something that is achievable with SecLang, it's not desirable for us because:

Looking forward to hearing from you :smiley:

Discussed in https://github.com/corazawaf/coraza/discussions/784

Originally posted by **buixor** May 4, 2023 Hello, First of all, thanks for the cool project. I have a few questions! While playing around with coraza (v3), I found the need to be able to use, for example, `RemoveRuleByID` at runtime. However, it seems that the `types.Transaction` object that you get from the `NewTransaction` method doesn't offer much "runtime configuration" methods, that **do** exist in the underlying internal `Transaction` object. For now, I have forked coraza to expose the `RemoveRuleByID` method in `types.Transaction` and it didn't seem to implode. Q: Am I missing something, is it intentional, or is it already planned?
jcchavezs commented 1 year ago

I have some reluctancies about this, adding a method RemoveRuleByID isn't hard to add but the UX is more complex than that. First of all, every rule is associated to a phase (ideally) and we need to define what would happen if the phase is already gone (can we easily inferr that from the rule? what feedback will user receive?). Also, what other Rule associated methods we need to expose.

Given the nature of this, not being able to check the correctness of the ruleset before runtime is desirable, adding this method invalids this assumption.

jcchavezs commented 1 year ago

Ping @buixor

buixor commented 1 year ago

Hello,

If I understand correctly, you're concerned it would be used during the request processing while the associated phase has already been evaluated. (I do believe it's something that might be detected at runtime, but that's another topic).

I do understand it's not suitable in all situations. That's why I'm going to open a PR to add it as an experimental package. In our situation, those RemoveRuleById (and eventually other methods) calls would only happen before the request is evaluated.

jcchavezs commented 1 year ago

Related: libcoraza also needs to add/remove methods dynamically https://github.com/corazawaf/libcoraza/pull/23 cc @potats0

potats0 commented 1 year ago

Related: libcoraza also needs to add/remove methods dynamically https://github.com/corazawaf/libcoraza/pull/23 cc @potats0

copy that

jptosso commented 11 months ago

@jcchavezs what about implementing experimental interfaces to add access to internal functions? We could add someething like

type TransactionFlowControl interface {
  RemoveRuleById(int)
  Skip(int)
  ...
}
jcchavezs commented 11 months ago

It could be a function that accepts a transaction and returns an interface after validating the methods can be applied given the phase of the transaction.