Skip to content

Commit

Permalink
Merge pull request dotnet#111 from carlossanlop/AnonymousPipeServerSt…
Browse files Browse the repository at this point in the history
…reamAcl

Add AnonymousPipeServerStream method that takes an ACL

The original corefx PR was already signed off, but the CI did not finish on time before the 5pm deadline: dotnet/corefx#42392

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 AnonymousPipeServerStream taking a PipeSecurity object, reusing code that can already perform this task.
  • Loading branch information
carlossanlop authored Nov 19, 2019
2 parents 2224f8b + 96a2daf commit a16ee22
Show file tree
Hide file tree
Showing 13 changed files with 328 additions and 208 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio 15
VisualStudioVersion = 15.0.27213.1
# Visual Studio Version 16
VisualStudioVersion = 16.0.29411.138
MinimumVisualStudioVersion = 10.0.40219.1
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.IO.Pipes.AccessControl.Tests", "tests\System.IO.Pipes.AccessControl.Tests.csproj", "{A0356E61-19E1-4722-A53D-5D2616E16312}"
ProjectSection(ProjectDependencies) = postProject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,9 @@ public void ResetAccessRule(System.IO.Pipes.PipeAccessRule rule) { }
public void SetAccessRule(System.IO.Pipes.PipeAccessRule rule) { }
public void SetAuditRule(System.IO.Pipes.PipeAuditRule rule) { }
}

public static class AnonymousPipeServerStreamAcl
{
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; }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
using System.Collections.Generic;
using System.Linq;
using System.Security.AccessControl;
using System.Security.Principal;
using Xunit;

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()
{
CreateAndVerifyAnonymousPipe(expectedSecurity: null).Dispose();
}

[Fact]
public void Create_NotSupportedPipeDirection()
{
Assert.Throws<NotSupportedException>(() =>
{
CreateAndVerifyAnonymousPipe(GetBasicPipeSecurity(), PipeDirection.InOut).Dispose();
});
}

[Theory]
[InlineData((PipeDirection)(int.MinValue))]
[InlineData((PipeDirection)0)]
[InlineData((PipeDirection)4)]
[InlineData((PipeDirection)(int.MaxValue))]
public void Create_InvalidPipeDirection(PipeDirection direction)
{
Assert.Throws<ArgumentOutOfRangeException>(() =>
{
CreateAndVerifyAnonymousPipe(GetBasicPipeSecurity(), direction).Dispose();
});
}

[Theory]
[InlineData((HandleInheritability)(int.MinValue))]
[InlineData((HandleInheritability)(-1))]
[InlineData((HandleInheritability)2)]
[InlineData((HandleInheritability)(int.MaxValue))]
public void Create_InvalidInheritability(HandleInheritability inheritability)
{
Assert.Throws<ArgumentOutOfRangeException>(() =>
{
CreateAndVerifyAnonymousPipe(GetBasicPipeSecurity(), inheritability: inheritability).Dispose();
});
}

[Theory]
[InlineData(int.MinValue)]
[InlineData(-1)]
public void Create_InvalidBufferSize(int bufferSize)
{
Assert.Throws<ArgumentOutOfRangeException>(() =>
{
CreateAndVerifyAnonymousPipe(GetBasicPipeSecurity(), bufferSize: bufferSize).Dispose();
});
}

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 bufferSize in new[] { 0, 1 }
select new object[] { direction, inheritability, bufferSize };

[Theory]
[MemberData(nameof(Create_ValidParameters_MemberData))]
public void Create_ValidParameters(PipeDirection direction, HandleInheritability inheritability, int bufferSize)
{
CreateAndVerifyAnonymousPipe(GetBasicPipeSecurity(), direction, inheritability, bufferSize).Dispose();
}

public static IEnumerable<object[]> Create_CombineRightsAndAccessControl_MemberData() =>
from rights in Enum.GetValues(typeof(PipeAccessRights)).Cast<PipeAccessRights>()
from accessControl in new[] { AccessControlType.Allow, AccessControlType.Deny }
select new object[] { rights, accessControl };

// These tests match NetFX behavior
[Theory]
[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)
{
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
else
{
PipeSecurity security = GetPipeSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl);
Assert.Throws<UnauthorizedAccessException>(() =>
{
AnonymousPipeServerStreamAcl.Create(DefaultPipeDirection, DefaultInheritability, DefaultBufferSize, security).Dispose();
});
}
}

