silverstripe / cwp-installer

CWP project template
BSD 3-Clause "New" or "Revised" License
0 stars 5 forks source link

X-Frame-Options in .htaccess interfers with Incapsula #3

Closed robbieaverill closed 4 years ago

robbieaverill commented 6 years ago

Migrated from GitLab

cc @phptek

The cwp-installer module contains lines in its .htaccess file that prevent Incapsula from properly leveraging its ability to cache assets. While the header itself has nothing whatsoever to do with caching, this would seem to be an Incapsula-specific issue.

This issue was raised in a bespoke CWP project by @ingo.schommer in a performance review.

The lines in question are:

<IfModule mod_headers.c>
    Header always append X-Frame-Options SAMEORIGIN
</IfModule>

Comments

Usernames adjusted for GitHub

Ingo Schommer @chillu commented 2 years ago

This was anecdotal behaviour, observed by checking access logs. Russ, contrary to what I've told you yesterday, the X-Frame-Options header wasn't specifically mentioned as uncacheable by Incapsula support, only my other recommendation (Vary header) was. Which is also documented on https://incapsula.zendesk.com/hc/en-us/articles/200627510-How-to-control-the-way-my-site-is-cached

My point was that as far as I can tell, X-Frame-Options only needs to apply for text/html responses, not assets - since the protection it provides e.g. from clickjacking assumes interactive elements like form inputs in the hosted content.

Russell Michell @phptek commented 2 years ago

Right, so shouldn't .htaccess be changed in the module accordingly - something like

<IfModule mod_headers.c>
    SetEnvIf (mime text/html) SS_APPEND_XFrameOptions=1
    Header always append X-Frame-Options SAMEORIGIN env=SS_APPEND_XFrameOptions
</IfModule>

Russell Michell @phptek commented 2 years ago

Pfft, whatever I do, I can't seem to get this going. According to most docs, the following should only make httpd send this header for text/html:

# Set an env var when request is for text/html Content-Type to flag to Header directive to add X-Frame-Options
SetEnvIf Content-Type ^text/html do-x-frame-opts
<IfModule mod_headers.c>
    Header always append X-Frame-Options SAMEORIGIN env=do-x-frame-opts
</IfModule>

Also tried usiong Request_URI and a series of exclusion file-extensions which worked for all, apart from JPEG's..

Ingo Schommer @chillu commented 2 years ago

It's a tough one. I've investigated this a bit further, and technically we need to send the header with SWF files as well since they can show interactive content - hence potential to trick the user into clickjacking. Mozilla has an Apache implementation which doesn't include any mime type filtering: https://developer.mozilla.org/en-US/docs/Web/HTTP/X-Frame-Options?redirectlocale=en-US&redirectslug=The_X-FRAME-OPTIONS_response_header

Additionally, Incapsula shows inconsistent behaviour with this header: GET requests to an image don't have the header when delivered through Incapsula, while a HEAD request does. I've asked Incapsula support to clarify.

maxime-rainville commented 4 years ago

Just double check cwp-installer 2, and the X-Frame-Options statement doesn't appear anywhere.

Unfortunately, Silverstripe CMS 3 has entered limited support in June 2018. This means we'll only be fixing critical bugs and security issues for Silverstripe CMS 3 going forward.

You can read the Silverstripe CMS Roadmap for more information on our support commitments.

chillu commented 4 years ago

This is now behind a config flag Security.frame_options and LeftAndMain.frame_options, both of which are controllers which shouldn't be cached anyway - so a non-issue. Thanks for following up Max!