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

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

Open
Tracked by #945
buixor opened this issue May 25, 2023 Discussed in #784 · 7 comments
Open
Tracked by #945

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

buixor opened this issue May 25, 2023 Discussed in #784 · 7 comments
Assignees

Comments

@buixor
Copy link

buixor commented May 25, 2023

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:

  • We don't want to expose the user to SecLang / make him touch the rules
  • Crowdsec+expr offers some possibilities that might not be achievable with SecLang

Looking forward to hearing from you 😃

Discussed in #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
Copy link
Member

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
Copy link
Member

Ping @buixor

@buixor
Copy link
Author

buixor commented Jun 16, 2023

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
Copy link
Member

Related: libcoraza also needs to add/remove methods dynamically corazawaf/libcoraza#23 cc @potats0

@potats0
Copy link
Contributor

potats0 commented Jun 26, 2023

Related: libcoraza also needs to add/remove methods dynamically corazawaf/libcoraza#23 cc @potats0

copy that

@jptosso
Copy link
Member

jptosso commented Dec 27, 2023

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

type TransactionFlowControl interface {
  RemoveRuleById(int)
  Skip(int)
  ...
}

@jptosso jptosso mentioned this issue Dec 27, 2023
16 tasks
@jcchavezs
Copy link
Member

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.

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

6 participants