From 70e81317e7957c0769c64d56a9b1fcdb5d395e69 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com> Date: Tue, 29 Oct 2019 15:48:49 -0700 Subject: [PATCH] Add file creation method that takes an ACL (dotnet/corefx#42099) Approved API Proposal: dotnet/corefx#41614 Related change for directory creation method that takes an ACL: dotnet/corefx#41834 -merged and ported to 3.1 Prev2 Description We have extension methods in System.IO.FileSystem.AclExtensions that let the user get and set ACLs for existing files, but we do not have methods that create files with predefined ACLs. .NET ACL (Access Control List) support is Windows specific. This change will reside inside the System.IO.FileSystem.AccessControl assembly. Customer impact Before this change, customers had to create a file or filestream, then set its ACLs. This presents a few problems: Potential security hole as files can be accessed between creation and modification. Porting difficulties as there isn't a 1-1 API replacement Stability issues with background processes (file filters) can prevent modifying ACLs right after creation (typically surfaces as a security exception). This change addresses those problems by adding a new extension method that allows creating a file and ensuring the provided ACLs are set during creation. This change is expected to be backported to 3.1. Commit migrated from https://github.com/dotnet/corefx/commit/508cbc4f1021e260b52981b8510cc5a5881441d3 --- .../System.IO.FileSystem.AccessControl.sln | 4 +- .../ref/System.IO.FileSystem.AccessControl.cs | 1 + .../src/Resources/Strings.resx | 6 + .../src/System.IO.FileSystem.AccessControl.csproj | 5 + .../src/System/IO/FileSystemAclExtensions.net46.cs | 11 + .../IO/FileSystemAclExtensions.netcoreapp.cs | 147 ++++++++++ .../IO/FileSystemAclExtensions.netstandard.cs | 5 + .../tests/FileSystemAclExtensionsTests.cs | 307 ++++++++++++++++++--- .../src/System/IO/FileSystem.Windows.cs | 4 +- .../tests/FileStream/CopyToAsync.cs | 5 +- .../System.IO.FileSystem/tests/FileStream/Pipes.cs | 6 +- .../tests/System.IO.FileSystem.Tests.csproj | 1 + 12 files changed, 456 insertions(+), 46 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/System.IO.FileSystem.AccessControl.sln b/src/libraries/System.IO.FileSystem.AccessControl/System.IO.FileSystem.AccessControl.sln index 734ecd4..e4e3294 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/System.IO.FileSystem.AccessControl.sln +++ b/src/libraries/System.IO.FileSystem.AccessControl/System.IO.FileSystem.AccessControl.sln @@ -30,8 +30,8 @@ Global {5915DD11-5D57-45A9-BFB0-56FEB7741E1F}.Debug|Any CPU.Build.0 = netcoreapp-Windows_NT-Debug|Any CPU {5915DD11-5D57-45A9-BFB0-56FEB7741E1F}.Release|Any CPU.ActiveCfg = netcoreapp-Windows_NT-Release|Any CPU {5915DD11-5D57-45A9-BFB0-56FEB7741E1F}.Release|Any CPU.Build.0 = netcoreapp-Windows_NT-Release|Any CPU - {D77FBA6C-1AA6-45A4-93E2-97A370672C53}.Debug|Any CPU.ActiveCfg = netstandard-Windows_NT-Debug|Any CPU - {D77FBA6C-1AA6-45A4-93E2-97A370672C53}.Debug|Any CPU.Build.0 = netstandard-Windows_NT-Debug|Any CPU + {D77FBA6C-1AA6-45A4-93E2-97A370672C53}.Debug|Any CPU.ActiveCfg = netcoreapp-Windows_NT-Debug|Any CPU + {D77FBA6C-1AA6-45A4-93E2-97A370672C53}.Debug|Any CPU.Build.0 = netcoreapp-Windows_NT-Debug|Any CPU {D77FBA6C-1AA6-45A4-93E2-97A370672C53}.Release|Any CPU.ActiveCfg = netcoreapp-Windows_NT-Release|Any CPU {D77FBA6C-1AA6-45A4-93E2-97A370672C53}.Release|Any CPU.Build.0 = netcoreapp-Windows_NT-Release|Any CPU {88A04AB0-F61E-4DD2-9E12-928DCA261263}.Debug|Any CPU.ActiveCfg = netstandard2.0-Debug|Any CPU diff --git a/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.cs b/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.cs index d5360c9..cad6ec0 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.cs @@ -10,6 +10,7 @@ namespace System.IO public static partial class FileSystemAclExtensions { public static void Create(this System.IO.DirectoryInfo directoryInfo, System.Security.AccessControl.DirectorySecurity directorySecurity) { } + public static System.IO.FileStream Create(this System.IO.FileInfo fileInfo, System.IO.FileMode mode, System.Security.AccessControl.FileSystemRights rights, System.IO.FileShare share, int bufferSize, System.IO.FileOptions options, System.Security.AccessControl.FileSecurity fileSecurity) { throw null; } public static System.Security.AccessControl.DirectorySecurity GetAccessControl(this System.IO.DirectoryInfo directoryInfo) { throw null; } public static System.Security.AccessControl.DirectorySecurity GetAccessControl(this System.IO.DirectoryInfo directoryInfo, System.Security.AccessControl.AccessControlSections includeSections) { throw null; } public static System.Security.AccessControl.FileSecurity GetAccessControl(this System.IO.FileInfo fileInfo) { throw null; } diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/Resources/Strings.resx b/src/libraries/System.IO.FileSystem.AccessControl/src/Resources/Strings.resx index 933334e..d71dfd0 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/Resources/Strings.resx +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/Resources/Strings.resx @@ -188,4 +188,10 @@ The path is empty. + + Positive number required. + + + Combining FileMode.{0} with FileSystemRights.{1} is invalid. + diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj b/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj index e9504d4..e63f5c2 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System.IO.FileSystem.AccessControl.csproj @@ -25,17 +25,21 @@ + + + + @@ -45,6 +49,7 @@ + diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.net46.cs b/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.net46.cs index 3f40542..25ec5c5 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.net46.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.net46.cs @@ -8,6 +8,17 @@ namespace System.IO { public static class FileSystemAclExtensions { + public static FileStream Create(this FileInfo fileInfo, FileMode mode, FileSystemRights rights, FileShare share, int bufferSize, FileOptions options, FileSecurity fileSecurity) + { + if (fileInfo == null) + throw new ArgumentNullException(nameof(fileInfo)); + + if (fileSecurity == null) + throw new ArgumentNullException(nameof(fileSecurity)); + + return new FileStream(fileInfo.FullName, mode, rights, share, bufferSize, options, fileSecurity); + } + public static void Create(this DirectoryInfo directoryInfo, DirectorySecurity directorySecurity) { if (directoryInfo == null) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netcoreapp.cs b/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netcoreapp.cs index e7bac24..cd5afca 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netcoreapp.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netcoreapp.cs @@ -2,7 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Diagnostics; +using System.Runtime.InteropServices; using System.Security.AccessControl; +using Microsoft.Win32.SafeHandles; namespace System.IO { @@ -14,6 +17,7 @@ namespace System.IO /// or is . /// Could not find a part of the path. /// Access to the path is denied. + /// This extension method was added to .NET Core to bring the functionality that was provided by the `System.IO.DirectoryInfo.Create(System.Security.AccessControl.DirectorySecurity)` .NET Framework method. public static void Create(this DirectoryInfo directoryInfo, DirectorySecurity directorySecurity) { if (directoryInfo == null) @@ -24,5 +28,148 @@ namespace System.IO FileSystem.CreateDirectory(directoryInfo.FullName, directorySecurity.GetSecurityDescriptorBinaryForm()); } + + /// + /// Creates a new file stream, ensuring it is created with the specified properties and security settings. + /// + /// The current instance describing a file that does not exist in disk yet. + /// One of the enumeration values that specifies how the operating system should open a file. + /// One of the enumeration values that defines the access rights to use when creating access and audit rules. + /// One of the enumeration values for controlling the kind of access other FileStream objects can have to the same file. + /// The number of bytes buffered for reads and writes to the file. + /// One of the enumeration values that describes how to create or overwrite the file. + /// An object that determines the access control and audit security for the file. + /// A file stream for the newly created file. + /// The and combination is invalid. + /// or is . + /// or are out of their legal enum range. + ///-or- + /// is not a positive number. + /// Could not find a part of the path. + /// An I/O error occurs. + /// Access to the path is denied. + /// This extension method was added to .NET Core to bring the functionality that was provided by the `System.IO.FileStream.#ctor(System.String,System.IO.FileMode,System.Security.AccessControl.FileSystemRights,System.IO.FileShare,System.Int32,System.IO.FileOptions,System.Security.AccessControl.FileSecurity)` .NET Framework constructor. + public static FileStream Create(this FileInfo fileInfo, FileMode mode, FileSystemRights rights, FileShare share, int bufferSize, FileOptions options, FileSecurity fileSecurity) + { + if (fileInfo == null) + { + throw new ArgumentNullException(nameof(fileInfo)); + } + + if (fileSecurity == null) + { + throw new ArgumentNullException(nameof(fileSecurity)); + } + + // don't include inheritable in our bounds check for share + FileShare tempshare = share & ~FileShare.Inheritable; + + if (mode < FileMode.CreateNew || mode > FileMode.Append) + { + throw new ArgumentOutOfRangeException(nameof(mode), SR.ArgumentOutOfRange_Enum); + } + + if (tempshare < FileShare.None || tempshare > (FileShare.ReadWrite | FileShare.Delete)) + { + throw new ArgumentOutOfRangeException(nameof(share), SR.ArgumentOutOfRange_Enum); + } + + if (bufferSize <= 0) + { + throw new ArgumentOutOfRangeException(nameof(bufferSize), SR.ArgumentOutOfRange_NeedPosNum); + } + + // Do not allow using combinations of non-writing file system rights with writing file modes + if ((rights & FileSystemRights.Write) == 0 && + (mode == FileMode.Truncate || mode == FileMode.CreateNew || mode == FileMode.Create || mode == FileMode.Append)) + { + throw new ArgumentException(SR.Format(SR.Argument_InvalidFileModeAndFileSystemRightsCombo, mode, rights)); + } + + SafeFileHandle handle = CreateFileHandle(fileInfo.FullName, mode, rights, share, options, fileSecurity); + + try + { + return new FileStream(handle, GetFileStreamFileAccess(rights), bufferSize, (options & FileOptions.Asynchronous) != 0); + } + catch + { + // If anything goes wrong while setting up the stream, make sure we deterministically dispose of the opened handle. + handle.Dispose(); + throw; + } + } + + // In the context of a FileStream, the only ACCESS_MASK ACE rights we care about are reading/writing data and the generic read/write rights. + // See: https://docs.microsoft.com/en-us/windows/win32/secauthz/access-mask + private static FileAccess GetFileStreamFileAccess(FileSystemRights rights) + { + FileAccess access = 0; + if ((rights & FileSystemRights.ReadData) != 0 || ((int)rights & Interop.Kernel32.GenericOperations.GENERIC_READ) != 0) + { + access = FileAccess.Read; + } + if ((rights & FileSystemRights.WriteData) != 0 || ((int)rights & Interop.Kernel32.GenericOperations.GENERIC_WRITE) != 0) + { + access = access == FileAccess.Read ? FileAccess.ReadWrite : FileAccess.Write; + } + return access; + } + + private static unsafe SafeFileHandle CreateFileHandle(string fullPath, FileMode mode, FileSystemRights rights, FileShare share, FileOptions options, FileSecurity security) + { + Debug.Assert(fullPath != null); + + // Must use a valid Win32 constant + if (mode == FileMode.Append) + { + mode = FileMode.OpenOrCreate; + } + + // For mitigating local elevation of privilege attack through named pipes make sure we always call CreateFile with SECURITY_ANONYMOUS so that the + // named pipe server can't impersonate a high privileged client security context (note that this is the effective default on CreateFile2) + // SECURITY_SQOS_PRESENT flags that a SECURITY_ flag is present. + int flagsAndAttributes = (int)options | Interop.Kernel32.SecurityOptions.SECURITY_SQOS_PRESENT | Interop.Kernel32.SecurityOptions.SECURITY_ANONYMOUS; + + SafeFileHandle handle; + + fixed (byte* pSecurityDescriptor = security.GetSecurityDescriptorBinaryForm()) + { + var secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES + { + nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES), + bInheritHandle = ((share & FileShare.Inheritable) != 0) ? Interop.BOOL.TRUE : Interop.BOOL.FALSE, + lpSecurityDescriptor = (IntPtr)pSecurityDescriptor + }; + + using (DisableMediaInsertionPrompt.Create()) + { + handle = Interop.Kernel32.CreateFile(fullPath, (int)rights, share, ref secAttrs, mode, flagsAndAttributes, IntPtr.Zero); + ValidateFileHandle(handle, fullPath); + } + } + + return handle; + } + + private static void ValidateFileHandle(SafeFileHandle handle, string fullPath) + { + if (handle.IsInvalid) + { + // Return a meaningful exception with the full path. + + // NT5 oddity - when trying to open "C:\" as a FileStream, + // we usually get ERROR_PATH_NOT_FOUND from the OS. We should + // probably be consistent w/ every other directory. + int errorCode = Marshal.GetLastWin32Error(); + + if (errorCode == Interop.Errors.ERROR_PATH_NOT_FOUND && fullPath.Length == Path.GetPathRoot(fullPath).Length) + { + errorCode = Interop.Errors.ERROR_ACCESS_DENIED; + } + + throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath); + } + } } } diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netstandard.cs b/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netstandard.cs index 760fb2b..4432071 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netstandard.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netstandard.cs @@ -8,6 +8,11 @@ namespace System.IO { public static partial class FileSystemAclExtensions { + public static FileStream Create(this FileInfo fileInfo, FileMode mode, FileSystemRights rights, FileShare share, int bufferSize, FileOptions options, FileSecurity fileSecurity) + { + throw new PlatformNotSupportedException(); + } + public static void Create(this DirectoryInfo directoryInfo, DirectorySecurity directorySecurity) { throw new PlatformNotSupportedException(); diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index 70213c5..b477fa3 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -4,19 +4,21 @@ using System.Collections.Generic; using System.Linq; -using System.Linq.Expressions; using System.Security.AccessControl; using System.Security.Principal; -using Microsoft.DotNet.PlatformAbstractions; -using Microsoft.VisualBasic; using Xunit; namespace System.IO { public class FileSystemAclExtensionsTests { + private const int DefaultBufferSize = 4096; + + #region Test methods + #region GetAccessControl + [Fact] public void GetAccessControl_DirectoryInfo_InvalidArguments() { @@ -120,6 +122,10 @@ namespace System.IO } } + #endregion + + #region SetAccessControl + [Fact] public void SetAccessControl_DirectoryInfo_DirectorySecurity_InvalidArguments() { @@ -195,69 +201,269 @@ namespace System.IO } } + #endregion + + #region DirectoryInfo Create + [Fact] public void DirectoryInfo_Create_NullDirectoryInfo() { DirectoryInfo info = null; DirectorySecurity security = new DirectorySecurity(); - if (PlatformDetection.IsFullFramework) + Assert.Throws("directoryInfo", () => { - Assert.Throws(() => FileSystemAclExtensions.Create(info, security)); - } - else - { - Assert.Throws(() => info.Create(security)); - } + if (PlatformDetection.IsFullFramework) + { + FileSystemAclExtensions.Create(info, security); + } + else + { + info.Create(security); + } + }); } [Fact] - public void DirectoryInfo_Create_DefaultDirectorySecurity() + public void DirectoryInfo_Create_NullDirectorySecurity() { - DirectorySecurity security = new DirectorySecurity(); - VerifyDirectorySecurity(security); + DirectoryInfo info = new DirectoryInfo("path"); + + Assert.Throws("directorySecurity", () => + { + if (PlatformDetection.IsFullFramework) + { + FileSystemAclExtensions.Create(info, null); + } + else + { + info.Create(null); + } + }); } [Fact] - public void DirectoryInfo_Create_NullDirectorySecurity() + public void DirectoryInfo_Create_NotFound() { - DirectoryInfo info = new DirectoryInfo("path"); - if (PlatformDetection.IsFullFramework) - { - Assert.Throws(() => FileSystemAclExtensions.Create(info, null)); - } - else + using var directory = new TempDirectory(); + string path = Path.Combine(directory.Path, Guid.NewGuid().ToString(), "ParentDoesNotExist"); + DirectoryInfo info = new DirectoryInfo(path); + DirectorySecurity security = new DirectorySecurity(); + + Assert.Throws(() => { - Assert.Throws(() => info.Create(null)); - } + if (PlatformDetection.IsFullFramework) + { + FileSystemAclExtensions.Create(info, security); + } + else + { + info.Create(security); + } + }); } [Fact] - public void DirectoryInfo_Create_NotFound() + public void DirectoryInfo_Create_DefaultDirectorySecurity() { - DirectoryInfo info = new DirectoryInfo(@"W:\\I\\Do\\Not\\Exist"); DirectorySecurity security = new DirectorySecurity(); - Assert.Throws(() => info.Create(security)); + VerifyDirectorySecurity(security); } [Theory] + [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.ReadAndExecute, AccessControlType.Allow)] + [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.ReadAndExecute, AccessControlType.Deny)] + [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.WriteData, AccessControlType.Allow)] + [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.WriteData, AccessControlType.Deny)] [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.FullControl, AccessControlType.Allow)] - [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.ReadData, AccessControlType.Allow)] - [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.Write, AccessControlType.Allow)] - [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.Write, AccessControlType.Deny)] [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.FullControl, AccessControlType.Deny)] public void DirectoryInfo_Create_DirectorySecurityWithSpecificAccessRule( WellKnownSidType sid, FileSystemRights rights, AccessControlType controlType) { - DirectorySecurity security = GetDirectorySecurity(sid, rights, controlType); VerifyDirectorySecurity(security); } #endregion + #region FileInfo Create + + [Fact] + public void FileInfo_Create_NullFileInfo() + { + FileInfo info = null; + FileSecurity security = new FileSecurity(); + + Assert.Throws("fileInfo", () => + { + if (PlatformDetection.IsFullFramework) + { + FileSystemAclExtensions.Create(info, FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, security); + } + else + { + info.Create(FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, security); + } + }); + } + + [Fact] + public void FileInfo_Create_NullFileSecurity() + { + FileInfo info = new FileInfo("path"); + + Assert.Throws("fileSecurity", () => + { + if (PlatformDetection.IsFullFramework) + { + FileSystemAclExtensions.Create(info, FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, null); + } + else + { + info.Create(FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, null); + } + }); + } + + [Fact] + public void FileInfo_Create_NotFound() + { + using var directory = new TempDirectory(); + string path = Path.Combine(directory.Path, Guid.NewGuid().ToString(), "file.txt"); + FileInfo info = new FileInfo(path); + FileSecurity security = new FileSecurity(); + + Assert.Throws(() => + { + if (PlatformDetection.IsFullFramework) + { + FileSystemAclExtensions.Create(info, FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, security); + } + else + { + info.Create(FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, security); + } + }); + } + + [Theory] + [InlineData((FileMode)int.MinValue)] + [InlineData((FileMode)0)] + [InlineData((FileMode)int.MaxValue)] + public void FileInfo_Create_FileSecurity_InvalidFileMode(FileMode invalidMode) + { + FileSecurity security = new FileSecurity(); + FileInfo info = new FileInfo("path"); + + Assert.Throws("mode", () => + { + if (PlatformDetection.IsFullFramework) + { + FileSystemAclExtensions.Create(info, invalidMode, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, security); ; + } + else + { + info.Create(invalidMode, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, security); + } + }); + } + + [Theory] + [InlineData((FileShare)(-1))] + [InlineData((FileShare)int.MaxValue)] + public void FileInfo_Create_FileSecurity_InvalidFileShare(FileShare invalidFileShare) + { + FileSecurity security = new FileSecurity(); + FileInfo info = new FileInfo("path"); + + Assert.Throws("share", () => + { + if (PlatformDetection.IsFullFramework) + { + FileSystemAclExtensions.Create(info, FileMode.Create, FileSystemRights.WriteData, invalidFileShare, DefaultBufferSize, FileOptions.None, security); + } + else + { + info.Create(FileMode.Create, FileSystemRights.WriteData, invalidFileShare, DefaultBufferSize, FileOptions.None, security); + } + }); + } + + [Theory] + [InlineData(int.MinValue)] + [InlineData(0)] + public void FileInfo_Create_FileSecurity_InvalidBufferSize(int invalidBufferSize) + { + FileSecurity security = new FileSecurity(); + FileInfo info = new FileInfo("path"); + + Assert.Throws("bufferSize", () => + { + if (PlatformDetection.IsFullFramework) + { + FileSystemAclExtensions.Create(info, FileMode.Create, FileSystemRights.WriteData, FileShare.Read, invalidBufferSize, FileOptions.None, security); + } + else + { + info.Create(FileMode.Create, FileSystemRights.WriteData, FileShare.Read, invalidBufferSize, FileOptions.None, security); + } + }); + } + + [Theory] + [InlineData(FileMode.Truncate, FileSystemRights.Read)] + [InlineData(FileMode.Truncate, FileSystemRights.ReadData)] + [InlineData(FileMode.CreateNew, FileSystemRights.Read)] + [InlineData(FileMode.CreateNew, FileSystemRights.ReadData)] + [InlineData(FileMode.Create, FileSystemRights.Read)] + [InlineData(FileMode.Create, FileSystemRights.ReadData)] + [InlineData(FileMode.Append, FileSystemRights.Read)] + [InlineData(FileMode.Append, FileSystemRights.ReadData)] + public void FileInfo_Create_FileSecurity_ForbiddenCombo_FileModeFileSystemSecurity(FileMode mode, FileSystemRights rights) + { + FileSecurity security = new FileSecurity(); + FileInfo info = new FileInfo("path"); + + Assert.Throws(() => + { + if (PlatformDetection.IsFullFramework) + { + FileSystemAclExtensions.Create(info, mode, rights, FileShare.Read, DefaultBufferSize, FileOptions.None, security); + } + else + { + info.Create(mode, rights, FileShare.Read, DefaultBufferSize, FileOptions.None, security); + } + }); + } + + [Fact] + public void FileInfo_Create_DefaultFileSecurity() + { + FileSecurity security = new FileSecurity(); + VerifyFileSecurity(security); + } + + [Theory] + [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.ReadAndExecute, AccessControlType.Allow)] + [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.ReadAndExecute, AccessControlType.Deny)] + [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.WriteData, AccessControlType.Allow)] + [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.WriteData, AccessControlType.Deny)] + [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.FullControl, AccessControlType.Allow)] + [InlineData(WellKnownSidType.BuiltinUsersSid, FileSystemRights.FullControl, AccessControlType.Deny)] + public void FileInfo_Create_FileSecurity_SpecificAccessRule(WellKnownSidType sid, FileSystemRights rights, AccessControlType controlType) + { + FileSecurity security = GetFileSecurity(sid, rights, controlType); + VerifyFileSecurity(security); + } + + #endregion + + #endregion + + #region Helper methods private DirectorySecurity GetDirectorySecurity(WellKnownSidType sid, FileSystemRights rights, AccessControlType controlType) @@ -274,24 +480,58 @@ namespace System.IO private void VerifyDirectorySecurity(DirectorySecurity expectedSecurity) { using var directory = new TempDirectory(); - string path = Path.Combine(directory.Path, "directory"); DirectoryInfo info = new DirectoryInfo(path); info.Create(expectedSecurity); Assert.True(Directory.Exists(path)); - Assert.Equal(typeof(FileSystemRights), expectedSecurity.AccessRightType); DirectoryInfo actualInfo = new DirectoryInfo(info.FullName); DirectorySecurity actualSecurity = actualInfo.GetAccessControl(); - VerifyDirectoryAccessSecurity(expectedSecurity, actualSecurity); + VerifyAccessSecurity(expectedSecurity, actualSecurity); + } + + private FileSecurity GetFileSecurity(WellKnownSidType sid, FileSystemRights rights, AccessControlType controlType) + { + FileSecurity security = new FileSecurity(); + + SecurityIdentifier identity = new SecurityIdentifier(sid, null); + FileSystemAccessRule accessRule = new FileSystemAccessRule(identity, rights, controlType); + security.AddAccessRule(accessRule); + + return security; + } + + private void VerifyFileSecurity(FileSecurity expectedSecurity) + { + VerifyFileSecurity(FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, expectedSecurity); } - private void VerifyDirectoryAccessSecurity(DirectorySecurity expectedSecurity, DirectorySecurity actualSecurity) + private void VerifyFileSecurity(FileMode mode, FileSystemRights rights, FileShare share, int bufferSize, FileOptions options, FileSecurity expectedSecurity) { + using var directory = new TempDirectory(); + + string path = Path.Combine(directory.Path, "file.txt"); + FileInfo info = new FileInfo(path); + + info.Create(mode, rights, share, bufferSize, options, expectedSecurity); + + Assert.True(File.Exists(path)); + + FileInfo actualInfo = new FileInfo(info.FullName); + + FileSecurity actualSecurity = actualInfo.GetAccessControl(); + + VerifyAccessSecurity(expectedSecurity, actualSecurity); + } + + private void VerifyAccessSecurity(CommonObjectSecurity expectedSecurity, CommonObjectSecurity actualSecurity) + { + Assert.Equal(typeof(FileSystemRights), expectedSecurity.AccessRightType); + Assert.Equal(typeof(FileSystemRights), actualSecurity.AccessRightType); List expectedAccessRules = expectedSecurity.GetAccessRules(includeExplicit: true, includeInherited: false, typeof(SecurityIdentifier)) @@ -300,7 +540,6 @@ namespace System.IO List actualAccessRules = actualSecurity.GetAccessRules(includeExplicit: true, includeInherited: false, typeof(SecurityIdentifier)) .Cast().ToList(); - // If DirectorySecurity is created without arguments, GetAccessRules will return zero rules Assert.Equal(expectedAccessRules.Count, actualAccessRules.Count); if (expectedAccessRules.Count > 0) { diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs index 62e8484..6adb20d 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs @@ -18,8 +18,6 @@ namespace System.IO { internal static partial class FileSystem { - internal const int GENERIC_READ = unchecked((int)0x80000000); - public static void CopyFile(string sourceFullPath, string destFullPath, bool overwrite) { int errorCode = Interop.Kernel32.CopyFile(sourceFullPath, destFullPath, !overwrite); @@ -32,7 +30,7 @@ namespace System.IO { // For a number of error codes (sharing violation, path not found, etc) we don't know if the problem was with // the source or dest file. Try reading the source file. - using (SafeFileHandle handle = Interop.Kernel32.CreateFile(sourceFullPath, GENERIC_READ, FileShare.Read, FileMode.Open, 0)) + using (SafeFileHandle handle = Interop.Kernel32.CreateFile(sourceFullPath, Interop.Kernel32.GenericOperations.GENERIC_READ, FileShare.Read, FileMode.Open, 0)) { if (handle.IsInvalid) fileName = sourceFullPath; diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/CopyToAsync.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/CopyToAsync.cs index feeb6bc..fd83079 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/CopyToAsync.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/CopyToAsync.cs @@ -237,7 +237,7 @@ namespace System.IO.Tests }); Assert.True(WaitNamedPipeW(@"\\.\pipe\" + name, -1)); - using (SafeFileHandle clientHandle = CreateFileW(@"\\.\pipe\" + name, GENERIC_READ, FileShare.None, IntPtr.Zero, FileMode.Open, (int)pipeOptions, IntPtr.Zero)) + using (SafeFileHandle clientHandle = CreateFileW(@"\\.\pipe\" + name, Interop.Kernel32.GenericOperations.GENERIC_READ, FileShare.None, IntPtr.Zero, FileMode.Open, (int)pipeOptions, IntPtr.Zero)) using (var client = new FileStream(clientHandle, FileAccess.Read, bufferSize: 3, isAsync: useAsync)) { Task copyTask = client.CopyToAsync(results, (int)totalLength); @@ -261,7 +261,7 @@ namespace System.IO.Tests Task serverTask = server.WaitForConnectionAsync(); Assert.True(WaitNamedPipeW(@"\\.\pipe\" + name, -1)); - using (SafeFileHandle clientHandle = CreateFileW(@"\\.\pipe\" + name, GENERIC_READ, FileShare.None, IntPtr.Zero, FileMode.Open, (int)PipeOptions.Asynchronous, IntPtr.Zero)) + using (SafeFileHandle clientHandle = CreateFileW(@"\\.\pipe\" + name, Interop.Kernel32.GenericOperations.GENERIC_READ, FileShare.None, IntPtr.Zero, FileMode.Open, (int)PipeOptions.Asynchronous, IntPtr.Zero)) using (var client = new FileStream(clientHandle, FileAccess.Read, bufferSize: 3, isAsync: true)) { await serverTask; @@ -327,7 +327,6 @@ namespace System.IO.Tests string lpFileName, int dwDesiredAccess, FileShare dwShareMode, IntPtr securityAttrs, FileMode dwCreationDisposition, int dwFlagsAndAttributes, IntPtr hTemplateFile); - internal const int GENERIC_READ = unchecked((int)0x80000000); #endregion } } diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/Pipes.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/Pipes.cs index 74f8bee..eaf1bf2 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/Pipes.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/Pipes.cs @@ -109,7 +109,7 @@ namespace System.IO.Tests }); WaitNamedPipeW(@"\\.\pipe\" + name, -1); - using (SafeFileHandle clientHandle = CreateFileW(@"\\.\pipe\" + name, GENERIC_WRITE, FileShare.None, IntPtr.Zero, FileMode.Open, (int)PipeOptions.Asynchronous, IntPtr.Zero)) + using (SafeFileHandle clientHandle = CreateFileW(@"\\.\pipe\" + name, Interop.Kernel32.GenericOperations.GENERIC_WRITE, FileShare.None, IntPtr.Zero, FileMode.Open, (int)PipeOptions.Asynchronous, IntPtr.Zero)) using (var client = new FileStream(clientHandle, FileAccess.Write, bufferSize: 3, isAsync: true)) { var data = new[] { new byte[] { 0, 1 }, new byte[] { 2, 3 }, new byte[] { 4, 5 } }; @@ -142,7 +142,7 @@ namespace System.IO.Tests }); WaitNamedPipeW(@"\\.\pipe\" + name, -1); - using (SafeFileHandle clientHandle = CreateFileW(@"\\.\pipe\" + name, GENERIC_READ, FileShare.None, IntPtr.Zero, FileMode.Open, (int)PipeOptions.Asynchronous, IntPtr.Zero)) + using (SafeFileHandle clientHandle = CreateFileW(@"\\.\pipe\" + name, Interop.Kernel32.GenericOperations.GENERIC_READ, FileShare.None, IntPtr.Zero, FileMode.Open, (int)PipeOptions.Asynchronous, IntPtr.Zero)) using (var client = new FileStream(clientHandle, FileAccess.Read, bufferSize: 3, isAsync: true)) { var arr = new byte[1]; @@ -174,8 +174,6 @@ namespace System.IO.Tests string lpFileName, int dwDesiredAccess, FileShare dwShareMode, IntPtr securityAttrs, FileMode dwCreationDisposition, int dwFlagsAndAttributes, IntPtr hTemplateFile); - internal const int GENERIC_READ = unchecked((int)0x80000000); - internal const int GENERIC_WRITE = 0x40000000; #endregion } } diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index 5671633..b9aff42 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -163,6 +163,7 @@ + Common\System\Buffers\NativeMemoryManager.cs -- 2.7.4