Roshan Piyush 12:02:11 UTC
👋
Juan Pablo Tosso 12:02:15 UTC
Hello and welcome to our monthly meeting
Juan Pablo Tosso 12:03:23 UTC
Our agenda for today is:
Juan Pablo Tosso 12:04:52 UTC
@fzipitria @bxlxx.wu @JC
Juan Pablo Tosso 12:05:49 UTC
Ok so regarding de project status. Two PRs has enhanced Coraza performance by more than a 200% using CRS There have been some upgrades for the collections engine and a full replacement of the aho-corasick implementation for the pm and pmf operators
Juan Pablo Tosso 12:06:17 UTC
Roshan Piyush 12:06:33 UTC
So do you think we need more more on the aho-corasick side?
Juan Pablo Tosso 12:07:00 UTC
So according to my research, modsecurity takes 90mb of memory to handle CRS
Juan Pablo Tosso 12:07:06 UTC
Coraza used to take 900mb thanks to aho-corasick
Juan Pablo Tosso 12:07:12 UTC
with the new implementation coraza takes 300mb
Juan Pablo Tosso 12:07:17 UTC
we still have to make some magic
Juan Pablo Tosso 12:08:00 UTC
I have created some benchmarks for CRS: corazawaf/coraza#301 I will soon merge them to v2 too
Juan Pablo Tosso 12:08:41 UTC
Also there is a new github action that will tell us if there is a performance regression alert:
Juan Pablo Tosso 12:11:14 UTC
Any question regarding the current status?
Juan Pablo Tosso 12:11:19 UTC
shall we pass to v3?
fzipitria 12:12:57 UTC
👋
↳ Juan Pablo Tosso 12:13:12 UTC
Welcome Felipe
↳ fzipitria 12:13:46 UTC
🙇
fzipitria 12:14:12 UTC
Just wanted so say that this has been an awesome effort, thanks @Juan Pablo Tosso!! 👏 👏
Juan Pablo Tosso 12:14:46 UTC
Thanks Felipe, we have received a lot of help from the community too 🙂
fzipitria 12:15:08 UTC
Of course 🙂
Juan Pablo Tosso 12:15:11 UTC
Regarding v3. There have been many improvements and API changes First of all, the variables engine has been transformed and now we have a system similar to modsecurity. There are multiple variable types with different helpers and generic helpers. It allows us to safely handle variables programmatically and transparently read them for rules
↳ Roshan Piyush 12:17:59 UTC
I promise to take this as a priority.
Juan Pablo Tosso 12:15:57 UTC
The only problem is that I have undone @Roshan Piyush fixes for case-sensitive/insensitive names
Juan Pablo Tosso 12:16:21 UTC
I have temporarily disabled tests using these features until fixed
Juan Pablo Tosso 12:18:32 UTC
We are trying to merge the v2 PR that replaces all tests with testify. Thanks @JC for the great effort in migrating ALL tests
Juan Pablo Tosso 12:20:02 UTC
We are also working in persistence, it’s a bit fuzzy yet but I think we have a clear idea on how to achieve it. https://github.com/corazawaf/coraza/tree/v3/dev-persistence
Juan Pablo Tosso 12:20:29 UTC
@fzipitria proposed https://github.com/dgraph-io/badger as the default persistence engine
Juan Pablo Tosso 12:22:42 UTC
There is a function called tx.ProcessRequests. it takes an http.Request object and processes phases from 1 to 3 https://github.com/corazawaf/coraza/blob/v3/dev/http/http.go
Juan Pablo Tosso 12:23:06 UTC
I think this is a very high-level function and it limits creativity from developers
Juan Pablo Tosso 12:23:11 UTC
that’s why I would like to remove it
Juan Pablo Tosso 12:23:35 UTC
Altough it’s a widely used function
Roshan Piyush 12:23:57 UTC
Why not leave it as a wrapper of what we like to implement?
Juan Pablo Tosso 12:24:52 UTC
we could leave it at the examples directory
fzipitria 12:25:47 UTC
IMHO, phases should register themselves
Juan Pablo Tosso 12:26:32 UTC
cool, so everyone agrees on transforming it into an example?
fzipitria 12:27:16 UTC
This should be a CoR pattern
fzipitria 12:27:42 UTC
So anyone can add phases, effectively letting devs adding phases at will
Juan Pablo Tosso 12:28:03 UTC
They can, they are just tempted by the automatic helper
Juan Pablo Tosso 12:29:18 UTC
I’m still not quite convinced we have an agreement on this one, so let’s take this discussion further
Juan Pablo Tosso 12:29:42 UTC
Regarding v3 response body processors
Juan Pablo Tosso 12:30:36 UTC
Now we support request and response body processors
Juan Pablo Tosso 12:30:52 UTC
if an API returns a JSON payload we can read it using ARGS_RESPONSE:json…..
Juan Pablo Tosso 12:31:33 UTC
We have implemented limited XML support with only two xpath expressions for compatibility with coreruleset
fzipitria 12:32:32 UTC
The only question is if we are trusting the answer on the declared content-type
fzipitria 12:32:59 UTC
And what happens when we cannot parse it
Juan Pablo Tosso 12:33:17 UTC
if we cannot parse it we fill RESBODY_ERROR_MSG
Juan Pablo Tosso 12:33:27 UTC
just like REQBODY_ERROR_MSG from request
Juan Pablo Tosso 12:33:49 UTC
also, Coraza by default doesn’t handle response body, a CTL must be used to force it
Juan Pablo Tosso 12:34:19 UTC
behavior is very similar to request bodies
Juan Pablo Tosso 12:34:40 UTC
the only problem is that when we get REQBODY_ERROR_MSG we cannot deny anymore
Juan Pablo Tosso 12:34:45 UTC
as the buffer has probably already been sent
Juan Pablo Tosso 12:35:20 UTC
for the caddy connector it would work, as the body is buffered for analysis
Juan Pablo Tosso 12:35:39 UTC
but we won’t be able to send a new status code, as it was already sent. just a different response body
fzipitria 12:35:48 UTC
Excellent
fzipitria 12:35:50 UTC
MAkes sense
Juan Pablo Tosso 12:37:14 UTC
Ok so I think that’s all regarding v3. Any question?
fzipitria 12:38:26 UTC
Looking good
Juan Pablo Tosso 12:39:39 UTC
@JC has made great efforts for the project, he has a strong interest in WAF and he has proven to be an amazing contributor, so I would like to nominate him for the core team
fzipitria 12:41:53 UTC
Awesome! Welcome @JC!
fzipitria 12:42:20 UTC
With a great power, comes a great responsibility! 😄
Juan Pablo Tosso 12:43:22 UTC
So last topics! I have worked in two features that I would like for V2 and V3
Juan Pablo Tosso 12:44:13 UTC
the @rest operator uses the same syntax as a OpenAPI spec path to match a URL pattern and fill ARGS_PATH. ARGS also includes ARGS_PATH
Juan Pablo Tosso 12:45:10 UTC
`SecRule REQUEST_URI “@rest /api/v1/users/{user_id}” “…msg:‘User id is %{ARGS_PATH.user_id}’”
Juan Pablo Tosso 12:45:24 UTC
Do we want this as a plugin or as a core feature?
fzipitria 12:46:54 UTC
Very good question
Roshan Piyush 12:47:40 UTC
IMO as a plugin would be good, as rest is not closely followed and someone may want to implement their own version of rest as they have been doing.
fzipitria 12:48:04 UTC
Plugins are more flexible probably
fzipitria 12:48:23 UTC
But I was wondering if making it core would help making the difference from modsec
Juan Pablo Tosso 12:48:40 UTC
the only problem with plugins is that we cannot programmatically create variables
Juan Pablo Tosso 12:49:15 UTC
Other security consideration, if we include ARGS_PATH in ARGS, it could be overlapped by ARGS_GET, /api/v1/users/5?user_id=4 would trick coraza
↳ Roshan Piyush 12:56:09 UTC
Ideally we should handle duplicates and not override here.
↳ Juan Pablo Tosso 12:56:31 UTC
In theory coraza would fill user_id: [5,4]
↳ fzipitria 12:57:46 UTC
Yes, parameter pollution handling needs to work properly
fzipitria 12:50:37 UTC
CRS is taking a second look at PATH
fzipitria 12:51:06 UTC
E.g coreruleset/coreruleset#2699
fzipitria 12:51:47 UTC
Just saying that more rules might include that in the near future
Juan Pablo Tosso 12:52:37 UTC
Cool, so let’s vote to bring it to the core or as a plugin: ✅=core 👀=plugin
Juan Pablo Tosso 12:54:52 UTC
Ok, we are keeping it in the loop as we don’t have enough support for the feature
Juan Pablo Tosso 12:55:34 UTC
We are running out of time, any other topics you would like to discuss?
fzipitria 12:56:17 UTC
No, I think we are doing good progress on the v3 release
fzipitria 12:56:46 UTC
And just a reminder for people to use the new testify framework when writing tests
fzipitria 12:56:57 UTC
… and reviewers to be aware of that
Juan Pablo Tosso 12:57:09 UTC
Yes, we should add it to the contributors guidline
fzipitria 12:57:10 UTC
Maybe also changing the PR template would help
Juan Pablo Tosso 12:57:56 UTC
Yes, I will work on that
Juan Pablo Tosso 12:58:17 UTC
Ok everyone, nice meetings, thanks for joining!