Add NamedPipeServerStream method that takes an ACL (#317)
authorCarlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com>
Mon, 2 Dec 2019 23:54:46 +0000 (15:54 -0800)
committerGitHub <noreply@github.com>
Mon, 2 Dec 2019 23:54:46 +0000 (15:54 -0800)
Add NamedPipeServerStream method that takes an ACL

Approved API proposal: dotnet/corefx#41657

We don't currently have a way to create a pipe with a given ACL in .NET Core. We can modify the ACL, but it would be more secure to have the proper ACL on the pipe from the start.

This PR adds a new static class and method that can create an NamedPipeServerStream taking a PipeSecurity object, reusing code that can already perform this task.

13 files changed:
src/libraries/System.IO.Pipes.AccessControl/ref/System.IO.Pipes.AccessControl.cs
src/libraries/System.IO.Pipes.AccessControl/tests/AnonymousPipeTests/AnonymousPipeServerStreamAclTests.cs
src/libraries/System.IO.Pipes.AccessControl/tests/NamedPipeTests/NamedPipeServerStreamAclTests.cs [new file with mode: 0644]
src/libraries/System.IO.Pipes.AccessControl/tests/PipeServerStreamAclTestBase.cs
src/libraries/System.IO.Pipes.AccessControl/tests/PipeTest.AclExtensions.cs
src/libraries/System.IO.Pipes.AccessControl/tests/System.IO.Pipes.AccessControl.Tests.csproj
src/libraries/System.IO.Pipes/src/MatchingRefApiCompatBaseline.txt
src/libraries/System.IO.Pipes/src/Resources/Strings.resx
src/libraries/System.IO.Pipes/src/System.IO.Pipes.csproj
src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Windows.cs
src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.cs
src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStreamAcl.cs [new file with mode: 0644]
src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Windows.cs

index ffce891..6f29979 100644 (file)
@@ -71,4 +71,9 @@ namespace System.IO.Pipes
     {
         public static System.IO.Pipes.AnonymousPipeServerStream Create(System.IO.Pipes.PipeDirection direction, System.IO.HandleInheritability inheritability, int bufferSize, System.IO.Pipes.PipeSecurity pipeSecurity) { throw null; }
     }
+
+    public static class NamedPipeServerStreamAcl
+    {
+        public static System.IO.Pipes.NamedPipeServerStream Create(string pipeName, System.IO.Pipes.PipeDirection direction, int maxNumberOfServerInstances, System.IO.Pipes.PipeTransmissionMode transmissionMode, System.IO.Pipes.PipeOptions options, int inBufferSize, int outBufferSize, System.IO.Pipes.PipeSecurity pipeSecurity, System.IO.HandleInheritability inheritability = System.IO.HandleInheritability.None, System.IO.Pipes.PipeAccessRights additionalAccessRights = default) { throw null; }
+    }
 }
