From 5415139a21d5f3089e646b07bc1e979fe330b92e Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 11 Jul 2024 20:53:37 +0200 Subject: [PATCH] fix(SetupCheck): Properly check public access to data directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../SetupChecks/DataDirectoryProtected.php | 16 +++++++--- lib/private/Repair.php | 2 ++ .../Repair/NC30/RemoveLegacyDatadirFile.php | 32 +++++++++++++++++++ lib/private/Setup.php | 7 ++-- lib/private/Updater.php | 7 ++-- lib/private/User/Manager.php | 2 +- lib/private/legacy/OC_Util.php | 8 ++--- 7 files changed, 61 insertions(+), 13 deletions(-) create mode 100644 lib/private/Repair/NC30/RemoveLegacyDatadirFile.php diff --git a/apps/settings/lib/SetupChecks/DataDirectoryProtected.php b/apps/settings/lib/SetupChecks/DataDirectoryProtected.php index 5afdfaaddd557..447c66d5c5de4 100644 --- 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/private/Repair.php b/lib/private/Repair.php index ed9cd400f617a..3c0d1d49f77fd 100644 --- 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; @@ -187,6 +188,7 @@ public static function getRepairSteps(): array { \OCP\Server::get(AddRemoveOldTasksBackgroundJob::class), \OCP\Server::get(AddMetadataGenerationJob::class), \OCP\Server::get(AddAppConfigLazyMigration::class), + \OCP\Server::get(RemoveLegacyDatadirFile::class), ]; } diff --git a/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php b/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php new file mode 100644 index 0000000000000..623163927bd3f --- /dev/null +++ b/lib/private/Repair/NC30/RemoveLegacyDatadirFile.php @@ -0,0 +1,32 @@ +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 index a568365544746..c991f4a8127cb 100644 --- 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 index 6d23e81aa6383..e26faf86f924a 100644 --- 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 index 639ce507f4d35..2c8cc10dc15ae 100644 --- 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 index b7836e86d7b7b..8e34eedd44b5f 100644 --- 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;