Changed toml file permissions to check 400 and 600 permissions only.

snowflakedb · Nov 6, 2024 · 08d40ba · 08d40ba
1 parent ca94da5
commit 08d40ba
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 22 deletions.
diff --git a/Snowflake.Data.Tests/UnitTests/Tools/FileOperationsTest.cs b/Snowflake.Data.Tests/UnitTests/Tools/FileOperationsTest.cs
@@ -2,21 +2,19 @@
  * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved.
  */
 
-
-using System;
+using System.Collections.Generic;
 using Snowflake.Data.Core;
+using System.IO;
+using System.Runtime.InteropServices;
+using Mono.Unix;
+using Mono.Unix.Native;
+using NUnit.Framework;
+using Snowflake.Data.Core.Tools;
+using static Snowflake.Data.Tests.UnitTests.Configuration.EasyLoggingConfigGenerator;
+using System.Security;
 
 namespace Snowflake.Data.Tests.Tools
 {
-    using System.IO;
-    using System.Runtime.InteropServices;
-    using Mono.Unix;
-    using Mono.Unix.Native;
-    using NUnit.Framework;
-    using Snowflake.Data.Core.Tools;
-    using static Snowflake.Data.Tests.UnitTests.Configuration.EasyLoggingConfigGenerator;
-    using System.Security;
-
     [TestFixture, NonParallelizable]
     public class FileOperationsTest
     {
@@ -59,7 +57,9 @@ public void TestReadAllTextOnWindows()
         }
 
         [Test]
-        public void TestReadAllTextCheckingPermissionsUsingTomlConfigurationFileValidations()
+        public void TestReadAllTextCheckingPermissionsUsingTomlConfigurationFileValidations(
+            [ValueSource(nameof(UserAllowedFilePermissions))]
+            FileAccessPermissions userAllowedFilePermissions)
         {
             if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
             {
@@ -68,8 +68,9 @@ public void TestReadAllTextCheckingPermissionsUsingTomlConfigurationFileValidati
 
             var content = "random text";
             var filePath = CreateConfigTempFile(s_workingDirectory, content);
-            var filePermissions = FileAccessPermissions.UserReadWriteExecute;
-            Syscall.chmod(filePath, (FilePermissions)filePermissions);
+            var filePermissions = userAllowedFilePermissions;
+
+        Syscall.chmod(filePath, (FilePermissions)filePermissions);
 
             // act
             var result = s_fileOperations.ReadAllText(filePath, TomlConnectionBuilder.ValidateFilePermissions);
@@ -79,7 +80,9 @@ public void TestReadAllTextCheckingPermissionsUsingTomlConfigurationFileValidati
         }
 
         [Test]
-        public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadConfigurationFile()
+        public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadConfigurationFile(
+            [ValueSource(nameof(UserAllowedFilePermissions))]
+            FileAccessPermissions userAllowedFilePermissions)
         {
             if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
             {
@@ -88,12 +91,20 @@ public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadConfiguration
 
             var content = "random text";
             var filePath = CreateConfigTempFile(s_workingDirectory, content);
-            var filePermissions = FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.OtherReadWriteExecute;
+            var filePermissions = userAllowedFilePermissions | FileAccessPermissions.OtherReadWriteExecute;
+
             Syscall.chmod(filePath, (FilePermissions)filePermissions);
 
             // act and assert
             Assert.Throws<SecurityException>(() => s_fileOperations.ReadAllText(filePath, TomlConnectionBuilder.ValidateFilePermissions),
                 "Attempting to read a file with too broad permissions assigned");
         }
+
+
+        public static IEnumerable<FileAccessPermissions> UserAllowedFilePermissions()
+        {
+            yield return FileAccessPermissions.UserRead;
+            yield return FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite;
+        }
     }
 }
diff --git a/Snowflake.Data.Tests/UnitTests/Tools/UnixOperationsTest.cs b/Snowflake.Data.Tests/UnitTests/Tools/UnixOperationsTest.cs
@@ -84,16 +84,16 @@ public void TestDetectGroupOrOthersNotWritablePermissions(
         }
 
         [Test]
-        public void TestReadAllTextCheckingPermissionsUsingTomlConfigurationFileValidations()
+        public void TestReadAllTextCheckingPermissionsUsingTomlConfigurationFileValidations(
+            [ValueSource(nameof(UserAllowedPermissions))] FilePermissions userAllowedPermissions)
         {
             if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
             {
                 Assert.Ignore("skip test on Windows");
             }
             var content = "random text";
             var filePath = CreateConfigTempFile(s_workingDirectory, content);
-            var filePermissions = FileAccessPermissions.UserReadWriteExecute;
-            Syscall.chmod(filePath, (FilePermissions)filePermissions);
+            Syscall.chmod(filePath, userAllowedPermissions);
 
             // act
             var result = s_unixOperations.ReadAllText(filePath, TomlConnectionBuilder.ValidateFilePermissions);
@@ -177,7 +177,13 @@ public static IEnumerable<FilePermissions> OtherNotWritablePermissions()
 
         public static IEnumerable<FilePermissions> UserReadWritePermissions()
         {
-            yield return FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR;
+            yield return FilePermissions.S_IRUSR | FilePermissions.S_IWUSR;
+        }
+
+        public static IEnumerable<FilePermissions> UserAllowedPermissions()
+        {
+            yield return FilePermissions.S_IRUSR;
+            yield return FilePermissions.S_IRUSR | FilePermissions.S_IWUSR;
         }
 
         public static IEnumerable<FilePermissions> GroupOrOthersReadablePermissions()

diff --git a/Snowflake.Data/Core/TomlConnectionBuilder.cs b/Snowflake.Data/Core/TomlConnectionBuilder.cs
@@ -5,6 +5,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Linq;
 using System.Security;
 using System.Text;
 using Mono.Unix;
@@ -154,12 +155,16 @@ private string ResolveConnectionTomlFile()
 
         internal static void ValidateFilePermissions(UnixStream stream)
         {
-            const FileAccessPermissions forbiddenPermissions = FileAccessPermissions.OtherReadWriteExecute | FileAccessPermissions.GroupReadWriteExecute;
+            var allowedPermissions = new FileAccessPermissions[]
+            {
+                FileAccessPermissions.UserRead | FileAccessPermissions.UserWrite,
+                FileAccessPermissions.UserRead
+            };
             if (stream.OwnerUser.UserId != Syscall.geteuid())
                 throw new SecurityException("Attempting to read a file not owned by the effective user of the current process");
             if (stream.OwnerGroup.GroupId != Syscall.getegid())
                 throw new SecurityException("Attempting to read a file not owned by the effective group of the current process");
-            if ((stream.FileAccessPermissions & forbiddenPermissions) != 0)
+            if (!(allowedPermissions.Any(a => stream.FileAccessPermissions == a)))
                 throw new SecurityException("Attempting to read a file with too broad permissions assigned");
         }
     }