index 4443a75..d92a23b 100644 (file)
@@ -8,10 +8,6 @@ namespace System.IO.Pipes.Tests
 {
     public class AnonymousPipeServerStreamAclTests : PipeServerStreamAclTestBase
     {
-        private const PipeDirection DefaultPipeDirection = PipeDirection.In;
-        private const HandleInheritability DefaultInheritability = HandleInheritability.None;
-        private const int DefaultBufferSize = 1;
-
         [Fact]
         public void Create_NullSecurity()
         {
@@ -28,10 +24,7 @@ namespace System.IO.Pipes.Tests
         }
 
         [Theory]
-        [InlineData((PipeDirection)(int.MinValue))]
-        [InlineData((PipeDirection)0)]
-        [InlineData((PipeDirection)4)]
-        [InlineData((PipeDirection)(int.MaxValue))]
+        [MemberData(nameof(Create_InvalidPipeDirection_MemberData))]
         public void Create_InvalidPipeDirection(PipeDirection direction)
         {
             Assert.Throws<ArgumentOutOfRangeException>(() =>
@@ -41,10 +34,7 @@ namespace System.IO.Pipes.Tests
         }
 
         [Theory]
-        [InlineData((HandleInheritability)(int.MinValue))]
-        [InlineData((HandleInheritability)(-1))]
-        [InlineData((HandleInheritability)2)]
-        [InlineData((HandleInheritability)(int.MaxValue))]
+        [MemberData(nameof(Create_InvalidInheritability_MemberData))]
         public void Create_InvalidInheritability(HandleInheritability inheritability)
         {
             Assert.Throws<ArgumentOutOfRangeException>(() =>
@@ -54,8 +44,7 @@ namespace System.IO.Pipes.Tests
         }
 
         [Theory]
-        [InlineData(int.MinValue)]
-        [InlineData(-1)]
+        [MemberData(nameof(Create_InvalidBufferSize_MemberData))]
         public void Create_InvalidBufferSize(int bufferSize)
         {
             Assert.Throws<ArgumentOutOfRangeException>(() =>
@@ -66,7 +55,7 @@ namespace System.IO.Pipes.Tests
 
         public static IEnumerable<object[]> Create_ValidParameters_MemberData() =>
             from direction in new[] { PipeDirection.In, PipeDirection.Out }
-            from inheritability in Enum.GetValues(typeof(HandleInheritability)).Cast<HandleInheritability>()
+            from inheritability in new[] { HandleInheritability.None, HandleInheritability.Inheritable }
             from bufferSize in new[] { 0, 1 }
             select new object[] { direction, inheritability, bufferSize };
 
@@ -78,7 +67,7 @@ namespace System.IO.Pipes.Tests
         }
 
         public static IEnumerable<object[]> Create_CombineRightsAndAccessControl_MemberData() =>
-            from rights in Enum.GetValues(typeof(PipeAccessRights)).Cast<PipeAccessRights>()
+            from rights in s_combinedPipeAccessRights
             from accessControl in new[] { AccessControlType.Allow, AccessControlType.Deny }
             select new object[] { rights, accessControl };
 
@@ -87,22 +76,13 @@ namespace System.IO.Pipes.Tests
         [MemberData(nameof(Create_CombineRightsAndAccessControl_MemberData))]
         public void Create_CombineRightsAndAccessControl(PipeAccessRights rights, AccessControlType accessControl)
         {
-            // These are the two cases that create a valid pipe when using Allow
-            if ((rights == PipeAccessRights.FullControl || rights == PipeAccessRights.ReadWrite) &&
-                accessControl == AccessControlType.Allow)
+            // These are the only two rights that allow creating a pipe when using Allow
+            if (accessControl == AccessControlType.Allow &&
+                (rights == PipeAccessRights.FullControl || rights == PipeAccessRights.ReadWrite))
             {
                 VerifyValidSecurity(rights, accessControl);
             }
-            // When creating the PipeAccessRule for the PipeSecurity, the PipeAccessRule constructor calls AccessMaskFromRights, which explicilty removes the Synchronize bit from rights when AccessControlType is Deny
-            // and rights is not FullControl, so using Synchronize with Deny is not allowed
-            else  if (rights == PipeAccessRights.Synchronize && accessControl == AccessControlType.Deny)
-            {
-                Assert.Throws<ArgumentException>("accessMask", () =>
-                {
-                    PipeSecurity security = GetPipeSecurity(WellKnownSidType.BuiltinUsersSid, PipeAccessRights.Synchronize, AccessControlType.Deny);
-                });
-            }
-            // Any other case is not authorized
+            // Any other combination is not authorized
             else
             {
                 PipeSecurity security = GetPipeSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl);
@@ -113,11 +93,12 @@ namespace System.IO.Pipes.Tests
             }
         }
 
-        [Theory]
-        [InlineData(PipeAccessRights.ReadWrite | PipeAccessRights.Synchronize, AccessControlType.Allow)]
-        public void Create_ValidBitwiseRightsSecurity(PipeAccessRights rights, AccessControlType accessControl)
+        [Fact]
+        public void Create_ValidBitwiseRightsSecurity()
         {
-            VerifyValidSecurity(rights, accessControl);
+            // Synchronize gets removed from the bitwise combination,
+            // but ReadWrite (an allowed right) should remain untouched
+            VerifyValidSecurity(PipeAccessRights.ReadWrite | PipeAccessRights.Synchronize, AccessControlType.Allow);
         }
 
         private void VerifyValidSecurity(PipeAccessRights rights, AccessControlType accessControl)
diff --git a/src/libraries/System.IO.Pipes.AccessControl/tests/NamedPipeTests/NamedPipeServerStreamAclTests.cs b/src/libraries/System.IO.Pipes.AccessControl/tests/NamedPipeTests/NamedPipeServerStreamAclTests.cs
new file mode 100644 (file)
index 0000000..893bda0
--- /dev/null
@@ -0,0 +1,316 @@
+using System.Collections.Generic;
+using System.Linq;
+using System.Security.AccessControl;
+using System.Security.Cryptography;
+using System.Security.Principal;
+using Xunit;
+
+namespace System.IO.Pipes.Tests
+{
+    public class NamedPipeServerStreamAclTests : PipeServerStreamAclTestBase
+    {
+        private const int DefaultNumberOfServerInstances = 1;
+        private const PipeTransmissionMode DefaultPipeTransmissionMode = PipeTransmissionMode.Byte;
+        private const PipeOptions DefaultPipeOptions = PipeOptions.None;
+
+        [Fact]
+        public void Create_NullSecurity()
+        {
+            CreateNamedPipe(GetRandomName(), expectedSecurity: null).Dispose();
+            CreateNamedPipe(GetRandomName(), expectedSecurity: null, options: PipeOptions.WriteThrough).Dispose();
+            CreateNamedPipe(GetRandomName(), expectedSecurity: null, options: PipeOptions.Asynchronous).Dispose();
+        }
+
+        [Theory]
+        [InlineData((PipeOptions)(-1))]
+        [InlineData((PipeOptions)1)]
+        [InlineData((PipeOptions)int.MaxValue)]
+        public void Create_InvalidOptions(PipeOptions options)
+        {
+            Assert.Throws<ArgumentOutOfRangeException>("options", () =>
+            {
+                CreateAndVerifyNamedPipe(GetRandomName(), GetBasicPipeSecurity(), options: options).Dispose();
+            });
+        }
+
+        [Theory]
+        [InlineData(PipeOptions.None)]
+        [InlineData(PipeOptions.Asynchronous)]
+        [InlineData(PipeOptions.WriteThrough)]
+        [InlineData(PipeOptions.Asynchronous | PipeOptions.WriteThrough)]
+        public void Create_ValidOptions(PipeOptions options)
+        {
+            CreateAndVerifyNamedPipe(GetRandomName(), GetBasicPipeSecurity(), options: options).Dispose();
+        }
+
+        // Creating a pipe with CurrentUserOnly should be allowed only when the passed pipeSecurity is null.
+        [Fact]
+        public void Create_NullSecurity_PipeOptionsCurrentUserOnly()
+        {
+            using NamedPipeServerStream pipe = CreateNamedPipe(GetRandomName(), null, options: PipeOptions.CurrentUserOnly);
+            PipeSecurity actualSecurity = pipe.GetAccessControl();
+            PipeSecurity expectedSecurity = GetPipeSecurityForCurrentUserOnly();
+            VerifyPipeSecurity(expectedSecurity, actualSecurity);
+        }
+
+        // We do not allow using PipeOptions.CurrentUserOnly and passing a PipeSecurity object at the same time,
+        // because the Create method will force the usage of a custom PipeSecurity instance assigned to the
+        // current user with full control allowed
+        [Fact]
+        public void Create_ValidSecurity_PipeOptionsCurrentUserOnly()
+        {
+            Assert.Throws<ArgumentException>("pipeSecurity", () =>
+            {
+                CreateNamedPipe(GetRandomName(), GetBasicPipeSecurity(), options: PipeOptions.CurrentUserOnly);
+            });
+        }
+
+        [Fact]
+        public void Create_InvalidName()
+        {
+            Assert.Throws<ArgumentException>(() =>
+            {
+                CreateNamedPipe(pipeName: "", GetBasicPipeSecurity());
+            });
+
+            Assert.Throws<ArgumentNullException>("pipeName", () =>
+            {
+                CreateNamedPipe(pipeName: null, GetBasicPipeSecurity());
+            });
+
+            Assert.Throws<ArgumentOutOfRangeException>("pipeName", () =>
+            {
+                CreateNamedPipe(pipeName: "anonymous", GetBasicPipeSecurity());
+            });
+        }
+
+        [Theory]
+        [MemberData(nameof(Create_InvalidPipeDirection_MemberData))]
+        public void Create_InvalidPipeDirection(PipeDirection direction)
+        {
+            Assert.Throws<ArgumentOutOfRangeException>(() =>
+            {
+                CreateAndVerifyNamedPipe(GetRandomName(), GetBasicPipeSecurity(), direction: direction).Dispose();
+            });
+        }
+
+        [Theory]
+        [InlineData(int.MinValue)]
+        [InlineData(0)]
+        [InlineData(255)]
+        [InlineData(int.MaxValue)]
+        public void Create_InvalidMaxNumberOfServerInstances(int maxNumberOfServerInstances)
+        {
+            Assert.Throws<ArgumentOutOfRangeException>("maxNumberOfServerInstances", () =>
+            {
+                CreateAndVerifyNamedPipe(GetRandomName(), GetBasicPipeSecurity(), maxNumberOfServerInstances: maxNumberOfServerInstances).Dispose();
+            });
+        }
+
+        [Theory]
+        [InlineData(-1)] // We interpret -1 as MaxAllowedServerInstances.
+        [InlineData(1)]
+        [InlineData(2)]
+        [InlineData(254)]
+        public void Create_ValidMaxNumberOfServerInstances(int instances)
+        {
+            CreateAndVerifyNamedPipe(GetRandomName(), GetBasicPipeSecurity(), maxNumberOfServerInstances: instances).Dispose();
+        }
+
+        [Theory]
+        [InlineData((PipeTransmissionMode)(-1))]
+        [InlineData((PipeTransmissionMode)2)]
+        public void Create_InvalidTransmissionMode(PipeTransmissionMode transmissionMode)
+        {
+            Assert.Throws<ArgumentOutOfRangeException>("transmissionMode", () =>
+            {
+                CreateAndVerifyNamedPipe(GetRandomName(), GetBasicPipeSecurity(), transmissionMode: transmissionMode).Dispose();
+            });
+        }
+
+        [Theory]
+        [MemberData(nameof(Create_InvalidBufferSize_MemberData))]
+        public void Create_InvalidInBufferSize(int inBufferSize)
+        {
+            Assert.Throws<ArgumentOutOfRangeException>(() =>
+            {
+                CreateAndVerifyNamedPipe(GetRandomName(), GetBasicPipeSecurity(), inBufferSize: inBufferSize).Dispose();
+            });
+        }
+
+        [Theory]
+        [MemberData(nameof(Create_InvalidBufferSize_MemberData))]
+        public void Create_InvalidOutBufferSize(int outBufferSize)
+        {
+            Assert.Throws<ArgumentOutOfRangeException>(() =>
+            {
+                CreateAndVerifyNamedPipe(GetRandomName(), GetBasicPipeSecurity(), outBufferSize: outBufferSize).Dispose();
+            });
+        }
+
+        [Theory]
+        [MemberData(nameof(Create_InvalidInheritability_MemberData))]
+        public void Create_InvalidInheritability(HandleInheritability inheritability)
+        {
+            Assert.Throws<ArgumentOutOfRangeException>(() =>
+            {
+                CreateAndVerifyNamedPipe(GetRandomName(), GetBasicPipeSecurity(), inheritability: inheritability).Dispose();
+            });
+        }
+
+        [Theory]
+        [InlineData(PipeAccessRights.Read)]
+        [InlineData(PipeAccessRights.ReadExtendedAttributes)]
+        [InlineData(PipeAccessRights.ReadAttributes)]
+        [InlineData(PipeAccessRights.ReadPermissions)]
+        [InlineData(PipeAccessRights.Write)]
+        [InlineData(PipeAccessRights.WriteExtendedAttributes)]
+        [InlineData(PipeAccessRights.WriteAttributes)]
+        public void Create_InvalidAdditionalAccessRights(PipeAccessRights additionalAccessRights)
+        {
+            // GetBasicPipeSecurity returns an object created with PipeAccessRights.ReadWrite as default. This enum is formed by:
+            //     - PipeAccessRights.Read: This enum is formed by:
+            //         - PipeAccessRights.ReadData | PipeAccessRights.ReadExtendedAttributes | PipeAccessRights.ReadAttributes | PipeAccessRights.ReadPermissions
+            //     - PipeAccessRights.Write: This enum is formed by:
+            //         - PipeAccessRights.WriteData | PipeAccessRights.WriteExtendedAttributes | PipeAccessRights.WriteAttributes
+
+            // additionalAccessRights gets bitwise merged with the 'dwOpenMode' parameter we pass to CreateNamedPipeW.
+            // This parameter can acquire any of the values described here: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createnamedpipea
+            // It's particularly important to mention that two of the accepted values collide with the value of two PipeAccessRights enum values:
+            // - ReadData (0x1): Same value as PIPE_ACCESS_INBOUND
+            // - WriteData (0x2): Same value as PIPE_ACCESS_OUTBOUND
+
+            // Any other value will throw with the message 'The parameter is incorrect.'
+            Assert.Throws<IOException>(() =>
+            {
+                Create_AdditionalAccessRights(additionalAccessRights).Dispose();
+            });
+        }
+
+        [Theory]
+        [InlineData(PipeAccessRights.CreateNewInstance)]
+        [InlineData(PipeAccessRights.Delete)]
+        public void Create_WindowsNotAcceptedAdditionalAccessRights(PipeAccessRights additionalAccessRights)
+        {
+            // Exception message: "The parameter is incorrect."
+            // Neither CreateNewInstance (0x4) nor Delete (0x10000) collide with any of the dwOpenMode values that get into the bitwise combination:
+            // PipeOptions, PipeDirection, Interop.Kernel32.FileOperations.FILE_FLAG_FIRST_PIPE_INSTANCE
+            // But Windows does not accept them anyway
+            Assert.Throws<IOException>(() =>
+            {
+                Create_AdditionalAccessRights(additionalAccessRights).Dispose();
+            });
+        }
+
+        [Fact]
+        public void Create_NotEnoughPrivilegesAdditionalAccessRights()
+        {
+            // Exception message: "A required privilege is not held by the client"
+            Assert.Throws<IOException>(() =>
+            {
+                Create_AdditionalAccessRights(PipeAccessRights.AccessSystemSecurity).Dispose();
+            });
+        }
+
+        [Theory]
+        [InlineData(PipeAccessRights.ReadData)]
+        [InlineData(PipeAccessRights.WriteData)]
+        [InlineData(PipeAccessRights.ChangePermissions)]
+        [InlineData(PipeAccessRights.TakeOwnership)]
+        public void Create_ValidAdditionalAccessRights(PipeAccessRights additionalAccessRights)
+        {
+            using var pipe = Create_AdditionalAccessRights(additionalAccessRights);
+
+            // This contains the rights added to BasicPipeSecurity plus the one we are testing
+            PipeSecurity expectedPipeSecurity = GetPipeSecurity(WellKnownSidType.BuiltinUsersSid, additionalAccessRights | PipeAccessRights.ReadWrite, AccessControlType.Allow);
+
+            // additional should be applied to the pipe, so actual should be identical to expected
+            PipeSecurity actualPipeSecurity = pipe.GetAccessControl();
+
+            VerifyPipeSecurity(expectedPipeSecurity, actualPipeSecurity);
+        }
+
+        private NamedPipeServerStream Create_AdditionalAccessRights(PipeAccessRights additionalAccessRights)
+        {
+            // GetBasicPipeSecurity returns an object created with PipeAccessRights.ReadWrite as default
+            PipeSecurity initialPipeSecurity = GetBasicPipeSecurity();
+            return CreateNamedPipe(GetRandomName(), initialPipeSecurity, additionalAccessRights: additionalAccessRights);
+        }
+
+        public static IEnumerable<object[]> Create_ValidParameters_MemberData() =>
+            from options in new[] { PipeOptions.None, PipeOptions.Asynchronous, PipeOptions.WriteThrough }
+            from direction in new[] { PipeDirection.In, PipeDirection.Out, PipeDirection.InOut }
+            from transmissionMode in new[] { PipeTransmissionMode.Byte, PipeTransmissionMode.Message }
+            from inheritability in new[] { HandleInheritability.None, HandleInheritability.Inheritable }
+            from inBufferSize in new[] { 0, 1 }
+            from outBufferSize in new[] { 0, 1 }
+            from maxNumberOfServerInstances in new[] { -1, 1, 254 }
+            from rights in s_combinedPipeAccessRights
+            from controlType in new[] { AccessControlType.Allow, AccessControlType.Deny }
+            select new object[] { options, direction, transmissionMode, inheritability, inBufferSize, outBufferSize, maxNumberOfServerInstances, rights, controlType };
+
+        [Theory]
+        [MemberData(nameof(Create_ValidParameters_MemberData))]
+        public void Create_ValidParameters(PipeOptions options, PipeDirection direction, PipeTransmissionMode transmissionMode, HandleInheritability inheritability, int inBufferSize, int outBufferSize, int maxNumberOfServerInstances, PipeAccessRights rights, AccessControlType controlType)
+        {
+            if (controlType != AccessControlType.Deny && (rights & ~PipeAccessRights.Synchronize) != 0)
+            {
+                PipeSecurity security = GetPipeSecurity(WellKnownSidType.BuiltinUsersSid, rights, controlType);
+                CreateAndVerifyNamedPipe(GetRandomName(), security, direction, maxNumberOfServerInstances, transmissionMode, options, inBufferSize, outBufferSize, inheritability, 0).Dispose();
+            }
+        }
+
+        private NamedPipeServerStream CreateAndVerifyNamedPipe(
+            string pipeName,
+            PipeSecurity expectedSecurity,
+            PipeDirection direction = DefaultPipeDirection,
+            int maxNumberOfServerInstances = DefaultNumberOfServerInstances,
+            PipeTransmissionMode transmissionMode = DefaultPipeTransmissionMode,
+            PipeOptions options = DefaultPipeOptions,
+            int inBufferSize = DefaultBufferSize,
+            int outBufferSize = DefaultBufferSize,
+            HandleInheritability inheritability = DefaultInheritability,
+            PipeAccessRights additionalAccessRights = 0)
+        {
+            NamedPipeServerStream pipe = CreateNamedPipe(pipeName, expectedSecurity, direction, maxNumberOfServerInstances, transmissionMode, options, inBufferSize, outBufferSize, inheritability, additionalAccessRights);
+
+            if (expectedSecurity != null)
+            {
+                PipeSecurity actualSecurity = pipe.GetAccessControl();
+                VerifyPipeSecurity(expectedSecurity, actualSecurity);
+            }
+            return pipe;
+        }
+
+        private NamedPipeServerStream CreateNamedPipe(
+            string pipeName,
+            PipeSecurity expectedSecurity,
+            PipeDirection direction = DefaultPipeDirection,
+            int maxNumberOfServerInstances = DefaultNumberOfServerInstances,
+            PipeTransmissionMode transmissionMode = DefaultPipeTransmissionMode,
+            PipeOptions options = DefaultPipeOptions,
+            int inBufferSize = DefaultBufferSize,
+            int outBufferSize = DefaultBufferSize,
+            HandleInheritability inheritability = DefaultInheritability,
+            PipeAccessRights additionalAccessRights = 0)
+        {
+            NamedPipeServerStream pipe = NamedPipeServerStreamAcl.Create(pipeName, direction, maxNumberOfServerInstances, transmissionMode, options, inBufferSize, outBufferSize, expectedSecurity, inheritability, additionalAccessRights);
+            Assert.NotNull(pipe);
+            return pipe;
+        }
+
+        // This is the code we use in the Create method called by the NamedPipeServerStream constructor
+        private PipeSecurity GetPipeSecurityForCurrentUserOnly()
+        {
+            PipeSecurity security = new PipeSecurity();
+
+            using WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent();
+            SecurityIdentifier identifier = currentIdentity.Owner;
+            PipeAccessRule rule = new PipeAccessRule(identifier, PipeAccessRights.FullControl, AccessControlType.Allow);
+            security.AddAccessRule(rule);
+            security.SetOwner(identifier);
+
+            return security;
+        }
+    }
+}
index 679710e..b1d1b13 100644 (file)
@@ -8,11 +8,50 @@ namespace System.IO.Pipes.Tests
 {
     public class PipeServerStreamAclTestBase
     {
+        protected const PipeDirection DefaultPipeDirection = PipeDirection.In;
+        protected const HandleInheritability DefaultInheritability = HandleInheritability.None;
+        protected const int DefaultBufferSize = 1;
+
+        // As it is documented in the source definition of the PipeAccessRights enum, we do not have a 0 value on purpose (can't grant nor deny "nothing").
+        // So ReadWrite will be used in these unit tests the sole minimum additional granted right, considering that AnonymousPipeServerStreams can only
+        // get created with either ReadWrite or FullControl.
+        protected const PipeAccessRights DefaultAccessRight = PipeAccessRights.ReadWrite;
+
+        // PipeAccessRights.Synchronize is not included in this arary because it is handled in a special way inside the PipeAccessRuleInstance constructor when creating the access mask: If Deny is specified, Synchronize gets removed from the rights.
+        // So this right's behavior is verified separately in the System.IO.Pipes.Tests.PipeTest_AclExtensions.PipeSecurity_VerifySynchronizeMasks unit test.
+        protected static readonly PipeAccessRights[] s_mostRights = new[]
+        {
+            PipeAccessRights.ReadData,
+            PipeAccessRights.WriteData,
+            PipeAccessRights.CreateNewInstance,
+            PipeAccessRights.ReadExtendedAttributes,
+            PipeAccessRights.WriteExtendedAttributes,
+            PipeAccessRights.ReadAttributes,
+            PipeAccessRights.WriteAttributes,
+            PipeAccessRights.Write,
+            PipeAccessRights.Delete,
+            PipeAccessRights.ReadPermissions,
+            PipeAccessRights.Read,
+            PipeAccessRights.ReadWrite,
+            PipeAccessRights.ChangePermissions,
+            PipeAccessRights.TakeOwnership,
+            PipeAccessRights.FullControl,
+            PipeAccessRights.AccessSystemSecurity
+        };
+
+        protected static readonly PipeAccessRights[] s_bitWisePipeAccessRights = new[]
+        {
+            PipeAccessRights.ChangePermissions | PipeAccessRights.ReadPermissions,
+            PipeAccessRights.ReadExtendedAttributes | PipeAccessRights.WriteExtendedAttributes
+        };
+
+        protected static IEnumerable<PipeAccessRights> s_combinedPipeAccessRights = s_mostRights.Concat(s_bitWisePipeAccessRights);
+
         protected PipeSecurity GetBasicPipeSecurity()
         {
             return GetPipeSecurity(
                 WellKnownSidType.BuiltinUsersSid,
-                PipeAccessRights.FullControl,
+                DefaultAccessRight,
                 AccessControlType.Allow);
         }
 
@@ -55,5 +94,38 @@ namespace System.IO.Pipes.Tests
                 expectedRule.InheritanceFlags  == actualRule.InheritanceFlags &&
                 expectedRule.PropagationFlags  == actualRule.PropagationFlags;
         }
+
+        protected string GetRandomName()
+        {
+            return Guid.NewGuid().ToString("N");
+        }
+
+        public static IEnumerable<object[]> Create_MostAccessRights_MemberData() =>
+            from rights in s_mostRights
+            select new object[] { rights };
+
+        public static IEnumerable<object[]> Create_InvalidPipeDirection_MemberData() =>
+            from direction in new[]
+            {
+                (PipeDirection)(int.MinValue),
+                (PipeDirection)0,
+                (PipeDirection)4,
+                (PipeDirection)(int.MaxValue)
+            }
+            select new object[] { direction };
+
+        public static IEnumerable<object[]> Create_InvalidInheritability_MemberData() =>
+            from inheritability in new[]
+            {
+                (HandleInheritability)int.MinValue,
+                (HandleInheritability)(-1),
+                (HandleInheritability)2,
+                (HandleInheritability)int.MaxValue
+            }
+            select new object[] { inheritability };
+
+        public static IEnumerable<object[]> Create_InvalidBufferSize_MemberData() =>
+            from bufferSize in new[] { int.MinValue, -1 }
+            select new object[] { bufferSize };
     }
 }
index 011207c..3b99f30 100644 (file)
@@ -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.Security.AccessControl;
+using System.Security.Principal;
 using Xunit;
 
 namespace System.IO.Pipes.Tests
@@ -81,5 +83,20 @@ namespace System.IO.Pipes.Tests
                 pair.writeablePipe.SetAccessControl(security);
             }
         }
+
+        // This test matches NetFX behavior
+        [Fact]
+        public void PipeSecurity_VerifySynchronizeMasks()
+        {
+            var si = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null);
+
+            // This is a valid mask that should not throw
+            new PipeAccessRule(si, PipeAccessRights.Synchronize, AccessControlType.Allow);
+
+            Assert.Throws<ArgumentException>("accessMask", () =>
+            {
+                new PipeAccessRule(si, PipeAccessRights.Synchronize, AccessControlType.Deny);
+            });
+        }
     }
 }
