fix: Fix Security headers setup check behavior

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
nextcloud · Mar 13, 2024 · 310377e · 310377e
1 parent 1fffdf4
commit 310377e
Show file tree

Hide file tree

Showing 2 changed files with 3 additions and 72 deletions.
diff --git a/apps/settings/lib/SetupChecks/SecurityHeaders.php b/apps/settings/lib/SetupChecks/SecurityHeaders.php
@@ -70,7 +70,7 @@ public function run(): SetupResult {
 
 		foreach ($urls as [$verb,$url,$validStatuses]) {
 			$works = null;
-			foreach ($this->runRequest($url, $verb) as $response) {
+			foreach ($this->runRequest($url, $verb, ['httpErrors' => false]) as $response) {
 				// Check that the response status matches
 				if (!in_array($response->getStatusCode(), $validStatuses)) {
 					$works = false;
@@ -95,7 +95,7 @@ public function run(): SetupResult {
 				}
 
 				$referrerPolicy = $response->getHeader('Referrer-Policy');
-				if ($referrerPolicy === null || !preg_match('/(no-referrer(-when-downgrade)?|strict-origin(-when-cross-origin)?|same-origin)(,|$)/', $referrerPolicy)) {
+				if (!preg_match('/(no-referrer(-when-downgrade)?|strict-origin(-when-cross-origin)?|same-origin)(,|$)/', $referrerPolicy)) {
 					$msg .= $this->l10n->t(
 						'- The `%1` HTTP header is not set to `%2`, `%3`, `%4`, `%5` or `%6`. This can leak referer information. See the {w3c-recommendation}.',
 						[
@@ -118,6 +118,7 @@ public function run(): SetupResult {
 					return SetupResult::warning($this->l10n->t('Some headers are not set correctly on your instance')."\n".$msg, descriptionParameters:$msgParameters);
 				}
 				// Skip the other requests if one works
+				$works = true;
 				break;
 			}
 			// If 'works' is null then we could not connect to the server

diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js
@@ -169,7 +169,6 @@
 			var deferred = $.Deferred();
 			var afterCall = function(data, statusText, xhr) {
 				var messages = [];
-				messages = messages.concat(self._checkSecurityHeaders(xhr));
 				messages = messages.concat(self._checkSSL(xhr));
 				deferred.resolve(messages);
 			};
@@ -183,75 +182,6 @@
 			return deferred.promise();
 		},
 
-		/**
-		 * Runs check for some generic security headers on the server side
-		 *
-		 * @param {Object} xhr
-		 * @return {Array} Array with error messages
-		 */
-		_checkSecurityHeaders: function(xhr) {
-			var messages = [];
-
-			if (xhr.status === 200) {
-				var securityHeaders = {
-					'X-Content-Type-Options': ['nosniff'],
-					'X-Robots-Tag': ['noindex, nofollow'],
-					'X-Frame-Options': ['SAMEORIGIN', 'DENY'],
-					'X-Permitted-Cross-Domain-Policies': ['none'],
-				};
-				for (var header in securityHeaders) {
-					var option = securityHeaders[header][0];
-					if(!xhr.getResponseHeader(header) || xhr.getResponseHeader(header).replace(/, /, ',').toLowerCase() !== option.replace(/, /, ',').toLowerCase()) {
-						var msg = t('core', 'The "{header}" HTTP header is not set to "{expected}". This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.', {header: header, expected: option});
-						if(xhr.getResponseHeader(header) && securityHeaders[header].length > 1 && xhr.getResponseHeader(header).toLowerCase() === securityHeaders[header][1].toLowerCase()) {
-							msg = t('core', 'The "{header}" HTTP header is not set to "{expected}". Some features might not work correctly, as it is recommended to adjust this setting accordingly.', {header: header, expected: option});
-						}
-						messages.push({
-							msg: msg,
-							type: OC.SetupChecks.MESSAGE_TYPE_WARNING
-						});
-					}
-				}
-
-				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
-					});
-				}
-
-				const referrerPolicy = xhr.getResponseHeader('Referrer-Policy')
-				if (referrerPolicy === null || !/(no-referrer(-when-downgrade)?|strict-origin(-when-cross-origin)?|same-origin)(,|$)/.test(referrerPolicy)) {
-					messages.push({
-						msg: t('core', 'The "{header}" HTTP header is not set to "{val1}", "{val2}", "{val3}", "{val4}" or "{val5}". This can leak referer information. See the {linkstart}W3C Recommendation ↗{linkend}.',
-							{
-								header: 'Referrer-Policy',
-								val1: 'no-referrer',
-								val2: 'no-referrer-when-downgrade',
-								val3: 'strict-origin',
-								val4: 'strict-origin-when-cross-origin',
-								val5: 'same-origin'
-							})
-							.replace('{linkstart}', '<a target="_blank" rel="noreferrer noopener" class="external" href="https://www.w3.org/TR/referrer-policy/">')
-							.replace('{linkend}', '</a>'),
-						type: OC.SetupChecks.MESSAGE_TYPE_INFO
-					})
-				}
-			} else {
-				messages.push({
-					msg: t('core', 'Error occurred while checking server setup'),
-					type: OC.SetupChecks.MESSAGE_TYPE_ERROR
-				});
-			}
-
-			return messages;
-		},
-
 		/**
 		 * Runs check for some SSL configuration issues on the server side
 		 *