feat: Migrate HSTS check to Security headers SetupCheck

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

5 files changed, 29 insertions, 266 deletions

diff --git a/apps/settings/lib/SetupChecks/SecurityHeaders.php b/apps/settings/lib/SetupChecks/SecurityHeaders.php
index 4d89b198f89..f1d66188744 100644
--- a/apps/settings/lib/SetupChecks/SecurityHeaders.php
+++ b/apps/settings/lib/SetupChecks/SecurityHeaders.php
@@ -113,8 +113,26 @@ class SecurityHeaders implements ISetupCheck {
 						'link' => 'https://www.w3.org/TR/referrer-policy/',
 					];
 				}
+
+				$transportSecurityValidity = $response->getHeader('Strict-Transport-Security');
+				$minimumSeconds = 15552000;
+				if (preg_match('/^max-age=(\d+)(;.*)?$/', $transportSecurityValidity, $m)) {
+					$transportSecurityValidity = (int)$m[1];
+					if ($transportSecurityValidity < $minimumSeconds) {
+						$msg .= $this->l10n->t('- The `Strict-Transport-Security` HTTP header is not set to at least `%d` seconds (current value: `%d`). For enhanced security, it is recommended to enable HSTS.', [$minimumSeconds, $transportSecurityValidity])."\n";
+					}
+				} elseif (!empty($transportSecurityValidity)) {
+					$msg .= $this->l10n->t('- The `Strict-Transport-Security` HTTP header is malformed: `%s`. For enhanced security, it is recommended to enable HSTS.', [$transportSecurityValidity])."\n";
+				} else {
+					$msg .= $this->l10n->t('- The `Strict-Transport-Security` HTTP header is not set (should be at least `%d` seconds). For enhanced security, it is recommended to enable HSTS.', [$minimumSeconds])."\n";
+				}
+
 				if (!empty($msg)) {
-					return SetupResult::warning($this->l10n->t('Some headers are not set correctly on your instance')."\n".$msg, descriptionParameters:$msgParameters);
+					return SetupResult::warning(
+						$this->l10n->t('Some headers are not set correctly on your instance')."\n".$msg,
+						$this->urlGenerator->linkToDocs('admin-security'),
+						$msgParameters,
+					);
 				}
 				// Skip the other requests if one works
 				$works = true;
@@ -124,12 +142,14 @@ class SecurityHeaders implements ISetupCheck {
 			if ($works === null) {
 				return SetupResult::info(
 					$this->l10n->t('Could not check that your web server serves security headers correctly. Please check manually.'),
+					$this->urlGenerator->linkToDocs('admin-security'),
 				);
 			}
 			// Otherwise if we fail we can abort here
 			if ($works === false) {
 				return SetupResult::warning(
 					$this->l10n->t("Could not check that your web server serves security headers correctly, unable to query `%s`", [$url]),
+					$this->urlGenerator->linkToDocs('admin-security'),
 				);
 			}
 		}