index 7bd84f2..30fdc15 100644 (file)
@@ -8,6 +8,7 @@
   <ItemGroup>
     <Compile Include="AnonymousPipeTests\AnonymousPipeServerStreamAclTests.cs" />
     <Compile Include="AnonymousPipeTests\AnonymousPipeTest.AclExtensions.cs" />
+    <Compile Include="NamedPipeTests\NamedPipeServerStreamAclTests.cs" />
     <Compile Include="NamedPipeTests\NamedPipeTest.AclExtensions.cs" />
     <Compile Include="PipeServerStreamAclTestBase.cs" />
     <Compile Include="PipeTest.AclExtensions.cs" />
index 4bf6540..45d7379 100644 (file)
@@ -1,5 +1,6 @@
 # Exposed public in System.IO.Pipes.AccessControl but implemented in System.IO.Pipes
 TypesMustExist : Type 'System.IO.Pipes.AnonymousPipeServerStreamAcl' does not exist in the reference but it does exist in the implementation.
+TypesMustExist : Type 'System.IO.Pipes.NamedPipeServerStreamAcl' does not exist in the reference but it does exist in the implementation.
 TypesMustExist : Type 'System.IO.Pipes.PipeAccessRights' does not exist in the reference but it does exist in the implementation.
 TypesMustExist : Type 'System.IO.Pipes.PipeAccessRule' does not exist in the reference but it does exist in the implementation.
 TypesMustExist : Type 'System.IO.Pipes.PipeAuditRule' does not exist in the reference but it does exist in the implementation.
