Banned IP addresses can still access cached pages

+++ b/core/modules/system/lib/Drupal/system/Tests/Session/SessionTest.phpundefined
@@ -152,7 +152,8 @@ function testEmptyAnonymousSession() {
+    // @todo Reinstate when REQUEST and RESPONSE events fire for cached pages.

i think this issue should be postponed for the same reason..do we actually have an issues for this though?

I'm pretty sure the problem here is that page caching hasn't been converted into the kernel.

When we do get to full HTTP-based caching, this is definitely going to still be the case. The cache will happen outside the kernel, meaning before there are any events to fire. That's actually consistent with using a proxy cache now, I would imagine. (Your Varnish server doesn't know what has been banned at the application layer.)

I think this is actually fine. It becomes consistent regardless of your caching tool, and if you were actually worried about a person DDOSing you you'd not want to use ban.module to block that anyway (that's what your network layer is for).

So I think all we need here is a change notice.

Hm. I've to admit I hoped for "more."

That response essentially means that we can send Ban module to /dev/null, since it won't work with HttpCache and it also won't work with reverse-proxies.

In turn, the configurations it "may" work with are edge-cases. E.g., development sites. Or sites on which people "forgot" to enable caching.

That said, I do wonder a bit why Symfony (CMF?) has a Firewall component, if all of that is irrational. Perhaps we should look into that some more?

Symfony's firewall setup is not an equivalent of ban module. It's more akin to the access callbacks on menu items, but centrally configured using regex and such rather than specified per-route. The naming makes that a big confusing.

ban.module would in this case really only be useful for preventing certain IPs from logging in or posting comments or otherwise interacting with the site. Once you have a cache that lives outside your application (be it HttpCache, Varnish, mod_cache, nginx, whatever), your application is now useless for enforcing network-level firewalls (eg, IP addresses). In a sense, Drupal <=7 has had its default configuration be the edge case, where we assume there's no caching going on at all so we're in complete control of the network layer, too. (And even then, I think anyone who knew what they were doing acknowledged that if you really wanted to ban a troublesome IP, you should do so at the network layer anyway for performance reasons.)

As that's not a particularly wise assumption in 2013, I think there's a good argument to be made for just euthanising ban.module, as it's redundant with the entire lower layers of your web stack, whatever they are.

I thought I would've seen documentation for the sf Firewall component on the net, detailing how to block IP addresses?

Perhaps I'm just flabbergasted by the misnomer though.

With an additional confirmation that Symfony doesn't provide any concepts for this (ideally by someone from Symfony) I'd thus opt for the conclusion that we just simply remove Ban module entirely from core, since we generally do not maintain edge-case material in core.

@webchick and potentially others will ask: "So what about the stored data/configuration and stuff?"

I do not have an answer to that. And #5 makes it sound as if we do not even want to provide one.

In any case, I think this is a pretty crazy change in Drupal's architecture, which silently happened as part of a less important API clean-up issue. I do not disagree with the proposed architectural change (because I assume it ultimately makes sense), but I do think that this issue and architectural change needs to be bumped to critical to raise some more awareness.

That is, because this change effectively means that the Drupal application layer is no longer in charge of caching, or alternatively, its output must always be compatible with external reverse-proxies.

That's the abstracted use-case of Ban module: Assume that every request lands in Drupal, which means that every possible request can be blocked by Drupal. That's the opposite of the reality today in advanced setups already, but at least, Drupal modules were still able to signify that a particular page must not be cached (including reverse-proxies).

This has an impact on all functionality that typically runs in the front-end layer. Everything that might end up on a page that is cached externally and which might rely on visitor/page-specific parameters will have to fundamentally rethink itself.

The ultimate fate is that we'll remove Ban module from core. We will also have to double-check and verify that all functionality that is supposed to work for anonymous users with enabled page/HTTP-level caching still works. (We have 0% test coverage for that.)

I agree that it's a not-small change. However, it's not a stealth change. Similar discussions already happened around statistics module when discussing the removal of hook_boot(). And "everything is an HTTP cache" has been the end-game for WSCCI for coming up on 3 years now. :-)

There are still ways to disable caching for selected responses if needed; just set the response object's cache headers directly to no-cache, or whatever other rules make sense for that specific page. I maintain the cacheexclude module and have already figured I'll be doing something like that in the D8 version. But yes, it does change the default assumption.

Statistics module correctly logs node counts from requests behind reverse proxies via an AJAX callback now so that's been fixed. The issue was moved to 7.x for backport because it was always broken with reverse proxies so it was really a bug fix. Ban module's a different use case though.

That response essentially means that we can send Ban module to /dev/null, since it won't work with HttpCache and it also won't work with reverse-proxies.

Yes. #1570102: [Policy] Deprecate Ban module.

None of this has ever worked with reverse proxies and none of the changes in 8.x have made it any more broken with them than it already was.

but at least, Drupal modules were still able to signify that a particular page must not be cached (including reverse-proxies).

You can still do that with drupal_page_is_cacheable(FALSE), or starting a session, that's not changed at all. If/when we move to HttpCache we'll need a similar mechanism since there's always going to be valid use cases for that.

This has an impact on all functionality that typically runs in the front-end layer. Everything that might end up on a page that is cached externally and which might rely on visitor/page-specific parameters will have to fundamentally rethink itself.

This is true since cacheable page headers were added to Drupal 7, and was true with Drupal 6 Pressflow as well. Any contrib or custom code that doesn't take care of this needs bug reports posting for it. Here's one from 2008 #339958: Cached pages returned in wrong language when browser language used.

There's nothing new here except that core is beginning to recognise reality whereas previously it ignored it. Individual sites or custom code can still disable page level caching if they need to (both internal caching and reverse proxies).

Agreed we should add an end-user facing change notification for ban module, so retitling issue for that.

catch: If we're going to just remove ban.module (which seems like a good idea), do we need a change notice or do we just mark this issue a dupe?

I think we need a change notice until that issue is fixed. Both webchick and Dries have been extremely resistant to removing IP blocking from core in the past so that issue needs a lot of help to actually get committed.

After the new stuff i just read from sun on #1911178-57: Remove hook_exit(), why dont we just fix our kernel and use Bundle::boot() for ban module?

I'm going to attempt a change notification for this one, this task was taken by myself here: http://core.drupalofficehours.org/task/1717

Attempted change notification:

Summary:

• IP addresses banned using the ban module will still be able to access cached pages
• Page caching is not in the kernel and so IPs do not get intercepted early enough to stop their viewing of cached pages.

Before:
In Drupal 7 IPs that were banned by the ban module in fact could not view cached pages.

After:
In Drupal 8 IPs that are banned can still view cached pages. Recommendations on banning IPs are to ban IPs on the network level. Banning IPs on the network level is preferable to banning on the Drupal level, even if and when this issue gets resolved.

Please review the draft of the change notification above. thanks.

using #13 done http://drupal.org/node/1941682

Added an extra note about .htaccess on shared hosting environments since not everyone has network configuration access. Moving this to fixed.

restoring status

Thanks @barnettech and @podarok!

In light of #2332983: Replace ban event subscriber with a ban stack I've deleted https://www.drupal.org/node/1941682