documentation icon indicating copy to clipboard operation
documentation copied to clipboard

Disable xss header and add CSP header

Open minecrawler opened this issue 3 years ago • 4 comments

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?

minecrawler avatar Sep 29 '22 11:09 minecrawler

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

HLFH avatar Dec 08 '22 10:12 HLFH

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 :)

minecrawler avatar Dec 08 '22 11:12 minecrawler

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.

HLFH avatar Dec 08 '22 11:12 HLFH

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

iasdeoupxe avatar Jun 13 '25 21:06 iasdeoupxe

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

welcome[bot] avatar Jul 01 '25 06:07 welcome[bot]

@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 avatar Jul 23 '25 07:07 Patta

@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

minecrawler avatar Jul 23 '25 10:07 minecrawler

Absolutely, that was an oversight. I will create a followup PR just now 🙂.

MichaIng avatar Jul 23 '25 10:07 MichaIng

#13445

In addition, the version in the line 1 is outdated.

Or shall we call it the "RIP Ozzy 🤘" version of the Nginx config? 😢🙏

MichaIng avatar Jul 23 '25 11:07 MichaIng