Skip to content

Commit 5415139

Browse files
committed
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>
1 parent 19c8c63 commit 5415139

File tree

7 files changed

+61
-13
lines changed

7 files changed

+61
-13
lines changed

apps/settings/lib/SetupChecks/DataDirectoryProtected.php

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,21 @@ public function getName(): string {
4242
public function run(): SetupResult {
4343
$datadir = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValue('datadirectory', ''));
4444

45-
$dataUrl = $this->urlGenerator->getWebroot() . '/' . $datadir . '/.ocdata';
45+
$dataUrl = $this->urlGenerator->getWebroot() . '/' . $datadir . '/.ncdata';
4646

4747
$noResponse = true;
48-
foreach ($this->runHEAD($dataUrl, httpErrors:false) as $response) {
48+
foreach ($this->runRequest('GET', $dataUrl, [ 'httpErrors' => false ]) as $response) {
4949
$noResponse = false;
50-
if ($response->getStatusCode() === 200) {
51-
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.'));
50+
if ($response->getStatusCode() < 400) {
51+
// Read the response body
52+
$body = $response->getBody();
53+
if (is_resource($body)) {
54+
$body = stream_get_contents($body, 64);
55+
}
56+
57+
if (str_contains($body, '# Nextcloud data directory')) {
58+
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.'));
59+
}
5260
} else {
5361
$this->logger->debug('[expected] Could not access data directory from outside.', ['url' => $dataUrl]);
5462
}

lib/private/Repair.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
use OC\Repair\NC22\LookupServerSendCheck;
4343
use OC\Repair\NC24\AddTokenCleanupJob;
4444
use OC\Repair\NC25\AddMissingSecretJob;
45+
use OC\Repair\NC30\RemoveLegacyDatadirFile;
4546
use OC\Repair\OldGroupMembershipShares;
4647
use OC\Repair\Owncloud\CleanPreviews;
4748
use OC\Repair\Owncloud\DropAccountTermsTable;
@@ -187,6 +188,7 @@ public static function getRepairSteps(): array {
187188
\OCP\Server::get(AddRemoveOldTasksBackgroundJob::class),
188189
\OCP\Server::get(AddMetadataGenerationJob::class),
189190
\OCP\Server::get(AddAppConfigLazyMigration::class),
191+
\OCP\Server::get(RemoveLegacyDatadirFile::class),
190192
];
191193
}
192194

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OC\Repair\NC30;
10+
11+
use OCP\IConfig;
12+
use OCP\Migration\IOutput;
13+
use OCP\Migration\IRepairStep;
14+
15+
class RemoveLegacyDatadirFile implements IRepairStep {
16+
17+
public function __construct(
18+
private IConfig $config,
19+
) {
20+
}
21+
22+
public function getName(): string {
23+
return 'Remove legacy ".ocdata" file';
24+
}
25+
26+
public function run(IOutput $output): void {
27+
$ocdata = $this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ocdata';
28+
if (file_exists($ocdata)) {
29+
unlink($ocdata);
30+
}
31+
}
32+
}

lib/private/Setup.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,9 +360,12 @@ public function install(array $options, ?IOutput $output = null): array {
360360
Installer::installShippedApps(false, $output);
361361

362362
// create empty file in data dir, so we can later find
363-
// out that this is indeed an ownCloud data directory
363+
// out that this is indeed a Nextcloud data directory
364364
$this->outputDebug($output, 'Setup data directory');
365-
file_put_contents($config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ocdata', '');
365+
file_put_contents(
366+
$config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ncdata',
367+
"# Nextcloud data directory\n# Do not change this file",
368+
);
366369

367370
// Update .htaccess files
368371
self::updateHtaccess();

lib/private/Updater.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,12 @@ private function doUpgrade(string $currentVersion, string $installedVersion): vo
208208
}
209209

210210
// create empty file in data dir, so we can later find
211-
// out that this is indeed an ownCloud data directory
211+
// out that this is indeed a Nextcloud data directory
212212
// (in case it didn't exist before)
213-
file_put_contents($this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ocdata', '');
213+
file_put_contents(
214+
$this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/.ncdata',
215+
"# Nextcloud data directory\n# Do not change this file",
216+
);
214217

215218
// pre-upgrade repairs
216219
$repair = \OCP\Server::get(Repair::class);

lib/private/User/Manager.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,7 @@ private function verifyUid(string $uid, bool $checkDataDirectory = false): bool
783783
'.htaccess',
784784
'files_external',
785785
'__groupfolders',
786-
'.ocdata',
786+
'.ncdata',
787787
'owncloud.log',
788788
'nextcloud.log',
789789
'updater.log',

lib/private/legacy/OC_Util.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,7 @@ public static function checkDataDirectoryPermissions($dataDirectory) {
687687

688688
/**
689689
* Check that the data directory exists and is valid by
690-
* checking the existence of the ".ocdata" file.
690+
* checking the existence of the ".ncdata" file.
691691
*
692692
* @param string $dataDirectory data directory path
693693
* @return array errors found
@@ -701,11 +701,11 @@ public static function checkDataDirectoryValidity($dataDirectory) {
701701
'hint' => $l->t('Check the value of "datadirectory" in your configuration.')
702702
];
703703
}
704-
if (!file_exists($dataDirectory . '/.ocdata')) {
704+
705+
if (!file_exists($dataDirectory . '/.ncdata')) {
705706
$errors[] = [
706707
'error' => $l->t('Your data directory is invalid.'),
707-
'hint' => $l->t('Ensure there is a file called ".ocdata"' .
708-
' in the root of the data directory.')
708+
'hint' => $l->t('Ensure there is a file called ".ncdata" in the root of the data directory.')
709709
];
710710
}
711711
return $errors;

0 commit comments

Comments
 (0)