Skip to content
This repository was archived by the owner on May 14, 2020. It is now read-only.

Conversation

@fgsch
Copy link
Contributor

@fgsch fgsch commented Mar 18, 2019

Related to #1121.

@dune73
Copy link
Contributor

dune73 commented Mar 18, 2019

Wow, this is impressive.

tx.msg in referenced in the 980xxx rules. Would you mind erasing it there as well?

I'm all for merging this.

@csanders-git
Copy link
Contributor

w00t I agree this will simplify rules greatly

@fgsch
Copy link
Contributor Author

fgsch commented Mar 18, 2019

@dune73 will do and update this PR.

@dune73
Copy link
Contributor

dune73 commented Mar 18, 2019

Appreciated. Thanks.

Also appreciate that this very useful PR carries the number 1327. 1327 is the year in which Umberto Eco's "Name of the Rose" happens. #FunFacts

@fgsch fgsch force-pushed the fgsch/remove-old-construct branch from a5a75e4 to b183530 Compare March 19, 2019 01:02
@fgsch
Copy link
Contributor Author

fgsch commented Mar 19, 2019

Updated.

@emphazer
Copy link
Contributor

@fgsch i wasn't able to find any issues after some tests.
good PR, thx.

@ghost
Copy link

ghost commented Mar 29, 2019 via email

@fgsch fgsch force-pushed the fgsch/remove-old-construct branch from b183530 to ba2c43e Compare March 29, 2019 15:51
@fgsch
Copy link
Contributor Author

fgsch commented Mar 29, 2019

Updated to cover latest changes in master and also the 2 scripts referencing these constructors.

@lifeforms
Copy link
Contributor

We like this PR, but the tx.%{rule.id} construct is used in one particular place, the RESPONSE-981-DEBUG.conf file. It spits out these variables for debugging. So this would break that old debug hack. But the debug hack is not widely known, and maybe nobody (except me) uses it. So then, we could clean up the technical debt.

Decision: I'll ask on the mailinglist if anyone is depending on that debug mode, if so then we do this merge. We will get back to it.

Then the file RESPONSE-981-DEBUG.conf can also be deleted from the repo.

@fgsch fgsch force-pushed the fgsch/remove-old-construct branch from ba2c43e to b6e6925 Compare April 7, 2019 11:28
@dune73
Copy link
Contributor

dune73 commented May 5, 2019

So what do we do @lifeforms?

@lifeforms
Copy link
Contributor

lifeforms commented May 5, 2019

Nobody cared about it (edit: the tx.{%ruleid} construct), let's delete it 👍

@na1ex
Copy link
Contributor

na1ex commented May 6, 2019

Nobody cared about it, let's delete it 👍

Maybe too late : I have some tooling to replay auditlog that makes use of it for fast (>200 test/sec) non-regression testing.

@fgsch
Copy link
Contributor Author

fgsch commented May 6, 2019

@na1ex is it using tx.%{rule.id}.., tx.msg, or both?

@na1ex
Copy link
Contributor

na1ex commented May 6, 2019

@na1ex is it using tx.%{rule.id}.., tx.msg, or both?

Tool uses tx.%{rule.id} only ( each matching rule-id is added in a single response header for check. )

@lifeforms
Copy link
Contributor

lifeforms commented May 6, 2019

Tool uses tx.%{rule.id} only ( each matching rule-id is added in a single response header for check. )

RESPONSE-981-DEBUG.conf uses this mechanism too. Sadly it will break when we remove this variable. As you say, it can make for very fast testing. However, not many people were using this feature (in fact you are the first I know!). That said, it's a tradeoff, but we had to choose, and we decided to remove these variables and make our rule writing easier for newbies.

@na1ex
Copy link
Contributor

na1ex commented May 6, 2019

Trade-off understood, just wanted to mention the use.
Note : these are magical rules gone to a treasure drawer to be re-discovered (someday :o).

@dune73
Copy link
Contributor

dune73 commented May 7, 2019

Anything else before we merge this PR?

@lifeforms
Copy link
Contributor

LGTM!

@dune73
Copy link
Contributor

dune73 commented May 7, 2019

Well, then let's get rolling!

Thank for this swift PR @fgs. This is huge.

@dune73 dune73 merged commit f70268a into SpiderLabs:v3.2/dev May 7, 2019
@lifeforms
Copy link
Contributor

Great work @fgsch !

@fgsch fgsch deleted the fgsch/remove-old-construct branch May 8, 2019 22:43
fgsch added a commit to fgsch/coreruleset-old that referenced this pull request May 9, 2019
csanders-git pushed a commit that referenced this pull request May 9, 2019
csanders-git pushed a commit to csanders-git/owasp-modsecurity-crs that referenced this pull request May 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants