The problem

In Drupal 7 we removed 90% of the 'access rules' functionality that used to live in user module, but IP blocking was given a reprieve by Dries.

In Drupal 8, the remaining IP blocking functionality was moved into Ban module #1161486: Move IP blocking into its own module.

Not much has happened to ban module since then, except when it gets touched by other cleanup issues (like converting hook_help()).

According to stats in #3158669: By default deprecate non-experimental modules that are used by less 5% of sites before the next major version ban module is used by approximately 8% of sites.

Since this issue was opened, there are now some equivalent modules in contrib for Drupal 8.

https://www.drupal.org/project/autoban

https://www.drupal.org/project/advban

https://www.drupal.org/project/restrict_by_ip

Advanced ban is a direct replacement of the core module, and has 6.5k users. Autoban integrates optionally wither either ban or advanced ban. Restrict by IP has 8k users and seems to be standalone.

Proposed solution

Move ban module to contrib as-is, deprecate it in Drupal 9 for removal in Drupal 10.

CommentFileSizeAuthor
#14 1570102_remove_ban_module.patch18.34 KBcweagans
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

"there are existing contrib modules that solve the same problem that are well established for doing it from within Drupal" What are these modules? Drupal.org makes extensive use of this feature for spammers. It's not practical to give the webmasters team access to edit .htaccess rules, nor is it practical to ping Narayan every time someone catches spam.

catch’s picture

Here's some:

http://drupal.org/project/badbehavior
http://drupal.org/project/mollom
http://drupal.org/project/troll
http://drupal.org/project/spambot

It'd also be easy to turn the existing core behaviour into a contrib module, I've already done most of that work in the core patch. I don't think "Drupal.org uses it" is a sufficient reason to keep something in core any longer.

webchick’s picture

Wasn't implying that it was; just pointing out there's a general need for this functionality, which none of those listed modules deal with, AFAIK. And that is, blocking IP addresses for bulk-spammers through a web interface. It's not safe to assume the person who can change .htaccess or firewall rules is the same person dealing with content and abusive users on the site. (For example, any shared host ever :P)

If there's legitimately a contributed project that deals with it that sites like Drupal.org could migrate to, that might be enough. Should we mark this postponed on #1161486: Move IP blocking into its own module?

catch’s picture

Troll contains this functionality according the project page, but looks like it's been unmaintained for some time (although still managing to report 875 active users back to Drupal.org despite that).

I'd like to discuss this one a bit more before marking postponed, because we may well want to in turn postpone #1479466: Convert IP address blocking to config system on one or both of these - since we definitely don't want to move IP blocking to the configuration system, but having all of bootstrap dependent on an ancillary feature in system module doesn't bode well for some of the other refactoring that's going on.

andypost’s picture

The issue #1161486: Move IP blocking into its own module mostly done except hook_help() which could be done latter

webchick’s picture

Docs are a gate, not a follow-up. :\

sun’s picture

While I generally agree with the thinking here, there are a couple of counter-arguments:

1) Many CMS/applications have this kind of functionality built-in. (Even the Symfony framework has a Security component with a firewall feature.)

2) If there was a slightly better API and slightly more functionality (e.g., IP ranges, perhaps even GeoIP?), then contributed modules (such as Mollom but also others) could actually leverage the built-in Ban module in Drupal core.

3) The new decoupled Ban module might be the only module we have in core that allows us to verify that modules operating in early bootstrap at hook_boot() time are working correctly. The current state of kernel/bootstrap/container/config stuff is anything but solid, proven by critical bugs like #1576322: page_cache_without_database doesn't return cached pages without the database and many other major issues that are currently in the queue.

In light of that, especially the last item, I'd prefer to keep Ban module in core for at least one more release.

Also, given #1036780: Drupal.org should collect stats on enabled sub-modules and core modules, we should soon be able to see actual usage data for individual modules in Drupal core. Thus, by decoupling the functionality into Ban module via #1161486: Move IP blocking into its own module for D8, we can actually make a more grounded decision on whether to keep it for D9, because we can learn how many sites have it enabled in D8. And, by D9, we'll (hopefully) have the early bootstrap problems figured out ;)

andypost’s picture

For me 3rd option is main, this is a only module that works with tests on early bootstrap.

Also I think we should remove dependency of Statistics module on the Ban btw to allow contrib to implement it's own ways. Probably we need a pluggable solution to find a better implementation

catch’s picture

