From 74095699940184048e4cdd84360f41cc08a12528 Mon Sep 17 00:00:00 2001 From: Martin Brugnara Date: Sat, 4 Jun 2022 00:24:35 +0200 Subject: [PATCH] Expose umask override value as config parameter: localstorage.umask Commit 451c06d introduced override for umask value. This is needed to avoid broken env configuration or dirty workers to mess with the permissions when creating new files. Most Nextcloud, that does not integrate with external software would work fine with an hard-coded value (451c06d set it at 022). Advanced install may require more flexibility, as such this commit exposes the "umask override value" as configuration parameter: `localstorage.umask` It defaults to 0022 both in code and in config/config.sample.php . Signed-off-by: Martin Brugnara --- config/config.sample.php | 12 ++++++++++++ lib/private/Files/Storage/Local.php | 14 +++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/config/config.sample.php b/config/config.sample.php index c3a0048625ca2..4cb19c8d9e9e3 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -1839,6 +1839,18 @@ */ 'localstorage.allowsymlinks' => false, +/** + * Nextcloud overrides umask to ensure suitable access permissions + * regardless of webserver/php-fpm configuration and worker state. + * WARNING: Modifying this value has security implications and + * may soft-break the installation. + * + * Most installs shall not modify this value. + * + * Defaults to ``0022`` + */ +'localstorage.umask' => 0022, + /** * EXPERIMENTAL: option whether to include external storage in quota * calculation, defaults to false. diff --git a/lib/private/Files/Storage/Local.php b/lib/private/Files/Storage/Local.php index ee8a8c7d16130..4996572a40e06 100644 --- a/lib/private/Files/Storage/Local.php +++ b/lib/private/Files/Storage/Local.php @@ -15,6 +15,7 @@ * @author Jörn Friedrich Dreyer * @author Klaas Freitag * @author Lukas Reschke + * @author Martin Brugnara * @author Michael Gapczynski * @author Morris Jobke * @author Robin Appelman @@ -66,6 +67,8 @@ class Local extends \OC\Files\Storage\Common { private IMimeTypeDetector $mimeTypeDetector; + private $defUMask; + public function __construct($arguments) { if (!isset($arguments['datadir']) || !is_string($arguments['datadir'])) { throw new \InvalidArgumentException('No data directory set for local storage'); @@ -84,6 +87,7 @@ public function __construct($arguments) { $this->dataDirLength = strlen($this->realDataDir); $this->config = \OC::$server->get(IConfig::class); $this->mimeTypeDetector = \OC::$server->get(IMimeTypeDetector::class); + $this->defUMask = $this->config->getSystemValue('localstorage.umask', 0022); } public function __destruct() { @@ -95,7 +99,7 @@ public function getId() { public function mkdir($path) { $sourcePath = $this->getSourcePath($path); - $oldMask = umask(022); + $oldMask = umask($this->defUMask); $result = @mkdir($sourcePath, 0777, true); umask($oldMask); return $result; @@ -273,7 +277,7 @@ public function touch($path, $mtime = null) { if ($this->file_exists($path) and !$this->isUpdatable($path)) { return false; } - $oldMask = umask(022); + $oldMask = umask($this->defUMask); if (!is_null($mtime)) { $result = @touch($this->getSourcePath($path), $mtime); } else { @@ -292,7 +296,7 @@ public function file_get_contents($path) { } public function file_put_contents($path, $data) { - $oldMask = umask(022); + $oldMask = umask($this->defUMask); $result = file_put_contents($this->getSourcePath($path), $data); umask($oldMask); return $result; @@ -365,7 +369,7 @@ public function copy($path1, $path2) { if ($this->is_dir($path1)) { return parent::copy($path1, $path2); } else { - $oldMask = umask(022); + $oldMask = umask($this->defUMask); $result = copy($this->getSourcePath($path1), $this->getSourcePath($path2)); umask($oldMask); return $result; @@ -373,7 +377,7 @@ public function copy($path1, $path2) { } public function fopen($path, $mode) { - $oldMask = umask(022); + $oldMask = umask($this->defUMask); $result = fopen($this->getSourcePath($path), $mode); umask($oldMask); return $result;