The intent of this patch is to provide an easy way to ban hosts that are consuming too much bandwidth. Ideal for over-aggressive crawlers.

CommentFileSizeAuthor
#11 ban_3.diff12.73 KBmoshe weitzman
#9 ban_2.diff10.49 KBmoshe weitzman
#7 ban_1.diff10.5 KBmoshe weitzman
#1 ban_0.diff5.95 KBmoshe weitzman
ban.diff5.94 KBmoshe weitzman

Comments

moshe weitzman’s picture

moshe weitzman commented
StatusFileSize
new ban_0.diff5.95 KB

Oops. Here is the latest version.

dries’s picture

dries commented

Any particular reason we are not storing this in the database? Storing it in the database sounds like the better option to me.

moshe weitzman’s picture

moshe weitzman commented

well, we can't store it in the sessions table because many crawlers don't persist cookies and thus get a new session for every request. so we would have to store in a new table and I figured that I didn't need to add a new query for every page view. it would be a fast query, so perhaps a new table is OK. any more comments about variable table versus new table?

breyten’s picture

breyten commented

Having a table would allow for a simple query like:

$result = db_query("SELECT * FROM {banned_hosts} WHERE ipaddr = '%s'", $_SERVER["REMOTE_ADDR"]);

which is probably faster than a combined in_array and array_keys I suppose.

dries’s picture

dries commented

How about including this in the 'administer - access control - account rules' page (?q=admin/access/rules). We'd have to rephrase, and possibly rework it a little to extend the e-mail and username rules with IP/hostname rules but it sounds like a logical place to maintain IP-bans. In addition, we would be able to reuse the existing SQL table. (Let's not use variables!) Thoughts?

breyten’s picture

breyten commented

Only a +1. It sounds very sensible :)

moshe weitzman’s picture

moshe weitzman commented
Component: statistics.module » user.module
StatusFileSize
new ban_1.diff10.5 KB

As suggested, this version integrates nicely into the admin/access/rules pages. You are now able to ban hosts from statistics page or the access rules page. I think there are use cases for both.

I moved renamed user_deny() tp drupal_deny() and moved it to bootstrap.inc in order to let us block hosts during the bootstrap. We check for a blocked host right after loading the database APi, which is the earliest possible moment.

This feature makes it possible now to restrict your site to a few approved IP addresses (or a range of addresses). Might be useful for high security sites.

Jaza’s picture

Jaza commented

This is a great start, but I think that this feature still needs improvement before it's ready to be used by joe shmoe administrator. The biggest danger that I foresee is administrators banning IPs that they really shouldn't be banning. "Great, so you've banned the aggressive bot from Kazakhstan, that put your throttle up to 3 when it slammed your site last night. But you've also banned GoogleBot! Now your site won't get indexed!"

There need to be mechanisms in place to stop this from happening. Possibilities:

- IP lookup, so you can see the resolved hostname before banning the host. Perhaps show the resolved hostname on the confirm dialog, e.g. "Are you sure you want to ban the IP 64.68.82.14 (crawler10.googlebot.com)?"
- Automatic rather than (or as well as) manual banning. This might create more problems than it solves, but in the long-term it's a better solution. Perhaps something similar to the filtering mechanisms used by the spam module?

But overall, +1 for this patch, as long as people know what they're doing when they use it.

moshe weitzman’s picture

moshe weitzman commented
StatusFileSize
new ban_2.diff10.49 KB

same patch, but removing some debug comments which slipped in

dries’s picture

dries commented

Moshe, can you do one more round of updates? The patch can and will go in after that.

1. q=admin/access/rules/check needs to be updated. Maybe use form_group() too.

2. The ban/unban URL in statistics.module is really ugly and not very Drupal-ish. Mind to tidy up the URL scheme. (Haven't checked but do we perform a check_url() on that URL with user data? Might be taken care of at a lower level.)

3. I'd rename ip_address to either hostname (what do we use in other tables?) or ip.

Otherwise this works perfect; I just banned myself. ;)

moshe weitzman’s picture

moshe weitzman commented
StatusFileSize
new ban_3.diff12.73 KB

I made the requested changes. I also optimized drupal_deny() so that it usually issues only 1 query.

dries’s picture

dries commented

Committed to HEAD. Great work. Thanks.

Anonymous’s picture

(not verified) commented