From 66decde8e79ddd547569e9e305719f4130f3283c Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com> Date: Wed, 6 Nov 2019 17:05:31 -0800 Subject: [PATCH] Add Mutex creation extension methods that take an ACL (dotnet/corefx#42281) Approved API Proposal: dotnet/corefx#41662 Description We don't currently have a way to create a Mutex 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 object from the start. Customer impact Before this change, customers had to create a Mutex, then set its ACLs. This presents a few problems: Potential security hole as mutexes can be accessed between creation and modification. Porting difficulties as there isn't a 1-1 API replacement This change addresses those problems by adding a new extension method that allows creating a Mutex and ensuring the provided ACLs are set during creation. Commit migrated from https://github.com/dotnet/corefx/commit/46edc58620f29557e23fea29ed8392a0f2c9e31c --- .../System.Threading.AccessControl.sln | 4 +- .../ref/System.Threading.AccessControl.cs | 5 + .../src/Resources/Strings.resx | 110 ++++++++++++++- .../src/System.Threading.AccessControl.csproj | 20 ++- .../src/System/Threading/MutexAcl.cs | 82 ++++++++++++ .../src/System/Threading/MutexAcl.net46.cs | 16 +++ .../tests/AclTests.cs | 14 ++ .../tests/MutexAclTests.cs | 147 +++++++++++++++++++++ .../System.Threading.AccessControl.Tests.csproj | 4 + 9 files changed, 395 insertions(+), 7 deletions(-) create mode 100644 src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs create mode 100644 src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs create mode 100644 src/libraries/System.Threading.AccessControl/tests/AclTests.cs create mode 100644 src/libraries/System.Threading.AccessControl/tests/MutexAclTests.cs diff --git a/src/libraries/System.Threading.AccessControl/System.Threading.AccessControl.sln b/src/libraries/System.Threading.AccessControl/System.Threading.AccessControl.sln index a199436..bb4c50c 100644 --- a/src/libraries/System.Threading.AccessControl/System.Threading.AccessControl.sln +++ b/src/libraries/System.Threading.AccessControl/System.Threading.AccessControl.sln @@ -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.Threading.AccessControl.Tests", "tests\System.Threading.AccessControl.Tests.csproj", "{458E445C-DF3C-4E4D-8E1D-F2FAC365BB40}" ProjectSection(ProjectDependencies) = postProject diff --git a/src/libraries/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs b/src/libraries/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs index 8d7efcd..e843a29 100644 --- a/src/libraries/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs +++ b/src/libraries/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs @@ -147,4 +147,9 @@ namespace System.Threading public static void SetAccessControl(this System.Threading.Mutex mutex, System.Security.AccessControl.MutexSecurity mutexSecurity) { } public static void SetAccessControl(this System.Threading.Semaphore semaphore, System.Security.AccessControl.SemaphoreSecurity semaphoreSecurity) { } } + + public static class MutexAcl + { + public static System.Threading.Mutex Create(bool initiallyOwned, string name, out bool createdNew, System.Security.AccessControl.MutexSecurity mutexSecurity) { throw null; } + } } diff --git a/src/libraries/System.Threading.AccessControl/src/Resources/Strings.resx b/src/libraries/System.Threading.AccessControl/src/Resources/Strings.resx index 265db70..2e0669e 100644 --- a/src/libraries/System.Threading.AccessControl/src/Resources/Strings.resx +++ b/src/libraries/System.Threading.AccessControl/src/Resources/Strings.resx @@ -1,4 +1,64 @@ - + + + @@ -63,4 +123,52 @@ Access Control List (ACL) APIs are part of resource management on Windows and are not supported on this platform. + + The length of the name exceeds the maximum limit. + + + Enum value was out of legal range. + + + Cannot create '{0}' because a file or directory with the same name already exists. + + + The file '{0}' already exists. + + + Unable to find the specified file. + + + Could not find file '{0}'. + + + Could not find a part of the path. + + + Could not find a part of the path '{0}'. + + + The specified file name or path is too long, or a component of the specified path is too long. + + + The path '{0}' is too long, or a component of the specified path is too long. + + + The process cannot access the file '{0}' because it is being used by another process. + + + The process cannot access the file because it is being used by another process. + + + Access to the path is denied. + + + Access to the path '{0}' is denied. + + + Argument cannot be null or empty. + + + A WaitHandle with system-wide name '{0}' cannot be created. A WaitHandle of a different type might have the same name. + \ No newline at end of file diff --git a/src/libraries/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj b/src/libraries/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj index 1485cdf..f4792d9 100644 --- a/src/libraries/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj +++ b/src/libraries/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj @@ -1,25 +1,37 @@ - + Windows_NT SR.PlatformNotSupported_AccessControl true net461-Windows_NT-Debug;net461-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 + true + + + - - Common\Interop\Windows\Interop.Errors.cs - + + + + + + + + + + + diff --git a/src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs b/src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs new file mode 100644 index 0000000..0d94b09 --- /dev/null +++ b/src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs @@ -0,0 +1,82 @@ +// 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.IO; +using System.Runtime.InteropServices; +using System.Security.AccessControl; +using Microsoft.Win32.SafeHandles; + +namespace System.Threading +{ + public static class MutexAcl + { + /// Gets or creates instance, allowing a to be optionally specified to set it during the mutex creation. + /// to give the calling thread initial ownership of the named system mutex if the named system mutex is created as a result of this call; otherwise, . + /// The optional name of the system mutex. If this argument is set to or , a local mutex is created. + /// When this method returns, this argument is always set to if a local mutex is created; that is, when is or . If has a valid non-empty value, this argument is set to when the system mutex is created, or it is set to if an existing system mutex is found with that name. This parameter is passed uninitialized. + /// The optional mutex access control security to apply. + /// An object that represents a system mutex, if named, or a local mutex, if nameless. + /// .NET Framework only: The length of the name exceeds the maximum limit. + /// A mutex handle with system-wide cannot be created. A mutex handle of a different type might have the same name. + public static unsafe Mutex Create(bool initiallyOwned, string name, out bool createdNew, MutexSecurity mutexSecurity) + { + if (mutexSecurity == null) + { + return new Mutex(initiallyOwned, name, out createdNew); + } + + uint mutexFlags = initiallyOwned ? Interop.Kernel32.CREATE_MUTEX_INITIAL_OWNER : 0; + + fixed (byte* pSecurityDescriptor = mutexSecurity.GetSecurityDescriptorBinaryForm()) + { + var secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES + { + nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES), + lpSecurityDescriptor = (IntPtr)pSecurityDescriptor + }; + + SafeWaitHandle handle = Interop.Kernel32.CreateMutexEx( + (IntPtr)(&secAttrs), + name, + mutexFlags, + (uint)MutexRights.FullControl // Equivalent to MUTEX_ALL_ACCESS + ); + + ValidateMutexHandle(handle, name, out createdNew); + + Mutex mutex = new Mutex(initiallyOwned); + SafeWaitHandle old = mutex.SafeWaitHandle; + mutex.SafeWaitHandle = handle; + old.Dispose(); + + return mutex; + } + } + + private static void ValidateMutexHandle(SafeWaitHandle mutexHandle, string name, out bool createdNew) + { + int errorCode = Marshal.GetLastWin32Error(); + + if (mutexHandle.IsInvalid) + { + mutexHandle.SetHandleAsInvalid(); + + if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) + { + // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8 + throw new ArgumentException(SR.Argument_WaitHandleNameTooLong, nameof(name)); + } + + if (errorCode == Interop.Errors.ERROR_INVALID_HANDLE) + { + throw new WaitHandleCannotBeOpenedException(SR.Format(SR.Threading_WaitHandleCannotBeOpenedException_InvalidHandle, name)); + } + + throw Win32Marshal.GetExceptionForWin32Error(errorCode, name); + } + + createdNew = (errorCode != Interop.Errors.ERROR_ALREADY_EXISTS); + } + } +} diff --git a/src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs b/src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs new file mode 100644 index 0000000..2c1a5ca --- /dev/null +++ b/src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs @@ -0,0 +1,16 @@ +// 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.Threading +{ + public static class MutexAcl + { + public static Mutex Create(bool initiallyOwned, string name, out bool createdNew, MutexSecurity mutexSecurity) + { + return new Mutex(initiallyOwned, name, out createdNew, mutexSecurity); + } + } +} diff --git a/src/libraries/System.Threading.AccessControl/tests/AclTests.cs b/src/libraries/System.Threading.AccessControl/tests/AclTests.cs new file mode 100644 index 0000000..496d8e7 --- /dev/null +++ b/src/libraries/System.Threading.AccessControl/tests/AclTests.cs @@ -0,0 +1,14 @@ +// 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.Threading.Tests +{ + public class AclTests + { + protected string GetRandomName() + { + return Guid.NewGuid().ToString("N"); + } + } +} diff --git a/src/libraries/System.Threading.AccessControl/tests/MutexAclTests.cs b/src/libraries/System.Threading.AccessControl/tests/MutexAclTests.cs new file mode 100644 index 0000000..f51f09e --- /dev/null +++ b/src/libraries/System.Threading.AccessControl/tests/MutexAclTests.cs @@ -0,0 +1,147 @@ +// 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.Collections.Generic; +using System.Linq; +using System.Security.AccessControl; +using System.Security.Principal; +using Xunit; + +namespace System.Threading.Tests +{ + public class MutexAclTests : AclTests + { + [Fact] + public void Mutex_Create_NullSecurity() + { + CreateAndVerifyMutex(initiallyOwned: true, GetRandomName(), expectedSecurity: null, expectedCreatedNew: true).Dispose(); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + public void Mutex_Create_NameMultipleNew(string name) + { + var security = GetBasicMutexSecurity(); + + using Mutex mutex1 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); + using Mutex mutex2 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); + } + + [Fact] + public void Mutex_Create_CreateNewExisting() + { + string name = GetRandomName(); + var security = GetBasicMutexSecurity(); + + using Mutex mutexNew = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); + using Mutex mutexExisting = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: false); + } + + [Fact] + public void Mutex_Create_BeyondMaxPathLength() + { + string name = new string('x', Interop.Kernel32.MAX_PATH + 100); + + if (PlatformDetection.IsFullFramework) + { + Assert.Throws(() => + { + CreateAndVerifyMutex(initiallyOwned: true, name, GetBasicMutexSecurity(), expectedCreatedNew: true).Dispose(); + }); + } + else + { + using Mutex created = CreateAndVerifyMutex(initiallyOwned: true, name, GetBasicMutexSecurity(), expectedCreatedNew: true); + using Mutex openedByName = Mutex.OpenExisting(name); + Assert.NotNull(openedByName); + } + } + + [Theory] + [InlineData(true, MutexRights.FullControl, AccessControlType.Allow)] + [InlineData(true, MutexRights.FullControl, AccessControlType.Deny)] + [InlineData(true, MutexRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, MutexRights.Synchronize, AccessControlType.Deny)] + [InlineData(true, MutexRights.Modify, AccessControlType.Allow)] + [InlineData(true, MutexRights.Modify, AccessControlType.Deny)] + [InlineData(true, MutexRights.Modify | MutexRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, MutexRights.Modify | MutexRights.Synchronize, AccessControlType.Deny)] + [InlineData(false, MutexRights.FullControl, AccessControlType.Allow)] + [InlineData(false, MutexRights.FullControl, AccessControlType.Deny)] + [InlineData(false, MutexRights.Synchronize, AccessControlType.Allow)] + [InlineData(false, MutexRights.Synchronize, AccessControlType.Deny)] + [InlineData(false, MutexRights.Modify, AccessControlType.Allow)] + [InlineData(false, MutexRights.Modify, AccessControlType.Deny)] + public void Mutex_Create_SpecificParameters(bool initiallyOwned, MutexRights rights, AccessControlType accessControl) + { + var security = GetMutexSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); + CreateAndVerifyMutex(initiallyOwned, GetRandomName(), security, expectedCreatedNew: true).Dispose(); + + } + + private MutexSecurity GetBasicMutexSecurity() + { + return GetMutexSecurity( + WellKnownSidType.BuiltinUsersSid, + MutexRights.FullControl, + AccessControlType.Allow); + } + + private MutexSecurity GetMutexSecurity(WellKnownSidType sid, MutexRights rights, AccessControlType accessControl) + { + var security = new MutexSecurity(); + SecurityIdentifier identity = new SecurityIdentifier(sid, null); + var accessRule = new MutexAccessRule(identity, rights, accessControl); + security.AddAccessRule(accessRule); + return security; + } + + private Mutex CreateAndVerifyMutex(bool initiallyOwned, string name, MutexSecurity expectedSecurity, bool expectedCreatedNew) + { + Mutex mutex = MutexAcl.Create(initiallyOwned, name, out bool createdNew, expectedSecurity); + Assert.NotNull(mutex); + Assert.Equal(createdNew, expectedCreatedNew); + + if (expectedSecurity != null) + { + MutexSecurity actualSecurity = mutex.GetAccessControl(); + VerifyMutexSecurity(expectedSecurity, actualSecurity); + } + + return mutex; + } + + private void VerifyMutexSecurity(MutexSecurity expectedSecurity, MutexSecurity actualSecurity) + { + Assert.Equal(typeof(MutexRights), expectedSecurity.AccessRightType); + Assert.Equal(typeof(MutexRights), actualSecurity.AccessRightType); + + List expectedAccessRules = expectedSecurity.GetAccessRules(includeExplicit: true, includeInherited: false, typeof(SecurityIdentifier)) + .Cast().ToList(); + + List actualAccessRules = actualSecurity.GetAccessRules(includeExplicit: true, includeInherited: false, typeof(SecurityIdentifier)) + .Cast().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); + }); + } + } + + private bool AreAccessRulesEqual(MutexAccessRule expectedRule, MutexAccessRule actualRule) + { + return + expectedRule.AccessControlType == actualRule.AccessControlType && + expectedRule.MutexRights == actualRule.MutexRights && + expectedRule.InheritanceFlags == actualRule.InheritanceFlags && + expectedRule.PropagationFlags == actualRule.PropagationFlags; + } + } +} diff --git a/src/libraries/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj b/src/libraries/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj index 62b99a1..8e31f48 100644 --- a/src/libraries/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj +++ b/src/libraries/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj @@ -3,6 +3,10 @@ netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;netfx-Windows_NT-Debug;netfx-Windows_NT-Release + + + + -- 2.7.4