Owasp-modsecurity-crs: Rule Cleanup

[dune73]

There are now several issues and even PRs that deal with a cleanup of rules. This issue is meant to keep all these tasks together by discussing them centrally and by governing the label "Rule Cleanup Project".

In focus are:

• Ordering of actions
• Indentation
• Clean whitespace
• Remove maturity action
• Remove accuracy action

@fgsch has volunteered to carry this project.

[csanders-git]

So what are we saying are the best practices? should we start a wiki page for that?

[dune73]

Creating a wiki page would probably make sense. Here on the fly the order that I used in the book:

phase
disruptive-action
status
capture
t:xxx
log
nolog
auditlog
noauditlog
msg
logdata
tag
sanitiseArg
sanitiseRequestHeader
sanitiseMatched
sanitiseMatchedBytes
ctl
setenv
setvar
chain
skip
skipAfter

What I think is essential is starting with id and phase and disruptive action. Other items are mostly optional and do not matter much for CRS. I have logfile definitions early as are the transformations. Msg and logdata are coming only afterwards. Chain is listed near the end to make a visible link to the chained rule that has indentation.

Of course the list above is only proposal. I think it has some merits, but whatever the reasonable order is that we want to apply, we need to make sure it is applied consistenly and that we check it automatically. @fgsch also speaks of lint.

[dune73]

I have not heard any feedback on the proposal above. I have transferred it into a wiki page: https://github.com/SpiderLabs/owasp-modsecurity-crs/wiki/Order-of-ModSecurity-Actions-in-CRS-rules

If there is no comment in the next few days, I assume the proposal has support and @fgsch is good to go.

[fgsch]

It's been 20 days since @dune73 comment.
I assume it's OK and I will proceed as per comment above.

[fzipi]

In addition to your proposal, I would like to propose a standard for indenting rules.

I noticed these inconsistencies trying to commit the ruleset to my own git. It began complaining in lots of places about tabs and spaces mix, and others.

Examples of actual inconsistency:
REQUEST-913-SCANNER-DETECTION.conf (33-34):

SecRule REQUEST_HEADERS:User-Agent "@pmFromFile scanners-user-agents.data" \
 "msg:'Found User-Agent associated with security scanner',\

In this case there are two spaces at the beggining of the second line. But in ./REQUEST-921-PROTOCOL-ATTACK.conf (line 36):

SecRule REQUEST_HEADERS:'/(Content-Length|Transfer-Encoding)/' "," \
        "msg:'HTTP Request Smuggling Attack.',\
        phase:request,\
        id:921100,\
        rev:'2',\

It is indented using tabs.

Anothe in REQUEST-920-PROTOCOL-ENFORCEMENT.conf (337-359):

SecRule REQUEST_HEADERS:Range|REQUEST_HEADERS:Request-Range "(\d+)\-(\d+)\," \
  "capture,\
   phase:request,\
   rev:'2',\
   ver:'OWASP_CRS/3.0.0',\
   maturity:'6',\
   accuracy:'8',\
   t:none,\
   block,\
   msg:'Range: Invalid Last Byte Value.',\
   logdata:'%{matched_var}',\
   severity:'WARNING',\
   id:920190,\
   tag:'application-multi',\
   tag:'language-multi',\
   tag:'platform-multi',\
   tag:'attack-protocol',\
   tag:'OWASP_CRS/PROTOCOL_VIOLATION/INVALID_HREQ',\
   chain"
        SecRule TX:2 "!@ge %{tx.1}" \
                "setvar:'tx.msg=%{rule.msg}',\
                setvar:tx.anomaly_score=+%{tx.warning_anomaly_score},\
                setvar:tx.%{rule.id}-OWASP_CRS/PROTOCOL_VIOLATION/INVALID_HREQ-%{matched_var_name}=%{matched_var}"

This case has a chained SecRule, and indentation begins at a different level, and below setvars are indented using tabs.

This are only a small part of the inconsistencies.

I don't wan't to start a 'tabs vs. spaces', I'm only concerned about consistency. I prefer tabs for this case, but ymmv.

So, in summary:

