From 89d91e8316ca213c5d184bcf469ed93977a5edf9 Mon Sep 17 00:00:00 2001 From: Piotr Bulawa Date: Wed, 29 Jan 2025 16:48:27 +0100 Subject: [PATCH] SNOW-1706569: Fix file permissions (#1089) --- .../IntegrationTests/MaxLobSizeIT.cs | 7 +- .../IntegrationTests/SFPutGetTest.cs | 8 +- .../Mock/MockUnixOperations.cs | 39 ++++ .../SFCredentialManagerFileImplTest.cs | 17 +- .../Logger/EasyLoggingStarterTest.cs | 16 +- .../UnitTests/SFAzureClientTest.cs | 4 +- .../UnitTests/SFBindUploaderTest.cs | 1 + .../Tools/DirectoryOperationsTest.cs | 123 ++++++++++++ .../UnitTests/Tools/FileOperationsTest.cs | 186 ++++++++++++++---- .../UnitTests/Tools/UnixOperationsTest.cs | 165 ++++++++++------ .../SFCredentialManagerFileImpl.cs | 51 ++--- .../Core/FileTransfer/EncryptionProvider.cs | 3 +- .../Core/FileTransfer/SFFileTransferAgent.cs | 3 +- .../FileTransfer/StorageClient/SFGCSClient.cs | 5 +- .../StorageClient/SFLocalStorageUtil.cs | 9 +- .../StorageClient/SFRemoteStorageUtil.cs | 20 +- .../FileTransfer/StorageClient/SFS3Client.cs | 5 +- .../StorageClient/SFSnowflakeAzureClient.cs | 17 +- .../Core/Session/EasyLoggingStarter.cs | 11 +- .../Core/Tools/DirectoryOperations.cs | 21 +- Snowflake.Data/Core/Tools/FileOperations.cs | 80 +++++++- Snowflake.Data/Core/Tools/TempUtil.cs | 24 +++ Snowflake.Data/Core/Tools/UnixOperations.cs | 70 ++++++- 23 files changed, 699 insertions(+), 186 deletions(-) create mode 100644 Snowflake.Data.Tests/Mock/MockUnixOperations.cs create mode 100644 Snowflake.Data.Tests/UnitTests/Tools/DirectoryOperationsTest.cs create mode 100644 Snowflake.Data/Core/Tools/TempUtil.cs diff --git a/Snowflake.Data.Tests/IntegrationTests/MaxLobSizeIT.cs b/Snowflake.Data.Tests/IntegrationTests/MaxLobSizeIT.cs index 448585f52..d4c6a4a39 100644 --- a/Snowflake.Data.Tests/IntegrationTests/MaxLobSizeIT.cs +++ b/Snowflake.Data.Tests/IntegrationTests/MaxLobSizeIT.cs @@ -11,6 +11,7 @@ using System.Data.Common; using System.IO; using System.Linq; +using Snowflake.Data.Core.Tools; namespace Snowflake.Data.Tests.IntegrationTests { @@ -301,7 +302,11 @@ void PrepareTest() t_inputFilePath = Path.GetTempPath() + t_fileName; var data = $"{t_colData[0]},{t_colData[1]},{t_colData[2]}"; - File.WriteAllText(t_inputFilePath, data); + using (var stream = FileOperations.Instance.Create(t_inputFilePath)) + using (var writer = new StreamWriter(stream)) + { + writer.Write(data); + } t_outputFilePath = $@"{s_outputDirectory}/{t_fileName}"; t_filesToDelete.Add(t_inputFilePath); diff --git a/Snowflake.Data.Tests/IntegrationTests/SFPutGetTest.cs b/Snowflake.Data.Tests/IntegrationTests/SFPutGetTest.cs index ccb39e26a..220ec1fe5 100644 --- a/Snowflake.Data.Tests/IntegrationTests/SFPutGetTest.cs +++ b/Snowflake.Data.Tests/IntegrationTests/SFPutGetTest.cs @@ -7,6 +7,7 @@ using System.Data.Common; using System.IO.Compression; using System.Text; +using Snowflake.Data.Core.Tools; using Snowflake.Data.Tests.Util; namespace Snowflake.Data.Tests.IntegrationTests @@ -807,7 +808,12 @@ private static void PrepareFileData(string file) var rawDataRow = string.Join(",", s_colData) + "\n"; var rawData = string.Concat(Enumerable.Repeat(rawDataRow, NumberOfRows)); - File.WriteAllText(file, rawData); + + using (var stream = FileOperations.Instance.Create(file)) + using (var writer = new StreamWriter(stream)) + { + writer.Write(rawData); + } t_filesToDelete.Add(file); } diff --git a/Snowflake.Data.Tests/Mock/MockUnixOperations.cs b/Snowflake.Data.Tests/Mock/MockUnixOperations.cs new file mode 100644 index 000000000..0a9c4e52e --- /dev/null +++ b/Snowflake.Data.Tests/Mock/MockUnixOperations.cs @@ -0,0 +1,39 @@ +using Mono.Unix; +using Snowflake.Data.Core.Tools; + +namespace Snowflake.Data.Tests.Mock +{ + internal class MockUnixOperations : UnixOperations + { + public long CurrentUserId { get; set; } = 0; + public long FileOwnerId { get; set; } = 0; + public long DirectoryOwnerId { get; set; } = 0; + public bool DirectoryPermissionsReturn { get; set; } = false; + public bool FilePermissionsReturn { get; set; } = false; + + public override bool CheckDirectoryHasAnyOfPermissions(string path, FileAccessPermissions permissions) + { + return DirectoryPermissionsReturn; + } + + public override bool CheckFileHasAnyOfPermissions(string path, FileAccessPermissions permissions) + { + return FilePermissionsReturn; + } + + public override long GetCurrentUserId() + { + return CurrentUserId; + } + + public override long GetOwnerIdOfDirectory(string path) + { + return DirectoryOwnerId; + } + + public override long GetOwnerIdOfFile(string path) + { + return FileOwnerId; + } + } +} diff --git a/Snowflake.Data.Tests/UnitTests/CredentialManager/SFCredentialManagerFileImplTest.cs b/Snowflake.Data.Tests/UnitTests/CredentialManager/SFCredentialManagerFileImplTest.cs index 079c02901..073ff5b04 100644 --- a/Snowflake.Data.Tests/UnitTests/CredentialManager/SFCredentialManagerFileImplTest.cs +++ b/Snowflake.Data.Tests/UnitTests/CredentialManager/SFCredentialManagerFileImplTest.cs @@ -6,7 +6,6 @@ using System.IO; using System.Security; using Mono.Unix; -using Mono.Unix.Native; using Moq; using NUnit.Framework; using Snowflake.Data.Core.CredentialManager.Infrastructure; @@ -66,8 +65,8 @@ public void TestThatThrowsErrorWhenCacheFailToCreateCacheFile() .Returns(false); t_unixOperations .Setup(u => u.CreateFileWithPermissions(s_customJsonPath, - FilePermissions.S_IRUSR | FilePermissions.S_IWUSR)) - .Returns(-1); + FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite)) + .Throws(); t_environmentOperations .Setup(e => e.GetEnvironmentVariable(SFCredentialManagerFileStorage.CredentialCacheDirectoryEnvironmentName)) .Returns(CustomJsonDir); @@ -83,6 +82,9 @@ public void TestThatThrowsErrorWhenCacheFailToCreateCacheFile() t_directoryOperations .Setup(d => d.GetDirectoryInfo(s_customLockPath)) .Returns(new DirectoryInformation(false, null)); + t_unixOperations + .Setup(u => u.CreateDirectoryWithPermissionsMkdir(s_customLockPath, FileAccessPermissions.UserRead)) + .Returns(0); _credentialManager = new SFCredentialManagerFileImpl(t_fileOperations.Object, t_directoryOperations.Object, t_unixOperations.Object, t_environmentOperations.Object); // act @@ -104,7 +106,7 @@ public void TestThatThrowsErrorWhenCacheFileCanBeAccessedByOthers() try { DirectoryOperations.Instance.CreateDirectory(tempDirectory); - UnixOperations.Instance.CreateFileWithPermissions(Path.Combine(tempDirectory, SFCredentialManagerFileStorage.CredentialCacheFileName), FilePermissions.ALLPERMS); + UnixOperations.Instance.CreateFileWithPermissions(Path.Combine(tempDirectory, SFCredentialManagerFileStorage.CredentialCacheFileName), FileAccessPermissions.AllPermissions); // act var thrown = Assert.Throws(() => _credentialManager.SaveCredentials("key", "token")); @@ -124,8 +126,7 @@ public void TestThatJsonFileIsCheckedIfAlreadyExists() // arrange t_unixOperations .Setup(u => u.CreateFileWithPermissions(s_customJsonPath, - FilePermissions.S_IRUSR | FilePermissions.S_IWUSR)) - .Returns(0); + FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite)); t_unixOperations .Setup(u => u.GetFilePermissions(s_customJsonPath)) .Returns(FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite); @@ -184,7 +185,7 @@ public void TestWritingIsUnavailableIfFailedToCreateDirLock() .Setup(u => u.GetCurrentUserId()) .Returns(UserId); t_unixOperations - .Setup(u => u.CreateDirectoryWithPermissions(s_customLockPath, SFCredentialManagerFileImpl.CredentialCacheLockDirPermissions)) + .Setup(u => u.CreateDirectoryWithPermissionsMkdir(s_customLockPath, SFCredentialManagerFileImpl.CredentialCacheLockDirPermissions)) .Returns(-1); _credentialManager = new SFCredentialManagerFileImpl(t_fileOperations.Object, t_directoryOperations.Object, t_unixOperations.Object, t_environmentOperations.Object); @@ -210,7 +211,7 @@ public void TestReadingIsUnavailableIfFailedToCreateDirLock() .Returns(false) .Returns(true); t_unixOperations - .Setup(u => u.CreateDirectoryWithPermissions(s_customLockPath, SFCredentialManagerFileImpl.CredentialCacheLockDirPermissions)) + .Setup(u => u.CreateDirectoryWithPermissionsMkdir(s_customLockPath, SFCredentialManagerFileImpl.CredentialCacheLockDirPermissions)) .Returns(-1); t_directoryOperations .Setup(d => d.GetParentDirectoryInfo(CustomJsonDir)) diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggingStarterTest.cs index d816455d5..b41c5eb95 100644 --- a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggingStarterTest.cs @@ -6,7 +6,6 @@ using System.IO; using System.Runtime.InteropServices; using Mono.Unix; -using Mono.Unix.Native; using Moq; using NUnit.Framework; using Snowflake.Data.Configuration; @@ -157,7 +156,7 @@ public void TestThatDoesNotFailWhenLogDirectoryPermissionIsNot700() // assert t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, - FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Never); + FileAccessPermissions.UserReadWriteExecute), Times.Never); } [Test] @@ -172,9 +171,12 @@ public void TestFailIfDirectoryCreationFails() t_easyLoggingProvider .Setup(provider => provider.ProvideConfig(ConfigPath)) .Returns(s_configWithErrorLevel); + t_directoryOperations + .Setup(d => d.CreateDirectory(s_expectedLogPath)) + .Throws(() => new Exception("Unable to create directory")); t_unixOperations - .Setup(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR)) - .Returns((int)Errno.EPERM); + .Setup(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, FileAccessPermissions.UserReadWriteExecute)) + .Throws(() => new Exception("Unable to create directory")); // act var thrown = Assert.Throws(() => t_easyLoggerStarter.Init(ConfigPath)); @@ -208,7 +210,7 @@ public void TestThatConfiguresEasyLoggingOnlyOnceWhenInitializedWithConfigPath() else { t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, - FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Once); + FileAccessPermissions.UserReadWriteExecute), Times.Once); } t_easyLoggerManager.Verify(manager => manager.ReconfigureEasyLogging(EasyLoggingLogLevel.Error, s_expectedLogPath), Times.Once); @@ -241,7 +243,7 @@ public void TestThatConfiguresEasyLoggingOnlyOnceForInitializationsWithoutConfig else { t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, - FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Once); + FileAccessPermissions.UserReadWriteExecute), Times.Once); } t_easyLoggerManager.Verify(manager => manager.ReconfigureEasyLogging(EasyLoggingLogLevel.Error, s_expectedLogPath), Times.Once); } @@ -268,7 +270,7 @@ public void TestThatReconfiguresEasyLoggingWithConfigPathIfNotGivenForTheFirstTi else { t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, - FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Once); + FileAccessPermissions.UserReadWriteExecute), Times.Once); } t_easyLoggerManager.Verify(manager => manager.ReconfigureEasyLogging(EasyLoggingLogLevel.Error, s_expectedLogPath), Times.Once); diff --git a/Snowflake.Data.Tests/UnitTests/SFAzureClientTest.cs b/Snowflake.Data.Tests/UnitTests/SFAzureClientTest.cs index cce7a586a..d000b0202 100644 --- a/Snowflake.Data.Tests/UnitTests/SFAzureClientTest.cs +++ b/Snowflake.Data.Tests/UnitTests/SFAzureClientTest.cs @@ -303,7 +303,7 @@ public void TestDownloadFile(HttpStatusCode httpStatusCode, ResultStatus expecte .Returns((blobName) => { var mockBlobClient = new Mock(); - mockBlobClient.Setup(client => client.DownloadTo(It.IsAny())) + mockBlobClient.Setup(client => client.DownloadTo(It.IsAny())) .Returns(() => { if (key == HttpStatusCode.OK.ToString()) @@ -350,7 +350,7 @@ public async Task TestDownloadFileAsync(HttpStatusCode httpStatusCode, ResultSta .Returns((blobName) => { var mockBlobClient = new Mock(); - mockBlobClient.Setup(client => client.DownloadToAsync(It.IsAny(), It.IsAny())) + mockBlobClient.Setup(client => client.DownloadToAsync(It.IsAny(), It.IsAny())) .Returns(async () => { if (key == HttpStatusCode.OK.ToString()) diff --git a/Snowflake.Data.Tests/UnitTests/SFBindUploaderTest.cs b/Snowflake.Data.Tests/UnitTests/SFBindUploaderTest.cs index 46e5b5b90..782b70175 100644 --- a/Snowflake.Data.Tests/UnitTests/SFBindUploaderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/SFBindUploaderTest.cs @@ -9,6 +9,7 @@ namespace Snowflake.Data.Tests.UnitTests { [TestFixture] + [SetCulture("en-US")] class SFBindUploaderTest { private readonly SFBindUploader _bindUploader = new SFBindUploader(null, "test"); diff --git a/Snowflake.Data.Tests/UnitTests/Tools/DirectoryOperationsTest.cs b/Snowflake.Data.Tests/UnitTests/Tools/DirectoryOperationsTest.cs new file mode 100644 index 000000000..cb362d8a1 --- /dev/null +++ b/Snowflake.Data.Tests/UnitTests/Tools/DirectoryOperationsTest.cs @@ -0,0 +1,123 @@ +using System.Collections.Generic; +using System.IO; +using Mono.Unix; +using Mono.Unix.Native; +using NUnit.Framework; +using Snowflake.Data.Core.Tools; +using Snowflake.Data.Tests.Mock; + +namespace Snowflake.Data.Tests.UnitTests.Tools +{ + [TestFixture, NonParallelizable] + public class DirectoryOperationsTest + { + private static DirectoryOperations s_directoryOperations; + private static readonly string s_relativeWorkingDirectory = $"directory_operations_test_{Path.GetRandomFileName()}"; + private static readonly string s_workingDirectory = Path.Combine(TempUtil.GetTempPath(), s_relativeWorkingDirectory); + private static readonly string s_dirName = "testdir"; + private static readonly string s_dirAbsolutePath = Path.Combine(s_workingDirectory, s_dirName); + + [SetUp] + public static void Before() + { + if (!Directory.Exists(s_workingDirectory)) + { + Directory.CreateDirectory(s_workingDirectory); + } + + s_directoryOperations = new DirectoryOperations(); + } + + [TearDown] + public static void After() + { + Directory.Delete(s_workingDirectory, true); + } + + [Test] + [Platform("Win")] + public void TestDirectoryIsSafeOnWindows() + { + // arrange + var absoluteFilePath = Path.Combine(s_workingDirectory, s_dirName); + Directory.CreateDirectory(absoluteFilePath); + + // act and assert + Assert.IsTrue(s_directoryOperations.IsDirectorySafe(absoluteFilePath)); + } + + [Test] + [Platform(Exclude = "Win")] + public void TestDirectoryIsNotSafeOnNotWindowsWhenPermissionsAreTooBroad( + [ValueSource(nameof(InsecurePermissions))] + FileAccessPermissions permissions) + { + // arrange + Syscall.mkdir(s_dirAbsolutePath, (FilePermissions)permissions); + + // act and assert + Assert.IsFalse(s_directoryOperations.IsDirectorySafe(s_dirAbsolutePath)); + } + + [Test] + public void TestShouldCreateDirectoryWithSafePermissions() + { + // act + s_directoryOperations.CreateDirectory(s_dirAbsolutePath); + + // assert + Assert.IsTrue(Directory.Exists(s_dirAbsolutePath)); + Assert.IsTrue(s_directoryOperations.IsDirectorySafe(s_dirAbsolutePath)); + } + + [Test] + [Platform(Exclude = "Win")] + public void TestOwnerIsCurrentUser() + { + // arrange + var mockUnixOperations = new MockUnixOperations{ CurrentUserId = 1, DirectoryOwnerId = 1}; + var dirOps = new DirectoryOperations(mockUnixOperations); + + // act and assert + Assert.IsTrue(dirOps.IsDirectoryOwnedByCurrentUser(s_dirAbsolutePath)); + } + + [Test] + [Platform(Exclude = "Win")] + public void TestOwnerIsNotCurrentUser() + { + // arrange + var mockUnixOperations = new MockUnixOperations{ CurrentUserId = 1, DirectoryOwnerId = 2}; + var dirOps = new DirectoryOperations(mockUnixOperations); + + // act and assert + Assert.IsFalse(dirOps.IsDirectoryOwnedByCurrentUser(s_dirAbsolutePath)); + } + + [Test] + [Platform(Exclude = "Win")] + public void TestDirectoryIsNotSecureWhenNotOwnedByCurrentUser() + { + // arrange + var mockUnixOperations = new MockUnixOperations{ CurrentUserId = 1, DirectoryOwnerId = 2}; + var dirOps = new DirectoryOperations(mockUnixOperations); + + // act and assert + Assert.IsFalse(dirOps.IsDirectorySafe(s_dirAbsolutePath)); + } + + // User permissions are required for all of the tests to be able to access directory information + public static IEnumerable InsecurePermissions() + { + yield return FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.GroupRead; + yield return FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.GroupRead | FileAccessPermissions.GroupWrite; + yield return FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.GroupExecute; + yield return FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.GroupReadWriteExecute; + yield return FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.OtherRead; + yield return FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.OtherRead | FileAccessPermissions.OtherWrite; + yield return FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.OtherExecute; + yield return FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.OtherReadWriteExecute; + yield return FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.AllPermissions; + } + } +} diff --git a/Snowflake.Data.Tests/UnitTests/Tools/FileOperationsTest.cs b/Snowflake.Data.Tests/UnitTests/Tools/FileOperationsTest.cs index b8b311357..c585b2bc6 100644 --- a/Snowflake.Data.Tests/UnitTests/Tools/FileOperationsTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Tools/FileOperationsTest.cs @@ -3,26 +3,29 @@ */ using System.Collections.Generic; -using Snowflake.Data.Core; using System.IO; -using System.Runtime.InteropServices; +using System.Security; using Mono.Unix; using Mono.Unix.Native; using NUnit.Framework; +using Snowflake.Data.Core; using Snowflake.Data.Core.Tools; +using Snowflake.Data.Tests.Mock; using static Snowflake.Data.Tests.UnitTests.Configuration.EasyLoggingConfigGenerator; -using System.Security; -namespace Snowflake.Data.Tests.Tools +namespace Snowflake.Data.Tests.UnitTests.Tools { [TestFixture, NonParallelizable] public class FileOperationsTest { private static FileOperations s_fileOperations; - private static readonly string s_workingDirectory = Path.Combine(Path.GetTempPath(), "file_operations_test_", Path.GetRandomFileName()); + private static readonly string s_relativeWorkingDirectory = $"file_operations_test_{Path.GetRandomFileName()}"; + private static readonly string s_workingDirectory = Path.Combine(TempUtil.GetTempPath(), s_relativeWorkingDirectory); + private static readonly string s_content = "random text"; + private static readonly string s_fileName = "testfile"; - [OneTimeSetUp] - public static void BeforeAll() + [SetUp] + public static void Before() { if (!Directory.Exists(s_workingDirectory)) { @@ -32,65 +35,50 @@ public static void BeforeAll() s_fileOperations = new FileOperations(); } - [OneTimeTearDown] - public static void AfterAll() + [TearDown] + public static void After() { Directory.Delete(s_workingDirectory, true); } [Test] + [Platform("Win")] public void TestReadAllTextOnWindows() { - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - Assert.Ignore("skip test only runs on Windows"); - } - - var content = "random text"; - var filePath = CreateConfigTempFile(s_workingDirectory, content); + var filePath = CreateConfigTempFile(s_workingDirectory, s_content); // act var result = s_fileOperations.ReadAllText(filePath, TomlConnectionBuilder.ValidateFilePermissions); // assert - Assert.AreEqual(content, result); + Assert.AreEqual(s_content, result); } [Test] + [Platform(Exclude = "Win")] public void TestReadAllTextCheckingPermissionsUsingTomlConfigurationFileValidations( [ValueSource(nameof(UserAllowedFilePermissions))] FileAccessPermissions userAllowedFilePermissions) { - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - Assert.Ignore("skip test on Windows"); - } - - var content = "random text"; - var filePath = CreateConfigTempFile(s_workingDirectory, content); + var filePath = CreateConfigTempFile(s_workingDirectory, s_content); var filePermissions = userAllowedFilePermissions; - Syscall.chmod(filePath, (FilePermissions)filePermissions); + Syscall.chmod(filePath, (FilePermissions) filePermissions); // act var result = s_fileOperations.ReadAllText(filePath, TomlConnectionBuilder.ValidateFilePermissions); // assert - Assert.AreEqual(content, result); + Assert.AreEqual(s_content, result); } [Test] + [Platform(Exclude = "Win")] public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadConfigurationFile( [ValueSource(nameof(UserAllowedFilePermissions))] FileAccessPermissions userAllowedFilePermissions) { - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - Assert.Ignore("skip test on Windows"); - } - - var content = "random text"; - var filePath = CreateConfigTempFile(s_workingDirectory, content); + var filePath = CreateConfigTempFile(s_workingDirectory, s_content); var filePermissions = userAllowedFilePermissions | FileAccessPermissions.OtherReadWriteExecute; Syscall.chmod(filePath, (FilePermissions)filePermissions); @@ -101,10 +89,142 @@ public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadConfiguration } + [Test] + [Platform(Exclude = "Win")] + public void TestFileIsSafeOnNotWindows() + { + // arrange + var absoluteFilePath = Path.Combine(s_workingDirectory, s_fileName); + Syscall.creat(absoluteFilePath, (FilePermissions) (FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite)); + + // act and assert + Assert.IsTrue(s_fileOperations.IsFileSafe(absoluteFilePath)); + } + + [Test] + [Platform(Exclude = "Win")] + public void TestFileIsNotSafeOnNotWindowsWhenTooBroadPermissionsAreUsed( + [ValueSource(nameof(InsecurePermissions))] + FileAccessPermissions permissions) + { + // arrange + var absoluteFilePath = Path.Combine(s_workingDirectory, s_fileName); + Syscall.creat(absoluteFilePath, (FilePermissions)permissions); + + // act and assert + Assert.IsFalse(s_fileOperations.IsFileSafe(absoluteFilePath)); + } + + [Test] + [Platform("Win")] + public void TestFileIsSafeOnWindows() + { + // arrange + var absoluteFilePath = Path.Combine(s_workingDirectory, s_fileName); + File.Create(absoluteFilePath).Close(); + + // act and assert + Assert.IsTrue(s_fileOperations.IsFileSafe(absoluteFilePath)); + } + + [Test] + [Platform(Exclude = "Win")] + public void TestOwnerIsCurrentUser() + { + // arrange + var absolutePath = Path.Combine(s_workingDirectory, s_fileName); + var mockUnixOperations = new MockUnixOperations{ CurrentUserId = 1, FileOwnerId = 1 }; + var fileOps = new FileOperations(mockUnixOperations); + + // act and assert + Assert.IsTrue(fileOps.IsFileOwnedByCurrentUser(absolutePath)); + } + + [Test] + [Platform(Exclude = "Win")] + public void TestOwnerIsNotCurrentUser() + { + // arrange + var absolutePath = Path.Combine(s_workingDirectory, s_fileName); + var mockUnixOperations = new MockUnixOperations{ CurrentUserId = 1, FileOwnerId = 2 }; + var fileOps = new FileOperations(mockUnixOperations); + + // act and assert + Assert.IsFalse(fileOps.IsFileOwnedByCurrentUser(absolutePath)); + } + + [Test] + [Platform(Exclude = "Win")] + public void TestFileIsNotSecureWhenNotOwnedByCurrentUser() + { + // arrange + var absolutePath = Path.Combine(s_workingDirectory, s_fileName); + var mockUnixOperations = new MockUnixOperations{ CurrentUserId = 1, FileOwnerId = 2 }; + var fileOps = new FileOperations(mockUnixOperations); + + // act and assert + Assert.IsFalse(fileOps.IsFileSafe(absolutePath)); + } + + [Test] + [Platform(Exclude = "Win")] + public void TestFileCopyUsesProperPermissions() + { + // arrange + const string SrcFile = "srcfile"; + var SrcFilePath = Path.Combine(s_workingDirectory, SrcFile); + const string DestFile = "destfile"; + var DestFilePath = Path.Combine(s_workingDirectory, DestFile); + + s_fileOperations.Create(SrcFilePath).Close(); + File.WriteAllText(SrcFilePath, s_content); + + // act + s_fileOperations.CopyFile(SrcFilePath, DestFilePath); + + // assert + Assert.IsTrue(File.Exists(DestFilePath)); + Assert.IsTrue(s_fileOperations.IsFileSafe(DestFilePath)); + Assert.AreEqual(s_content, File.ReadAllText(DestFilePath)); + } + + [Test] + [Platform(Exclude = "Win")] + public void TestFileCopyShouldThrowExecptionIfTooBroadPermissionsAreUsed() + { + // arrange + const string SrcFile = "srcfile"; + var SrcFilePath = Path.Combine(s_workingDirectory, SrcFile); + const string DestFile = "destfile"; + var DestFilePath = Path.Combine(s_workingDirectory, DestFile); + + s_fileOperations.Create(SrcFilePath).Close(); + Syscall.chmod(SrcFilePath, (FilePermissions) FileAccessPermissions.AllPermissions); + File.WriteAllText(SrcFilePath, s_content); + + // act and assert + Assert.Throws(() => s_fileOperations.CopyFile(SrcFilePath, DestFilePath), $"File ${SrcFilePath} is not safe to read."); + } + + public static IEnumerable UserAllowedFilePermissions() { yield return FileAccessPermissions.UserRead; yield return FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite; } + + public static IEnumerable InsecurePermissions() + { + yield return FileAccessPermissions.GroupRead; + yield return FileAccessPermissions.OtherRead; + yield return FileAccessPermissions.GroupRead | FileAccessPermissions.OtherRead; + yield return FileAccessPermissions.GroupRead | FileAccessPermissions.GroupWrite; + yield return FileAccessPermissions.OtherRead | FileAccessPermissions.OtherWrite; + yield return FileAccessPermissions.GroupRead | FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherRead | FileAccessPermissions.OtherWrite; + yield return FileAccessPermissions.GroupReadWriteExecute; + yield return FileAccessPermissions.OtherReadWriteExecute; + yield return FileAccessPermissions.GroupReadWriteExecute | FileAccessPermissions.OtherReadWriteExecute; + yield return FileAccessPermissions.AllPermissions; + } } } diff --git a/Snowflake.Data.Tests/UnitTests/Tools/UnixOperationsTest.cs b/Snowflake.Data.Tests/UnitTests/Tools/UnixOperationsTest.cs index 9c471965f..716724c14 100644 --- a/Snowflake.Data.Tests/UnitTests/Tools/UnixOperationsTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Tools/UnixOperationsTest.cs @@ -10,7 +10,7 @@ using Snowflake.Data.Core.Tools; using static Snowflake.Data.Tests.UnitTests.Configuration.EasyLoggingConfigGenerator; -namespace Snowflake.Data.Tests.Tools +namespace Snowflake.Data.Tests.UnitTests.Tools { [TestFixture, NonParallelizable] public class UnixOperationsTest @@ -41,15 +41,15 @@ public static void AfterAll() [Test] [Platform(Exclude = "Win")] public void TestDetectGroupOrOthersWritablePermissions( - [ValueSource(nameof(GroupOrOthersWritablePermissions))] FilePermissions groupOrOthersWritablePermissions, - [ValueSource(nameof(GroupNotWritablePermissions))] FilePermissions groupNotWritablePermissions, - [ValueSource(nameof(OtherNotWritablePermissions))] FilePermissions otherNotWritablePermissions) + [ValueSource(nameof(GroupOrOthersWritablePermissions))] FileAccessPermissions groupOrOthersWritablePermissions, + [ValueSource(nameof(GroupNotWritablePermissions))] FileAccessPermissions groupNotWritablePermissions, + [ValueSource(nameof(OtherNotWritablePermissions))] FileAccessPermissions otherNotWritablePermissions) { // arrange var filePath = CreateConfigTempFile(s_workingDirectory, "random text"); - var readWriteUserPermissions = FilePermissions.S_IRUSR | FilePermissions.S_IWUSR; + var readWriteUserPermissions = FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite; var filePermissions = readWriteUserPermissions | groupOrOthersWritablePermissions | groupNotWritablePermissions | otherNotWritablePermissions; - Syscall.chmod(filePath, filePermissions); + Syscall.chmod(filePath, (FilePermissions) filePermissions); // act var result = s_unixOperations.CheckFileHasAnyOfPermissions(filePath, FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite); @@ -61,13 +61,13 @@ public void TestDetectGroupOrOthersWritablePermissions( [Test] [Platform(Exclude = "Win")] public void TestDetectGroupOrOthersNotWritablePermissions( - [ValueSource(nameof(UserPermissions))] FilePermissions userPermissions, - [ValueSource(nameof(GroupNotWritablePermissions))] FilePermissions groupNotWritablePermissions, - [ValueSource(nameof(OtherNotWritablePermissions))] FilePermissions otherNotWritablePermissions) + [ValueSource(nameof(UserPermissions))] FileAccessPermissions userPermissions, + [ValueSource(nameof(GroupNotWritablePermissions))] FileAccessPermissions groupNotWritablePermissions, + [ValueSource(nameof(OtherNotWritablePermissions))] FileAccessPermissions otherNotWritablePermissions) { var filePath = CreateConfigTempFile(s_workingDirectory, "random text"); var filePermissions = userPermissions | groupNotWritablePermissions | otherNotWritablePermissions; - Syscall.chmod(filePath, filePermissions); + Syscall.chmod(filePath, (FilePermissions) filePermissions); // act var result = s_unixOperations.CheckFileHasAnyOfPermissions(filePath, FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite); @@ -79,11 +79,11 @@ public void TestDetectGroupOrOthersNotWritablePermissions( [Test] [Platform(Exclude = "Win")] public void TestReadAllTextCheckingPermissionsUsingTomlConfigurationFileValidations( - [ValueSource(nameof(UserAllowedPermissions))] FilePermissions userAllowedPermissions) + [ValueSource(nameof(UserAllowedPermissions))] FileAccessPermissions userAllowedPermissions) { var content = "random text"; var filePath = CreateConfigTempFile(s_workingDirectory, content); - Syscall.chmod(filePath, userAllowedPermissions); + Syscall.chmod(filePath, (FilePermissions) userAllowedPermissions); // act var result = s_unixOperations.ReadAllText(filePath, TomlConnectionBuilder.ValidateFilePermissions); @@ -95,11 +95,11 @@ public void TestReadAllTextCheckingPermissionsUsingTomlConfigurationFileValidati [Test] [Platform(Exclude = "Win")] public void TestWriteAllTextCheckingPermissionsUsingSFCredentialManagerFileValidations( - [ValueSource(nameof(UserAllowedWritePermissions))] FilePermissions userAllowedPermissions) + [ValueSource(nameof(UserAllowedWritePermissions))] FileAccessPermissions userAllowedPermissions) { var content = "random text"; var filePath = CreateConfigTempFile(s_workingDirectory, content); - Syscall.chmod(filePath, userAllowedPermissions); + Syscall.chmod(filePath, (FilePermissions) userAllowedPermissions); // act and assert Assert.DoesNotThrow(() => s_unixOperations.WriteAllText(filePath,"test", SFCredentialManagerFileImpl.Instance.ValidateFilePermissions)); @@ -107,9 +107,9 @@ public void TestWriteAllTextCheckingPermissionsUsingSFCredentialManagerFileValid [Test] [Platform(Exclude = "Win")] - public void TestFailIfGroupOrOthersHavePermissionsToFileWhileReadingWithUnixValidationsUsingTomlConfig([ValueSource(nameof(UserReadWritePermissions))] FilePermissions userPermissions, - [ValueSource(nameof(GroupPermissions))] FilePermissions groupPermissions, - [ValueSource(nameof(OthersPermissions))] FilePermissions othersPermissions) + public void TestFailIfGroupOrOthersHavePermissionsToFileWithTomlConfigurationValidations([ValueSource(nameof(UserReadWritePermissions))] FileAccessPermissions userPermissions, + [ValueSource(nameof(GroupPermissions))] FileAccessPermissions groupPermissions, + [ValueSource(nameof(OthersPermissions))] FileAccessPermissions othersPermissions) { if(groupPermissions == 0 && othersPermissions == 0) { @@ -120,7 +120,7 @@ public void TestFailIfGroupOrOthersHavePermissionsToFileWhileReadingWithUnixVali var filePath = CreateConfigTempFile(s_workingDirectory, content); var filePermissions = userPermissions | groupPermissions | othersPermissions; - Syscall.chmod(filePath, filePermissions); + Syscall.chmod(filePath, (FilePermissions) filePermissions); // act and assert Assert.Throws(() => s_unixOperations.ReadAllText(filePath, TomlConnectionBuilder.ValidateFilePermissions), "Attempting to read a file with too broad permissions assigned"); @@ -128,9 +128,9 @@ public void TestFailIfGroupOrOthersHavePermissionsToFileWhileReadingWithUnixVali [Test] [Platform(Exclude = "Win")] - public void TestFailIfGroupOrOthersHavePermissionsToFileWhileWritingWithUnixValidationsForCredentialManagerFile([ValueSource(nameof(UserReadWritePermissions))] FilePermissions userPermissions, - [ValueSource(nameof(GroupPermissions))] FilePermissions groupPermissions, - [ValueSource(nameof(OthersPermissions))] FilePermissions othersPermissions) + public void TestFailIfGroupOrOthersHavePermissionsToFileWhileWritingWithUnixValidationsForCredentialManagerFile([ValueSource(nameof(UserReadWritePermissions))] FileAccessPermissions userPermissions, + [ValueSource(nameof(GroupPermissions))] FileAccessPermissions groupPermissions, + [ValueSource(nameof(OthersPermissions))] FileAccessPermissions othersPermissions) { if(groupPermissions == 0 && othersPermissions == 0) { @@ -141,83 +141,128 @@ public void TestFailIfGroupOrOthersHavePermissionsToFileWhileWritingWithUnixVali var filePath = CreateConfigTempFile(s_workingDirectory, content); var filePermissions = userPermissions | groupPermissions | othersPermissions; - Syscall.chmod(filePath, filePermissions); + Syscall.chmod(filePath, (FilePermissions) filePermissions); // act and assert Assert.Throws(() => s_unixOperations.WriteAllText(filePath, "test", SFCredentialManagerFileImpl.Instance.ValidateFilePermissions), "Attempting to read or write a file with too broad permissions assigned"); } - public static IEnumerable UserPermissions() + [Test] + [Platform(Exclude = "Win")] + public void TestCreateFileWithUserRwPermissions() + { + // arrange + var filePath = Path.Combine(s_workingDirectory, "testfile"); + + // act + s_unixOperations.CreateFileWithPermissions(filePath, FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite); + + // assert + var result = s_unixOperations.CheckFileHasAnyOfPermissions(filePath, FileAccessPermissions.AllPermissions & ~(FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite)); + Assert.IsFalse(result); + } + + [Test] + [Platform(Exclude = "Win")] + public void TestCreateDirectoryWithUserRwxPermissions() + { + // arrange + var dirPath = Path.Combine(s_workingDirectory, "testdir"); + + // act + s_unixOperations.CreateDirectoryWithPermissions(dirPath, FileAccessPermissions.UserReadWriteExecute); + + // assert + var result = s_unixOperations.CheckFileHasAnyOfPermissions(dirPath, FileAccessPermissions.AllPermissions & ~FileAccessPermissions.UserReadWriteExecute); + Assert.IsFalse(result); + } + + [Test] + [Platform(Exclude = "Win")] + public void TestNestedDir() + { + // arrange + var dirPath = Path.Combine(s_workingDirectory, "testdir", "a", "b", "c"); + s_unixOperations.CreateDirectoryWithPermissions(dirPath, FileAccessPermissions.UserReadWriteExecute); + + // act + var result = s_unixOperations.CheckFileHasAnyOfPermissions(dirPath, FileAccessPermissions.AllPermissions & ~FileAccessPermissions.UserReadWriteExecute); + + // assert + Assert.IsFalse(result); + } + + public static IEnumerable UserPermissions() { - yield return FilePermissions.S_IRUSR; - yield return FilePermissions.S_IWUSR; - yield return FilePermissions.S_IXUSR; - yield return FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR; + yield return FileAccessPermissions.UserRead; + yield return FileAccessPermissions.UserWrite; + yield return FileAccessPermissions.UserExecute; + yield return FileAccessPermissions.UserReadWriteExecute; } - public static IEnumerable GroupPermissions() + public static IEnumerable GroupPermissions() { yield return 0; - yield return FilePermissions.S_IRGRP; - yield return FilePermissions.S_IWGRP; - yield return FilePermissions.S_IXGRP; - yield return FilePermissions.S_IRGRP | FilePermissions.S_IWGRP | FilePermissions.S_IXGRP; + yield return FileAccessPermissions.GroupRead; + yield return FileAccessPermissions.GroupWrite; + yield return FileAccessPermissions.GroupExecute; + yield return FileAccessPermissions.GroupReadWriteExecute; } - public static IEnumerable OthersPermissions() + public static IEnumerable OthersPermissions() { yield return 0; - yield return FilePermissions.S_IROTH; - yield return FilePermissions.S_IWOTH; - yield return FilePermissions.S_IXOTH; - yield return FilePermissions.S_IROTH | FilePermissions.S_IWOTH | FilePermissions.S_IXOTH; + yield return FileAccessPermissions.GroupRead; + yield return FileAccessPermissions.GroupWrite; + yield return FileAccessPermissions.GroupExecute; + yield return FileAccessPermissions.GroupReadWriteExecute; } - public static IEnumerable GroupOrOthersWritablePermissions() + public static IEnumerable GroupOrOthersWritablePermissions() { - yield return FilePermissions.S_IWGRP; - yield return FilePermissions.S_IWOTH; - yield return FilePermissions.S_IWGRP | FilePermissions.S_IWOTH; + yield return FileAccessPermissions.GroupWrite; + yield return FileAccessPermissions.OtherWrite; + yield return FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite; } - public static IEnumerable GroupNotWritablePermissions() + public static IEnumerable GroupNotWritablePermissions() { yield return 0; - yield return FilePermissions.S_IRGRP; - yield return FilePermissions.S_IXGRP; - yield return FilePermissions.S_IRGRP | FilePermissions.S_IXGRP; + yield return FileAccessPermissions.GroupRead; + yield return FileAccessPermissions.GroupExecute; + yield return FileAccessPermissions.GroupRead | FileAccessPermissions.GroupExecute; } - public static IEnumerable OtherNotWritablePermissions() + public static IEnumerable OtherNotWritablePermissions() { yield return 0; - yield return FilePermissions.S_IROTH; - yield return FilePermissions.S_IXOTH; - yield return FilePermissions.S_IROTH | FilePermissions.S_IXOTH; + yield return FileAccessPermissions.OtherRead; + yield return FileAccessPermissions.OtherExecute; + yield return FileAccessPermissions.OtherRead | FileAccessPermissions.OtherExecute; } - public static IEnumerable UserReadWritePermissions() + public static IEnumerable UserReadWritePermissions() { - yield return FilePermissions.S_IRUSR | FilePermissions.S_IWUSR; + yield return FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite; } - public static IEnumerable UserAllowedPermissions() + public static IEnumerable UserAllowedPermissions() { - yield return FilePermissions.S_IRUSR; - yield return FilePermissions.S_IRUSR | FilePermissions.S_IWUSR; + yield return FileAccessPermissions.UserRead; + yield return FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite; } - public static IEnumerable UserAllowedWritePermissions() + public static IEnumerable UserAllowedWritePermissions() { - yield return FilePermissions.S_IRUSR | FilePermissions.S_IWUSR; + yield return FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite; } - public static IEnumerable GroupOrOthersReadablePermissions() + public static IEnumerable GroupOrOthersReadablePermissions() { yield return 0; - yield return FilePermissions.S_IRGRP; - yield return FilePermissions.S_IROTH; - yield return FilePermissions.S_IRGRP | FilePermissions.S_IROTH; + yield return FileAccessPermissions.GroupRead; + yield return FileAccessPermissions.OtherRead; + yield return FileAccessPermissions.GroupRead | FileAccessPermissions.OtherRead; } } } diff --git a/Snowflake.Data/Core/CredentialManager/Infrastructure/SFCredentialManagerFileImpl.cs b/Snowflake.Data/Core/CredentialManager/Infrastructure/SFCredentialManagerFileImpl.cs index 89581483f..6ba55c2fc 100644 --- a/Snowflake.Data/Core/CredentialManager/Infrastructure/SFCredentialManagerFileImpl.cs +++ b/Snowflake.Data/Core/CredentialManager/Infrastructure/SFCredentialManagerFileImpl.cs @@ -3,7 +3,6 @@ */ using Mono.Unix; -using Mono.Unix.Native; using Newtonsoft.Json; using Snowflake.Data.Client; using Snowflake.Data.Core.Tools; @@ -21,7 +20,7 @@ internal class SFCredentialManagerFileImpl : ISnowflakeCredentialManager { internal const int CredentialCacheLockDurationSeconds = 60; - internal const FilePermissions CredentialCacheLockDirPermissions = FilePermissions.S_IRUSR; + internal const FileAccessPermissions CredentialCacheLockDirPermissions = FileAccessPermissions.UserRead; private static readonly SFLogger s_logger = SFLoggerFactory.GetLogger(); @@ -171,13 +170,16 @@ private void WriteContentToJsonFile(string content) else { s_logger.Info($"Creating the json file for credential cache in {_fileStorage.JsonCacheFilePath}"); - var createFileResult = _unixOperations.CreateFileWithPermissions(_fileStorage.JsonCacheFilePath, - FilePermissions.S_IRUSR | FilePermissions.S_IWUSR); - if (createFileResult == -1) + try + { + _unixOperations.CreateFileWithPermissions(_fileStorage.JsonCacheFilePath, + FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite); + } + catch (Exception e) { var errorMessage = "Failed to create the JSON token cache file"; s_logger.Error(errorMessage); - throw new Exception(errorMessage); + throw new Exception(errorMessage, e); } } _fileOperations.Write(_fileStorage.JsonCacheFilePath, content, ValidateFilePermissions); @@ -212,38 +214,20 @@ private void InitializeFileStorageIfNeeded() if (_fileStorage != null) return; var fileStorage = new SFCredentialManagerFileStorage(_environmentOperations); - PrepareParentDirectory(fileStorage.JsonCacheDirectory); - PrepareSecureDirectory(fileStorage.JsonCacheDirectory); + _directoryOperations.CreateDirectory(fileStorage.JsonCacheDirectory); + SetSecureDirectory(fileStorage.JsonCacheDirectory); _fileStorage = fileStorage; } - private void PrepareParentDirectory(string directory) - { - var parentDirectory = _directoryOperations.GetParentDirectoryInfo(directory); - if (!parentDirectory.Exists) - { - _directoryOperations.CreateDirectory(parentDirectory.FullName); - } - } - - private void PrepareSecureDirectory(string directory) + private void SetSecureDirectory(string directory) { var unixDirectoryInfo = _unixOperations.GetDirectoryInfo(directory); - if (unixDirectoryInfo.Exists) - { - var userId = _unixOperations.GetCurrentUserId(); - if (!unixDirectoryInfo.IsSafeExactly(userId)) - { - SetSecureOwnershipAndPermissions(directory, userId); - } - } - else + if (!unixDirectoryInfo.Exists) return; + + var userId = _unixOperations.GetCurrentUserId(); + if (!unixDirectoryInfo.IsSafeExactly(userId)) { - var createResult = _unixOperations.CreateDirectoryWithPermissions(directory, FilePermissions.S_IRWXU); - if (createResult == -1) - { - throw new SecurityException($"Could not create directory: {directory}"); - } + SetSecureOwnershipAndPermissions(directory, userId); } } @@ -292,7 +276,8 @@ private bool AcquireLock() { return false; } - var result = _unixOperations.CreateDirectoryWithPermissions(_fileStorage.JsonCacheLockPath, CredentialCacheLockDirPermissions); + + var result = _unixOperations.CreateDirectoryWithPermissionsMkdir(_fileStorage.JsonCacheLockPath, CredentialCacheLockDirPermissions); return result == 0; } diff --git a/Snowflake.Data/Core/FileTransfer/EncryptionProvider.cs b/Snowflake.Data/Core/FileTransfer/EncryptionProvider.cs index b625f80d3..7370078e7 100644 --- a/Snowflake.Data/Core/FileTransfer/EncryptionProvider.cs +++ b/Snowflake.Data/Core/FileTransfer/EncryptionProvider.cs @@ -6,6 +6,7 @@ using System; using Snowflake.Data.Log; using System.Security.Cryptography; +using Snowflake.Data.Core.Tools; namespace Snowflake.Data.Core.FileTransfer { @@ -203,7 +204,7 @@ public static string DecryptFile( ivBytes, transferConfiguration)) { - using (var decryptedFileStream = File.Create(tempFileName)) + using (var decryptedFileStream = FileOperations.Instance.CreateTempFile(tempFileName)) { var decryptedBytesStream = decryptedBytesStreamPair.MainStream; decryptedBytesStream.Position = 0; diff --git a/Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs b/Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs index 86395992f..f63b41ea5 100644 --- a/Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs +++ b/Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs @@ -14,6 +14,7 @@ using System.Security.Cryptography; using System.Threading; using System.Threading.Tasks; +using Snowflake.Data.Core.Tools; namespace Snowflake.Data.Core { @@ -898,7 +899,7 @@ private void compressFileWithGzip(SFFileMetadata fileMetadata) if ((File.GetAttributes(fileToCompress.FullName) & FileAttributes.Hidden) != FileAttributes.Hidden) { - using (FileStream compressedFileStream = File.Create(fileMetadata.realSrcFilePath)) + using (var compressedFileStream = FileOperations.Instance.Create(fileMetadata.realSrcFilePath)) { using (GZipStream compressionStream = new GZipStream(compressedFileStream, CompressionMode.Compress)) diff --git a/Snowflake.Data/Core/FileTransfer/StorageClient/SFGCSClient.cs b/Snowflake.Data/Core/FileTransfer/StorageClient/SFGCSClient.cs index 4e0f62f35..ec07edb43 100644 --- a/Snowflake.Data/Core/FileTransfer/StorageClient/SFGCSClient.cs +++ b/Snowflake.Data/Core/FileTransfer/StorageClient/SFGCSClient.cs @@ -12,6 +12,7 @@ using System.Net; using Google.Apis.Storage.v1; using Google.Cloud.Storage.V1; +using Snowflake.Data.Core.Tools; namespace Snowflake.Data.Core.FileTransfer.StorageClient { @@ -378,7 +379,7 @@ public void DownloadFile(SFFileMetadata fileMetadata, string fullDstPath, int ma using (HttpWebResponse response = (HttpWebResponse)request.GetResponse()) { // Write to file - using (var fileStream = File.Create(fullDstPath)) + using (var fileStream = FileOperations.Instance.Create(fullDstPath)) { using (var responseStream = response.GetResponseStream()) { @@ -412,7 +413,7 @@ public async Task DownloadFileAsync(SFFileMetadata fileMetadata, string fullDstP using (HttpWebResponse response = (HttpWebResponse)await request.GetResponseAsync().ConfigureAwait(false)) { // Write to file - using (var fileStream = File.Create(fullDstPath)) + using (var fileStream = FileOperations.Instance.Create(fullDstPath)) { using (var responseStream = response.GetResponseStream()) { diff --git a/Snowflake.Data/Core/FileTransfer/StorageClient/SFLocalStorageUtil.cs b/Snowflake.Data/Core/FileTransfer/StorageClient/SFLocalStorageUtil.cs index 6b98f9fb1..0d5160713 100644 --- a/Snowflake.Data/Core/FileTransfer/StorageClient/SFLocalStorageUtil.cs +++ b/Snowflake.Data/Core/FileTransfer/StorageClient/SFLocalStorageUtil.cs @@ -3,6 +3,7 @@ */ using System.IO; +using Snowflake.Data.Core.Tools; namespace Snowflake.Data.Core.FileTransfer { @@ -20,7 +21,7 @@ internal static void UploadOneFileWithRetry(SFFileMetadata fileMetadata) // Create directory if doesn't exist if (!Directory.Exists(fileMetadata.stageInfo.location)) { - Directory.CreateDirectory(fileMetadata.stageInfo.location); + DirectoryOperations.Instance.CreateDirectory(fileMetadata.stageInfo.location); } // Create reader stream @@ -36,7 +37,7 @@ internal static void UploadOneFileWithRetry(SFFileMetadata fileMetadata) stream.Position = 0; // Write stream to file - using (var fileStream = File.Create(Path.Combine(fileMetadata.stageInfo.location, fileMetadata.destFileName))) + using (var fileStream = FileOperations.Instance.Create(Path.Combine(fileMetadata.stageInfo.location, fileMetadata.destFileName))) { stream.CopyTo(fileStream); } @@ -58,12 +59,12 @@ internal static void DownloadOneFile(SFFileMetadata fileMetadata) // Create directory if doesn't exist if (!Directory.Exists(fileMetadata.localLocation)) { - Directory.CreateDirectory(fileMetadata.localLocation); + DirectoryOperations.Instance.CreateDirectory(fileMetadata.localLocation); } // Create stream object for reader and writer Stream stream = new MemoryStream(File.ReadAllBytes(realSrcFilePath)); - using (var fileStream = File.Create(output)) + using (var fileStream = FileOperations.Instance.Create(output)) { // Write file stream.CopyTo(fileStream); diff --git a/Snowflake.Data/Core/FileTransfer/StorageClient/SFRemoteStorageUtil.cs b/Snowflake.Data/Core/FileTransfer/StorageClient/SFRemoteStorageUtil.cs index 7ca2ee27d..bb40c762c 100644 --- a/Snowflake.Data/Core/FileTransfer/StorageClient/SFRemoteStorageUtil.cs +++ b/Snowflake.Data/Core/FileTransfer/StorageClient/SFRemoteStorageUtil.cs @@ -7,6 +7,7 @@ using System.IO; using System.Threading; using System.Threading.Tasks; +using Snowflake.Data.Core.Tools; namespace Snowflake.Data.Core.FileTransfer { @@ -312,7 +313,6 @@ internal static async Task UploadOneFileWithRetryAsync(SFFileMetadata fileMetada /// /// Download one file. /// - /// /// The file metadata of the file to download internal static void DownloadOneFile(SFFileMetadata fileMetadata) { @@ -322,7 +322,7 @@ internal static void DownloadOneFile(SFFileMetadata fileMetadata) // Check local location exists if (!Directory.Exists(fileMetadata.localLocation)) { - Directory.CreateDirectory(fileMetadata.localLocation); + DirectoryOperations.Instance.CreateDirectory(fileMetadata.localLocation); } ISFRemoteStorageClient client = fileMetadata.client; @@ -364,12 +364,7 @@ internal static void DownloadOneFile(SFFileMetadata fileMetadata) encryptionMetadata, FileTransferConfiguration.FromFileMetadata(fileMetadata)); - File.Delete(fullDstPath); - - // Copy decrypted tmp file to target destination path - File.Copy(tmpDstName, fullDstPath); - - // Delete tmp file + FileOperations.Instance.CopyFile(tmpDstName, fullDstPath); File.Delete(tmpDstName); } @@ -411,7 +406,7 @@ internal static async Task DownloadOneFileAsync(SFFileMetadata fileMetadata, Can // Check local location exists if (!Directory.Exists(fileMetadata.localLocation)) { - Directory.CreateDirectory(fileMetadata.localLocation); + DirectoryOperations.Instance.CreateDirectory(fileMetadata.localLocation); } ISFRemoteStorageClient client = fileMetadata.client; @@ -453,12 +448,7 @@ internal static async Task DownloadOneFileAsync(SFFileMetadata fileMetadata, Can encryptionMetadata, FileTransferConfiguration.FromFileMetadata(fileMetadata)); - File.Delete(fullDstPath); - - // Copy decrypted tmp file to target destination path - File.Copy(tmpDstName, fullDstPath); - - // Delete tmp file + FileOperations.Instance.CopyFile(tmpDstName, fullDstPath); File.Delete(tmpDstName); } diff --git a/Snowflake.Data/Core/FileTransfer/StorageClient/SFS3Client.cs b/Snowflake.Data/Core/FileTransfer/StorageClient/SFS3Client.cs index 524dc23c1..91fb5aa5c 100644 --- a/Snowflake.Data/Core/FileTransfer/StorageClient/SFS3Client.cs +++ b/Snowflake.Data/Core/FileTransfer/StorageClient/SFS3Client.cs @@ -13,6 +13,7 @@ using System.Net; using System.Threading; using System.Threading.Tasks; +using Snowflake.Data.Core.Tools; namespace Snowflake.Data.Core.FileTransfer.StorageClient { @@ -469,7 +470,7 @@ public void DownloadFile(SFFileMetadata fileMetadata, string fullDstPath, int ma using (GetObjectResponse response = task.Result) { // Write to file - using (var fileStream = File.Create(fullDstPath)) + using (var fileStream = FileOperations.Instance.Create(fullDstPath)) { response.ResponseStream.CopyTo(fileStream); } @@ -503,7 +504,7 @@ public async Task DownloadFileAsync(SFFileMetadata fileMetadata, string fullDstP using (GetObjectResponse response = await client.GetObjectAsync(getObjectRequest, cancellationToken).ConfigureAwait(false)) // Write to file - using (var fileStream = File.Create(fullDstPath)) + using (var fileStream = FileOperations.Instance.Create(fullDstPath)) { response.ResponseStream.CopyTo(fileStream); } diff --git a/Snowflake.Data/Core/FileTransfer/StorageClient/SFSnowflakeAzureClient.cs b/Snowflake.Data/Core/FileTransfer/StorageClient/SFSnowflakeAzureClient.cs index 272f1e48c..1c76ab527 100644 --- a/Snowflake.Data/Core/FileTransfer/StorageClient/SFSnowflakeAzureClient.cs +++ b/Snowflake.Data/Core/FileTransfer/StorageClient/SFSnowflakeAzureClient.cs @@ -14,6 +14,7 @@ using System.Threading; using System.Threading.Tasks; using System.Net; +using Snowflake.Data.Core.Tools; namespace Snowflake.Data.Core.FileTransfer.StorageClient { @@ -325,11 +326,15 @@ public void DownloadFile(SFFileMetadata fileMetadata, string fullDstPath, int ma try { - // Issue the GET request - blobClient.DownloadTo(fullDstPath); + using (var fileStream = FileOperations.Instance.Create(fullDstPath)) + { + // Issue the GET request + blobClient.DownloadTo(fileStream); + } } catch (RequestFailedException ex) { + File.Delete(fullDstPath); fileMetadata = HandleDownloadFileErr(ex, fileMetadata); return; } @@ -354,11 +359,15 @@ public async Task DownloadFileAsync(SFFileMetadata fileMetadata, string fullDstP try { - // Issue the GET request - await blobClient.DownloadToAsync(fullDstPath, cancellationToken).ConfigureAwait(false); + using (var fileStream = FileOperations.Instance.Create(fullDstPath)) + { + // Issue the GET request + await blobClient.DownloadToAsync(fileStream, cancellationToken).ConfigureAwait(false); + } } catch (RequestFailedException ex) { + File.Delete(fullDstPath); fileMetadata = HandleDownloadFileErr(ex, fileMetadata); return; } diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index f0ff20852..3b07d349c 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -6,7 +6,6 @@ using System.IO; using System.Runtime.InteropServices; using Mono.Unix; -using Mono.Unix.Native; using Snowflake.Data.Configuration; using Snowflake.Data.Core.Tools; using Snowflake.Data.Log; @@ -145,9 +144,13 @@ private string GetLogPath(string logPath) { Directory.CreateDirectory(logPathOrDefault); } - var createDirResult = _unixOperations.CreateDirectoryWithPermissions(pathWithDotnetSubdirectory, - FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR); - if (createDirResult != 0) + + try + { + _unixOperations.CreateDirectoryWithPermissions(pathWithDotnetSubdirectory, + FileAccessPermissions.UserReadWriteExecute); + } + catch (Exception) { s_logger.Error($"Failed to create logs directory: {pathWithDotnetSubdirectory}"); throw new Exception("Failed to create logs directory"); diff --git a/Snowflake.Data/Core/Tools/DirectoryOperations.cs b/Snowflake.Data/Core/Tools/DirectoryOperations.cs index 46254c85d..9c25f15b8 100644 --- a/Snowflake.Data/Core/Tools/DirectoryOperations.cs +++ b/Snowflake.Data/Core/Tools/DirectoryOperations.cs @@ -4,6 +4,7 @@ using System.IO; using System.Runtime.InteropServices; +using Mono.Unix; namespace Snowflake.Data.Core.Tools { @@ -23,7 +24,19 @@ internal DirectoryOperations(UnixOperations unixOperations) public virtual bool Exists(string path) => Directory.Exists(path); - public virtual DirectoryInfo CreateDirectory(string path) => Directory.CreateDirectory(path); + public virtual string CreateDirectory(string path) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Directory.CreateDirectory(path); + } + else + { + _unixOperations.CreateDirectoryWithPermissions(path, FileAccessPermissions.UserReadWriteExecute); + } + + return path; + } public virtual void Delete(string path, bool recursive) => Directory.Delete(path, recursive); @@ -40,5 +53,11 @@ public virtual bool IsDirectorySafe(string path) var unixInfo = _unixOperations.GetDirectoryInfo(path); return unixInfo.IsSafe(_unixOperations.GetCurrentUserId()); } + + public virtual bool IsDirectoryOwnedByCurrentUser(string path) + { + return RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || + _unixOperations.GetOwnerIdOfDirectory(path) == _unixOperations.GetCurrentUserId(); + } } } diff --git a/Snowflake.Data/Core/Tools/FileOperations.cs b/Snowflake.Data/Core/Tools/FileOperations.cs index 1324423f2..8c20c3485 100644 --- a/Snowflake.Data/Core/Tools/FileOperations.cs +++ b/Snowflake.Data/Core/Tools/FileOperations.cs @@ -5,15 +5,28 @@ using System; using System.IO; using System.Runtime.InteropServices; +using System.Security; using Mono.Unix; +using Snowflake.Data.Log; namespace Snowflake.Data.Core.Tools { internal class FileOperations { + private static readonly SFLogger s_logger = SFLoggerFactory.GetLogger(); public static readonly FileOperations Instance = new FileOperations(); - private readonly UnixOperations _unixOperations = UnixOperations.Instance; + private readonly UnixOperations _unixOperations; + private const FileAccessPermissions NotSafePermissions = FileAccessPermissions.AllPermissions & ~(FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite); + + internal FileOperations() : this(UnixOperations.Instance) + { + } + + internal FileOperations(UnixOperations unixOperations) + { + _unixOperations = unixOperations; + } public virtual bool Exists(string path) { @@ -42,5 +55,70 @@ public virtual string ReadAllText(string path, Action validator) var contentFile = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || validator == null ? File.ReadAllText(path) : _unixOperations.ReadAllText(path, validator); return contentFile; } + + public virtual Stream CreateTempFile(string filePath) + { + var absolutePath = Path.Combine(TempUtil.GetTempPath(), filePath); + + return Create(absolutePath); + } + + public virtual Stream Create(string filePath) + { + var absolutePath = Path.GetFullPath(filePath); + + return RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? + File.Create(absolutePath) : + _unixOperations.CreateFileWithPermissions(absolutePath, FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite); + } + + public virtual void CopyFile(string src, string dst) + { + File.Delete(dst); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + File.Copy(src, dst); + return; + } + + if (!IsFileSafe(src)) + { + throw new SecurityException($"File ${src} is not safe to read."); + } + + using (var srcStream = File.OpenRead(src)) + using (var dstStream = Create(dst)) + { + srcStream.CopyTo(dstStream); + } + } + + public virtual bool IsFileSafe(string path) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return true; + } + + if (_unixOperations.CheckFileHasAnyOfPermissions(path, NotSafePermissions)) + { + s_logger.Warn($"File '{path}' permissions are too broad. It could be potentially accessed by group or others."); + return false; + } + + if (!IsFileOwnedByCurrentUser(path)) + { + s_logger.Warn($"File '{path}' is not owned by the current user."); + return false; + } + + return true; + } + + public virtual bool IsFileOwnedByCurrentUser(string path) + { + return RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || + _unixOperations.GetOwnerIdOfFile(path) == _unixOperations.GetCurrentUserId(); + } } } diff --git a/Snowflake.Data/Core/Tools/TempUtil.cs b/Snowflake.Data/Core/Tools/TempUtil.cs new file mode 100644 index 000000000..6391da133 --- /dev/null +++ b/Snowflake.Data/Core/Tools/TempUtil.cs @@ -0,0 +1,24 @@ +using System.IO; + +namespace Snowflake.Data.Core.Tools +{ + internal static class TempUtil + { + private static readonly string s_tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + + static TempUtil() + { + DirectoryOperations.Instance.CreateDirectory(s_tempDir); + } + + public static string GetTempPath() + { + if (!Directory.Exists(s_tempDir)) + { + DirectoryOperations.Instance.CreateDirectory(s_tempDir); + } + + return s_tempDir; + } + } +} diff --git a/Snowflake.Data/Core/Tools/UnixOperations.cs b/Snowflake.Data/Core/Tools/UnixOperations.cs index 5133255da..076b12b8a 100644 --- a/Snowflake.Data/Core/Tools/UnixOperations.cs +++ b/Snowflake.Data/Core/Tools/UnixOperations.cs @@ -4,11 +4,11 @@ using System; using System.IO; -using System.Linq; -using System.Security; + using System.Text; using Mono.Unix; using Mono.Unix.Native; +using Snowflake.Data.Log; namespace Snowflake.Data.Core.Tools { @@ -16,15 +16,55 @@ namespace Snowflake.Data.Core.Tools internal class UnixOperations { public static readonly UnixOperations Instance = new UnixOperations(); + private static readonly SFLogger s_logger = SFLoggerFactory.GetLogger(); + + public virtual void CreateDirectoryWithPermissions(string path, FileAccessPermissions permissions) + { + var fullPath = Path.GetFullPath(path); + var splitDirectories = fullPath.Split(Path.DirectorySeparatorChar); + + var dirPath = Path.DirectorySeparatorChar.ToString(); + foreach (var dir in splitDirectories) + { + dirPath = Path.Combine(dirPath, dir); + + if (Directory.Exists(dirPath) || dirPath == Path.PathSeparator.ToString()) + { + continue; + } - public virtual int CreateFileWithPermissions(string path, FilePermissions permissions) + CreateSingleDirectory(dirPath, permissions); + } + } + + public virtual int CreateDirectoryWithPermissionsMkdir(string path, FileAccessPermissions permissions) + { + return Syscall.mkdir(path, (FilePermissions)permissions); + } + + private static void CreateSingleDirectory(string path, FileAccessPermissions permissions) { - return Syscall.creat(path, permissions); + s_logger.Debug($"Creating a directory {path} with permissions: {permissions}"); + try + { + new UnixDirectoryInfo(path).Create(permissions); + } + catch (Exception e) + { + throw new IOException("Unable to create directory", e); + } } - public virtual int CreateDirectoryWithPermissions(string path, FilePermissions permissions) + public virtual Stream CreateFileWithPermissions(string path, FileAccessPermissions permissions) { - return Syscall.mkdir(path, permissions); + var dirPath = Path.GetDirectoryName(path); + if (dirPath != null) + { + CreateDirectoryWithPermissions(dirPath, FileAccessPermissions.UserReadWriteExecute); + } + + s_logger.Debug($"Creating a file {path} with permissions: {permissions}"); + return new UnixFileInfo(path).Create(permissions); } public virtual FileAccessPermissions GetFilePermissions(string path) @@ -55,6 +95,12 @@ public virtual bool CheckFileHasAnyOfPermissions(string path, FileAccessPermissi return (permissions & fileInfo.FileAccessPermissions) != 0; } + public virtual bool CheckDirectoryHasAnyOfPermissions(string path, FileAccessPermissions permissions) + { + var directoryInfo = new UnixDirectoryInfo(path); + return (permissions & directoryInfo.FileAccessPermissions) != 0; + } + public string ReadAllText(string path, Action validator) { var fileInfo = new UnixFileInfo(path: path); @@ -92,5 +138,17 @@ public virtual long GetCurrentGroupId() { return Syscall.getgid(); } + + public virtual long GetOwnerIdOfDirectory(string path) + { + var dirInfo = new UnixDirectoryInfo(path); + return dirInfo.OwnerUser.UserId; + } + + public virtual long GetOwnerIdOfFile(string path) + { + var fileInfo = new UnixFileInfo(path); + return fileInfo.OwnerUser.UserId; + } } }