[Theory]
[InlineData(PipeAccessRights.ReadWrite | PipeAccessRights.Synchronize, AccessControlType.Allow)]
public void Create_ValidBitwiseRightsSecurity(PipeAccessRights rights, AccessControlType accessControl)
{
VerifyValidSecurity(rights, accessControl);
}

private void VerifyValidSecurity(PipeAccessRights rights, AccessControlType accessControl)
{
PipeSecurity security = GetPipeSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl);
CreateAndVerifyAnonymousPipe(security).Dispose();
}

private AnonymousPipeServerStream CreateAndVerifyAnonymousPipe(
PipeSecurity expectedSecurity,
PipeDirection direction = DefaultPipeDirection,
HandleInheritability inheritability = DefaultInheritability,
int bufferSize = DefaultBufferSize)
{
AnonymousPipeServerStream pipe = AnonymousPipeServerStreamAcl.Create(direction, inheritability, bufferSize, expectedSecurity);
Assert.NotNull(pipe);

if (expectedSecurity != null)
{
PipeSecurity actualSecurity = pipe.GetAccessControl();
VerifyPipeSecurity(expectedSecurity, actualSecurity);
}

return pipe;
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
using System.Collections.Generic;
using System.Linq;
using System.Security.AccessControl;
using System.Security.Principal;
using Xunit;

namespace System.IO.Pipes.Tests
{
public class PipeServerStreamAclTestBase
{
protected PipeSecurity GetBasicPipeSecurity()
{
return GetPipeSecurity(
WellKnownSidType.BuiltinUsersSid,
PipeAccessRights.FullControl,
AccessControlType.Allow);
}

protected PipeSecurity GetPipeSecurity(WellKnownSidType sid, PipeAccessRights rights, AccessControlType accessControl)
{
var security = new PipeSecurity();
SecurityIdentifier identity = new SecurityIdentifier(sid, null);
var accessRule = new PipeAccessRule(identity, rights, accessControl);
security.AddAccessRule(accessRule);
return security;
}

protected void VerifyPipeSecurity(PipeSecurity expectedSecurity, PipeSecurity actualSecurity)
{
Assert.Equal(typeof(PipeAccessRights), expectedSecurity.AccessRightType);
Assert.Equal(typeof(PipeAccessRights), actualSecurity.AccessRightType);

List<PipeAccessRule> expectedAccessRules = expectedSecurity.GetAccessRules(includeExplicit: true, includeInherited: false, typeof(SecurityIdentifier))
.Cast<PipeAccessRule>().ToList();

List<PipeAccessRule> actualAccessRules = actualSecurity.GetAccessRules(includeExplicit: true, includeInherited: false, typeof(SecurityIdentifier))
.Cast<PipeAccessRule>().ToList();

Assert.Equal(expectedAccessRules.Count, actualAccessRules.Count);
if (expectedAccessRules.Count > 0)
{
Assert.All(expectedAccessRules, actualAccessRule =>
{
int count = expectedAccessRules.Count(expectedAccessRule => AreAccessRulesEqual(expectedAccessRule, actualAccessRule));
Assert.True(count > 0);
});
}
}

protected bool AreAccessRulesEqual(PipeAccessRule expectedRule, PipeAccessRule actualRule)
{
return
expectedRule.AccessControlType == actualRule.AccessControlType &&
expectedRule.PipeAccessRights == actualRule.PipeAccessRights &&
expectedRule.InheritanceFlags == actualRule.InheritanceFlags &&
expectedRule.PropagationFlags == actualRule.PropagationFlags;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
<AssembliesBeingTested Include="System.IO.Pipes" />
</ItemGroup>
<ItemGroup>
<Compile Include="AnonymousPipeTests\AnonymousPipeServerStreamAclTests.cs" />
<Compile Include="AnonymousPipeTests\AnonymousPipeTest.AclExtensions.cs" />
<Compile Include="NamedPipeTests\NamedPipeTest.AclExtensions.cs" />
<Compile Include="PipeServerStreamAclTestBase.cs" />
<Compile Include="PipeTest.AclExtensions.cs" />
<Compile Include="..\..\System.IO.Pipes\tests\PipeTestBase.cs" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# 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.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.
Expand Down
Loading

0 comments on commit a16ee22

Please sign in to comment.