1. Use tabs for indentation. This could be aided by uploading an '.editorconfig' with simple rules.
2. Whenever there is a continuation at the end ('\'), the next line will be indented using tab.
3. If there is a rule with a chain, the next chained rule begins on the next line with the same indenting, and if there is another continuation, indent again using tab.
4. Align with commas at the begin of the rule, don't add a space. Just tabs.

Example:

SecRule &REQUEST_HEADERS:Host "@eq 0" \
        "msg:'Request Missing a Host Header',\
        severity:'WARNING',\
        ...\
        chain"
        SecRule REQUEST_METHOD "!^OPTIONS$" \
                "chain"
                SecRule REQUEST_HEADERS:User-Agent "SomeStuff" \
                        "t:none,\
                        setvar:'tx.msg=%{rule.msg}',\
                        setvar:tx.anomaly_score=+%{tx.notice_anomaly_score},\
                        setvar:tx.%{rule.id}-OWASP_CRS/PROTOCOL_VIOLATION/MISSING_HEADER-%{matched_var_name}=%{matched_var}"

Let me know what you think. It isn't written in stone :simple_smile:

[fgsch]

This was already mentioned in the first comment.
Personally I'd prefer spaces to keep the width limited to (or close to) 80 cols but I don't mind either way. Changing this one way or another should be rather trivial.

[fzipi]

I was trying just to put it down some criteria which could be used afterwards on a CONTRIBUTING file, or something like that.

Spaces is fine for me also, btw! 4?

[fzipi]

So, I created #859 (dev branch), let me know what you think or if there is a need to change something.

[csanders-git]

we can add tests to the travis-ci tests to enforce these standards as well once we solidify. I think this feedback is great. I like spaces personally but don't feel strongly one way or the other.

[fgsch]

I like 4 spaces but I'd like to see a 4 space indentation for each SecRule in the chain. i.e.

SecRule .. ..
   "...."
   SecRule .. ..
       "..."
       SecRule .. ..
       ..

Am I the only one?

[fzipi]

I concur. I updated my patch.

Another thing I've noticed is that when there is a only one action there is no "" surrounding it. For example whenever there is a chain alone. e.g:

SecRule .... \
    chain
    SecRule
        ...

It is better visually for me if this is used as "chain" (coloring helps) also, e.g:

SecRule ..... \
    "chain"

So I fixed that also.

[fgsch]

Cool. There are a few places were there is 8 spaces (or one tab, have not checked) instead of 4.
I've left come comments on the PR.

[spartantri]

cool, this should be part of a full document detiling all the rule writing cosmetics 👍

[fzipi]

I've made all the changes discussed about the whitespacing, so maybe #859 can be merged now. To document this, should we create a wiki page or a CONTRIBUTING.md file (or both)?

[csanders-git]

We should create a contributing file first and then link that to any PRs :-). If it gets complicated enough we can make a wiki article.

[dune73]

I've covered this effort in the September CRS newsletter (-> https://coreruleset.org/20170907/crs-project-news-september-2017/). We support your decision to go with spaces instead of tabs and generally want to make sure you do not hit a roadblock with your effort. The fact that we do not contribute much here, does not mean that we do not care.

@emphazer volunteered to help you out if you need anything from the core project. I'm assigning him. But that does not mean he'll do the work. It's more to make sure he receives notifications from this issue.

Please keep up the good work!

[fzipi]

@fgsch Are you ok if I go for:

• Remove maturity action

• Remove accuracy action

? Then we can focus on reordering last.

[lifeforms]

+1 on removing those!

[fgsch]

@fzipi ofc, please go ahead!

[fzipi]

878 removes maturity & accuracy for v3.1/dev.

[fgsch]

Another thing I'd like to address is the usage of @rx - some rules use it, some don't thus using implicitly.

Can we reach some consensus and try to unify its usage?

[dune73]

I prefer explicit rx. It does not hurt us and it's easier for newbies.

P.S. There is a guy with the github handle @ + rx. He appreciates if we do not use @ + rx too often. :)

[fgsch]

I prefer explicit rx. It does not hurt us and it's easier for newbies.

Do I take it'd be a good exercise to add it where needed?

P.S. There is a guy with the github handle @ + rx. He appreciates if we do not use @ + rx too often. :)

Oops!

[dune73]

Depends on the goal of the exercise. If the goal is the development of a one-liner, then it's definitely a good exercise.