index b22a3ea..c8193e4 100644 (file)
@@ -1,4 +1,5 @@
-<root>
+<?xml version="1.0" encoding="utf-8"?>
+<root>
   <!-- 
     Microsoft ResX Schema 
     
   <data name="UnauthorizedAccess_ClientIsNotCurrentUser" xml:space="preserve">
     <value>Client connection (user id {0}) was refused because it was not owned by the current user (id {1}).</value>
   </data>
+  <data name="NotSupported_PipeSecurityIsCurrentUserOnly" xml:space="preserve">
+    <value>'pipeSecurity' must be null when 'options' contains 'PipeOptions.CurrentUserOnly'. </value>
+  </data>
 </root>
index 1bd0ed0..58efb92 100644 (file)
@@ -61,6 +61,7 @@
     <Compile Include="System\IO\Pipes\AnonymousPipeServerStreamAcl.cs" />
     <Compile Include="System\IO\Pipes\AnonymousPipeServerStream.Windows.cs" />
     <Compile Include="System\IO\Pipes\ConnectionCompletionSource.cs" />
+    <Compile Include="System\IO\Pipes\NamedPipeServerStreamAcl.cs" />
     <Compile Include="System\IO\Pipes\NamedPipeClientStream.Windows.cs" />
     <Compile Include="System\IO\Pipes\NamedPipeServerStream.Windows.cs" />
     <Compile Include="System\IO\Pipes\PipeAccessRights.cs" />
