Juan Pablo Tosso 12:02:03 UTC
Hello and welcome to our monthly meeting corazawaf/coraza#351
Juan Pablo Tosso 12:04:09 UTC
Let’s begin. It’s been a crazy month, a lot has happened
Juan Pablo Tosso 12:05:13 UTC
we have implemented a new feature, SecDatasets: corazawaf/coraza#361 (
Juan Pablo Tosso 12:05:36 UTC
There is a new debug interface for custom debug corazawaf/coraza#337
Juan Pablo Tosso 12:05:49 UTC
The golang URL processor was replaced corazawaf/coraza#326
Juan Pablo Tosso 12:05:59 UTC
We are back at Core ruleset 100% compatibility
Juan Pablo Tosso 12:06:14 UTC
And now Coraza can be compiled using tinygo
JC 12:06:25 UTC
Yay
Juan Pablo Tosso 12:08:12 UTC
Current benchmark results, I will share the link of the repo at the end of the meeting so you can review the methodology and we can publish it
Juan Pablo Tosso 12:09:52 UTC
Great, any questions or comments?
Juan Pablo Tosso 12:10:07 UTC
Btw congratulations to everyone who has helped to make this happen
Juan Pablo Tosso 12:10:18 UTC
specially our newest contributor, @Anuraag Agrawal
Juan Pablo Tosso 12:11:33 UTC
ok to keep up with the agenda
Juan Pablo Tosso 12:12:02 UTC
@Anuraag Agrawal or @JC, could you give us some insight into how tinygo compatibility was achieved and what the benefits are?
Anuraag Agrawal 12:12:26 UTC
Sure
JC 12:14:37 UTC
You go Rag
Anuraag Agrawal 12:16:18 UTC
For context, TinyGo is a compiler of Go code originally to create lighter weight binaries for microcontrollers. A target for WebAssembly was added because it also benefits from lightweight implementations.
Unfortunately, that means it is for a large part a reimplementation of the standard Go compiler, and features that are specific to compilation continue to get implemented over time. One significant gap vs normal Go is support for the reflect package. And lacking that, encoding/json doesn’t work for most cases.
Being able to compile Coraza with TinyGo would be great though because by compiling to WebAssembly, it can be loaded with Envoy, which supports extensions in wasm. So we went for it 🙂
We replaced some implementations with non-reflection, e.g. JSON parsing uses a library called gjson in TinyGo, but XML is currently unsupported with TinyGo and we’ve flagged the implementation and tests off for TinyGo. Also file system access isn’t supported by Envoy so we flagged it too to be noop when compiling with TinyGo.
It isn’t a perfect reproduction of coraza’s features, but thanks to this, we have been able to load Coraza in Envoy and have a WAF there
Anuraag Agrawal 12:16:53 UTC
reflect will get better support over time for sure, and we can expect gaps to close over time
Anuraag Agrawal 12:18:11 UTC
One thing I am anxious about is if people submit changes to coraza but run into one of TinyGo’s unimplemented features - we are now running CI with TinyGo enabled. If something mysterious comes up, just ping me asap so we can work together as it can get quite tricky
Juan Pablo Tosso 12:18:48 UTC
thank you for the update Anuraag, btw gjson might become our official replacement for encoding/json, seems to be way faster and it fits all of our use cases
Anuraag Agrawal 12:18:49 UTC
EOF - happy to answer questions 🙂
Juan Pablo Tosso 12:20:07 UTC
Regarding future regression, I think we should be clear on our contributors.md about tinygo. We can always mark new features as “experimental” using build constrains so everything can be tested. Please open discussion for this
Juan Pablo Tosso 12:20:46 UTC
Ok so now we are done with the past, and lets discuss the future
Juan Pablo Tosso 12:21:30 UTC
If we have a limitation to read files, then we cannot use pmFromFile or other important operators used by CRS
Juan Pablo Tosso 12:21:38 UTC
That’s why we came with SecDataset: corazawaf/coraza#361
Juan Pablo Tosso 12:22:04 UTC
SecDatasets allows us to create inline datasets without having to create .data files. it’s compatible with all file consuming operators
Juan Pablo Tosso 12:23:15 UTC
It might be a great feature and I would like to read if there are any comments. Could we make this better?
Juan Pablo Tosso 12:25:16 UTC
The other issue regarding pmfromfile is Snort suricata syntax support
Juan Pablo Tosso 12:25:27 UTC
It is an interesting modsecurity feature but we haven’t had any request to implement it
Juan Pablo Tosso 12:25:45 UTC
maybe it’s not that important? Maybe we should implement it in other operator?
fzipitria 12:25:53 UTC
Files are shared in a common memory area, right?
fzipitria 12:26:03 UTC
Data I meant
Juan Pablo Tosso 12:26:23 UTC
data is stored into a map of slices in the bootstrap configuration
fzipitria 12:26:41 UTC
About the suricata syntax, I don’t know if adds too much in our case
fzipitria 12:26:55 UTC
It is a plus, but the CRS doesn’t use it per se
fzipitria 12:27:45 UTC
Unless there is a clear win from implementing it, we can push it to the backlog
Juan Pablo Tosso 12:28:01 UTC
great, we will keep avoiding suricata syntax, feel free to create an issue or comment here if you think there are reasons to implement it
Anuraag Agrawal 12:28:41 UTC
We are back at Core ruleset 100% compatibilityI think this tends to be an important feature when evaluating a change in tech. If suricata isn’t important for it, then it’s probably not a big deal
Juan Pablo Tosso 12:29:03 UTC
It’s your time @fzipitria: Rate limiting: new operator, new action, or new specific rule?
fzipitria 12:29:16 UTC
Yes
fzipitria 12:29:31 UTC
All the discussion in the ticket has been regarding the implementation details
fzipitria 12:29:44 UTC
Which honestly, I don’t is relevant yet
fzipitria 12:30:05 UTC
This is what we have as requirements in the issue:
the url to protect
the rate limit per second/minute/etc.
the action when the limit is reached:
return http code 429
send it to a tarpit
also, when to clear the rate limit
I’m rethinking if we want to have:
fzipitria 12:30:39 UTC
Also, there is some advanced rate limiting that can make our life easier in the future
fzipitria 12:30:53 UTC
For distributed attacks, for example
fzipitria 12:31:13 UTC
So, the classic base RL is about IPs
fzipitria 12:31:32 UTC
Whatever URL some IP is hitting, we do the RL in that IP
fzipitria 12:31:54 UTC
But for distributed brute forcing, for example, that technique doesn’t apply well
fzipitria 12:32:29 UTC
So we might want to have something different as counting expression for tracking what to block
fzipitria 12:33:20 UTC
Could be a parameter (e.g. login?username=, or some header (Authorization: Bearer ...)
fzipitria 12:33:56 UTC
So the question. is if the design using a SecRule is enough
fzipitria 12:34:27 UTC
Or we leverage the new SecRateLimit with extended options, that can give us more freedom afterwards
fzipitria 12:34:40 UTC
I see both working good
fzipitria 12:34:57 UTC
But for growing, probably the second option with a new directive is better
fzipitria 12:35:29 UTC
Or we start simply with a ratelimit action for SecRule
Juan Pablo Tosso 12:36:04 UTC
I think there is no problem with Directives being SecRule aliases, so maybe we can cover both
Juan Pablo Tosso 12:37:10 UTC
But ratelimit as a disruptive action looks interesting
Juan Pablo Tosso 12:37:26 UTC
in the future we might integrate it with the persistence engine to create variables
fzipitria 12:37:41 UTC
Definitely it should be integrated with persistence
fzipitria 12:37:53 UTC
If we want complex cases to be covered
fzipitria 12:38:21 UTC
But also because we might end up with a distributed coraza deployment
fzipitria 12:38:45 UTC
And we want to rate limit horizontally across the cluster
Juan Pablo Tosso 12:39:07 UTC
exactly
Juan Pablo Tosso 12:39:18 UTC
in the meantime single node blocking should work
Anuraag Agrawal 12:40:33 UTC
Just for context, more parameters does make it tough. e.g., if accepting username then I would expect that all requests to that username are handled by the same rule. This might not be commonly achievable
Anuraag Agrawal 12:41:06 UTC
But more general global rate limits without state would be easy
fzipitria 12:41:43 UTC
You need state for RL
fzipitria 12:41:58 UTC
I mean, it can be in memory
fzipitria 12:42:18 UTC
You cannot RL stateless
Anuraag Agrawal 12:43:17 UTC
Ah sorry I meant global state. Rate limiting without global/shared state is important, but single-node can still have some limitations, e.g. it can’t implement a SAS API rate limit usually
Anuraag Agrawal 12:43:25 UTC
So just need to be clear about that
Juan Pablo Tosso 12:44:13 UTC
great, I think this is great progress on the discussion Felipe could we move this to a github discussion?
JC 12:45:26 UTC
I would like to propose Rag (anuraaga) to be a core team member due to his great contributions in v3 and also as one who is devoted to take coraza wasm to reality.
fzipitria 12:47:05 UTC
I think is it a good idea, and he is very welcomed as core team member!
fzipitria 12:48:03 UTC
Just to be transparent with all members on this channel, we welcome all of you to be part of the core team.
fzipitria 12:49:09 UTC
We did setup our rules in our first meeting 🙂
fzipitria 12:50:17 UTC
The only thing we ask our contributors is to ask for additional reviews when there is people working in the same company, or working together in PRs.
fzipitria 12:50:47 UTC
The more eyes we have, the stronger our project is going to be
Juan Pablo Tosso 12:51:18 UTC
Thank you @fzipitria for the explanation
fzipitria 12:51:24 UTC
🙇
Juan Pablo Tosso 12:51:40 UTC
@JC, we should wait for @Roshan Piyush or @bxlxx.wu for the final vote
Juan Pablo Tosso 12:51:46 UTC
but you have my vote and @fzipitria
Anuraag Agrawal 12:52:43 UTC
We did setup our rules in our first meeting 🙂Are these written on GitHub anywhere? Would be great to refer to, for example at these times. If not, yet we could possibly create corazawaf/community repo for collecting community guidelines
Juan Pablo Tosso 12:53:18 UTC
Anuraag Agrawal 12:53:48 UTC
Thanks for the reference!
Juan Pablo Tosso 12:54:29 UTC
Ok so we are moving to the next topics
Juan Pablo Tosso 12:54:51 UTC
we will go fast as I only have a few minutes
Juan Pablo Tosso 12:54:57 UTC
Juan Pablo Tosso 12:55:52 UTC
Although I think it would make life of many people easier, I think users should still download the package and setup it up by themselves. Otherwise they would miss a lot of documentation and requirements. Also it will be hard to maintain. This is my humble opinion
Anuraag Agrawal 12:57:23 UTC
I think we would decide by what we expect users to fall into
If 1., we would generally expect them to get very familiar to coraza / modsec. If 2., a simpler experience will help them I think
Anuraag Agrawal 12:58:23 UTC
If we decide to focus on 1., then I agree that it’s good to absorb the concept of rules, etc. But I have also heard that WAF is becoming more important for compliance reasons, i.e. in US some standards will require it. So if we expect apps to add it for compliance reason, an easy entry could help them
fzipitria 12:58:26 UTC
I think this might be a part of coraza + caddy, if anything
Juan Pablo Tosso 12:58:33 UTC
Complex features like this could be easily implemented using plugins. We can merge plugins into the core if they are ok
Anuraag Agrawal 12:58:52 UTC
But I do tend to agree that embedding not in coraza, but in the proxy integration layers, should satisfy almost if not all users
fzipitria 12:59:20 UTC
Basic users just want caddy + coraza
fzipitria 12:59:28 UTC
Enterprise users want a strong API
Juan Pablo Tosso 12:59:44 UTC
exactly
Juan Pablo Tosso 13:00:04 UTC
Let’s move this discussion to the issue
Juan Pablo Tosso 13:00:50 UTC
Juan Pablo Tosso 13:01:11 UTC
it is a major change, it will break some code but I think v3 is the moment to do it
Anuraag Agrawal 13:02:14 UTC
For context, I think my recent corazawaf/coraza#375 breaks everyone anyways so collating them will not be a huge delta, probably
Juan Pablo Tosso 13:02:28 UTC
yes, but it had to happen at some point
Juan Pablo Tosso 13:02:56 UTC
Ok, next issue is corazawaf/coraza#374
Juan Pablo Tosso 13:03:12 UTC
I agree with this, we should start moving all types to specific packages inside the types/ directory
Juan Pablo Tosso 13:03:29 UTC
but not just from types
Juan Pablo Tosso 13:03:41 UTC
we should try to remove most dmetadata from the core package and move it to a type
Juan Pablo Tosso 13:03:47 UTC
to avoid circular dependencies
↳ fzipitria 13:04:02 UTC
🙏
Juan Pablo Tosso 13:04:16 UTC
Juan Pablo Tosso 13:04:22 UTC
this one should become a long discussion
Juan Pablo Tosso 13:04:32 UTC
although I like it, I think it breaks A LOT and it requires a major refactor
Juan Pablo Tosso 13:06:07 UTC
Ok everyone, thank you very much for joining
Juan Pablo Tosso 13:06:17 UTC
please help me write comments in the monthly meeting issue to keep track
Juan Pablo Tosso 13:06:30 UTC
Also please have a look at corazawaf/coraza#336
Juan Pablo Tosso 13:06:58 UTC
Also @fzipitria created a calendar event: https://calendar.google.com/calendar/ical/c_s1vf6auene1ehionfslv010tk8%40group.calendar.google.com/public/basic.ics
Anuraag Agrawal 13:09:21 UTC
Thanks @Juan Pablo Tosso - if going with corazawaf/coraza#371 I think the schedule on corazawaf/coraza#336 is impossible, so we would probably need a go or no go for that. Should we set a deadline on a decision while debating on corazawaf/coraza#371?
Anuraag Agrawal 13:09:41 UTC
(impossible=should require one extra month IMO)
Juan Pablo Tosso 13:11:39 UTC
I agree, feel free to propose new dates
Anuraag Agrawal 13:12:22 UTC
Thanks everyone, see you on GitHub 🙂
fzipitria 13:18:27 UTC
👋 See you next one!