[fgsch]

I meant if we should do a sweep and added where needed as part of this issue.

[dune73]

Yes, I think it belongs into this issue. The status is unclean / inconsistent now and you are proposing an improvement. That is certainly welcome. If we go with explicit or implicit needs a bit more thought / opinions, but I personally prefer explicit.

[fzipi]

I agree on adding @ + rx explicitly. Will try later this week to fix that.

[fgsch]

@fzipi I was planning on working on that myself when/iff consensus was reached :)

[fzipi]

@fgsch Please go ahead! I will begin then with the actions reordering stuff 🙂

[fzipi]

Finally, reordering is in PR #901.

[fgsch]

This is almost completed AFAICT.

I can see a few rules missing and forgot to cover pass. Any comments on the latter?

[dune73]

I am not sure I am following you. What's the problem with "pass". Please give an example rule id to make it clear.

[fgsch]

The pass action order was not defined in the list above and as such it's sometimes after phase, others after nolog, etc.

Where should it stand considering the order you mentioned in https://github.com/SpiderLabs/owasp-modsecurity-crs/issues/808#issuecomment-308061656 ?

[dune73]

Hmm. I took it for granted that disruptive action would include pass. However it did not occur to me that it has been overlooked.

[fzipi]

I think I understood. The one liners. We didn't fix the one liners yet. The idea was to merge first the big reordering, and we can manage the one liners as a separated PR.

BTW, the 'pass' is a disruptive action.

[fgsch]

Ah. Duh! I will get that sorted as well.

[fzipi]

One of the thing that I think we need to discuss is that if that one-liners should be transformed to lines with continuations or not.

For the visual effect, I think that they should stay as they are now (but of course, ordered). What do you think?

[fgsch]

I think one liners for the rules checking the paranoia level is fine and in fact that's what I'm doing (keeping it as a single line) wrt pass.

[fzipi]

One more thing that I found is that 'setvar', 'severity', and others, sometimes have quoted strings and sometimes not.

Severity hasn't got single quotes in REQUEST-949-BLOCKING-EVALUATION.conf.
Setvar is mixed in many places e.g:

setvar:tx.anomaly_score=+%{tx.critical_anomaly_score},\
setvar:'tx.msg=%{rule.msg}',\

I think we should always quote them.

[fgsch]

I think we should always quote them.

That's my opinion as well.
Comments?

[dune73]

I do not like these quotes from an aesthetic viewpoint , but I think you are right.

But maybe somebody has better arguments one way or the other.

[fzipi]

I do not like quoting at all either, but I suppose that some directives could have spaces (e.g: msg) and that may be the reason that quotes began to appear.

Reading the mod_security code, it seems that parameters are automatically quoted if there are spaces (in the sense of isspace())in the params (see file re.c line 673 in master from modsecurity v2).

if (isspace(action->param[j])) {
                    use_quotes = 1;
                    break;
                }

Also, for integer fields like 'rev' it doesn't make any sense to have quoting, but I suppose its easer to quote them all than to say "quote them unless they are integers or don't have spaces".

So now I am having doubts on adding quotes everywhere. Maybe it just make sense to have quoting on msg which 99% of the cases will have spaces on it.

[fgsch]

As a developer, I'd say that numbers should not be quoted, strings (with or without spaces) should.
This makes it easier to decide when to apply them. YMMV.

[spartantri]

As you are standardizing stuff, severity can use words and numbers, I prefer words having a 0 in the log for me is not appealing as URGENT, in fact make me think of something not important that should be totally ignored.

[dune73]

If the paranoia one-liners are not yet settled: I think they should remain one-liners. It's a repetition of the same thing over and over and things at that should eat as little room as possible.

[dune73]

As for the quoting, I think we do not need to set up a general ModSec convention, but simply cover what we are using in CRS right now. Severity is a string now, phase is a number. And while seeing @spartantri's point with the severity, I think we should stick to the string.

[fzipi]

