diff --git a/Snowflake.Data.Tests/IntegrationTests/EasyLoggingIT.cs b/Snowflake.Data.Tests/IntegrationTests/EasyLoggingIT.cs index fd2e79409..f0a43ea26 100644 --- a/Snowflake.Data.Tests/IntegrationTests/EasyLoggingIT.cs +++ b/Snowflake.Data.Tests/IntegrationTests/EasyLoggingIT.cs @@ -1,3 +1,4 @@ +using System; using System.Data; using System.IO; using System.Runtime.InteropServices; @@ -73,13 +74,9 @@ public void TestFailToEnableEasyLoggingForWrongConfiguration() } [Test] + [Platform(Exclude="Win", Reason="skip test on Windows")] public void TestFailToEnableEasyLoggingWhenConfigHasWrongPermissions() { - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - Assert.Ignore("skip test on Windows"); - } - // arrange var configFilePath = CreateConfigTempFile(s_workingDirectory, Config("WARN", s_workingDirectory)); Syscall.chmod(configFilePath, FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IWGRP); @@ -91,19 +88,15 @@ public void TestFailToEnableEasyLoggingWhenConfigHasWrongPermissions() var thrown = Assert.Throws(() => conn.Open()); // assert - Assert.That(thrown.Message, Does.Contain("Connection string is invalid: Unable to initialize session")); + Assert.That(thrown.Message, Does.Contain("Attempting to read a file with too broad permissions assigned")); Assert.IsFalse(EasyLoggerManager.HasEasyLoggingAppender()); } } [Test] + [Platform(Exclude="Win", Reason="skip test on Windows")] public void TestFailToEnableEasyLoggingWhenLogDirectoryNotAccessible() { - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - Assert.Ignore("skip test on Windows"); - } - // arrange var configFilePath = CreateConfigTempFile(s_workingDirectory, Config("WARN", "/")); using (IDbConnection conn = new SnowflakeDbConnection()) diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index b23fbbf0e..1552745a7 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -39,13 +39,12 @@ public class EasyLoggingConfigFinderTest public void Setup() { t_fileOperations = new Mock(); - t_unixOperations = new Mock(); t_environmentOperations = new Mock(); - t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_unixOperations.Object, t_environmentOperations.Object); + t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_environmentOperations.Object); MockHomeDirectory(); MockExecutionDirectory(); } - + [Test] public void TestThatTakesFilePathFromTheInput() { @@ -53,10 +52,10 @@ public void TestThatTakesFilePathFromTheInput() MockFileFromEnvironmentalVariable(); MockFileOnDriverPath(); MockFileOnHomePath(); - + // act var filePath = t_finder.FindConfigFilePath(InputConfigFilePath); - + // assert Assert.AreEqual(InputConfigFilePath, filePath); t_fileOperations.VerifyNoOtherCalls(); @@ -71,14 +70,14 @@ public void TestThatTakesFilePathFromEnvironmentVariableIfInputNotPresent( MockFileFromEnvironmentalVariable(); MockFileOnDriverPath(); MockFileOnHomePath(); - + // act var filePath = t_finder.FindConfigFilePath(inputFilePath); - + // assert Assert.AreEqual(EnvironmentalConfigFilePath, filePath); } - + [Test] public void TestThatTakesFilePathFromDriverLocationWhenNoInputParameterNorEnvironmentVariable() { @@ -88,20 +87,20 @@ public void TestThatTakesFilePathFromDriverLocationWhenNoInputParameterNorEnviro // act var filePath = t_finder.FindConfigFilePath(null); - + // assert Assert.AreEqual(s_driverConfigFilePath, filePath); } - + [Test] public void TestThatTakesFilePathFromHomeLocationWhenNoInputParamEnvironmentVarNorDriverLocation() { // arrange MockFileOnHomePath(); - + // act var filePath = t_finder.FindConfigFilePath(null); - + // assert Assert.AreEqual(s_homeConfigFilePath, filePath); } @@ -119,32 +118,12 @@ public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible() Assert.AreEqual(s_homeConfigFilePath, filePath); } - [Test] - public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile() - { - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - Assert.Ignore("skip test on Windows"); - } - - // arrange - MockFileOnHomePath(); - MockHasFlagReturnsTrue(); - - // act - var thrown = Assert.Throws(() => t_finder.FindConfigFilePath(null)); - - // assert - Assert.IsNotNull(thrown); - Assert.AreEqual(thrown.Message, $"Error due to other users having permission to modify the config file: {s_homeConfigFilePath}"); - } - [Test] public void TestThatReturnsNullIfNoWayOfGettingTheFile() { // act var filePath = t_finder.FindConfigFilePath(null); - + // assert Assert.IsNull(filePath); } @@ -157,7 +136,7 @@ public void TestThatDoesNotFailWhenSearchForOneOfDirectoriesFails() // act var filePath = t_finder.FindConfigFilePath(null); - + // assert Assert.IsNull(filePath); t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once); @@ -186,7 +165,7 @@ public void TestThatDoesNotFailWhenHomeDirectoryDoesNotExist() // act var filePath = t_finder.FindConfigFilePath(null); - + // assert Assert.IsNull(filePath); t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once); @@ -220,7 +199,7 @@ private static void MockExecutionDirectory() .Setup(e => e.GetExecutionDirectory()) .Returns(DriverDirectory); } - + private static void MockFileOnHomePathDoesNotExist() { t_fileOperations diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigParserTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigParserTest.cs index 847d57c51..2d516cc97 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigParserTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigParserTest.cs @@ -33,7 +33,7 @@ public static void AfterAll() { Directory.Delete(s_workingDirectory, true); } - + [Test] public void TestThatParsesConfigFile() { @@ -59,12 +59,12 @@ public void TestThatParsesConfigFileWithNullValues(string filePath) // act var config = parser.Parse(filePath); - + // assert Assert.IsNotNull(config); Assert.IsNotNull(config.CommonProps); Assert.IsNull(config.CommonProps.LogLevel); - Assert.IsNull(config.CommonProps.LogPath); + Assert.IsNull(config.CommonProps.LogPath); } [Test] @@ -77,23 +77,23 @@ public void TestThatReturnsNullWhenNothingToParse(string noFilePath) // act var config = parser.Parse(noFilePath); - + // assert Assert.IsNull(config); } - + [Test] public void TestThatFailsWhenTheFileDoesNotExist() { // arrange var parser = new EasyLoggingConfigParser(); - + // act - var thrown = Assert.Throws(() => parser.Parse(NotExistingFilePath)); - + var thrown = Assert.Throws(() => parser.Parse(NotExistingFilePath)); + // assert Assert.IsNotNull(thrown); - Assert.AreEqual("Finding easy logging configuration failed", thrown.Message); + Assert.AreEqual("No such file or directory", thrown.Message); } [Test, TestCaseSource(nameof(WrongConfigFiles))] diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index d417eb4d8..6d2060a52 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -19,15 +19,13 @@ internal class EasyLoggingConfigFinder internal const string ClientConfigEnvironmentName = "SF_CLIENT_CONFIG_FILE"; private readonly FileOperations _fileOperations; - private readonly UnixOperations _unixOperations; private readonly EnvironmentOperations _environmentOperations; - public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, UnixOperations.Instance, EnvironmentOperations.Instance); + public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, EnvironmentOperations.Instance); - internal EasyLoggingConfigFinder(FileOperations fileOperations, UnixOperations unixFileOperations, EnvironmentOperations environmentOperations) + internal EasyLoggingConfigFinder(FileOperations fileOperations, EnvironmentOperations environmentOperations) { _fileOperations = fileOperations; - _unixOperations = unixFileOperations; _environmentOperations = environmentOperations; } @@ -38,16 +36,12 @@ internal EasyLoggingConfigFinder() public virtual string FindConfigFilePath(string configFilePathFromConnectionString) { var configFilePath = GetFilePathFromInputParameter(configFilePathFromConnectionString, "connection string") - ?? GetFilePathEnvironmentVariable() - ?? GetFilePathFromDriverLocation() - ?? GetFilePathFromHomeDirectory(); - if (configFilePath != null) - { - CheckIfValidPermissions(configFilePath); - } + ?? GetFilePathEnvironmentVariable() + ?? GetFilePathFromDriverLocation() + ?? GetFilePathFromHomeDirectory(); return configFilePath; } - + private string GetFilePathEnvironmentVariable() { var filePath = _environmentOperations.GetEnvironmentVariable(ClientConfigEnvironmentName); @@ -100,19 +94,5 @@ private string OnlyIfFileExists(string filePath, string directoryDescription) } return null; } - - private void CheckIfValidPermissions(string filePath) - { - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - return; - - // Check if others have permissions to modify the file and fail if so - if (_unixOperations.CheckFileHasAnyOfPermissions(filePath, FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite)) - { - var errorMessage = $"Error due to other users having permission to modify the config file: {filePath}"; - s_logger.Error(errorMessage); - throw new Exception(errorMessage); - } - } } } diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs index dbc6820b4..582badd9d 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs @@ -7,8 +7,17 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.InteropServices; +using System.Security; +using System.Text; +using Microsoft.IdentityModel.Tokens; +using Mono.Unix; +using Mono.Unix.Native; using Newtonsoft.Json; using Newtonsoft.Json.Linq; +using Snowflake.Data.Client; +using Snowflake.Data.Core; +using Snowflake.Data.Core.Tools; using Snowflake.Data.Log; namespace Snowflake.Data.Configuration @@ -22,30 +31,68 @@ internal class EasyLoggingConfigParser public virtual ClientConfig Parse(string filePath) { var configFile = TryToReadFile(filePath); - return configFile == null ? null : TryToParseFile(configFile); + return configFile.IsNullOrEmpty() ? null : TryToParseFile(configFile); } + /// + /// ReadAllText function reads contents of a file at the making sure, in a way not prone to race-conditions, that: + /// - A file is owned by the same user as effective user of the current process. + /// - A file is owned by the same group as effective group of the current process. + /// - A file permissions do not include `forbiddenPermissions` (any others' permissions by default) + /// + /// + /// The file path of the configuration file + /// + /// An exception will be thrown if the file is no owned by the user or group private string TryToReadFile(string filePath) { if (string.IsNullOrEmpty(filePath)) { - return null; + return String.Empty; } + try { - return File.ReadAllText(filePath); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + var streamReader = new StreamReader(filePath, Encoding.Default); + return streamReader.ReadToEnd(); + } + else + { + var handle = VerifyUnixPermissions(filePath); + + var streamReader = new StreamReader(handle, Encoding.Default); + return streamReader.ReadToEnd(); + } } catch (Exception e) { var errorMessage = "Finding easy logging configuration failed"; s_logger.Error(errorMessage, e); - throw new Exception(errorMessage); + throw; } } + private static UnixStream VerifyUnixPermissions(string filePath) + { + FileAccessPermissions forbiddenPermissions = FileAccessPermissions.OtherWrite | FileAccessPermissions.GroupWrite; + var fileInfo = new UnixFileInfo(path: filePath); + + var handle = fileInfo.OpenRead(); + if (handle.OwnerUser.UserId != Syscall.geteuid()) + throw new SnowflakeDbException(SFError.INTERNAL_ERROR, "Attempting to read a file not owned by the effective user of the current process"); + if (handle.OwnerGroup.GroupId != Syscall.getegid()) + throw new SnowflakeDbException(SFError.INTERNAL_ERROR, "Attempting to read a file not owned by the effective group of the current process"); + if ((handle.FileAccessPermissions & forbiddenPermissions) != 0) + throw new SnowflakeDbException(SFError.INTERNAL_ERROR, "Attempting to read a file with too broad permissions assigned"); + return handle; + } + private ClientConfig TryToParseFile(string fileContent) { - try { + try + { var config = JsonConvert.DeserializeObject(fileContent); Validate(config); CheckForUnknownFields(fileContent); diff --git a/Snowflake.Data/Snowflake.Data.csproj b/Snowflake.Data/Snowflake.Data.csproj index 01fc2891e..c72636fac 100644 --- a/Snowflake.Data/Snowflake.Data.csproj +++ b/Snowflake.Data/Snowflake.Data.csproj @@ -13,7 +13,7 @@ Snowflake 4.0.0 Full - 7.3 + 8.0