Disable xss header and add CSP header
By now, no modern browser supports the XSS-Protection header anymore - and with good reason. It actually has known security vulnerabilities. Instead, it is recommended to disable it and use CSP instead. See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-XSS-Protection
Hence, this patch disables the XSS header and enables CSP instead. The CSP part was copied from an earlier PR (#1714), which was closed because of an incompatible browser matrix. That issue does not exist anymore. Is it possible to merge these changes?
It would be great to merge the first commit.
For the second commit, as Nextcloud devs said in the previous PR: they set the CSP from Nextcloud. And removing the X-Frame-Options depends quite from this issue: https://github.com/nextcloud/server/issues/34748
I can reverse the second commit and rebase the changes, if that's what it takes.
Also, if this commit still needs other changes (like the check for the presence of the header), please tell me what and where and I can add it :)
For the first commit, this code should be updated I think: https://github.com/nextcloud/server/blob/master/core/js/setupchecks.js
var xssfields = xhr.getResponseHeader('X-XSS-Protection') ? xhr.getResponseHeader('X-XSS-Protection').split(';').map(function(item) { return item.trim(); }) : [];
if (xssfields.length === 0 || xssfields.indexOf('1') === -1 || xssfields.indexOf('mode=block') === -1) {
messages.push({
msg: t('core', 'The "{header}" HTTP header does not contain "{expected}". This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.',
{
header: 'X-XSS-Protection',
expected: '1; mode=block'
}),
type: OC.SetupChecks.MESSAGE_TYPE_WARNING
});
}
That would be necessary before updating the nginx documentation for Nextcloud.
Server side change (remove it completely not just setting it to 0):
- nextcloud/server#37154
- nextcloud/server#53476
Could also require an update here:
https://github.com/nextcloud/documentation/blob/47a9a28ca920b8be1920b7f999c1585bd73241da/admin_manual/installation/harden_server.rst#serve-security-related-headers-by-the-web-server
Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/documentation/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22
@minecrawler Shouldn't the header also be removed in the rules under # Serve static files?https://github.com/nextcloud/documentation/blob/dbde0fdd992c9f5118d56d5fc334a3fe19ed93ca/admin_manual/installation/nginx-root.conf.sample#L182
In addition, the version in the line 1 is outdated. https://github.com/nextcloud/documentation/blob/dbde0fdd992c9f5118d56d5fc334a3fe19ed93ca/admin_manual/installation/nginx-root.conf.sample#L1
@Patta well, it's a 3(!) years old PR and it's merged, now. So if there's potential for improvement, we should create a new PR and hope it's merged quicker
Absolutely, that was an oversight. I will create a followup PR just now 🙂.
#13445
In addition, the version in the line 1 is outdated.
Or shall we call it the "RIP Ozzy 🤘" version of the Nginx config? 😢🙏