I am proposing then (per https://github.com/SpiderLabs/owasp-modsecurity-crs/issues/808#issuecomment-333598302):

Strings should be quoted, and numbers not.

Therefore, we need to change:

• 'id' unquoted number (just three)

    "id:'9001180',\
    "id:'9001182',\
    "id:'9001184',\

• 'rev' unquoted number (this need a massive substitution, today revision is quoted in CRS)

     53     rev:'1',\
     97     rev:'2',\
     29     rev:'3',\
      7     rev:'4',\
      1     rev:'5',\

• 'severity' is a quoted string (only 8 without quotes, and one that passed various eyes)

      8     severity:CRITICAL,\
      1     tag:'PCI/6.5.6',severity:'ERROR',\

• 'setvar', 'expirevar' and 'tag' are quoted strings

tags are well quoted, but the others need quoting.

     1  "setvar:global.alerted_970018_iisDefLoc,\
      2  setvar:ip.dos_block=1,\
      1  setvar:ip.dos_block_counter=0,\
      1  "setvar:ip.dos_block_counter=+1,\
      1  setvar:ip.dos_block_counter=+1"
      1  setvar:ip.dos_block_flag=1,\
      1  "setvar:ip.dos_burst_counter=1,\
      1  "setvar:ip.dos_burst_counter=2,\
      2  setvar:!ip.dos_counter,\
      1  "setvar:ip.dos_counter=+1"
      5  setvar:ip.previous_rbl_check=1,\
     11  setvar:ip.reput_block_flag=1,\
      1  setvar:tx.anomaly_score=0,\
      1  "setvar:tx.anomaly_score=+%{tx.critical_anomaly_score},\
    154  setvar:tx.anomaly_score=+%{tx.critical_anomaly_score},\
     10  setvar:tx.anomaly_score=+%{tx.error_anomaly_score},\
      1  setvar:tx.anomaly_score=+%{tx.error_anomaly_score}"
      1  setvar:tx.anomaly_score=+%{tx.error_anomaly_score},setvar:tx.%{rule.id}-OWASP_CRS/LEAKAGE/ERRORS-%{matched_var_name}=%{tx.0}"
      8  setvar:tx.anomaly_score=+%{tx.notice_anomaly_score},\
     19  setvar:tx.anomaly_score=+%{tx.warning_anomaly_score},\
      1  setvar:tx.critical_anomaly_score=5"
      1  setvar:tx.do_reput_block=0"
      1  setvar:tx.dos_block_counter=%{ip.dos_block_counter},\
      1  setvar:tx.error_anomaly_score=4"
      1  setvar:tx.extension=.%{tx.1}/,\
      1  setvar:tx.extension=/%{TX.1}/,\
      1  setvar:tx.httpbl_msg=%{tx.0},\
      1  setvar:tx.httpbl_msg=%{tx.1}"
      1  setvar:tx.http_violation_score=0,\
      1  setvar:tx.http_violation_score=+%{tx.critical_anomaly_score},\
      1  setvar:tx.inbound_anomaly_score=0,\
      1  setvar:tx.inbound_anomaly_score_threshold=5"
      2  setvar:tx.inbound_anomaly_score=%{tx.anomaly_score}"
      1  "setvar:tx.inbound_tx_msg=%{tx.msg},\
      1  setvar:tx.inbound_tx_msg=%{tx.msg},\
      1  setvar:tx.lfi_score=0,\
      4  setvar:tx.lfi_score=+%{tx.critical_anomaly_score},\
      1  setvar:tx.msg=%{rule.msg},\
      1  setvar:tx.notice_anomaly_score=2"
      1  setvar:tx.outbound_anomaly_score=0,\
      1  setvar:tx.outbound_anomaly_score_threshold=4"
     16  setvar:tx.outbound_anomaly_score=+%{tx.critical_anomaly_score},\
     11  setvar:tx.outbound_anomaly_score=+%{tx.error_anomaly_score},\
      1  setvar:tx.paranoia_level=1"
      1  setvar:tx.php_injection_score=0,\
     13  setvar:tx.php_injection_score=+%{tx.critical_anomaly_score},\
      1  setvar:tx.rce_score=0,\
     12  setvar:tx.rce_score=+%{tx.critical_anomaly_score},\
      1  setvar:tx.real_ip=%{remote_addr}"
      1  setvar:tx.reput_block_duration=300"
      1  setvar:tx.rfi_score=0,\
      4  setvar:tx.rfi_score=+%{tx.critical_anomaly_score},\
      6  setvar:tx.%{rule.id}-AUTOMATION/MALICIOUS-%{matched_var_name}=%{matched_var},\
      1  setvar:tx.%{rule.id}-AUTOMATION/MALICIOUS-%{matched_var_name}=%{matched_var}"
      2  setvar:tx.%{rule.id}-AVAILABILITY/APP_NOT_AVAIL-%{matched_var_name}=%{tx.0}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/AUTOMATION/CRAWLER-%{matched_var_name}=%{matched_var},\
      1  setvar:tx.%{rule.id}-OWASP_CRS/AUTOMATION/SCRIPTING-%{matched_var_name}=%{matched_var},\
      3  setvar:tx.%{rule.id}-OWASP_CRS/AUTOMATION/SECURITY_SCANNER-%{matched_var_name}=%{matched_var},\
     19  setvar:tx.%{rule.id}-OWASP_CRS/LEAKAGE/ERRORS-%{matched_var_name}=%{tx.0}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/LEAKAGE/INFO-%{matched_var_name}=%{tx.0}"
      3  setvar:tx.%{rule.id}-OWASP_CRS/LEAKAGE/SOURCE_CODE-%{matched_var_name}=%{tx.0}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/POLICY/CONTENT_TYPE_NOT_ALLOWED-%{matched_var_name}=%{matched_var}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/POLICY/EXT_RESTRICTED-%{matched_var_name}=%{matched_var}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/POLICY/IP_HOST-%{matched_var_name}=%{matched_var}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/POLICY/METHOD_NOT_ALLOWED-%{matched_var_name}=%{matched_var}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/POLICY/PROTOCOL_NOT_ALLOWED-%{matched_var_name}=%{matched_var}"
      6  setvar:tx.%{rule.id}-OWASP_CRS/POLICY/SIZE_LIMIT-%{matched_var_name}=%{matched_var}"
     10  setvar:tx.%{rule.id}-OWASP_CRS/PROTOCOL_VIOLATION/EVASION-%{matched_var_name}=%{matched_var}"
      5  setvar:tx.%{rule.id}-OWASP_CRS/PROTOCOL_VIOLATION/INVALID_HREQ-%{matched_var_name}=%{matched_var}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/PROTOCOL_VIOLATION/MISSING_HEADER-%{matched_var_name}=%{matched_var},\
      7  setvar:tx.%{rule.id}-OWASP_CRS/PROTOCOL_VIOLATION/MISSING_HEADER-%{matched_var_name}=%{matched_var}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/ABNORMAL-ESCAPE-%{matched_var_name}=%{matched_var}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/COMMAND_INJECTION-%{matched_var_name}=%{tx.0}"
      2  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/FILE_INJECTION-%{matched_var_name}=%{tx.0}"
      4  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/HEADER_INJECTION-%{matched_var_name}=%{tx.0}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/HTTP_PARAMETER_POLLUTION-%{matched_var_name}=%{tx.0}"
     13  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/PHP_INJECTION-%{matched_var_name}=%{tx.0}"
     12  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/RCE-%{matched_var_name}=%{tx.0}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/REQUEST_SMUGGLING-%{matched_var_name}=%{tx.0}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/REQUEST-SMUGGLING-%{matched_var_name}=%{tx.0}"
      2  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/RESPONSE_SPLITTING-%{matched_var_name}=%{tx.0}"
      5  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/RESTRICTED_SQLI_CHARS-%{matched_var_name}=%{tx.0}"
      3  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/RFI-%{matched_var_name}=%{tx.0}"
      1  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/RFI-%{matched_var_name}=%{tx.1}"
      3  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/SESSION_FIXATION-%{matched_var_name}=%{tx.0}"
     11  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/SQL_INJECTION-%{matched_var_name}=%{tx.0}"
     27  setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/XSS-%{matched_var_name}=%{tx.0}"
      1  setvar:tx.sampling_percentage=100"
      1  setvar:TX.sampling_rnd100=%{MATCHED_VAR}"
      1  setvar:TX.sampling_rnd100=%{TX.1}"
      1  setvar:TX.sampling_rnd100=%{TX.1}%{TX.2}"
      1  setvar:TX.sampling_rnd100=%{TX.sampling_rnd100}%{TX.1}"
      1  setvar:tx.session_fixation_score=0,\
      3  setvar:tx.session_fixation_score=+%{tx.critical_anomaly_score},\
      1  setvar:tx.sql_error_match=0"
      1  setvar:tx.sql_error_match=1"
      1  setvar:tx.sql_injection_score=0,\
     49  setvar:tx.sql_injection_score=+%{tx.critical_anomaly_score},\
      6  setvar:tx.sql_injection_score=+%{tx.warning_anomaly_score},\
      1  setvar:tx.ua_hash=%{matched_var}"
      1  setvar:tx.warning_anomaly_score=3"
      1  setvar:tx.xss_score=0,\
     27  setvar:tx.xss_score=+%{tx.critical_anomaly_score},\

I think this could be changed after #890 and #907.

[dune73]

This sounds like a plan. Thank you for the report @fzipi.

In fact we tried to get rid of the quotes for the id for 3.0. And they seem to pop up again and again. We need to have a travis test. ;)

