Fixes SecConnWriteStateLimit

[nicjansma]

Since a15f881, I don't believe SecConnWriteStateLimit has been working properly, which means that "slow read" attacks are not protected against.

With ModSecurity 2.9.2 on Apache 2.4.27, I was seeing ModSecurity properly blocking a Slowloris attack (slow writing headers) via slowhttptest -H:

slowhttptest -u http://127.0.0.1/a -H -c 1000

Apache error_log with SecConnReadStateLimit 50:

[Wed Aug 23 19:20:38.902507 2017] [:warn] [pid 2712] [client 127.0.0.1:21192] ModSecurity:
Access denied with code 400. Too many threads [51] of 50 allowed in READ state from
127.0.0.1 - Possible DoS Consumption Attack [Rejected]

We see above that the 51st and all subsequent connections are properly dropped.

However, "slow read" attacks (slowhttptest with -X or -B) aren't currently protected. With SecConnWriteStateLimit 50, ModSecurity currently logs warnings but doesn't drop connections.

slowhttptest -u http://127.0.0.1/a -X -c 1000

[Wed Aug 23 17:09:50.294791 2017] [:warn] [pid 11072] [client 127.0.0.1:44254] ModSecurity:
Access denied with code 400. Too many threads [226] of 50 allowed in WRITE state from
127.0.0.1 - Possible DoS Consumption Attack [Rejected]

(note 226 out of 50 allowed)

It looks like the line I've changed in this PR had inversed logic (should have been the same as the SecConnReadStateLimit logic), but a15f881 swapped it on accident.

With this one-character fix, I now see slow read attacks being blocked.

CC @zimmerle

[nicjansma]

(not sure why the Travis check failed? can I kick it off again?)

Any feedback? Hoping this is a straightforward fix to get in.

[marcstern]

nicjansma is correct, this is an important bug

[zimmerle]

Hi @nicjansma,

Thanks for the patch. We currently support two different ModSecurity versions on this very same repository. The v2 and the v3 family. Those different versions are using different buildbots, for some reason the v3 was shot to verify your commit and it failed as expected. I am running the v2 family buildbot manually to check your fix.

[zimmerle]

Merged ;) thanks!

[drmuey]

Greetings!

1. Will this fix go out in 2.9.3?
2. Is there an ETA for when that will version will be available?

[victorhora]

@drmuey

1. Yes, the fix is already in the mainline and will go to 2.9.3. See commit d50f840

2. There's still no ETA for 2.9.3. I don't think it will happen before year end as current focus is on ModSecurity 3.0.

[drmuey]

thanks @victorhora , much appreciated 👍

[nicjansma]

@victorhora @zimmerle Would we be able to move forward with a 2.9.3 release soon?

[nicjansma]

@victorhora @zimmerle do you have an estimate for when a 2.9.3 release will be available?

[victorhora]

Hi @nicjansma

We would like to have finished both 3.0.3 and 2.9.3 releases by end of September, but unfortunately some of the issues took more time and effort than we anticipated.

There's milestones set for both releases here:

2.9.3: https://github.com/SpiderLabs/ModSecurity/milestone/10

3.0.3 https://github.com/SpiderLabs/ModSecurity/milestone/12

And we also have a number of pull requests to be fully reviewed/tested and merged prior to the next release: https://github.com/SpiderLabs/ModSecurity/pulls

Ideally we would like to fix/close most (or all) of those issues before the next release. Hopefully soon, but I don't have a date yet, but certainly before end of October.

In the meantime, any help in the sense of pull requests, reviews, results of tests and comments on the aforementioned issues/PRs are appreciated :)

[nicjansma]

That's great @victorhora, thank you for the update!

That seems to be quite a few issues being addressed in 2.9.3. Would you consider moving forward with a few smaller 2.9.x releases?

[victorhora]

Hi @nicjansma, you're welcome :)

Well, the thing is, as you may have heard development is focused on the v3 branch. And we are planning to do releases for this branch on a shorter period of time. Ideally every 3 months or so.

For 2.x releases we don't have a fixed release schedule set at the moment. 2.x should be at maintenance level. Meaning that new releases should be based on addressing either security issues or critical bugs.

But we also believe that a reasonable amount of changes also warrants for a release. For instance, we have >20 fixes between 2.9.2 and current master (pre-2.9.3). Hence why we are working on releasing 2.9.3 as soon as possible :)

[nicjansma]

Glad to see 3.0.3 has been released! Are you planning a 2.9.3 release soon?

[victorhora]

Hi @nicjansma

Yes, we want to release 2.9.3 as soon as possible. You can track the progress of the milestone for 2.9.3 here: https://github.com/SpiderLabs/ModSecurity/milestone/10

As soon as that last issue is solved we will start cooking the 2.x release. Thanks.

[victorhora]

@nicjansma

I believe that by now you probably notice that 2.9.3 is out? :)

https://github.com/SpiderLabs/ModSecurity/releases/tag/v2.9.3

Thanks for your contribution on this one! :)

Cheers

[nicjansma]

Much obliged!