From 310377e496ef049340e10b318bd9498b0fa85f0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 7 Mar 2024 18:39:38 +0100 Subject: [PATCH] fix: Fix Security headers setup check behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../lib/SetupChecks/SecurityHeaders.php | 5 +- core/js/setupchecks.js | 70 ------------------- 2 files changed, 3 insertions(+), 72 deletions(-) diff --git a/apps/settings/lib/SetupChecks/SecurityHeaders.php b/apps/settings/lib/SetupChecks/SecurityHeaders.php index d5239d5a1b13a..9079df7e39bf3 100644 --- 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 index 00120d678a8d0..d11f05858c4f9 100644 --- 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}', '') - .replace('{linkend}', ''), - 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 *