Matteo Pace 12:55:41 UTC
Are there some ways to make it more an automatic experience (or at least more coupled with the PRs)?
Juan Pablo Tosso 12:56:27 UTC
yes we could do more automation, for example, we could create documentation right from the comments
↳ unknown user 13:35:01 UTC
Yeah I think this is what we should do, otherwise maintaining the docs will be a nightmare.
Juan Pablo Tosso 12:56:46 UTC
that would be a nice project
Juan Pablo Tosso 12:57:57 UTC
Ok no more comments around this I think?
Juan Pablo Tosso 12:57:59 UTC
Juan Pablo Tosso 12:58:21 UTC
Added by @Matteo Pace
Matteo Pace 12:59:29 UTC
About this, just wanted to agree on the log levels before updating the PR. Right now The v3 implementation, documentation and coraza recommended.conf are not aligned
Matteo Pace 13:00:10 UTC
The PR proposes to reorder the levels to: trace -> debug -> info -> warning -> error
Matteo Pace 13:01:29 UTC
The main question is, do we want to keep compatibility with ModSecurity? Adding some levels that we are currently not using (no log and fatal if I am not wrong) and sticking with the number correspondence (even if it is weird because it skips some numbers?)
Juan Pablo Tosso 13:02:17 UTC
it looks good to me
Juan Pablo Tosso 13:02:20 UTC
we should update docs
fzipitria 13:02:31 UTC
Updating the docs is a reasonable path
Juan Pablo Tosso 13:02:45 UTC
yes we should avoid fatal as we should not os.Exit(..) nor panic from coraza
fzipitria 13:02:57 UTC
Definitely
Matteo Pace 13:04:09 UTC
Good! I can take another look, but I think that the PR is pretty aligned to what you are saying
Juan Pablo Tosso 13:04:42 UTC
maybe maintain modsec standard for regression ?
Juan Pablo Tosso 13:04:45 UTC
0: Fatal 1: Panic 2: Error 3: Warning 4: details of how transactions are handled 5: log everything
Juan Pablo Tosso 13:04:53 UTC
we point fatal and panic to Error
Juan Pablo Tosso 13:05:16 UTC
0-3: Error, 3 warning, …
↳ Matteo Pace 13:07:35 UTC
Ok, makes sense!
Juan Pablo Tosso 13:06:47 UTC
Ok I have to fix my summer time event 😅 sorry for the delay
Juan Pablo Tosso 13:07:03 UTC
If you have any additional topic to discuss, all discussions are still open here! feel free to contribute
Juan Pablo Tosso 13:07:05 UTC
Thank you everyone
Matteo Pace 13:09:41 UTC
About corazawaf/coraza#572, are we happy with at least a fallback that takes the server name from the headers? Or could you elaborate a little about the additional helper? @Juan Pablo Tosso
Matteo Pace 13:10:36 UTC
I wish to provide an initial fix to it considering that SERVER_NAME is currently never working right now
Juan Pablo Tosso 13:32:10 UTC
That’s a big one… modsecurity relies on the headers
Juan Pablo Tosso 13:32:43 UTC
Maybe we could add a SetServerName helper to transactions, it would make sense as reverse proxies hides the host name
↳ unknown user 13:35:54 UTC
Sounds good to me.
↳ Juan Pablo Tosso 13:39:34 UTC
Maybe it would make more sense processServerName? To follow naming convention @Matteo Pace
↳ Matteo Pace 13:41:46 UTC
Yep, got the point :)
↳ unknown user 14:09:30 UTC
Quick question: is this the name of the own server? Or the client? If so, maybe instead of letting the customer setting the server we want to pass a function to extract the server from headers on the waf itself.
↳ unknown user 14:09:48 UTC
My main concern is the mutability
↳ Juan Pablo Tosso 14:09:58 UTC
That’s the problem, you can’t use the server name
↳ Juan Pablo Tosso 14:10:03 UTC
It could be a virtual host
↳ unknown user 14:10:21 UTC
Like we pass that function when building the waf and then on every transaction before we process phase 1 we set the server name from headers.
↳ Juan Pablo Tosso 14:10:24 UTC
And golang http reverse proxy library removes the host header
↳ Juan Pablo Tosso 14:10:38 UTC
Also caddy
↳ unknown user 14:10:58 UTC
Ok, so my question would be, can we do this at waf level instead of transaction level?
↳ Juan Pablo Tosso 14:12:05 UTC
Wouldn’t work for cases like spoa
↳ Juan Pablo Tosso 14:12:13 UTC
A single waf instance might feed multiple apps
↳ Juan Pablo Tosso 14:12:36 UTC
It’s a transaction thing
↳ unknown user 14:23:32 UTC
But you can set on all wags.
↳ unknown user 14:23:35 UTC
*wafs
↳ unknown user 14:24:05 UTC
My main point is that lets not make the transaction mutablr.
↳ unknown user 14:24:36 UTC
If we set a callback on the waf that will be applied "when logging" then we have the info and still don't mutate the transaction.
↳ unknown user 14:24:43 UTC
Let me scratch a Pr
↳ unknown user 14:25:13 UTC
Because this is about logging right?
↳ Matteo Pace 14:26:06 UTC
It just not only about logging, rules may try to match the SERVER_NAME variable, that currently is never populated
↳ unknown user 14:26:55 UTC
Ok, still that can happen after adding the headers.
↳ Matteo Pace 14:27:03 UTC
So it is something that should be populated at phase 1
↳ unknown user 14:27:34 UTC
If we do wafconfig.WithServerExtractor(func(headers) string)
↳ Juan Pablo Tosso 14:28:07 UTC
but that should be the default behavior
↳ Juan Pablo Tosso 14:28:17 UTC
unless we process the server name
↳ Juan Pablo Tosso 14:29:23 UTC
AddRequestHeaders looks for the host
↳ unknown user 14:36:46 UTC
Forget about my concerns. Tx is mutable already. Let's just add the method.
Matteo Pace 13:38:17 UTC
Great, thanks. I will update the PR following this approach. Retrieving the server name from the headers can be something delegated to the connectors before calling the setter
Juan Pablo Tosso 14:38:56 UTC
should we start tagging nightlies for the unstable v3/dev branch? There are a lot of people using v3, and it would be easier to handle breaking changes (We only tag if there was a merged PR)