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

Adding WAF Coraza+Caddy parser/scenario #942

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

Barnoux
Copy link

@Barnoux Barnoux commented Jan 29, 2024

Hello,

Hope this parser will find some love.
The goal of this parser is to parse waf alert from coraza when coraza is integreted as a plugin in caddy.
the scenario is triggered based on the treshold of the inbound anomaly score setup by the users in the crs-setup.conf file.

The work on the parser is based on the work done by https://github.com/crowdsecurity/hub/blob/master/parsers/s01-parse/crowdsecurity/modsecurity.yaml

It's time for me to eat a cake and take a nap.

@Barnoux Barnoux changed the title Adding WAF Coraza+Caddy parser Adding WAF Coraza+Caddy parser/scenario Jan 29, 2024
@LaurenceJJones LaurenceJJones added the needs-tests needs functional tests to be merged label Jan 29, 2024
@LaurenceJJones
Copy link
Contributor

Hey 👋🏻 Thank you for opening a PR!

We going to be need some tests for the parsers and scenarios. I left an initial comment since coraza is a modsecurity implementation meaning a scenario on the rule id might not be best since you dont have to use CRS.

@Barnoux
Copy link
Author

Barnoux commented Jan 31, 2024

Hey 👋🏻 Thank you for opening a PR!

We going to be need some tests for the parsers and scenarios. I left an initial comment since coraza is a modsecurity implementation meaning a scenario on the rule id might not be best since you dont have to use CRS.

What type of input do you need for testing ? The rule id chosed in the scenario is based on the inbound anomaly score that is triggered and can be tuned by the user (https://coreruleset.org/docs/concepts/anomaly_scoring/#anomaly-score-thresholds). I did't find a better way to handle this. The crowdsec modsecurity scenario is trigger based on the severity of the alert and it is too restrictive approach.

@LaurenceJJones LaurenceJJones removed the needs-tests needs functional tests to be merged label Feb 17, 2024
@LaurenceJJones
Copy link
Contributor

Okay, the last thing is adding a collection of

Parser
Scenarios (could include our standard modsec one 🤷🏻 )

@Barnoux
Copy link
Author

Barnoux commented Feb 29, 2024

Okay, the last thing is adding a collection of

Parser Scenarios (could include our standard modsec one 🤷🏻 )

I add the collection, is it looking good ?

@ubergeek77
Copy link

Hey there, just passing by. I came across this PR as it's exactly what I'm looking for.

@LaurenceJJones , hope you don't mind the ping after so long, but are any more changes needed to get this merged? 🙂

@LaurenceJJones
Copy link
Contributor

Hey there, just passing by. I came across this PR as it's exactly what I'm looking for.

@LaurenceJJones , hope you don't mind the ping after so long, but are any more changes needed to get this merged? 🙂

The only issue I have with is the scenario as its close to the original one we have for modsecurity. However, I did forget about this, so ill do this now.

@Barnoux
Copy link
Author

Barnoux commented Nov 4, 2024

The only issue I have with is the scenario as its close to the original one we have for modsecurity. However, I did forget about this, so ill do this now.

What can i do to help you on this one ?

@LaurenceJJones
Copy link
Contributor

LaurenceJJones commented Nov 4, 2024

I will look into it, the only thing I dont like is the modification that was needed to the original caddy parser (which I had to do because we just unmarshal the json and move on)

Maybe we can move this too s02 level instead cause if at any point they change the log handler name it will break the chain. Moving to s02 means we can still keep the original filter and then let the natural chain processing happen. The only thing is we have to execute before the original enrichers but this can be looked into.

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

Successfully merging this pull request may close these issues.

4 participants