Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Allowlist-based GET query parameter caching #850

Open
lkraav opened this issue Nov 17, 2016 · 4 comments
Open

Feature: Allowlist-based GET query parameter caching #850

lkraav opened this issue Nov 17, 2016 · 4 comments

Comments

@lkraav
Copy link

lkraav commented Nov 17, 2016

(From wpsharks/comet-cache-pro#293 (comment))

#639 has landed implementing a blocklist for optimized GET query parameter caching.

One of my scenarios would actually benefit from allowlisting instead.

COMET_CACHE_IGNORE_GET_REQUEST_VARS === *

COMET_CACHE_ALLOW_GET_REQUEST_VARS === [ 'date', 'location' ] etc.

In this scenario, the default is to route cached requests as if the request never had query parameters.

Only if ALLOW list exists, those specific parameters would create new cache entries.

This would lower DoS surface quite a bit, helping eliminate the possibility that an attacker could hit URLs with query strings just to avoid your cache engine and eat up more processing time.

To be clear, this stops untargeted random stuff. If someone targets a specific site, they will spend time to find out how to burn cycles within allowed GET parameters, too. But since the followup implementation costs here for having allowlisting have become negligible, I think overall a clear net win to be had.

@lkraav
Copy link
Author

lkraav commented Nov 17, 2016

PS I'm looking at our high-traffic content marketing blog analytics at http://conversionxl.com and filtering for query parameters. The incoming marketing toolset used (or just tested) over time can be massively wide, it's not feasible to maintain a blacklist of even legit set of parameters. This confirms that a global blacklist with a controlled allow list is the way to go.

@raamdev
Copy link
Contributor

raamdev commented Nov 17, 2016

@lkraav Thank you for opening this GitHub issue. :-)

@jaswsinc Could I get an estimate on this?

@jaswrks
Copy link

jaswrks commented Nov 18, 2016

Estimate: Two days


Note to self: This feature, when it's made available, could be prone to misuse. So we need to be careful about how we introduce this and provide some warnings. Silently ignoring all GET variables in the cache by default can easily lead to cache files that are served for future requests with query strings interpreted by plugins, themes, or by WP core itself, and if the query string is effectively ignored by Comet Cache, a cache file could be served without those variables taken into consideration as they should be. Site owners need to know exactly what they're doing to use this.

@lkraav
Copy link
Author

lkraav commented Nov 20, 2016

It could be an incremental release process. First step for implementing the necessary logic update could provide only a developer plugin API to hook into for enabling. This would allow at least one cycle of proper vetting by only knowledgeable people.

@raamdev raamdev modified the milestones: Next Release, Future Release Nov 22, 2016
@raamdev raamdev modified the milestones: Next Release, Future Release Dec 21, 2016
@raamdev raamdev modified the milestones: Next Release, Future Release Jun 22, 2017
@lkraav lkraav changed the title Feature: Whitelist-based GET query parameter caching Feature: Allowlist-based GET query parameter caching Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants