Skip to content

Commit

Permalink
Add file creation method that takes an ACL (dotnet/corefx#42099)
Browse files Browse the repository at this point in the history
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 dotnet/corefx@508cbc4
  • Loading branch information
carlossanlop authored Oct 29, 2019
1 parent 01aeaa5 commit 70e8131
Show file tree
Hide file tree
Showing 12 changed files with 456 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,4 +188,10 @@
<data name="Arg_PathEmpty" xml:space="preserve">
<value>The path is empty.</value>
</data>
<data name="ArgumentOutOfRange_NeedPosNum" xml:space="preserve">
<value>Positive number required.</value>
</data>
<data name="Argument_InvalidFileModeAndFileSystemRightsCombo" xml:space="preserve">
<value>Combining FileMode.{0} with FileSystemRights.{1} is invalid.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,21 @@
</ItemGroup>
<ItemGroup Condition="'$(TargetsNetCoreApp)' == 'true'">
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Interop.BOOL.cs" Link="Common\CoreLib\Interop\Windows\Interop.BOOL.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.CreateFile.cs" Link="Common\CoreLib\Interop\Windows\Interop.CreateFile.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.FILE_TIME.cs" Link="Common\CoreLib\Interop\Windows\Interop.FILE_TIME.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.FileAttributes.cs" Link="Common\Interop\Windows\Interop.FileAttributes.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.FileTypes.cs" Link="Common\Interop\Windows\Interop.FileTypes.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.FindClose.cs" Link="Common\CoreLib\Interop\Windows\Interop.FindClose.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.FindFirstFileEx.cs" Link="Common\CoreLib\Interop\Windows\Interop.FindFirstFileEx.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.FormatMessage.cs" Link="Common\Interop\Windows\Interop.FormatMessage.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.GET_FILEEX_INFO_LEVELS.cs" Link="Common\CoreLib\Interop\Windows\Interop.GET_FILEEX_INFO_LEVELS.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.GetFileAttributesEx.cs" Link="Common\Interop\Windows\Interop.GetFileAttributesEx.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.GetFileType_SafeHandle.cs" Link="Common\Interop\Windows\Interop.GetFileType_SafeHandle.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.GetFullPathNameW.cs" Link="Common\Interop\Windows\Interop.GetFullPathNameW.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.GetLongPathNameW.cs" Link="Common\Interop\Windows\Interop.GetLongPathNameW.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.GetLogicalDrives.cs" Link="Common\Interop\Windows\Interop.GetLogicalDrives.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.MAX_PATH.cs" Link="Common\CoreLib\Interop\Windows\Interop.MAX_PATH.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.SecurityOptions.cs" Link="Common\CoreLib\Interop\Windows\Interop.SecurityOptions.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.SECURITY_ATTRIBUTES.cs" Link="Common\CoreLib\Interop\Windows\Interop.SECURITY_ATTRIBUTES.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.SetThreadErrorMode.cs" Link="Common\CoreLib\Interop\Windows\Interop.SetThreadErrorMode.cs" />
<Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.WIN32_FILE_ATTRIBUTE_DATA.cs" Link="Common\CoreLib\Interop\Windows\Interop.WIN32_FILE_ATTRIBUTE_DATA.cs" />
Expand All @@ -45,6 +49,7 @@
<Compile Include="$(CommonPath)\CoreLib\System\IO\PathInternal.Windows.cs" Link="Common\CoreLib\System\IO\PathInternal.Windows.cs" />
<Compile Include="$(CommonPath)\CoreLib\System\IO\Win32Marshal.cs" Link="Common\CoreLib\System\IO\Win32Marshal.cs" />
<Compile Include="$(CommonPath)\CoreLib\System\Text\ValueStringBuilder.cs" Link="Common\CoreLib\System\Text\ValueStringBuilder.cs" />
<Compile Include="$(CommonPath)\Interop\Windows\Kernel32\Interop.GenericOperations.cs" Link="Common\Interop\Windows\Interop.GenericOperations.cs" />
<Compile Include="$(CommonPath)\Interop\Windows\Interop.Libraries.cs" Link="Common\Interop\Windows\Interop.Libraries.cs" />
<Compile Include="$(CommonPath)\Interop\Windows\Interop.LongFileTime.cs" Link="Common\Interop\Windows\Interop.LongFileTime.cs" />
<Compile Include="$(CommonPath)\Interop\Windows\Kernel32\Interop.CreateDirectory.cs" Link="Common\Interop\Windows\Interop.CreateDirectory.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -14,6 +17,7 @@ public static partial class FileSystemAclExtensions
/// <exception cref="ArgumentNullException"><paramref name="directoryInfo" /> or <paramref name="directorySecurity" /> is <see langword="null" />.</exception>
/// <exception cref="DirectoryNotFoundException">Could not find a part of the path.</exception>
/// <exception cref="UnauthorizedAccessException">Access to the path is denied.</exception>
/// <remarks>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.</remarks>
public static void Create(this DirectoryInfo directoryInfo, DirectorySecurity directorySecurity)
{
if (directoryInfo == null)
Expand All @@ -24,5 +28,148 @@ public static void Create(this DirectoryInfo directoryInfo, DirectorySecurity di

FileSystem.CreateDirectory(directoryInfo.FullName, directorySecurity.GetSecurityDescriptorBinaryForm());
}

/// <summary>
/// Creates a new file stream, ensuring it is created with the specified properties and security settings.
/// </summary>
/// <param name="fileInfo">The current instance describing a file that does not exist in disk yet.</param>
/// <param name="mode">One of the enumeration values that specifies how the operating system should open a file.</param>
/// <param name="rights">One of the enumeration values that defines the access rights to use when creating access and audit rules.</param>
/// <param name="share">One of the enumeration values for controlling the kind of access other FileStream objects can have to the same file.</param>
/// <param name="bufferSize">The number of bytes buffered for reads and writes to the file.</param>
/// <param name="options">One of the enumeration values that describes how to create or overwrite the file.</param>
/// <param name="fileSecurity">An object that determines the access control and audit security for the file.</param>
/// <returns>A file stream for the newly created file.</returns>
/// <exception cref="ArgumentException">The <paramref name="rights" /> and <paramref name="mode" /> combination is invalid.</exception>
/// <exception cref="ArgumentNullException"><paramref name="fileInfo" /> or <paramref name="fileSecurity" /> is <see langword="null" />.</exception>
/// <exception cref="ArgumentOutOfRangeException"><paramref name="mode" /> or <paramref name="share" /> are out of their legal enum range.
///-or-
/// <paramref name="bufferSize" /> is not a positive number.</exception>
/// <exception cref="DirectoryNotFoundException">Could not find a part of the path.</exception>
/// <exception cref="IOException">An I/O error occurs.</exception>
/// <exception cref="UnauthorizedAccessException">Access to the path is denied.</exception>
/// <remarks>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.</remarks>
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);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading

0 comments on commit 70e8131

Please sign in to comment.