If there was a slightly better API and slightly more functionality (e.g., IP ranges, perhaps even GeoIP?), then contributed modules (such as Mollom but also others) could actually leverage the built-in Ban module in Drupal core.

We used to have IP ranges, but the way it was implemented meant an uncacheable, un-indexed query on every request, which is why most of the old 'access rules' was stripped out about the first month of Drupal 7, apart from them confusing people to no end. Dries wanted this particular feature kept in though, not sure how he feels about that now.

lpalgarvio’s picture

i fully agree with sun on #7: 3)
decisions about replacing the module or removing it on D9 seem a better fit for D9 i guess

Crell’s picture

As noted in #1929506-5: Banned IP addresses can still access cached pages, Symfony's firewall system has nothing to do with firewalls like a network admin would use them. That's Symfony's access control system. The name is a bit weird.

Also as noted in that issue, there's only a small set of use cases where ban.module is even functional right now, and those cases are likely to go away, rendering ban.module useless except for logged in users... for which there are better access systems anyway.

Let's just put this module to bed and return this functionality to the network stack where it belongs.

sun’s picture

Title: Remove IP blocking from core » Remove Ban module (IP address blocking) from core
Component: base system » ban.module
Issue tags: +API clean-up

Yes, the discussion here is a bit older. Please ignore #7.