[fzipi]

This, and another things have been arising when trying to write a parser and things keep stubbornly being different... :|

[dune73]

Writing a parser is soo damn hard. And whatever looked homogenous when you started develops a life of its own when looking closer.

[fgsch]

Ok, I've started doing the cleanup wrt quotes and also ordering a few things that were missed in the previous rounds.

It's mostly mechanical so I will wait for #907 to get in before opening a PR.

[emphazer]

@fgsch
we are currently working on #907 and #887.
it should be done soon

[fgsch]

Since I'm at it, neither rev nor severity have been specified in the list above and thus are not ordered. @dune73 want to propose where they should be position-wise?

[fzipi]

Sorry @fgsch , its was proposed in my reordering, along with ver. See https://github.com/SpiderLabs/owasp-modsecurity-crs/pull/901#issue-259686170.
I will be adding this to the contributing file also.

[fgsch]

Thanks @fzipi.
Adding here for reference:

phase
disruptive-action
status
capture
t:xxx
log
nolog
auditlog
noauditlog
msg
logdata
tag
sanitiseArg
sanitiseRequestHeader
sanitiseMatched
sanitiseMatchedBytes
ctl
setenv
*rev
*ver
*severity
setvar
expirevar
chain
skip
skipAfter

[fgsch]

FTR, this is pending the merge of #907.