diff --git a/apps/settings/src/admin.js b/apps/settings/src/admin.js
index 09034495529..8b5ae1080e3 100644
--- a/apps/settings/src/admin.js
+++ b/apps/settings/src/admin.js
@@ -103,9 +103,8 @@ window.addEventListener('DOMContentLoaded', () => {
 		$.when(
 			OC.SetupChecks.checkWebDAV(),
 			OC.SetupChecks.checkSetup(),
-			OC.SetupChecks.checkGeneric(),
-		).then((check1, check2, check3) => {
-			const messages = [].concat(check1, check2, check3)
+		).then((check1, check2) => {
+			const messages = [].concat(check1, check2)
 			const $el = $('#postsetupchecks')
 			$('#security-warning-state-loading').addClass('hidden')
 
diff --git a/apps/settings/tests/SetupChecks/SecurityHeadersTest.php b/apps/settings/tests/SetupChecks/SecurityHeadersTest.php
index 4f3304a081d..0856cca38ca 100644
--- a/apps/settings/tests/SetupChecks/SecurityHeadersTest.php
+++ b/apps/settings/tests/SetupChecks/SecurityHeadersTest.php
@@ -120,6 +120,9 @@ class SecurityHeadersTest extends TestCase {
 			'referrer-strict-origin' => [['Referrer-Policy' => 'strict-origin']],
 			'referrer-strict-origin-when-cross-origin' => [['Referrer-Policy' => 'strict-origin-when-cross-origin']],
 			'referrer-same-origin' => [['Referrer-Policy' => 'same-origin']],
+			'hsts-minimum' => [['Strict-Transport-Security' => 'max-age=15552000']],
+			'hsts-include-subdomains' => [['Strict-Transport-Security' => 'max-age=99999999; includeSubDomains']],
+			'hsts-include-subdomains-preload' => [['Strict-Transport-Security' => 'max-age=99999999; preload; includeSubDomains']],
 		];
 	}
 
@@ -161,6 +164,9 @@ class SecurityHeadersTest extends TestCase {
 			'referrer-origin' => [['Referrer-Policy' => 'origin'], "- The `Referrer-Policy` HTTP header is not set to `no-referrer`, `no-referrer-when-downgrade`, `strict-origin`, `strict-origin-when-cross-origin` or `same-origin`. This can leak referer information. See the {w3c-recommendation}.\n"],
 			'referrer-origin-when-cross-origin' => [['Referrer-Policy' => 'origin-when-cross-origin'], "- The `Referrer-Policy` HTTP header is not set to `no-referrer`, `no-referrer-when-downgrade`, `strict-origin`, `strict-origin-when-cross-origin` or `same-origin`. This can leak referer information. See the {w3c-recommendation}.\n"],
 			'referrer-unsafe-url' => [['Referrer-Policy' => 'unsafe-url'], "- The `Referrer-Policy` HTTP header is not set to `no-referrer`, `no-referrer-when-downgrade`, `strict-origin`, `strict-origin-when-cross-origin` or `same-origin`. This can leak referer information. See the {w3c-recommendation}.\n"],
+			'hsts-missing' => [['Strict-Transport-Security' => ''], "- The `Strict-Transport-Security` HTTP header is not set (should be at least `15552000` seconds). For enhanced security, it is recommended to enable HSTS.\n"],
+			'hsts-too-low' => [['Strict-Transport-Security' => 'max-age=15551999'], "- The `Strict-Transport-Security` HTTP header is not set to at least `15552000` seconds (current value: `15551999`). For enhanced security, it is recommended to enable HSTS.\n"],
+			'hsts-malformed' => [['Strict-Transport-Security' => 'iAmABogusHeader342'], "- The `Strict-Transport-Security` HTTP header is malformed: `iAmABogusHeader342`. For enhanced security, it is recommended to enable HSTS.\n"],
 		];
 	}
 
diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js
index d11f05858c4..0c0e673eae7 100644
--- a/core/js/setupchecks.js
+++ b/core/js/setupchecks.js
@@ -156,73 +156,5 @@
 				})
 			}
 		},
-
-		/**
-		 * Runs generic checks on the server side, the difference to dedicated
-		 * methods is that we use the same XHR object for all checks to save
-		 * requests.
-		 *
-		 * @return $.Deferred object resolved with an array of error messages
-		 */
-		checkGeneric: function() {
-			var self = this;
-			var deferred = $.Deferred();
-			var afterCall = function(data, statusText, xhr) {
-				var messages = [];
-				messages = messages.concat(self._checkSSL(xhr));
-				deferred.resolve(messages);
-			};
-
-			$.ajax({
-				type: 'GET',
-				url: OC.generateUrl('heartbeat'),
-				allowAuthErrors: true
-			}).then(afterCall, afterCall);
-
-			return deferred.promise();
-		},
-
-		/**
-		 * Runs check for some SSL configuration issues on the server side
-		 *
-		 * @param {Object} xhr
-		 * @return {Array} Array with error messages
-		 */
-		_checkSSL: function(xhr) {
-			var messages = [];
-
-			if (xhr.status === 200) {
-				var tipsUrl = OC.theme.docPlaceholderUrl.replace('PLACEHOLDER', 'admin-security');
-				if(OC.getProtocol() === 'https') {
-					// Extract the value of 'Strict-Transport-Security'
-					var transportSecurityValidity = xhr.getResponseHeader('Strict-Transport-Security');
-					if(transportSecurityValidity !== null && transportSecurityValidity.length > 8) {
-						var firstComma = transportSecurityValidity.indexOf(";");
-						if(firstComma !== -1) {
-							transportSecurityValidity = transportSecurityValidity.substring(8, firstComma);
-						} else {
-							transportSecurityValidity = transportSecurityValidity.substring(8);
-						}
-					}
-
-					var minimumSeconds = 15552000;
-					if(isNaN(transportSecurityValidity) || transportSecurityValidity <= (minimumSeconds - 1)) {
-						messages.push({
-							msg: t('core', 'The "Strict-Transport-Security" HTTP header is not set to at least "{seconds}" seconds. For enhanced security, it is recommended to enable HSTS as described in the {linkstart}security tips ↗{linkend}.', {'seconds': minimumSeconds})
-								.replace('{linkstart}', '<a target="_blank" rel="noreferrer noopener" class="external" href="' + tipsUrl + '">')
-								.replace('{linkend}', '</a>'),
-							type: OC.SetupChecks.MESSAGE_TYPE_WARNING
-						});
-					}
-				}
-			} else {
-				messages.push({
-					msg: t('core', 'Error occurred while checking server setup'),
-					type: OC.SetupChecks.MESSAGE_TYPE_ERROR
-				});
-			}
-
-			return messages;
-		}
 	};
 })();
diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js
index b027bfd21bd..99f72754ace 100644
--- a/core/js/tests/specs/setupchecksSpec.js
+++ b/core/js/tests/specs/setupchecksSpec.js
@@ -320,198 +320,4 @@ describe('OC.SetupChecks tests', function() {
 			});
 		});
 	});
-
-	describe('checkGeneric', function() {
-		it('should return an error if the response has no statuscode 200', function(done) {
-			var async = OC.SetupChecks.checkGeneric();
-
-			suite.server.requests[0].respond(
-				500,
-				{
-					'Content-Type': 'application/json'
-				}
-			);
-
-			async.done(function( data, s, x ){
-				expect(data).toEqual([{
-					msg: 'Error occurred while checking server setup',
-					type: OC.SetupChecks.MESSAGE_TYPE_ERROR
-				}]);
-				done();
-			});
-		});
-	});
-
-	it('should return an error if the response has no statuscode 200', function(done) {
-		var async = OC.SetupChecks.checkGeneric();
-
-		suite.server.requests[0].respond(
-			500,
-			{
-				'Content-Type': 'application/json'
-			},
-			JSON.stringify({data: {serverHasInternetConnectionProblems: true}})
-		);
-		async.done(function( data, s, x ){
-			expect(data).toEqual([{
-				msg: 'Error occurred while checking server setup',
-				type: OC.SetupChecks.MESSAGE_TYPE_ERROR
-			}]);
-			done();
-		});
-	});
-
-	it('should return a SSL warning if SSL used without Strict-Transport-Security-Header', function(done) {
-		protocolStub.returns('https');
-		var async = OC.SetupChecks.checkGeneric();
-
-		suite.server.requests[0].respond(200,
-			{
-				'X-XSS-Protection': '1; mode=block',
-				'X-Content-Type-Options': 'nosniff',
-				'X-Robots-Tag': 'noindex, nofollow',
-				'X-Frame-Options': 'SAMEORIGIN',
-				'X-Permitted-Cross-Domain-Policies': 'none',
-				'Referrer-Policy': 'no-referrer',
-			}
-		);
-
-		async.done(function( data, s, x ){
-			expect(data).toEqual([{
-				msg: 'The "Strict-Transport-Security" HTTP header is not set to at least "15552000" seconds. For enhanced security, it is recommended to enable HSTS as described in the <a target="_blank" rel="noreferrer noopener" class="external" href="https://docs.example.org/admin-security">security tips ↗</a>.',
-				type: OC.SetupChecks.MESSAGE_TYPE_WARNING
-			}]);
-			done();
-		});
-	});
-
-	it('should return a SSL warning if SSL used with to small Strict-Transport-Security-Header', function(done) {
-		protocolStub.returns('https');
-		var async = OC.SetupChecks.checkGeneric();
-
-		suite.server.requests[0].respond(200,
-			{
-				'Strict-Transport-Security': 'max-age=15551999',
-				'X-XSS-Protection': '1; mode=block',
-				'X-Content-Type-Options': 'nosniff',
-				'X-Robots-Tag': 'noindex, nofollow',
-				'X-Frame-Options': 'SAMEORIGIN',
-				'X-Permitted-Cross-Domain-Policies': 'none',
-				'Referrer-Policy': 'no-referrer',
-			}
-		);
-
-		async.done(function( data, s, x ){
-			expect(data).toEqual([{
-				msg: 'The "Strict-Transport-Security" HTTP header is not set to at least "15552000" seconds. For enhanced security, it is recommended to enable HSTS as described in the <a target="_blank" rel="noreferrer noopener" class="external" href="https://docs.example.org/admin-security">security tips ↗</a>.',
-				type: OC.SetupChecks.MESSAGE_TYPE_WARNING
-			}]);
-			done();
-		});
-	});
-
-	it('should return a SSL warning if SSL used with to a bogus Strict-Transport-Security-Header', function(done) {
-		protocolStub.returns('https');
-		var async = OC.SetupChecks.checkGeneric();
-
-		suite.server.requests[0].respond(200,
-			{
-				'Strict-Transport-Security': 'iAmABogusHeader342',
-				'X-XSS-Protection': '1; mode=block',
-				'X-Content-Type-Options': 'nosniff',
-				'X-Robots-Tag': 'noindex, nofollow',
-				'X-Frame-Options': 'SAMEORIGIN',
-				'X-Permitted-Cross-Domain-Policies': 'none',
-				'Referrer-Policy': 'no-referrer',
-			}
-		);
-
-		async.done(function( data, s, x ){
-			expect(data).toEqual([{
-				msg: 'The "Strict-Transport-Security" HTTP header is not set to at least "15552000" seconds. For enhanced security, it is recommended to enable HSTS as described in the <a target="_blank" rel="noreferrer noopener" class="external" href="https://docs.example.org/admin-security">security tips ↗</a>.',
-				type: OC.SetupChecks.MESSAGE_TYPE_WARNING
-			}]);
-			done();
-		});
-	});
-
-	it('should return no SSL warning if SSL used with to exact the minimum Strict-Transport-Security-Header', function(done) {
-		protocolStub.returns('https');
-		var async = OC.SetupChecks.checkGeneric();
-
-		suite.server.requests[0].respond(200, {
-			'Strict-Transport-Security': 'max-age=15768000',
-			'X-XSS-Protection': '1; mode=block',
-			'X-Content-Type-Options': 'nosniff',
-			'X-Robots-Tag': 'noindex, nofollow',
-			'X-Frame-Options': 'SAMEORIGIN',
-			'X-Permitted-Cross-Domain-Policies': 'none',
-			'Referrer-Policy': 'no-referrer',
-		});
-
-		async.done(function( data, s, x ){
-			expect(data).toEqual([]);
-			done();
-		});
-	});
-
-	it('should return no SSL warning if SSL used with to more than the minimum Strict-Transport-Security-Header', function(done) {
-		protocolStub.returns('https');
-		var async = OC.SetupChecks.checkGeneric();
-
-		suite.server.requests[0].respond(200, {
-			'Strict-Transport-Security': 'max-age=99999999',
-			'X-XSS-Protection': '1; mode=block',
-			'X-Content-Type-Options': 'nosniff',
-			'X-Robots-Tag': 'noindex, nofollow',
-			'X-Frame-Options': 'SAMEORIGIN',
-			'X-Permitted-Cross-Domain-Policies': 'none',
-			'Referrer-Policy': 'no-referrer',
-		});
-
-		async.done(function( data, s, x ){
-			expect(data).toEqual([]);
-			done();
-		});
-	});
-
-	it('should return no SSL warning if SSL used with to more than the minimum Strict-Transport-Security-Header and includeSubDomains parameter', function(done) {
-		protocolStub.returns('https');
-		var async = OC.SetupChecks.checkGeneric();
-
-		suite.server.requests[0].respond(200, {
-			'Strict-Transport-Security': 'max-age=99999999; includeSubDomains',
-			'X-XSS-Protection': '1; mode=block',
-			'X-Content-Type-Options': 'nosniff',
-			'X-Robots-Tag': 'noindex, nofollow',
-			'X-Frame-Options': 'SAMEORIGIN',
-			'X-Permitted-Cross-Domain-Policies': 'none',
-			'Referrer-Policy': 'no-referrer',
-		});
-
-		async.done(function( data, s, x ){
-			expect(data).toEqual([]);
-			done();
-		});
-	});
-
-	it('should return no SSL warning if SSL used with to more than the minimum Strict-Transport-Security-Header and includeSubDomains and preload parameter', function(done) {
-		protocolStub.returns('https');
-		var async = OC.SetupChecks.checkGeneric();
-
-		suite.server.requests[0].respond(200, {
-			'Strict-Transport-Security': 'max-age=99999999; preload; includeSubDomains',
-			'X-XSS-Protection': '1; mode=block',
-			'X-Content-Type-Options': 'nosniff',
-			'X-Robots-Tag': 'noindex, nofollow',
-			'X-Frame-Options': 'SAMEORIGIN',
-			'X-Permitted-Cross-Domain-Policies': 'none',
-			'Referrer-Policy': 'no-referrer',
-		});
-
-		async.done(function( data, s, x ){
-			expect(data).toEqual([]);
-			done();
-		});
-	});
 });