Clarifying a couple of issue properties. (That said, we'll run into a weird situation with the new ban.module component; I think all issues need to be moved back to 7.x + system.module :P)

adammalone’s picture

When username/email blocking was removed from core in D6->D7 the User Restrictions module took over that functionality. It did not take over IP blocking as that was in core.

If Ban is removed from D8 I presume we'll just add IP banning functionality to User Restrictions making it essentially a clone of the D6 'Access Rules'.

Personally all D7 sites I have use:

$conf['blocked_ips'] = array();

in the settings.php and instead block IP addresses and ranges somewhere more efficient at doing so (ie iptables). This isn't a solution for users on shared hosts however.

At present I'm indifferent although slightly erring towards removal provided it doesn't break any tests/functionality.

cweagans’s picture

Status: Active » Needs review
FileSize
18.34 KB

I agree that ban module should be moved to contrib, as well, especially since you'd have to look at your logs to figure out what IP to ban anyway. There's not a lot of use that you can really get out of blocking IP addresses at the application level. This should definitely be at the network level, or at least the web server level.

In contrib, this module could be expanded to, for instance, use SNMP to reach out to the hardware firewall and add a new rule to ban a given IP address, or perhaps modify a .htaccess file to block an IP address. Leaving this in core is not only a bad idea from an architecture standpoint, it's bad from a flexibility/functionality standpoint for this module.

I'm really tired, so sorry for the rambling.

Status: Needs review » Needs work

The last submitted patch, 1570102_remove_ban_module.patch, failed testing.

klonos’s picture

...This should definitely be at the network level, or at least the web server level.

Sure, that'd be ideal, but you need certain level of access to the hosting server for that. Not everybody builds their site on VPS.

Is the namespace available? The removal should be followed by the creation of a project with the module moved there.

cweagans’s picture

Namespace is available.

The only access you need for banning at the server level is to dump rules into .htaccess. This does not require any additional access.

klonos’s picture

Yep. when you said "network level" I took you were referring to the firewall (iptables).

cweagans’s picture

Yes. And you obviously wouldn't have access to that on shared hosting, and since that's not an option, you can block it at the server level. The point is, this shouldn't happen at the application level.

artfulrobot’s picture

Issue summary: View changes

#17 "The only access you need for banning at the server level is to dump rules into .htaccess. This does not require any additional access."

You're note proposing that you let a module update .htaccess, surely? If you have your .htaccess file writable by the user that runs the webserver then that would be a big security risk (acknowledged elsewhere, e.g. when Drupal warns if settings.php is writable).

catch’s picture

@artfulrobot no cweagans is saying that if you have access to the Drupal files yourself, you can also edit .htaccess. This is the case on any self-hosted install. It may not be the case where the Drupal install is managed by a third party, but then they're the ones who should be worrying about DDOS in that case.

artfulrobot’s picture

@catch OK, all clear, thanks.

bfr’s picture

No one in their right mind would allow their server to crawl every directory on every page request just to search for some crazy server configuration override files, so .htaccess-things generally apply only for shared hosting, and even then only if they run on Apache.

That said, i'm still not sure how to proceed with my Ip Ranges module. I really hoped there would be some event replacing hook_boot(). Maybe just not upgrade it at all, especially if core team is working on some contrib already.

adammalone’s picture

@bfr, we can subscribe to the request event within IP Ranges module. This is, in effect, a replacement to hook_boot.

Take a look at my fast 404 port to 8.x and note how I intercept the request very early on in order to deny access to paths. I'm more than willing to work with you an IP Ranges port if you like :)

bfr’s picture

@typhonius Excellent, i will gladly make you a co-maintainer if you are interested.
However, let's continue this at the module issue queue.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

cilefen’s picture

Project: Drupal core » Drupal core ideas
Version: 8.2.x-dev »
Component: ban.module » Idea
Category: Task » Plan
Status: Needs work » Active
yoroy’s picture

Category: Plan » Feature request

Some rough usage data for core modules: #2867597: Top Drupal 7 and Drupal 8 core sub-modules

catch’s picture

Issue summary: View changes
catch’s picture

A fair bit has happened in contrib and core since this was last discussed 7-9 years ago.

'early bootstrap' is covered by page_cache module and our other middleware implementations.

There is at least one (Drupal 8/9) module that's a direct replacement for Ban module (advanced ban), one that is similar-ish with a slightly different feature set (read only as well as outright bans), and one module which supports integration with both ban and advanced ban.

Added some of this to the issue summary.

catch’s picture

Category: Feature request » Task
catch’s picture

Title: Remove Ban module (IP address blocking) from core » Deprecate Ban module in Drupal 9 and move it to contrib for Drupal 10
quietone’s picture

Title: Deprecate Ban module in Drupal 9 and move it to contrib for Drupal 10 » [Policy] Deprecate Ban module in Drupal 10 and move it to contrib for Drupal 11

Update title.

xjm’s picture

Removing a module from core requires the signoff of the subsystem maintainer when there is one, as well as all the committer roles.

The committer team recently reviewed our scoring exercise on all core modules, and there was not clear consensus to remove Ban, so it needs to be discussed further (both by the committers, and with input from the community).

Thanks!

catch’s picture

Just for posterity we're six weeks from the 15 year anniversary of me first opening an issue to try to remove this from core #228594: UMN Usability: split access rules into an optional module.

larowlan’s picture

From a framework manager point of view, I'm happy for it to be moved to contrib. There are a number of contrib solutions, and larger sites will using something at the WAF/CDN layer. Not removing the tag as giving others a chance to chime in.

tsotoodeh’s picture

There are a number of contrib solutions, and larger sites will using something at the WAF/CDN layer.

Totally agreed on this. In my point of view, from the perspective of webmaster banning an IP address is not a webmaster task but rather the task of network administration of the project, which as you mentioned should be done at the WAF or CDN level. The task itself, with today's technology could be easily done and resolved when appropriate credentials are provided.
In addition, from the security stand point it is best suited to prevent the malicious/intruder traffic stopped at the edge not the production environment. Also there are more means to mitigate bad actors.
I suggest, Drupal as a content management system be more focused on providing modules that offer best digital experience.

jungle’s picture

I was thinking of introducing a Ban entity to make it more extendable. The current ban_ip table has two fields iid(unique ID for IP addresses) and ip. -- It's a simple module, unlikely to have many issues reported as other core modules. Inactivity makes sense to me.

There are a number of contrib solutions

Yes, there are related contrib modules, but I prefer none of them. E.g., I hope to see a reason why the IP is banned. autoban, advban etc. can not show me that.

and larger sites will using something at the WAF/CDN layer.

There are small sites built with Drupal without WAF or CDN, e.g. some personal blog sites.

In short, I hope it can adopt entity, instead of being removed.

xjm’s picture

Regarding the proposal in #39, that could also be done if/when the module is moved to contrib. :)

yoroy’s picture

The two obvious alternatives in contrib appear to be in good shape, both with releases made this year
https://www.drupal.org/project/autoban
https://www.drupal.org/project/advban

Lets just go ahead and do this? Removing the framework manager review tag as @catch opened this issue and @larowlan chimed in at #1570102-37: [Policy] Deprecate Ban module

yoroy’s picture

Lets not be too eager here Roy!

larowlan’s picture

quietone’s picture

Title: [Policy] Deprecate Ban module in Drupal 10 and move it to contrib for Drupal 11 » [Policy] Deprecate Ban module

Removing versions from the title. They can be added back if this is approved.