API Proposal: Add file and directory creation methods that take an ACL

[JeremyKuhne]

Summary

.NET ACL (Access Control List) support is Windows specific (FileSecurity and DirectorySecurity). We keep support in System.IO.FileSystem.AccessControl and do not have references to the ACL types in base System.IO types in .NET Core (e.g. File, DirectoryInfo, FileStream). We have extension methods in System.IO.FileSystemAclExtensions to get and set ACLs instead. We do not, however, have methods that let you create files and directories with a specific ACL set.

This presents a few problems:

• Potential security hole as files/directories 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)

We need to add helper methods that prevent these problems. Adding back API isn't feasible as FileStream lives in CoreLib and would require pulling down a rather large closure (above and beyond putting Windows specific API in front of Unix/cross-plat developers). Having these as extension methods also facilitates writing .NET Standard libraries.

Proposal

using System.Security.AccessControl;

namespace System.IO
{
    public static class FileSystemAclExtensions
    {
        // Add
        public static FileStream Create(
            this FileInfo fileInfo,
            FileMode mode,
            FileSystemRights rights,
            FileShare share,
            int bufferSize,
            FileOptions options,
            FileSecurity fileSecurity);

        public static void Create(
            this DirectoryInfo directoryInfo,
            DirectorySecurity directorySecurity);

        public static FileStream CreateFile(
            this FileSecurity fileSecurity,
            string path,
            FileMode mode,
            FileSystemRights rights,
            FileShare share,
            int bufferSize,
            FileOptions options);

        public static DirectoryInfo CreateDirectory(
            this DirectorySecurity directorySecurity
            string path);

        // Existing
        public static DirectorySecurity GetAccessControl(this DirectoryInfo directoryInfo);
        public static DirectorySecurity GetAccessControl(this DirectoryInfo directoryInfo, AccessControlSections includeSections);
        public static FileSecurity GetAccessControl(this FileInfo fileInfo);
        public static FileSecurity GetAccessControl(this FileInfo fileInfo, AccessControlSections includeSections);
        public static FileSecurity GetAccessControl(this FileStream fileStream);
        public static void SetAccessControl(this DirectoryInfo directoryInfo, DirectorySecurity directorySecurity);
        public static void SetAccessControl(this FileInfo fileInfo, FileSecurity fileSecurity);
        public static void SetAccessControl(this FileStream fileStream, FileSecurity fileSecurity);
}

Details

Heuristics would follow .NET Framework as closely as possible. Directory creation is the most important of the two as you can prevent malicious file access if the directory is locked from the start.

For directory creation we would likely include the same sources that System.IO.FileSystem uses as the directory creation helper there already sets a "default" security object and would just need to have an overload added (which would allow creation heuristics to match properly, including error states).

File creation is a bit more complicated as the logic may take a bit more refactoring, but we should strive to include the same sources as well.

Marking as 3.1 as internal partners are hitting this in porting efforts.

Related Issues

#27021

CC: @danmosemsft, @ericstj, @terrajobst

[carlossanlop]

       public static void Create(
            this DirectoryInfo,
            DirectorySecurity directorySecurity);

Why is this method defined as void, but the one for this FileInfo returns a FileStream? Should this one return something?
Also, one small nit: the parameter name is missing for this DirectoryInfo.

[JeremyKuhne]

Why is this method defined as void, but the one for this FileInfo returns a FileStream? Should this one return something?

Because you're passing in the DirectoryInfo, which would be what was passed back anyway. DirectoryInfo.Create doesn't return anything.

[terrajobst]

The one thing that is odd is that you have to go through the info classes (as opposed to a pure static with a path).

What pattern would we use for other entities, such as threading primitives?

[JeremyKuhne]

The one thing that is odd is that you have to go through the info classes (as opposed to a pure static with a path).

@terrajobst We could also add a new static class here I suppose, but I can't think of a great name for it or think of how that would grow. We could also add these methods to FileSecurity and DirectorySecurity but I'm unclear on how we'd technically manage that for .NET Standard and the existing type forwards (just my ignorance here).

What we really need are static class extension methods. ;)

[JeremyKuhne]

What pattern would we use for other entities, such as threading primitives?

Are you talking about synchronization stuff? (MutexSecurity, etc.)

[nguerrera]

https://github.com/dotnet/corefx/issues/33719 was closed against https://github.com/dotnet/corefx/issues/31488, which was then closed against this.

Does this cover the case of using existing .NET Framework libs that call the API in their original location in .NET Framework? If not, then either #28009 should be re-opened or at least a comment added there that in fact that is being closed as "won't fix" and not addressed by the linked issue after all.

[JeremyKuhne]

Does this cover the case of using existing .NET Framework libs that call the API in their original location in .NET Framework?

No, it does not. Thanks for pointing that out.

[terrajobst]

Video

We considered adding the APIs back to the relevant types (i.e. as constructors). While this would be preferred from a porting/backwards compabitility case, it would mean we'd have to pull-in a large amount of ACL / Windows security into System.Private.Corelib.

We think these APIs are weird and shouldn't be exposed:

namespace System.IO
{
    public static class FileSystemAclExtensions
    {
        public static FileStream CreateFile(
            this FileSecurity fileSecurity,
            string path,
            FileMode mode,
            FileSystemRights rights,
            FileShare share,
            int bufferSize,
            FileOptions options);

        public static DirectoryInfo CreateDirectory(
            this DirectorySecurity directorySecurity
            string path);
    }
 }

These APIs look fine (assuming they match the existing instance methods of .NET Framework in terms of naming and order):

namespace System.IO
{
    public static class FileSystemAclExtensions
    {
        // Add
        public static FileStream Create(
            this FileInfo fileInfo,
            FileMode mode,
            FileSystemRights rights,
            FileShare share,
            int bufferSize,
            FileOptions options,
            FileSecurity fileSecurity);

        public static void Create(
            this DirectoryInfo directoryInfo,
            DirectorySecurity directorySecurity);
    }
}

[carlossanlop]

Reopening because I still have to merge the file creation method (#41909).