Remove XSS-Protection header from Nginx configs

[minecrawler]

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?

[HLFH]

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: nextcloud/server#34748

[minecrawler]

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

[HLFH]

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.

[iasdeoupxe]

Server side change (remove it completely not just setting it to 0):

• [Bug]: Setup check for X-XSS-Protection recommendation is deprecated server#37154
• Fix: Remove X-XSS-Protection use, check and recommendation 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

[MichaIng]

Blocking until nextcloud/server#53476 has been merged, but this is the complementary needed docs part then.

I hope no one has a problem with me adjusting and squashing the commits:

• While disabling XSS explicitly is safe as well, I think it makes more sense to remove and forget this header completely. OWASP recommends to do either of both. For more details see the server PR.
• CSP should not be set at webserver level: It would overwrite or duplicate what Nextcloud sets in PHP already, hence can break things or weaken security. CSP, the way it is designed with very selective permissions all as single header value, needs to be set in the backend as individually. There is no generic safe but functional value that could be applied on webserver level for the whole site.

[MichaIng]

Is this still accurate?

https://github.com/nextcloud/server/blob/master/lib/versioncheck.php

[welcome]

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

[Patta]

@minecrawler Shouldn't the header also be removed in the rules under # Serve static files?

documentation/admin_manual/installation/nginx-root.conf.sample

Line 182 in dbde0fd

add_header X-XSS-Protection "1; mode=block" always;

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

documentation/admin_manual/installation/nginx-root.conf.sample

Line 1 in dbde0fd

# Version 2024-07-17

[minecrawler]

@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

[MichaIng]

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

[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? 😢🙏