From 2c76afb839b91bba23d17160c7c747bc476cefa1 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Sat, 21 Nov 2020 22:14:58 +0100 Subject: [PATCH] Fixed disk permission tests --- docker/tests/mono/complete/Dockerfile | 3 + docker/tests/mono/sonarr/Dockerfile | 3 + .../DiskProviderTests/DiskProviderFixture.cs | 132 ++++++++++++++++-- src/NzbDrone.Mono/Disk/DiskProvider.cs | 6 +- 4 files changed, 130 insertions(+), 14 deletions(-) diff --git a/docker/tests/mono/complete/Dockerfile b/docker/tests/mono/complete/Dockerfile index 5f4e68de9..272aed8ca 100644 --- a/docker/tests/mono/complete/Dockerfile +++ b/docker/tests/mono/complete/Dockerfile @@ -17,6 +17,9 @@ RUN fromdos /startup.sh WORKDIR /data/ VOLUME ["/data/_tests_linux", "/data/_output_linux", "/data/_tests_results"] + +RUN groupadd sonarrtst -g 4020 && useradd sonarrtst -u 4021 -g 4020 -m -s /bin/bash +USER sonarrtst CMD bash /startup.sh diff --git a/docker/tests/mono/sonarr/Dockerfile b/docker/tests/mono/sonarr/Dockerfile index a8cb42cdc..f2ec42a9e 100644 --- a/docker/tests/mono/sonarr/Dockerfile +++ b/docker/tests/mono/sonarr/Dockerfile @@ -25,5 +25,8 @@ RUN fromdos /startup.sh WORKDIR /data/ VOLUME ["/data/_tests_linux", "/data/_output_linux", "/data/_tests_results"] +RUN groupadd sonarrtst -g 4020 && useradd sonarrtst -u 4021 -g 4020 -m -s /bin/bash +USER sonarrtst + CMD bash /startup.sh diff --git a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs index 500eab149..e4dfdabf4 100644 --- a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs +++ b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs @@ -17,11 +17,31 @@ namespace NzbDrone.Mono.Test.DiskProviderTests [Platform("Mono")] public class DiskProviderFixture : DiskProviderFixtureBase { + private string _tempPath; + public DiskProviderFixture() { MonoOnly(); } + [TearDown] + public void MonoDiskProviderFixtureTearDown() + { + // Give ourselves back write permissions so we can delete it + if (_tempPath != null) + { + if (Directory.Exists(_tempPath)) + { + Syscall.chmod(_tempPath, FilePermissions.S_IRWXU); + } + else if (File.Exists(_tempPath)) + { + Syscall.chmod(_tempPath, FilePermissions.S_IRUSR | FilePermissions.S_IWUSR); + } + _tempPath = null; + } + } + protected override void SetWritePermissions(string path, bool writable) { if (Environment.UserName == "root") @@ -29,17 +49,39 @@ namespace NzbDrone.Mono.Test.DiskProviderTests Assert.Inconclusive("Need non-root user to test write permissions."); } + SetWritePermissionsInternal(path, writable, false); + } + + protected void SetWritePermissionsInternal(string path, bool writable, bool setgid) + { // Remove Write permissions, we're still owner so we can clean it up, but we'll have to do that explicitly. - var entry = UnixFileSystemInfo.GetFileSystemEntry(path); + Stat stat; + Syscall.stat(path, out stat); + FilePermissions mode = stat.st_mode; if (writable) { - entry.FileAccessPermissions |= FileAccessPermissions.UserWrite | FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite; + mode |= FilePermissions.S_IWUSR | FilePermissions.S_IWGRP | FilePermissions.S_IWOTH; } else { - entry.FileAccessPermissions &= ~(FileAccessPermissions.UserWrite | FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite); + mode &= ~(FilePermissions.S_IWUSR | FilePermissions.S_IWGRP | FilePermissions.S_IWOTH); + } + + + if (setgid) + { + mode |= FilePermissions.S_ISGID; + } + else + { + mode &= ~FilePermissions.S_ISGID; + } + + if (stat.st_mode != mode) + { + Syscall.chmod(path, mode); } } @@ -164,7 +206,8 @@ namespace NzbDrone.Mono.Test.DiskProviderTests var tempFile = GetTempFilePath(); File.WriteAllText(tempFile, "File1"); - SetWritePermissions(tempFile, false); + SetWritePermissionsInternal(tempFile, false, false); + _tempPath = tempFile; // Verify test setup Syscall.stat(tempFile, out var fileStat); @@ -189,7 +232,8 @@ namespace NzbDrone.Mono.Test.DiskProviderTests var tempPath = GetTempFilePath(); Directory.CreateDirectory(tempPath); - SetWritePermissions(tempPath, false); + SetWritePermissionsInternal(tempPath, false, false); + _tempPath = tempPath; // Verify test setup Syscall.stat(tempPath, out var fileStat); @@ -199,14 +243,6 @@ namespace NzbDrone.Mono.Test.DiskProviderTests Syscall.stat(tempPath, out fileStat); NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0755"); - Subject.SetPermissions(tempPath, "0755", null); - Syscall.stat(tempPath, out fileStat); - NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0755"); - - Subject.SetPermissions(tempPath, "1775", null); - Syscall.stat(tempPath, out fileStat); - NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("1775"); - Subject.SetPermissions(tempPath, "775", null); Syscall.stat(tempPath, out fileStat); NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0775"); @@ -215,6 +251,66 @@ namespace NzbDrone.Mono.Test.DiskProviderTests Syscall.stat(tempPath, out fileStat); NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0750"); + Subject.SetPermissions(tempPath, "051", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0051"); + } + + [Test] + public void should_preserve_setgid_on_set_folder_permissions() + { + var tempPath = GetTempFilePath(); + + Directory.CreateDirectory(tempPath); + SetWritePermissionsInternal(tempPath, false, true); + _tempPath = tempPath; + + // Verify test setup + Syscall.stat(tempPath, out var fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("2555"); + + Subject.SetPermissions(tempPath, "755", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("2755"); + + Subject.SetPermissions(tempPath, "775", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("2775"); + + Subject.SetPermissions(tempPath, "750", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("2750"); + + Subject.SetPermissions(tempPath, "051", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("2051"); + } + + [Test] + public void should_clear_setgid_on_set_folder_permissions() + { + var tempPath = GetTempFilePath(); + + Directory.CreateDirectory(tempPath); + SetWritePermissionsInternal(tempPath, false, true); + _tempPath = tempPath; + + // Verify test setup + Syscall.stat(tempPath, out var fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("2555"); + + Subject.SetPermissions(tempPath, "0755", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0755"); + + Subject.SetPermissions(tempPath, "0775", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0775"); + + Subject.SetPermissions(tempPath, "0750", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0750"); + Subject.SetPermissions(tempPath, "0051", null); Syscall.stat(tempPath, out fileStat); NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0051"); @@ -229,6 +325,16 @@ namespace NzbDrone.Mono.Test.DiskProviderTests Subject.IsValidFolderPermissionMask("4755").Should().BeFalse(); Subject.IsValidFolderPermissionMask("7755").Should().BeFalse(); + // Folder should be readable and writeable by owner + Subject.IsValidFolderPermissionMask("000").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("100").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("200").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("300").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("400").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("500").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("600").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("700").Should().BeTrue(); + // Folder should be readable and writeable by owner Subject.IsValidFolderPermissionMask("0000").Should().BeFalse(); Subject.IsValidFolderPermissionMask("0100").Should().BeFalse(); diff --git a/src/NzbDrone.Mono/Disk/DiskProvider.cs b/src/NzbDrone.Mono/Disk/DiskProvider.cs index 628df8d59..f185ac1a0 100644 --- a/src/NzbDrone.Mono/Disk/DiskProvider.cs +++ b/src/NzbDrone.Mono/Disk/DiskProvider.cs @@ -85,7 +85,11 @@ namespace NzbDrone.Mono.Disk throw new LinuxPermissionsException("Error getting current permissions: " + error); } - permissions |= curStat.st_mode & ~FilePermissions.ACCESSPERMS; + // Preserve existing non-access permissions unless mask is 4 digits + if (mask.Length < 4) + { + permissions |= curStat.st_mode & ~FilePermissions.ACCESSPERMS; + } if (Syscall.chmod(path, permissions) < 0) {