index 82f1eb3..7a988f4 100644 (file)
@@ -19,6 +19,29 @@ namespace System.IO.Pipes
     /// </summary>
     public sealed partial class NamedPipeServerStream : PipeStream
     {
+        internal NamedPipeServerStream(
+            string pipeName,
+            PipeDirection direction,
+            int maxNumberOfServerInstances,
+            PipeTransmissionMode transmissionMode,
+            PipeOptions options,
+            int inBufferSize,
+            int outBufferSize,
+            PipeSecurity pipeSecurity,
+            HandleInheritability inheritability = HandleInheritability.None,
+            PipeAccessRights additionalAccessRights = default)
+            : base(direction, transmissionMode, outBufferSize)
+        {
+            ValidateParameters(pipeName, direction, maxNumberOfServerInstances, transmissionMode, options, inBufferSize, outBufferSize, inheritability);
+
+            if (pipeSecurity != null && IsCurrentUserOnly)
+            {
+                throw new ArgumentException(SR.NotSupported_PipeSecurityIsCurrentUserOnly, nameof(pipeSecurity));
+            }
+
+            Create(pipeName, direction, maxNumberOfServerInstances, transmissionMode, options, inBufferSize, outBufferSize, pipeSecurity, inheritability, additionalAccessRights);
+        }
+
         private void Create(string pipeName, PipeDirection direction, int maxNumberOfServerInstances,
                 PipeTransmissionMode transmissionMode, PipeOptions options, int inBufferSize, int outBufferSize,
                 HandleInheritability inheritability)
@@ -87,7 +110,7 @@ namespace System.IO.Pipes
             GCHandle pinningHandle = default;
             try
             {
-                Interop.Kernel32.SECURITY_ATTRIBUTES secAttrs = PipeStream.GetSecAttrs(inheritability, pipeSecurity, ref pinningHandle);
+                Interop.Kernel32.SECURITY_ATTRIBUTES secAttrs = GetSecAttrs(inheritability, pipeSecurity, ref pinningHandle);
                 SafePipeHandle handle = Interop.Kernel32.CreateNamedPipe(fullPipeName, openMode, pipeModes,
                     maxNumberOfServerInstances, outBufferSize, inBufferSize, 0, ref secAttrs);
 
index 996dcf5..d4cae06 100644 (file)
@@ -2,12 +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 Microsoft.Win32.SafeHandles;
 using System.Diagnostics.CodeAnalysis;
-using System.Security;
 using System.Threading;
 using System.Threading.Tasks;
-using System.Diagnostics;
+using Microsoft.Win32.SafeHandles;
 
 namespace System.IO.Pipes
 {
@@ -73,11 +71,30 @@ namespace System.IO.Pipes
         /// </param>
         /// <param name="outBufferSize">Outgoing buffer size, 0 or higher (see above)</param>
         /// <param name="inheritability">Whether handle is inheritable</param>
-        private NamedPipeServerStream(string pipeName, PipeDirection direction, int maxNumberOfServerInstances,
-                PipeTransmissionMode transmissionMode, PipeOptions options, int inBufferSize, int outBufferSize,
-                HandleInheritability inheritability)
+        private NamedPipeServerStream(string pipeName,
+            PipeDirection direction,
+            int maxNumberOfServerInstances,
+            PipeTransmissionMode transmissionMode,
+            PipeOptions options,
+            int inBufferSize,
+            int outBufferSize,
+            HandleInheritability inheritability)
             : base(direction, transmissionMode, outBufferSize)
         {
+            ValidateParameters(pipeName, direction, maxNumberOfServerInstances, transmissionMode, options, inBufferSize, outBufferSize, inheritability);
+            Create(pipeName, direction, maxNumberOfServerInstances, transmissionMode, options, inBufferSize, outBufferSize, inheritability);
+        }
+
+        private void ValidateParameters(
+            string pipeName,
+            PipeDirection direction,
+            int maxNumberOfServerInstances,
+            PipeTransmissionMode transmissionMode,
+            PipeOptions options,
+            int inBufferSize,
+            int outBufferSize,
+            HandleInheritability inheritability)
+        {
             if (pipeName == null)
             {
                 throw new ArgumentNullException(nameof(pipeName));
@@ -86,6 +103,14 @@ namespace System.IO.Pipes
             {
                 throw new ArgumentException(SR.Argument_NeedNonemptyPipeName);
             }
+            if (direction < PipeDirection.In || direction > PipeDirection.InOut)
+            {
+                throw new ArgumentOutOfRangeException(nameof(direction), SR.ArgumentOutOfRange_DirectionModeInOutOrInOut);
+            }
+            if (transmissionMode < PipeTransmissionMode.Byte || transmissionMode > PipeTransmissionMode.Message)
+            {
+                throw new ArgumentOutOfRangeException(nameof(transmissionMode), SR.ArgumentOutOfRange_TransmissionModeByteOrMsg);
+            }
             if ((options & ~(PipeOptions.WriteThrough | PipeOptions.Asynchronous | PipeOptions.CurrentUserOnly)) != 0)
             {
                 throw new ArgumentOutOfRangeException(nameof(options), SR.ArgumentOutOfRange_OptionsInvalid);
@@ -94,6 +119,10 @@ namespace System.IO.Pipes
             {
                 throw new ArgumentOutOfRangeException(nameof(inBufferSize), SR.ArgumentOutOfRange_NeedNonNegNum);
             }
+            if (outBufferSize < 0)
+            {
+                throw new ArgumentOutOfRangeException(nameof(outBufferSize), SR.ArgumentOutOfRange_NeedNonNegNum);
+            }
             if ((maxNumberOfServerInstances < 1 || maxNumberOfServerInstances > 254) && (maxNumberOfServerInstances != MaxAllowedServerInstances))
             {
                 // win32 allows fixed values of 1-254 or 255 to mean max allowed by system. We expose 255 as -1 (unlimited)
@@ -114,9 +143,6 @@ namespace System.IO.Pipes
             {
                 IsCurrentUserOnly = true;
             }
-
-            Create(pipeName, direction, maxNumberOfServerInstances, transmissionMode,
-            options, inBufferSize, outBufferSize, inheritability);
         }
 
         // Create a NamedPipeServerStream from an existing server pipe handle.
diff --git a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStreamAcl.cs b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStreamAcl.cs
new file mode 100644 (file)
index 0000000..53980df
--- /dev/null
@@ -0,0 +1,51 @@
+// 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.
+
+namespace System.IO.Pipes
+{
+    public static class NamedPipeServerStreamAcl
+    {
+        /// <summary>
+        /// Creates a new instance of the <see cref="NamedPipeServerStream" /> class with the specified pipe name, pipe direction, maximum number of server instances, transmission mode, pipe options, recommended in and out buffer sizes, pipe security, inheritability mode, and pipe access rights.
+        /// </summary>
+        /// <param name="pipeName">The name of the pipe.</param>
+        /// <param name="direction">One of the enumeration values that determines the direction of the pipe.</param>
+        /// <param name="maxNumberOfServerInstances">The maximum number of server instances that share the same name. You can pass <see cref="NamedPipeServerStream.MaxAllowedServerInstances" /> for this value.</param>
+        /// <param name="transmissionMode">One of the enumeration values that determines the transmission mode of the pipe.</param>
+        /// <param name="options">One of the enumeration values that determines how to open or create the pipe.</param>
+        /// <param name="inBufferSize">The input buffer size.</param>
+        /// <param name="outBufferSize">The output buffer size.</param>
+        /// <param name="pipeSecurity">An object that determines the access control and audit security for the pipe.</param>
+        /// <param name="inheritability">One of the enumeration values that determines whether the underlying handle can be inherited by child processes.</param>
+        /// <param name="additionalAccessRights">One of the enumeration values that specifies the access rights of the pipe.</param>
+        /// <returns>A new named pipe server stream instance.</returns>
+        /// <exception cref="ArgumentNullException"><paramref name="pipeName" /> is <see langword="null" />.</exception>
+        /// <exception cref="ArgumentException"><paramref name="pipeName" /> is empty.</exception>
+        /// <exception cref="IOException"><paramref name="options" /> is <see cref="PipeOptions.None" />.</exception>
+        /// <exception cref="ArgumentOutOfRangeException"><paramref name="options" /> contains an invalid flag.
+        /// -or-
+        /// <paramref name="inBufferSize" /> is a negative number.
+        /// -or
+        /// <paramref name="maxNumberOfServerInstances" /> is not a valid number: it should be greater or equal than 1 and less or equal than 254, or should be set to the value of <see cref="NamedPipeServerStream.MaxAllowedServerInstances" />.
+        /// -or-
+        /// <paramref name="inheritability" /> contains an invalid enum value.
+        /// -or
+        /// <paramref name="pipeName" /> is 'anonymous', which is reserved.</exception>
+        /// <remarks>If `options` contains &lt;xref:System.IO.Pipes.PipeOptions.CurrentUserOnly&gt;, the passed `pipeSecurity` is ignored and the returned &lt;xref:System.IO.Pipes.NamedPipeServerStream&gt; object is created using a custom &lt;xref:System.IO.Pipes.PipeSecurity&gt; instance assigned to the current Windows user as its only owner with full control of the pipe.</remarks>
+        public static NamedPipeServerStream Create(
+            string pipeName,
+            PipeDirection direction,
+            int maxNumberOfServerInstances,
+            PipeTransmissionMode transmissionMode,
+            PipeOptions options,
+            int inBufferSize,
+            int outBufferSize,
+            PipeSecurity pipeSecurity,
+            HandleInheritability inheritability = HandleInheritability.None,
+            PipeAccessRights additionalAccessRights = default)
+        {
+            return new NamedPipeServerStream(pipeName, direction, maxNumberOfServerInstances, transmissionMode, options, inBufferSize, outBufferSize, pipeSecurity, inheritability, additionalAccessRights);
+        }
+    }
+}
index 3973970..c79bdc7 100644 (file)
@@ -399,25 +399,18 @@ namespace System.IO.Pipes
 
         internal static unsafe Interop.Kernel32.SECURITY_ATTRIBUTES GetSecAttrs(HandleInheritability inheritability)
         {
-            Interop.Kernel32.SECURITY_ATTRIBUTES secAttrs = default(Interop.Kernel32.SECURITY_ATTRIBUTES);
-            if ((inheritability & HandleInheritability.Inheritable) != 0)
+            Interop.Kernel32.SECURITY_ATTRIBUTES secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES
             {
-                secAttrs = default;
-                secAttrs.nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES);
-                secAttrs.bInheritHandle = Interop.BOOL.TRUE;
-            }
+                nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES),
+                bInheritHandle = ((inheritability & HandleInheritability.Inheritable) != 0) ? Interop.BOOL.TRUE : Interop.BOOL.FALSE
+            };
+
             return secAttrs;
         }
 
         internal static unsafe Interop.Kernel32.SECURITY_ATTRIBUTES GetSecAttrs(HandleInheritability inheritability, PipeSecurity pipeSecurity, ref GCHandle pinningHandle)
         {
-            Interop.Kernel32.SECURITY_ATTRIBUTES secAttrs = default(Interop.Kernel32.SECURITY_ATTRIBUTES);
-            secAttrs.nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES);
-
-            if ((inheritability & HandleInheritability.Inheritable) != 0)
-            {
-                secAttrs.bInheritHandle = Interop.BOOL.TRUE;
-            }
+            Interop.Kernel32.SECURITY_ATTRIBUTES secAttrs = GetSecAttrs(inheritability);
 
             if (pipeSecurity != null)
             {