corazawaf / coraza

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

Exposing some of the hidden packages behind `internal` folder. #1094

Open s3rj1k opened 2 months ago

s3rj1k commented 2 months ago

I think that this internal folder that hides most of exported modules is an unfortunate situation.

Consider a use-case when someone wants to make a rules parser to help migrate off libmodsecurity, in current state this can only be done by actually launching waf via NewWAF function, this works but is a bit uncomfortable.

Furthermore, considering the case above, when broken rule is encountered, in some cases, error message is so obscured with error wrapping and lacks the actual information that can help to find the actual broken rule as opposed to how libmodsecurity reports parse errors (with partial rule and file:linenumber).

By exposing so of the internal packages to public it will allow to reuse them more comfortably and will help improve error messaging, at least there will be no multiple/useless error wrapping for parser code.

jptosso commented 2 months ago

Hey! The reason behind this is long-term maintenance; if you want a specific API or set of APIs, please mention them on this issue so we can review this. We don't have any problem with exposing additional APIs as long as it's safe, it doesn't break compatibility, and there is a clear use case.

There are certain limitations, for example, we cannot expose additional functions for existing immutable interfaces, as it breaks compatibility. In which case we would have to release Wrappers to access to them. Like:

tx := tx.(types.TransactionState)
tx = tx.(otherpackage.TransactionExtended)
s3rj1k commented 2 months ago

In my case described above I guess I would want access to:

fzipi commented 2 months ago

Do you want to create a new parser, or just add some directives?

s3rj1k commented 2 months ago

No, some utility that checks rules validity and has a bit more verbose error messages in regards to where the actual broken rule is (file:linenumber).

jcchavezs commented 2 months ago

Thanks for raising this @s3rj1k. There are a few reasons why we don't expose the parser and one of them is the API. The parser being internal allows us to accommodate the API aiming the best for the WAF, exporting it means we need to maintain compatibility and settle an API. I propose you the following: We can export the parser through the experimental API and then you can consume it and propose improvements when it comes to erroring and UX.

On Fri, Jul 12, 2024 at 11:02 PM s3rj1k @.***> wrote:

No, some utility that checks rules validity and has a bit more verbose error messages in records to where the actual broken rule is (file:linenumber).

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

s3rj1k commented 2 months ago

@jptosso Another solution maybe just standalone CLI utility in repository that just loads ruleset and reports error if any, I am fine with doing PRs as long as there is some kind of simplified entry point for rule testing that is.

Also this will be similar to what has libmodsecurity, they ship standalone CLI to just do rule verification.

This way there is no need to wraparound into experimental API.

jcchavezs commented 2 months ago

This way there is no need to wraparound into experimental API.

experimental API isn't a workaround, it is the official way for us to expose APIs we don't currently expose. Providing a CLI is not something we are inclined to for a couple of reasons: 1. you are the first person asking for it and 2. It is a maintenance burden for us (volunteering project). How about we expose the parser through an experimental API which we reshape if needed and you maintain the CLI and if the CLI becomes stable enough we could think to foster it under coraza? Does that make sense?

On Mon, Jul 15, 2024 at 11:50 PM s3rj1k @.***> wrote:

@jptosso https://github.com/jptosso Another solution maybe just standalone CLI utility in repository that just loads ruleset and reports error if any, I am fine with doing PRs as long as there is some kind of simplified entry point for rule testing that is.

Also this will be similar to what has libmodsecurity, the also ship standalone CLI to just do rule verification.

This way there is no need to wraparound into experimental API.

— Reply to this email directly, view it on GitHub https://github.com/corazawaf/coraza/issues/1094#issuecomment-2229498546, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAWQAZVWNRV7L47VGTDZMQ7ZBAVCNFSM6AAAAABKWX5VGSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRZGQ4TQNJUGY . You are receiving this because you commented.Message ID: @.***>

s3rj1k commented 2 months ago

@jptosso Yea, sounds also good

jcchavezs commented 2 months ago

Check https://github.com/corazawaf/coraza/pull/1101/files