varnishd: add a feature flag to disable bans in VCL

[walid-git]

This is useful on multi-tenant environments to prevent a single user from wiping the whole cache or other user's cache.

[nigoroll]

I am not against, but is this a generic solution to the problem? Would it not make much more sense to add some sort of cache partitioning where a certain value is forcibly added to the hash and bans?

[walid-git]

I am not against, but is this a generic solution to the problem? Would it not make much more sense to add some sort of cache partitioning where a certain value is forcibly added to the hash and bans?

I agree that there is probably much more improvements to do in this area, but this is a low hanging fruit that can already help in a lot of use cases, and I think it makes sense to have it.

[gquintard]

Aren't the two ideas a bit different? One removes the ability altogether, the other silos it. Both have merits, but they're different

[nigoroll]

I agree with both Walid's and Guiallaume's comments, but having this as a global flag really seems to be quite limited. Next up, someone might want feature +disable_bans_for_labeled_vcls or +disable_bans_unless_on_admin_acl. I would hope that just investing a little more could make this much more useful.

[bsdphk]

I'm -1 on this one. It doesn't seem hard to do it properly (see todays IRC log)

[dridi]

I agree with both Walid's and Guiallaume's comments, but having this as a global flag really seems to be quite limited. Next up, someone might want feature +disable_bans_for_labeled_vcls or +disable_bans_unless_on_admin_acl. I would hope that just investing a little more could make this much more useful.

How about a new vcl_feature bits parameter?

Initial default value: +ban,-connect

Since VCL bans simply exist today, we keep them enabled by default for compatibility, but new features like CONNECT support (https://github.com/varnishcache/varnish-cache/pull/4152#issuecomment-2283253436) can be introduced opt-in.

Then we can add a new option to vcl.load to override VCL flags on a per-VCL basis:

param.set vcl_feature -ban
vcl.load customer-a-1 customer-a.vcl
vcl.load -f +ban admin-1 admin.vcl
vcl.label customer-a customer-a-1
vcl.label admin admin-1
vcl.load dispatch-1 main.vcl
vcl.use dispatch

FYI I would be against adding an override at the label level.

We can keep track of VCL overrides separately for enabled and disabled features (with a simple sanity check of (enabled&disabled) == 0) and have a special case VCL_FEATURE(vcl, x) macro.

[nigoroll]

@dridi per vcl feature flags sound like a very good idea. I'd like to keep the CONNECT discussion separate, but regarding BANs I think your proposal is sound!

[dridi]

I only mentioned CONNECT to connect (pun obviously intended) this feature (pun intended) to other sensitive VCL capabilities.

Another example could be "import ... from":

param.set vcl_feature -import_from

[nigoroll]

bugwash: This needs additional implementation of per-vcl feature flags