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

[ENH] Sitemap output caching (for security reasons) #4899

Open
mdmontesinos opened this issue Dec 5, 2024 · 3 comments
Open

[ENH] Sitemap output caching (for security reasons) #4899

mdmontesinos opened this issue Dec 5, 2024 · 3 comments

Comments

@mdmontesinos
Copy link
Contributor

mdmontesinos commented Dec 5, 2024

Oqtane Info

Version - 6.0.0
Render Mode - Static
Interactivity - Server
Database - SQL Server

Describe the enhancement

Following what was mentioned on #2748, I still believe it's interesting to have some sort of caching for the sitemap.

Imagine a website with thousands of news or other entities that need to be added to the sitemap that is generated dynamically. Through the application, this data is possibly displayed with paging or other caching mechanisms, but as the sitemap requires all the entities to be loaded, this could imply heavy database calls that spike the server load.

This is a potential vector for DDoS attacks, as hundreds of simultaneous requests could be made to this page and exhaust the resources.

Therefore, I was thinking it would be interesting to at least have a short-lifed output cache of the page (i.e. 5 minutes) to avoid this kind of attacks. In this way, you still allow the sitemap to refresh automatically after a few minutes to display changes made to the site, without a significant impact on SEO. Also, there's no need for the complex cache invalidation that was mentioned in the previous issue, as it expires after a short time anyway.

As a fallback, there could be a button somewhere in the admin menus to clear that cache if someone needs to refresh it immediately.

Anything else?

The implementation should be simple with the Output Cache middleware, as it's basically a razor page. I can submit a PR if you approve the change.

@sbwalker
Copy link
Member

sbwalker commented Dec 7, 2024

Note that DDoS is an attack, not a vulnerability. If an attacker directs enough traffic at any application, it will eventually be overloaded.

I do think it would be a good idea to enable some caching for the sitemap to improve overall performance and scalability. I guess the question is whether to use IMemoryCache (which is already available), Output Caching middleware, or Response Caching middleware.

@mdmontesinos
Copy link
Contributor Author

@sbwalker

Note that DDoS is an attack, not a vulnerability. If an attacker directs enough traffic at any application, it will eventually be overloaded.

Yes, you're right, but depending on the application, the sitemap could be one of the most resource-consuming operations, so it's a good target for DDoS attacks to quickly exhaust the server.

I guess the question is whether to use IMemoryCache (which is already available), Output Caching middleware, or Response Caching middleware.

Response Caching should be discarded, as according to official Microsoft guidelines:

Is typically not beneficial for UI apps such as Razor Pages because browsers generally set request headers that prevent caching. Output caching, which is available in ASP.NET Core 7.0 and later, benefits UI apps. With output caching, configuration decides what should be cached independently of HTTP headers.

Now from IMemoryCache and Output cache, I believe the best option for this case is using Output cache, as it makes the most sense. The memory cache would only cache the internal data used by the sitemap and always return a 200 OK response (with no cache stampede prevention until HybridCache is used). On the other hand, Output cache provides the following benefits:

  • Resource locking mitigates the risk of cache stampede and thundering herd.
  • Cache revalidation minimizes bandwidth usage. (returning a 304 Not Modified if possible)

Adding Output cache middleware shouldn't present any issues, as it won't affect anything that is not decorated with the corresponding attribute. Perhaps we could even use it in other places to improve performance.

@mdmontesinos
Copy link
Contributor Author

@sbwalker If you agree that OutputCache is the right option, I will submit a PR to handle this enhancement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants