From 6d1ec2ab17e29e48ad847e7d0997e0ea364a99ef Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com> Date: Wed, 13 Nov 2019 12:26:12 -0800 Subject: [PATCH] Fix NetStandard issue in System.IO.FileSystem.AccessControl in 5.0 (dotnet/corefx#42546) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Fix NetStandard issue in System.IO.FileSystem.AccessControl We built the fix incorrectly so that it would only apply for netcoreapp3.0 but customer needs it on netstandard2.0 (desktop + core). Customer impact is that they’ll see a PlatformNotSupported exception rather than the new API. Commit migrated from https://github.com/dotnet/corefx/commit/70150e6750c35c9a67803e9cddfda7b1db2ec487 --- .../src/System/IO/FileSystem.Attributes.Windows.cs | 2 +- .../IO/FileSystem.DirectoryCreation.Windows.cs | 2 +- .../Common/src/System/IO/PathInternal.Unix.cs | 1 - .../src/System.IO.FileSystem.AccessControl.csproj | 61 ++++--- .../src/System/IO/FileSystemAclExtensions.cs | 163 +++++++++++++++++++ .../IO/FileSystemAclExtensions.netcoreapp.cs | 175 --------------------- .../IO/FileSystemAclExtensions.netstandard.cs | 21 --- 7 files changed, 195 insertions(+), 230 deletions(-) delete mode 100644 src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netcoreapp.cs delete mode 100644 src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netstandard.cs diff --git a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs index be7ddee..26cefbc 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.Attributes.Windows.cs @@ -57,7 +57,7 @@ namespace System.IO int errorCode = Interop.Errors.ERROR_SUCCESS; // Neither GetFileAttributes or FindFirstFile like trailing separators - path = Path.TrimEndingDirectorySeparator(path); + path = PathInternal.TrimEndingDirectorySeparator(path); using (DisableMediaInsertionPrompt.Create()) { diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index bd77c7e..f127e3d 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -42,7 +42,7 @@ namespace System.IO int length = fullPath.Length; // We need to trim the trailing slash or the code will try to create 2 directories of the same name. - if (length >= 2 && Path.EndsInDirectorySeparator(fullPath.AsSpan())) + if (length >= 2 && PathInternal.EndsInDirectorySeparator(fullPath.AsSpan())) { length--; } diff --git a/src/libraries/Common/src/System/IO/PathInternal.Unix.cs b/src/libraries/Common/src/System/IO/PathInternal.Unix.cs index db0e38d..8aff492 100644 --- a/src/libraries/Common/src/System/IO/PathInternal.Unix.cs +++ b/src/libraries/Common/src/System/IO/PathInternal.Unix.cs @@ -35,7 +35,6 @@ namespace System.IO return c == Path.DirectorySeparatorChar; } - internal static bool IsPartiallyQualified(string path) { // This is much simpler than Windows where paths can be rooted, but not fully qualified (such as Drive Relative) 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 2a0038b..dbd19fd 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 @@ -4,7 +4,11 @@ true SR.PlatformNotSupported_AccessControl net461-Windows_NT-Debug;net461-Windows_NT-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;netfx-Windows_NT-Debug;netfx-Windows_NT-Release;netstandard2.0-Debug;netstandard2.0-Release;netstandard2.0-Windows_NT-Debug;netstandard2.0-Windows_NT-Release + annotations + + + @@ -17,33 +21,32 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -52,14 +55,10 @@ - - - - diff --git a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.cs b/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.cs index aaab98d..1de6cd4 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.cs @@ -2,6 +2,8 @@ // 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; @@ -72,5 +74,166 @@ namespace System.IO fileSecurity.Persist(handle, fileStream.Name); } + + /// Creates a new directory, ensuring it is created with the specified directory security. If the directory already exists, nothing is done. + /// The object describing a directory that does not exist in disk yet. + /// An object that determines the access control and audit security for the directory. + /// 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) + throw new ArgumentNullException(nameof(directoryInfo)); + + if (directorySecurity == null) + throw new ArgumentNullException(nameof(directorySecurity)); + + 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.netcoreapp.cs b/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netcoreapp.cs deleted file mode 100644 index cd5afca..0000000 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netcoreapp.cs +++ /dev/null @@ -1,175 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// 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 -{ - public static partial class FileSystemAclExtensions - { - /// Creates a new directory, ensuring it is created with the specified directory security. If the directory already exists, nothing is done. - /// The object describing a directory that does not exist in disk yet. - /// An object that determines the access control and audit security for the directory. - /// 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) - throw new ArgumentNullException(nameof(directoryInfo)); - - if (directorySecurity == null) - throw new ArgumentNullException(nameof(directorySecurity)); - - 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 deleted file mode 100644 index 4432071..0000000 --- a/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.netstandard.cs +++ /dev/null @@ -1,21 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// 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.Security.AccessControl; - -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(); - } - } -} -- 2.7.4