From bd5d8da929b9f154f8ebd6e42d1f8dad267cf12c Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-lf <115584722+sfc-gh-ext-simba-lf@users.noreply.github.com> Date: Mon, 5 Feb 2024 09:02:21 -0800 Subject: [PATCH] SNOW-990111: Add easy logging improvements (#849) ### Description Modify easy logging based on list of security improvements ### Checklist - [x] Code compiles correctly - [x] Code is formatted according to [Coding Conventions](../CodingConventions.md) - [x] Created tests which fail without the change (if possible) - [x] All tests passing (`dotnet test`) - [x] Extended the README / documentation, if necessary - [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name --- .../EasyLoggingConfigFinderTest.cs | 87 +++++-- .../UnitTests/Logger/EasyLoggerManagerTest.cs | 35 ++- .../UnixFilePermissionsConverterTest.cs | 224 ++++++++++++++++++ .../Session/EasyLoggingStarterTest.cs | 130 +++++++++- .../Configuration/EasyLoggingConfigFinder.cs | 67 ++++-- .../Configuration/EasyLoggingConfigParser.cs | 19 ++ .../Core/Session/EasyLoggingStarter.cs | 63 ++++- .../Core/Tools/HomeDirectoryProvider.cs | 31 +++ Snowflake.Data/Core/Tools/UnixOperations.cs | 31 +++ .../Logger/UnixFilePermissionsConverter .cs | 51 ++++ Snowflake.Data/Snowflake.Data.csproj | 1 + 11 files changed, 689 insertions(+), 50 deletions(-) create mode 100644 Snowflake.Data.Tests/UnitTests/Logger/UnixFilePermissionsConverterTest.cs create mode 100644 Snowflake.Data/Core/Tools/HomeDirectoryProvider.cs create mode 100644 Snowflake.Data/Core/Tools/UnixOperations.cs create mode 100644 Snowflake.Data/Logger/UnixFilePermissionsConverter .cs diff --git a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs index c73feab9a..bea1e14d0 100644 --- a/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs @@ -4,6 +4,8 @@ using System; using System.IO; +using System.Runtime.InteropServices; +using Mono.Unix; using Moq; using NUnit.Framework; using Snowflake.Data.Configuration; @@ -17,13 +19,16 @@ public class EasyLoggingConfigFinderTest private const string InputConfigFilePath = "input_config.json"; private const string EnvironmentalConfigFilePath = "environmental_config.json"; private const string HomeDirectory = "/home/user"; - private static readonly string s_driverConfigFilePath = Path.Combine(".", EasyLoggingConfigFinder.ClientConfigFileName); + private const string DriverDirectory = "."; + private static readonly string s_driverConfigFilePath = Path.Combine(DriverDirectory, EasyLoggingConfigFinder.ClientConfigFileName); private static readonly string s_homeConfigFilePath = Path.Combine(HomeDirectory, EasyLoggingConfigFinder.ClientConfigFileName); - private static readonly string s_tempConfigFilePath = Path.Combine(Path.GetTempPath(), EasyLoggingConfigFinder.ClientConfigFileName); [ThreadStatic] private static Mock t_fileOperations; + [ThreadStatic] + private static Mock t_unixOperations; + [ThreadStatic] private static Mock t_environmentOperations; @@ -34,8 +39,9 @@ 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_environmentOperations.Object); + t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_unixOperations.Object, t_environmentOperations.Object); MockHomeDirectory(); } @@ -46,7 +52,6 @@ public void TestThatTakesFilePathFromTheInput() MockFileFromEnvironmentalVariable(); MockFileOnDriverPath(); MockFileOnHomePath(); - MockFileOnTempPath(); // act var filePath = t_finder.FindConfigFilePath(InputConfigFilePath); @@ -65,7 +70,6 @@ public void TestThatTakesFilePathFromEnvironmentVariableIfInputNotPresent( MockFileFromEnvironmentalVariable(); MockFileOnDriverPath(); MockFileOnHomePath(); - MockFileOnTempPath(); // act var filePath = t_finder.FindConfigFilePath(inputFilePath); @@ -80,7 +84,6 @@ public void TestThatTakesFilePathFromDriverLocationWhenNoInputParameterNorEnviro // arrange MockFileOnDriverPath(); MockFileOnHomePath(); - MockFileOnTempPath(); // act var filePath = t_finder.FindConfigFilePath(null); @@ -94,7 +97,6 @@ public void TestThatTakesFilePathFromHomeLocationWhenNoInputParamEnvironmentVarN { // arrange MockFileOnHomePath(); - MockFileOnTempPath(); // act var filePath = t_finder.FindConfigFilePath(null); @@ -104,16 +106,36 @@ public void TestThatTakesFilePathFromHomeLocationWhenNoInputParamEnvironmentVarN } [Test] - public void TestThatTakesFilePathFromTempDirectoryWhenNoOtherWaysPossible() + public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible() { // arrange - MockFileOnTempPath(); - + MockFileOnHomePath(); + // act var filePath = t_finder.FindConfigFilePath(null); - + + // assert + 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.AreEqual(s_tempConfigFilePath, filePath); + Assert.IsNotNull(thrown); + Assert.AreEqual(thrown.Message, $"Error due to other users having permission to modify the config file: {s_homeConfigFilePath}"); } [Test] @@ -139,13 +161,28 @@ public void TestThatDoesNotFailWhenSearchForOneOfDirectoriesFails() Assert.IsNull(filePath); t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once); } - + [Test] - public void TestThatDoesNotFailWhenOneOfDirectoriesNotDefined() + public void TestThatDoesNotFailWhenHomeDirectoryReturnsNull() { // arrange MockHomeDirectoryReturnsNull(); + // act + var filePath = t_finder.FindConfigFilePath(null); + + // assert + Assert.IsNull(filePath); + t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once); + } + + [Test] + public void TestThatDoesNotFailWhenHomeDirectoryDoesNotExist() + { + // arrange + MockFileOnHomePath(); + MockFileOnHomePathDoesNotExist(); + // act var filePath = t_finder.FindConfigFilePath(null); @@ -154,6 +191,14 @@ public void TestThatDoesNotFailWhenOneOfDirectoriesNotDefined() t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once); } + private static void MockHasFlagReturnsTrue() + { + t_unixOperations + .Setup(f => f.CheckFileHasPermissions(s_homeConfigFilePath, + It.Is(p => p.Equals(FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite)))) + .Returns(true); + } + private static void MockHomeDirectory() { t_environmentOperations @@ -168,6 +213,13 @@ private static void MockHomeDirectoryFails() .Throws(() => new Exception("No home directory")); } + private static void MockFileOnHomePathDoesNotExist() + { + t_fileOperations + .Setup(f => f.Exists(s_homeConfigFilePath)) + .Returns(false); + } + private static void MockHomeDirectoryReturnsNull() { t_environmentOperations @@ -195,12 +247,5 @@ private static void MockFileOnHomePath() .Setup(f => f.Exists(s_homeConfigFilePath)) .Returns(true); } - - private static void MockFileOnTempPath() - { - t_fileOperations - .Setup(f => f.Exists(s_tempConfigFilePath)) - .Returns(true); - } } } diff --git a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs index feeb728b9..4dfba8c83 100644 --- a/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs @@ -5,6 +5,7 @@ using System; using System.IO; using System.Linq; +using System.Runtime.InteropServices; using NUnit.Framework; using Snowflake.Data.Configuration; using Snowflake.Data.Core; @@ -21,7 +22,7 @@ public class EasyLoggerManagerTest private const string WarnMessage = "Easy logging Warn message"; private const string ErrorMessage = "Easy logging Error message"; private const string FatalMessage = "Easy logging Fatal message"; - private static readonly string s_logsDirectory = Path.GetTempPath(); + private static readonly string s_logsDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); [ThreadStatic] private static string t_directoryLogPath; @@ -92,6 +93,38 @@ public void TestThatLogsToProperFileWithProperLogLevelOnly() Assert.That(logLines, Has.Exactly(1).Matches(s => s.Contains(FatalMessage))); } + [Test] + public void TestThatOnlyUnknownFieldsAreLogged() + { + // arrange + string expectedFakeLogField = "fake_log_field"; + string ConfigWithUnknownFields = $@"{{ + ""common"": {{ + ""LOG_LEVEL"": ""warn"", + ""lOg_PaTh"": ""path"", + ""{expectedFakeLogField}_1"": ""abc"", + ""{expectedFakeLogField}_2"": ""123"" + }} + }}"; + var configFilePath = Guid.NewGuid().ToString() + ".json"; + using (var writer = File.CreateText(configFilePath)) + { + writer.Write(ConfigWithUnknownFields); + } + EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Warn, t_directoryLogPath); + var parser = new EasyLoggingConfigParser(); + + // act + parser.Parse(configFilePath); + + // assert + var logLines = File.ReadLines(FindLogFilePath(t_directoryLogPath)); + Assert.That(logLines, Has.Exactly(2).Matches(s => s.Contains($"Unknown field from config: {expectedFakeLogField}"))); + + // cleanup + File.Delete(configFilePath); + } + private static string RandomLogsDirectoryPath() { var randomName = Path.GetRandomFileName(); diff --git a/Snowflake.Data.Tests/UnitTests/Logger/UnixFilePermissionsConverterTest.cs b/Snowflake.Data.Tests/UnitTests/Logger/UnixFilePermissionsConverterTest.cs new file mode 100644 index 000000000..12f4ec756 --- /dev/null +++ b/Snowflake.Data.Tests/UnitTests/Logger/UnixFilePermissionsConverterTest.cs @@ -0,0 +1,224 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. + */ + +using Mono.Unix; +using NUnit.Framework; +using Snowflake.Data.Log; +using System.Collections.Generic; + +namespace Snowflake.Data.Tests.UnitTests.Logger +{ + [TestFixture] + public class UnixFilePermissionsConverterTest + { + [Test] + public void TestConversionForAllPermissionCombinations( + [ValueSource(nameof(UserPermissionTestCases))] PermissionTestCase userTestCase, + [ValueSource(nameof(GroupPermissionTestCases))] PermissionTestCase groupTestCase, + [ValueSource(nameof(OtherPermissionTestCases))] PermissionTestCase otherTestCase) + { + // arrange + var permissions = userTestCase.permissions | groupTestCase.permissions | otherTestCase.permissions; + var expectedPermissions = userTestCase.expectedPermissions + groupTestCase.expectedPermissions + otherTestCase.expectedPermissions; + + // act + var convertedPermissions = UnixFilePermissionsConverter.ConvertFileAccessPermissionsToInt(permissions); + + // assert + Assert.AreEqual(expectedPermissions, convertedPermissions); + } + + public static IEnumerable UserPermissionTestCases() + { + var noPermissionTestCase = new PermissionTestCase() + { + expectedPermissions = 000 + }; + + var readPermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserRead, + expectedPermissions = 400 + }; + + var writePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserWrite, + expectedPermissions = 200 + }; + + var executePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserExecute, + expectedPermissions = 100 + }; + + var readWritePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite, + expectedPermissions = 600 + }; + + var readExecutePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserRead | FileAccessPermissions.UserExecute, + expectedPermissions = 500 + }; + + var writeExecutePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserWrite | FileAccessPermissions.UserExecute, + expectedPermissions = 300 + }; + + var allPermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.UserReadWriteExecute, + expectedPermissions = 700 + }; + + return new[] + { + noPermissionTestCase, + readPermissionsTestCase, + writePermissionsTestCase, + executePermissionsTestCase, + readWritePermissionsTestCase, + readExecutePermissionsTestCase, + writeExecutePermissionsTestCase, + allPermissionsTestCase + }; + } + + public static IEnumerable GroupPermissionTestCases() + { + var noPermissionTestCase = new PermissionTestCase() + { + expectedPermissions = 000 + }; + + var readPermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupRead, + expectedPermissions = 040 + }; + + var writePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupWrite, + expectedPermissions = 020 + }; + + var executePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupExecute, + expectedPermissions = 010 + }; + + var readWritePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupRead | FileAccessPermissions.GroupWrite, + expectedPermissions = 060 + }; + + var readExecutePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupRead | FileAccessPermissions.GroupExecute, + expectedPermissions = 050 + }; + + var writeExecutePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupWrite | FileAccessPermissions.GroupExecute, + expectedPermissions = 030 + }; + + var allPermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.GroupReadWriteExecute, + expectedPermissions = 070 + }; + + return new[] + { + noPermissionTestCase, + readPermissionsTestCase, + writePermissionsTestCase, + executePermissionsTestCase, + readWritePermissionsTestCase, + readExecutePermissionsTestCase, + writeExecutePermissionsTestCase, + allPermissionsTestCase + }; + } + + public static IEnumerable OtherPermissionTestCases() + { + var noPermissionTestCase = new PermissionTestCase() + { + expectedPermissions = 000 + }; + + var readPermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherRead, + expectedPermissions = 004 + }; + + var writePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherWrite, + expectedPermissions = 002 + }; + + var executePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherExecute, + expectedPermissions = 001 + }; + + var readWritePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherRead | FileAccessPermissions.OtherWrite, + expectedPermissions = 006 + }; + + var readExecutePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherRead | FileAccessPermissions.OtherExecute, + expectedPermissions = 005 + }; + + var writeExecutePermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherWrite | FileAccessPermissions.OtherExecute, + expectedPermissions = 003 + }; + + var allPermissionsTestCase = new PermissionTestCase() + { + permissions = FileAccessPermissions.OtherReadWriteExecute, + expectedPermissions = 007 + }; + + return new[] + { + noPermissionTestCase, + readPermissionsTestCase, + writePermissionsTestCase, + executePermissionsTestCase, + readWritePermissionsTestCase, + readExecutePermissionsTestCase, + writeExecutePermissionsTestCase, + allPermissionsTestCase + }; + } + + public class PermissionTestCase + { + internal FileAccessPermissions permissions; + internal int expectedPermissions; + } + } +} diff --git a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs index 00a2ed9b7..3b96339a0 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/EasyLoggingStarterTest.cs @@ -4,6 +4,9 @@ using System; using System.IO; +using System.Runtime.InteropServices; +using Mono.Unix; +using Mono.Unix.Native; using Moq; using NUnit.Framework; using Snowflake.Data.Configuration; @@ -16,8 +19,8 @@ namespace Snowflake.Data.Tests.UnitTests.Session [TestFixture] public class EasyLoggingStarterTest { - - private const string LogPath = "/some-logs-path/some-folder"; + private static readonly string HomeDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + private static readonly string LogPath = Path.Combine(HomeDirectory, "some-logs-path/some-folder"); private const string ConfigPath = "/some-path/config.json"; private const string AnotherConfigPath = "/another/path"; private static readonly string s_expectedLogPath = Path.Combine(LogPath, "dotnet"); @@ -39,16 +42,30 @@ public class EasyLoggingStarterTest LogPath = LogPath } }; - + + private static readonly ClientConfig s_configWithNoLogPath = new ClientConfig + { + CommonProps = new ClientConfigCommonProps + { + LogLevel = "Info" + } + }; + [ThreadStatic] private static Mock t_easyLoggingProvider; [ThreadStatic] private static Mock t_easyLoggerManager; + [ThreadStatic] + private static Mock t_unixOperations; + [ThreadStatic] private static Mock t_directoryOperations; + [ThreadStatic] + private static Mock t_environmentOperations; + [ThreadStatic] private static EasyLoggingStarter t_easyLoggerStarter; @@ -57,10 +74,83 @@ public void BeforeEach() { t_easyLoggingProvider = new Mock(); t_easyLoggerManager = new Mock(); + t_unixOperations = new Mock(); t_directoryOperations = new Mock(); - t_easyLoggerStarter = new EasyLoggingStarter(t_easyLoggingProvider.Object, t_easyLoggerManager.Object, t_directoryOperations.Object); + t_environmentOperations = new Mock(); + t_easyLoggerStarter = new EasyLoggingStarter( + t_easyLoggingProvider.Object, + t_easyLoggerManager.Object, + t_unixOperations.Object, + t_directoryOperations.Object, + t_environmentOperations.Object); } - + + [Test] + public void TestThatThrowsErrorWhenLogPathAndHomeDirectoryIsNotSet() + { + // arrange + t_easyLoggingProvider + .Setup(provider => provider.ProvideConfig(ConfigPath)) + .Returns(s_configWithNoLogPath); + t_environmentOperations + .Setup(env => env.GetFolderPath(Environment.SpecialFolder.UserProfile)) + .Returns(""); + + // act + var thrown = Assert.Throws(() => t_easyLoggerStarter.Init(ConfigPath)); + + // assert + Assert.IsNotNull(thrown); + Assert.AreEqual(thrown.Message, "No log path found for easy logging. Home directory is not configured and log path is not provided"); + } + + [Test] + public void TestThatThrowsErrorWhenLogPathIsNotSetAndHomeDirectoryThrowsAnException() + { + // arrange + var ex = new Exception("No home directory"); + t_easyLoggingProvider + .Setup(provider => provider.ProvideConfig(ConfigPath)) + .Returns(s_configWithNoLogPath); + t_environmentOperations + .Setup(env => env.GetFolderPath(Environment.SpecialFolder.UserProfile)) + .Throws(() => ex); + + // act + var thrown = Assert.Throws(() => t_easyLoggerStarter.Init(ConfigPath)); + + // assert + Assert.IsNotNull(thrown); + Assert.AreEqual(thrown.Message, "No log path found for easy logging. Home directory is not configured and log path is not provided"); + } + + [Test] + public void TestThatDoesNotFailWhenLogDirectoryPermissionIsNot700() + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.Ignore("skip test on Windows"); + } + + // arrange + t_easyLoggingProvider + .Setup(provider => provider.ProvideConfig(ConfigPath)) + .Returns(s_configWithInfoLevel); + t_directoryOperations + .Setup(dir => dir.Exists(s_expectedLogPath)) + .Returns(true); + t_unixOperations + .Setup(unix => unix.GetDirPermissions(s_expectedLogPath)) + .Returns(FileAccessPermissions.AllPermissions); + + // act + t_easyLoggerStarter.Init(ConfigPath); + + // assert + t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, + FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Never); + } + [Test] public void TestThatConfiguresEasyLoggingOnlyOnceWhenInitializedWithConfigPath() { @@ -79,7 +169,15 @@ public void TestThatConfiguresEasyLoggingOnlyOnceWhenInitializedWithConfigPath() t_easyLoggerStarter.Init(ConfigPath); // assert - t_directoryOperations.Verify(d => d.CreateDirectory(s_expectedLogPath), Times.Once); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + t_directoryOperations.Verify(d => d.CreateDirectory(s_expectedLogPath), Times.Once); + } + else + { + t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, + FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Once); + } t_easyLoggerManager.Verify(manager => manager.ReconfigureEasyLogging(EasyLoggingLogLevel.Error, s_expectedLogPath), Times.Once); // act @@ -104,7 +202,15 @@ public void TestThatConfiguresEasyLoggingOnlyOnceForInitializationsWithoutConfig t_easyLoggerStarter.Init(null); // assert - t_directoryOperations.Verify(d => d.CreateDirectory(s_expectedLogPath), Times.Once); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + t_directoryOperations.Verify(d => d.CreateDirectory(s_expectedLogPath), Times.Once); + } + else + { + t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, + FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Once); + } t_easyLoggerManager.Verify(manager => manager.ReconfigureEasyLogging(EasyLoggingLogLevel.Error, s_expectedLogPath), Times.Once); } @@ -123,7 +229,15 @@ public void TestThatReconfiguresEasyLoggingWithConfigPathIfNotGivenForTheFirstTi t_easyLoggerStarter.Init(null); // assert - t_directoryOperations.Verify(d => d.CreateDirectory(s_expectedLogPath), Times.Once); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + t_directoryOperations.Verify(d => d.CreateDirectory(s_expectedLogPath), Times.Once); + } + else + { + t_unixOperations.Verify(u => u.CreateDirectoryWithPermissions(s_expectedLogPath, + FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR), Times.Once); + } t_easyLoggerManager.Verify(manager => manager.ReconfigureEasyLogging(EasyLoggingLogLevel.Error, s_expectedLogPath), Times.Once); // act diff --git a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs index 1d0c375b3..3fed8850e 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs @@ -4,6 +4,8 @@ using System; using System.IO; +using System.Runtime.InteropServices; +using Mono.Unix; using Snowflake.Data.Core.Tools; using Snowflake.Data.Log; @@ -17,13 +19,15 @@ 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, EnvironmentOperations.Instance); + public static readonly EasyLoggingConfigFinder Instance = new EasyLoggingConfigFinder(FileOperations.Instance, UnixOperations.Instance, EnvironmentOperations.Instance); - internal EasyLoggingConfigFinder(FileOperations fileOperations, EnvironmentOperations environmentOperations) + internal EasyLoggingConfigFinder(FileOperations fileOperations, UnixOperations unixFileOperations, EnvironmentOperations environmentOperations) { _fileOperations = fileOperations; + _unixOperations = unixFileOperations; _environmentOperations = environmentOperations; } @@ -33,26 +37,36 @@ internal EasyLoggingConfigFinder() public virtual string FindConfigFilePath(string configFilePathFromConnectionString) { - return GetFilePathFromInputParameter(configFilePathFromConnectionString) - ?? GetFilePathEnvironmentVariable() - ?? GetFilePathFromDriverLocation() - ?? GetFilePathFromHomeDirectory() - ?? GetFilePathFromTempDirectory(); + var configFilePath = GetFilePathFromInputParameter(configFilePathFromConnectionString, "connection string") + ?? GetFilePathEnvironmentVariable() + ?? GetFilePathFromDriverLocation() + ?? GetFilePathFromHomeDirectory(); + if (configFilePath != null) + { + CheckIfValidPermissions(configFilePath); + } + return configFilePath; } private string GetFilePathEnvironmentVariable() { var filePath = _environmentOperations.GetEnvironmentVariable(ClientConfigEnvironmentName); - return GetFilePathFromInputParameter(filePath); + return GetFilePathFromInputParameter(filePath, "environment variable"); } - - private string GetFilePathFromTempDirectory() => SearchForConfigInDirectory(Path.GetTempPath, "temp"); private string GetFilePathFromHomeDirectory() => SearchForConfigInDirectory(GetHomeDirectory, "home"); - - private string GetFilePathFromInputParameter(string filePath) => string.IsNullOrEmpty(filePath) ? null : filePath; - private string GetHomeDirectory() =>_environmentOperations.GetFolderPath(Environment.SpecialFolder.UserProfile); + private string GetFilePathFromInputParameter(string filePath, string inputDescription) + { + if (string.IsNullOrEmpty(filePath)) + { + return null; + } + s_logger.Info($"Using config file specified from {inputDescription}: {filePath}"); + return filePath; + } + + private string GetHomeDirectory() => HomeDirectoryProvider.HomeDirectory(_environmentOperations); private string GetFilePathFromDriverLocation() => SearchForConfigInDirectory(() => ".", "driver"); @@ -63,11 +77,12 @@ private string SearchForConfigInDirectory(Func directoryProvider, string var directory = directoryProvider.Invoke(); if (string.IsNullOrEmpty(directory)) { + s_logger.Warn($"The {directoryDescription} directory could not be determined and will be skipped"); return null; } var filePath = Path.Combine(directory, ClientConfigFileName); - return OnlyIfFileExists(filePath); + return OnlyIfFileExists(filePath, directoryDescription); } catch (Exception e) { @@ -76,6 +91,28 @@ private string SearchForConfigInDirectory(Func directoryProvider, string } } - private string OnlyIfFileExists(string filePath) => _fileOperations.Exists(filePath) ? filePath : null; + private string OnlyIfFileExists(string filePath, string directoryDescription) + { + if (_fileOperations.Exists(filePath)) + { + s_logger.Info($"Using config file specified from {directoryDescription} directory: {filePath}"); + return filePath; + } + 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.CheckFileHasPermissions(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 f21af2302..dbc6820b4 100644 --- a/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs +++ b/Snowflake.Data/Configuration/EasyLoggingConfigParser.cs @@ -3,8 +3,12 @@ */ using System; +using System.Collections.Generic; using System.IO; +using System.Linq; +using System.Reflection; using Newtonsoft.Json; +using Newtonsoft.Json.Linq; using Snowflake.Data.Log; namespace Snowflake.Data.Configuration @@ -44,6 +48,7 @@ private ClientConfig TryToParseFile(string fileContent) try { var config = JsonConvert.DeserializeObject(fileContent); Validate(config); + CheckForUnknownFields(fileContent); return config; } catch (Exception e) @@ -61,5 +66,19 @@ private void Validate(ClientConfig config) EasyLoggingLogLevelExtensions.From(config.CommonProps.LogLevel); } } + + private void CheckForUnknownFields(string fileContent) + { + // Parse the specified config file and get the key-value pairs from the "common" section + List knownProperties = typeof(ClientConfigCommonProps).GetProperties() + .Select(property => property.GetCustomAttribute().PropertyName) + .ToList(); + + JObject.Parse(fileContent).GetValue("common", StringComparison.OrdinalIgnoreCase)? + .Cast() + .Where(property => !knownProperties.Contains(property.Name, StringComparer.OrdinalIgnoreCase)) + .ToList() + .ForEach(unknownKey => s_logger.Warn($"Unknown field from config: {unknownKey.Name}")); + } } } diff --git a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs index 9800e5186..cd82d3904 100644 --- a/Snowflake.Data/Core/Session/EasyLoggingStarter.cs +++ b/Snowflake.Data/Core/Session/EasyLoggingStarter.cs @@ -2,7 +2,11 @@ * Copyright (c) 2023 Snowflake Computing Inc. All rights reserved. */ +using System; 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; @@ -17,23 +21,31 @@ internal class EasyLoggingStarter private readonly EasyLoggerManager _easyLoggerManager; + private readonly UnixOperations _unixOperations; + private readonly DirectoryOperations _directoryOperations; + private readonly EnvironmentOperations _environmentOperations; + private readonly object _lockForExclusiveInit = new object(); private EasyLoggingInitTrialParameters _initTrialParameters = null; public static readonly EasyLoggingStarter Instance = new EasyLoggingStarter(EasyLoggingConfigProvider.Instance, - EasyLoggerManager.Instance, DirectoryOperations.Instance); + EasyLoggerManager.Instance, UnixOperations.Instance, DirectoryOperations.Instance, EnvironmentOperations.Instance); internal EasyLoggingStarter( EasyLoggingConfigProvider easyLoggingConfigProvider, EasyLoggerManager easyLoggerManager, - DirectoryOperations directoryOperations) + UnixOperations unixOperations, + DirectoryOperations directoryOperations, + EnvironmentOperations environmentOperations) { _easyLoggingConfigProvider = easyLoggingConfigProvider; _easyLoggerManager = easyLoggerManager; + _unixOperations = unixOperations; _directoryOperations = directoryOperations; + _environmentOperations = environmentOperations; } internal EasyLoggingStarter() @@ -48,6 +60,14 @@ public virtual void Init(string configFilePathFromConnectionString) { return; } + if (string.IsNullOrEmpty(configFilePathFromConnectionString)) + { + s_logger.Info($"Attempting to enable easy logging without a config file specified from connection string"); + } + else + { + s_logger.Info($"Attempting to enable easy logging using config file specified from connection string: {configFilePathFromConnectionString}"); + } var config = _easyLoggingConfigProvider.ProvideConfig(configFilePathFromConnectionString); if (config == null) { @@ -56,6 +76,8 @@ public virtual void Init(string configFilePathFromConnectionString) } var logLevel = GetLogLevel(config.CommonProps.LogLevel); var logPath = GetLogPath(config.CommonProps.LogPath); + s_logger.Info($"LogLevel set to {logLevel}"); + s_logger.Info($"LogPath set to {logPath}"); _easyLoggerManager.ReconfigureEasyLogging(logLevel, logPath); _initTrialParameters = new EasyLoggingInitTrialParameters(configFilePathFromConnectionString); } @@ -90,17 +112,48 @@ private string GetLogPath(string logPath) var logPathOrDefault = logPath; if (string.IsNullOrEmpty(logPath)) { - s_logger.Warn("LogPath in client config not found. Using temporary directory as a default value"); - logPathOrDefault = Path.GetTempPath(); + s_logger.Warn("LogPath in client config not found. Using home directory as a default value"); + logPathOrDefault = HomeDirectoryProvider.HomeDirectory(_environmentOperations); + if (string.IsNullOrEmpty(logPathOrDefault)) + { + throw new Exception("No log path found for easy logging. Home directory is not configured and log path is not provided"); + } } var pathWithDotnetSubdirectory = Path.Combine(logPathOrDefault, "dotnet"); if (!_directoryOperations.Exists(pathWithDotnetSubdirectory)) { - _directoryOperations.CreateDirectory(pathWithDotnetSubdirectory); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + _directoryOperations.CreateDirectory(pathWithDotnetSubdirectory); + } + else + { + if (!Directory.Exists(logPathOrDefault)) + { + Directory.CreateDirectory(logPathOrDefault); + } + _unixOperations.CreateDirectoryWithPermissions(pathWithDotnetSubdirectory, + FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR); + } } + CheckDirPermissionsOnlyAllowUser(pathWithDotnetSubdirectory); return pathWithDotnetSubdirectory; } + + private void CheckDirPermissionsOnlyAllowUser(string dirPath) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + return; + + var dirPermissions = _unixOperations.GetDirPermissions(dirPath); + if (dirPermissions != FileAccessPermissions.UserReadWriteExecute) + { + s_logger.Warn($"Access permission for the logs directory is currently " + + $"{UnixFilePermissionsConverter.ConvertFileAccessPermissionsToInt(dirPermissions)} " + + $"and is potentially accessible to users other than the owner of the logs directory"); + } + } } internal class EasyLoggingInitTrialParameters diff --git a/Snowflake.Data/Core/Tools/HomeDirectoryProvider.cs b/Snowflake.Data/Core/Tools/HomeDirectoryProvider.cs new file mode 100644 index 000000000..a39a0987c --- /dev/null +++ b/Snowflake.Data/Core/Tools/HomeDirectoryProvider.cs @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. + */ + +using Snowflake.Data.Log; +using System; + +namespace Snowflake.Data.Core.Tools +{ + internal class HomeDirectoryProvider + { + private static readonly SFLogger s_logger = SFLoggerFactory.GetLogger(); + + public static string HomeDirectory(EnvironmentOperations _environmentOperations) { + try + { + var homeDirectory = _environmentOperations.GetFolderPath(Environment.SpecialFolder.UserProfile); + if (string.IsNullOrEmpty(homeDirectory) || homeDirectory.Equals("/")) + { + return null; + } + return homeDirectory; + } + catch (Exception e) + { + s_logger.Error($"Error while trying to retrieve the home directory: {e}"); + return null; + } + } + } +} diff --git a/Snowflake.Data/Core/Tools/UnixOperations.cs b/Snowflake.Data/Core/Tools/UnixOperations.cs new file mode 100644 index 000000000..d0e1fdf36 --- /dev/null +++ b/Snowflake.Data/Core/Tools/UnixOperations.cs @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. + */ + +using Mono.Unix; +using Mono.Unix.Native; + +namespace Snowflake.Data.Core.Tools +{ + internal class UnixOperations + { + public static readonly UnixOperations Instance = new UnixOperations(); + + public virtual void CreateDirectoryWithPermissions(string path, FilePermissions permissions) + { + Syscall.mkdir(path, permissions); + } + + public virtual FileAccessPermissions GetDirPermissions(string path) + { + var dirInfo = new UnixDirectoryInfo(path); + return dirInfo.FileAccessPermissions; + } + + public virtual bool CheckFileHasPermissions(string path, FileAccessPermissions permissions) + { + var fileInfo = new UnixFileInfo(path); + return fileInfo.FileAccessPermissions.HasFlag(permissions); + } + } +} diff --git a/Snowflake.Data/Logger/UnixFilePermissionsConverter .cs b/Snowflake.Data/Logger/UnixFilePermissionsConverter .cs new file mode 100644 index 000000000..d7278164f --- /dev/null +++ b/Snowflake.Data/Logger/UnixFilePermissionsConverter .cs @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. + */ + +using System; +using Mono.Unix; + +namespace Snowflake.Data.Log +{ + internal class UnixFilePermissionsConverter + { + internal static int ConvertFileAccessPermissionsToInt(FileAccessPermissions permissions) + { + int userPermission = 0; + int groupPermission = 0; + int otherPermission = 0; + + foreach (FileAccessPermissions perm in Enum.GetValues(typeof(FileAccessPermissions))) + { + // Check if enum exists in the directory permission + if ((permissions & perm) != 0) + { + string permName = perm.ToString(); + if (!permName.Contains("ReadWriteExecute")) + { + if (permName.Contains("User")) + { + userPermission += (int)perm; + } + else if (permName.Contains("Group")) + { + groupPermission += (int)perm; + } + else if (permName.Contains("Other")) + { + otherPermission += (int)perm; + } + } + } + } + + // Divide by 2^6 + userPermission /= 64; + + // Divide by 2^3 + groupPermission /= 8; + + return Convert.ToInt32(string.Format("{0}{1}{2}", userPermission, groupPermission, otherPermission)); + } + } +} diff --git a/Snowflake.Data/Snowflake.Data.csproj b/Snowflake.Data/Snowflake.Data.csproj index 02ec98f6d..490360792 100644 --- a/Snowflake.Data/Snowflake.Data.csproj +++ b/Snowflake.Data/Snowflake.Data.csproj @@ -24,6 +24,7 @@ +