[fzipi]

After merging #940, I can add some of my small changes.

[fgsch]

@fzipi I still have a few other changes waiting for #940 to be merged in, mostly around the order of the rules that were missed.

What are your changes about?

[fzipi]

@fgsch Per our own CONTRIBUTING.md, we need to change SecMarker and skipAfter to quoted and with hyphens instead of underscoring... so it is a rewriting of that.

• format your SecMarker between double quotes, using UPPERCASE and separating words using hyphens. Examples are

    SecMarker "END-RESPONSE-959-BLOCKING-EVALUATION"
    SecMarker "END-REQUEST-910-IP-REPUTATION"

Bugs like rules/REQUEST-910-IP-REPUTATION.conf, line 79:

-            setvar:'ip.reput_block_reason=%{rule.msg}'\
+            setvar:'ip.reput_block_reason=%{rule.msg}',\

And more casing and simple changes. Mainly simple stuff that arose writing my (very) simple parser (https://github.com/fzipi/secrules-parser) 🙂

[fgsch]

Perfect, doesn't sounds like any of the changes I have for the follow-up.

I guess we need #940 in now. Any reviewers?

[fzipi]

I've reviewed #940, and it's merged. Thanks to @victorhora for that permissions!

[fgsch]

@fzipi @victorhora thanks! I will take a look at #956 and open hopefully the last PR to address the remaining things afterwards.

[fgsch]

@fzipi commented.
Where is the CONTRIBUTING.md file?

[fzipi]

@fgsch Is at the 3.1/dev branch: https://github.com/SpiderLabs/owasp-modsecurity-crs/blob/v3.1/dev/CONTRIBUTING.md

[fzipi]

@fgsch @dune73 What is the status of this one? Do we need to do something more?

[dune73]

I've lost the overview a bit (or never had it ;)

So I'll take the word of @fgsch if this is done or not. If it's over, then let's close this issue and start a round of applause!

[fgsch]

I believe all the actions in this ticket were completed.