[BUG] bot1_connlimit and bot1_reqlimitip not defined

[dkrutsko]

trafficstars

According to the documentation, in order to activate less restrictive rate limiting on $bad_bot with a value of 1, you have to comment out the following two lines in blockbots.conf:

limit_conn bot1_connlimit 100;
limit_req  zone=bot1_reqlimitip burst=50;

Unfortunately I'm unable to find any instance of a pool for bot1_connlimit and bot1_reqlimitip being defined. Furthermore, wouldn't $bot_iplimit need to also be updated in order to map 1 values to $binary_remote_addr?

I'm thinking that perhaps this feature doesn't belong as part of the globalblacklist.conf file and instead, should be implemented as part of a different file in a more complete way, especially since the $ratelimited variable doesn't appear to be used as reported in #331. The globalblacklist.conf file should really only contain: $bad_bot, $bad_words, $bad_referer, $validate_client. With bad words sort to being a little bit useless since it doesn't contain anything. It would make it easier to integrate into large Nginx projects.

[mitchellkrogza]

Hi @dkrutsko I will have a look see, been so long ago but it does indeed work in its current form although maybe not perfect for large projects.

You are welcome to send a PR as my time is just very limited at the moment. The PR has to pass all checks in Travis-CI.

For this purpose I have added test versions of the blocker files to test any breaking changes, they are all in the dev-tools folder.

You can also see during tests run in a build how the rate limiting does indeed block any specified bot once it hits the rate limit.

Sorry my answer is a bit short and patchy I am tied up on other things but will try answer you better during the week.

It's important to note any introduction of a new include file, could very well cause major breaking changes for thousands of users of this project.

So any new include file would need to first be added to the include_filelist.txt so that all users who do use the update script will have the file pulled before they update, reload and cause an EMERG.

Unfortunately there are many users who might not be using the update script, so it can be complicated and potentially very bad with new changes that require new includes. My best suggestion would be a completely new branch for this purpose.

[dkrutsko]

Indeed, I initially thought about making a new repo which would use the list files directly to solve my problems, but it felt a bit overkill, instead I'll probably just end up generating new conf files using the list files from this project. Especially since some of my projects might end up using a small combination of those list files rather than all of them.

I did have a question though, would you be open to having another list file for known good "services". This would include IP ranges of services like Sentry and GitHub, which our company currently uses and white lists. The only thing I would suggest is giving each service their own "ID". For instance, GitHub might be 1 and Sentry might be 2. That way, we can choose which services to allow, instead of allowing all of them.

Moreover, it's very curious that the tests pass, honestly I haven't actually dug that deep into them, I only checked for references, for which I could fine none. So it's very interesting that the tests pass for something that looks like dead code on the surface. I'll see if I can take another look.

[mitchellkrogza]

Hi @dkrutsko I introduced test cases in my builds to test each and every feature of the blocker. The rate limiting test (https://travis-ci.org/mitchellkrogza/nginx-ultimate-bad-bot-blocker/jobs/633015708#L1279-L1653) where I actually specify GoogleBot as a rate limited bot gets rate limited indeed. Have a look through the Travis log to see all the barrage of tests that are run before a new build is released.