fix(SetupCheck): Properly check public access to data directory

When checking for public (web) access to the data directory the status is not enough as you might have a webserver that forwards to e.g. a login page. So instead check that the content of the file matches. For this the `.ncdata` file (renamed from `.ocdata`¹) has minimal text content to allow checking. ¹The file was renamed from the legacy `.ocdata`, there is a repair step to remove the old one. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
nextcloud · Jul 30, 2024 · 9cecc20 · 9cecc20
1 parent 21f558b
commit 9cecc20
Show file tree

Hide file tree

Showing 9 changed files with 63 additions and 13 deletions.
diff --git a/apps/settings/lib/SetupChecks/DataDirectoryProtected.php b/apps/settings/lib/SetupChecks/DataDirectoryProtected.php
@@ -42,13 +42,21 @@ public function getName(): string {
 	public function run(): SetupResult {
 		$datadir = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValue('datadirectory', ''));
 
-		$dataUrl = $this->urlGenerator->getWebroot() . '/' . $datadir . '/.ocdata';
+		$dataUrl = $this->urlGenerator->getWebroot() . '/' . $datadir . '/.ncdata';
 
 		$noResponse = true;
-		foreach ($this->runHEAD($dataUrl, httpErrors:false) as $response) {
+		foreach ($this->runRequest('GET', $dataUrl, [ 'httpErrors' => false ]) as $response) {
 			$noResponse = false;
-			if ($response->getStatusCode() === 200) {
-				return SetupResult::error($this->l10n->t('Your data directory and files are probably accessible from the internet. The .htaccess file is not working. It is strongly recommended that you configure your web server so that the data directory is no longer accessible, or move the data directory outside the web server document root.'));
+			if ($response->getStatusCode() < 400) {
+				// Read the response body
+				$body = $response->getBody();
+				if (is_resource($body)) {
+					$body = stream_get_contents($body, 64);
+				}
+
+				if (str_contains($body, '# Nextcloud data directory')) {
+					return SetupResult::error($this->l10n->t('Your data directory and files are probably accessible from the internet. The .htaccess file is not working. It is strongly recommended that you configure your web server so that the data directory is no longer accessible, or move the data directory outside the web server document root.'));
+				}
 			} else {
 				$this->logger->debug('[expected] Could not access data directory from outside.', ['url' => $dataUrl]);
 			}

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
@@ -1776,6 +1776,7 @@
     'OC\\Repair\\NC22\\LookupServerSendCheck' => $baseDir . '/lib/private/Repair/NC22/LookupServerSendCheck.php',
     'OC\\Repair\\NC24\\AddTokenCleanupJob' => $baseDir . '/lib/private/Repair/NC24/AddTokenCleanupJob.php',
     'OC\\Repair\\NC25\\AddMissingSecretJob' => $baseDir . '/lib/private/Repair/NC25/AddMissingSecretJob.php',
+    'OC\\Repair\\NC30\\RemoveLegacyDatadirFile' => $baseDir . '/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php',
     'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.php',
     'OC\\Repair\\Owncloud\\CleanPreviews' => $baseDir . '/lib/private/Repair/Owncloud/CleanPreviews.php',
     'OC\\Repair\\Owncloud\\CleanPreviewsBackgroundJob' => $baseDir . '/lib/private/Repair/Owncloud/CleanPreviewsBackgroundJob.php',

diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
@@ -1809,6 +1809,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Repair\\NC22\\LookupServerSendCheck' => __DIR__ . '/../../..' . '/lib/private/Repair/NC22/LookupServerSendCheck.php',
         'OC\\Repair\\NC24\\AddTokenCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC24/AddTokenCleanupJob.php',
         'OC\\Repair\\NC25\\AddMissingSecretJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC25/AddMissingSecretJob.php',
+        'OC\\Repair\\NC30\\RemoveLegacyDatadirFile' => __DIR__ . '/../../..' . '/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php',
         'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php',
         'OC\\Repair\\Owncloud\\CleanPreviews' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/CleanPreviews.php',
         'OC\\Repair\\Owncloud\\CleanPreviewsBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/CleanPreviewsBackgroundJob.php',

diff --git a/lib/private/Repair.php b/lib/private/Repair.php
@@ -42,6 +42,7 @@
 use OC\Repair\NC22\LookupServerSendCheck;
 use OC\Repair\NC24\AddTokenCleanupJob;
 use OC\Repair\NC25\AddMissingSecretJob;
+use OC\Repair\NC30\RemoveLegacyDatadirFile;
 use OC\Repair\OldGroupMembershipShares;
 use OC\Repair\Owncloud\CleanPreviews;
 use OC\Repair\Owncloud\DropAccountTermsTable;
@@ -189,6 +190,7 @@ public static function getRepairSteps(): array {
 			\OCP\Server::get(AddMetadataGenerationJob::class),
 			\OCP\Server::get(AddAppConfigLazyMigration::class),
 			\OCP\Server::get(RepairLogoDimension::class),
+			\OCP\Server::get(RemoveLegacyDatadirFile::class),
 		];
 	}
 

diff --git a/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php b/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php
@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OC\Repair\NC30;
+
+use OCP\IConfig;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+
+class RemoveLegacyDatadirFile implements IRepairStep {
+
+	public function __construct(
+		private IConfig $config,
+	) {
+	}
+
+	public function getName(): string {
+		return 'Remove legacy ".ocdata" file';
+	}
+
+	public function run(IOutput $output): void {
+		$ocdata = $this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ocdata';
+		if (file_exists($ocdata)) {
+			unlink($ocdata);
+		}
+	}
+}
diff --git a/lib/private/Setup.php b/lib/private/Setup.php
@@ -360,9 +360,12 @@ public function install(array $options, ?IOutput $output = null): array {
 		Installer::installShippedApps(false, $output);
 
 		// create empty file in data dir, so we can later find
-		// out that this is indeed an ownCloud data directory
+		// out that this is indeed a Nextcloud data directory
 		$this->outputDebug($output, 'Setup data directory');
-		file_put_contents($config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ocdata', '');
+		file_put_contents(
+			$config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ncdata',
+			"# Nextcloud data directory\n# Do not change this file",
+		);
 
 		// Update .htaccess files
 		self::updateHtaccess();

diff --git a/lib/private/Updater.php b/lib/private/Updater.php
@@ -208,9 +208,12 @@ private function doUpgrade(string $currentVersion, string $installedVersion): vo
 		}
 
 		// create empty file in data dir, so we can later find
-		// out that this is indeed an ownCloud data directory
+		// out that this is indeed a Nextcloud data directory
 		// (in case it didn't exist before)
-		file_put_contents($this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ocdata', '');
+		file_put_contents(
+			$this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ncdata',
+			"# Nextcloud data directory\n# Do not change this file",
+		);
 
 		// pre-upgrade repairs
 		$repair = \OCP\Server::get(Repair::class);

diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php
@@ -783,7 +783,7 @@ private function verifyUid(string $uid, bool $checkDataDirectory = false): bool
 			'.htaccess',
 			'files_external',
 			'__groupfolders',
-			'.ocdata',
+			'.ncdata',
 			'owncloud.log',
 			'nextcloud.log',
 			'updater.log',

diff --git a/lib/private/legacy/OC_Util.php b/lib/private/legacy/OC_Util.php
@@ -687,7 +687,7 @@ public static function checkDataDirectoryPermissions($dataDirectory) {
 
 	/**
 	 * Check that the data directory exists and is valid by
-	 * checking the existence of the ".ocdata" file.
+	 * checking the existence of the ".ncdata" file.
 	 *
 	 * @param string $dataDirectory data directory path
 	 * @return array errors found
@@ -701,11 +701,11 @@ public static function checkDataDirectoryValidity($dataDirectory) {
 				'hint' => $l->t('Check the value of "datadirectory" in your configuration.')
 			];
 		}
-		if (!file_exists($dataDirectory . '/.ocdata')) {
+
+		if (!file_exists($dataDirectory . '/.ncdata')) {
 			$errors[] = [
 				'error' => $l->t('Your data directory is invalid.'),
-				'hint' => $l->t('Ensure there is a file called ".ocdata"' .
-					' in the root of the data directory.')
+				'hint' => $l->t('Ensure there is a file called ".ncdata" in the root of the data directory.')
 			];
 		}
 		return $errors;