From 5c7137c54512d7298752203cfd3986c9975a2b9c Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Mon, 26 Feb 2018 16:52:17 -0800 Subject: [PATCH] NamedPipe: CurrentUserOnly, quick fixes for Unix (#27463) * NamedPipe: CurrentUserOnly, quick fixes for Unix * The path for the directory of when using current user only was wrong and not using the intended folder. * Added getpeerid validation on the server side. * Clean some dirty changes used for quick validation. * PR feedback round #1 --- .../src/Resources/Strings.resx | 3 +++ .../IO/Pipes/NamedPipeServerStream.Unix.cs | 17 ++++++++++++++ .../src/System/IO/Pipes/PipeStream.Unix.cs | 2 +- ...amedPipeTest.CurrentUserOnly.netcoreapp.cs | 22 +++++++++++++++---- 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/System.IO.Pipes/src/Resources/Strings.resx b/src/System.IO.Pipes/src/Resources/Strings.resx index e35951ae3d6c..4316ce959f75 100644 --- a/src/System.IO.Pipes/src/Resources/Strings.resx +++ b/src/System.IO.Pipes/src/Resources/Strings.resx @@ -285,4 +285,7 @@ Could not connect to the pipe because it was not owned by the current user. + + Client connection (user id {0}) was refused because it was not owned by the current user (id {1}). + \ No newline at end of file diff --git a/src/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs b/src/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs index d8430991c449..25c8d8a5d684 100644 --- a/src/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs +++ b/src/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs @@ -83,8 +83,25 @@ async Task WaitForConnectionAsyncCore() => private void HandleAcceptedSocket(Socket acceptedSocket) { var serverHandle = new SafePipeHandle(acceptedSocket); + try { + if (IsCurrentUserOnly) + { + uint serverEUID = Interop.Sys.GetEUid(); + + uint peerID; + if (Interop.Sys.GetPeerID(serverHandle, out peerID) == -1) + { + throw CreateExceptionForLastError(_instance?.PipeName); + } + + if (serverEUID != peerID) + { + throw new UnauthorizedAccessException(string.Format(SR.UnauthorizedAccess_ClientIsNotCurrentUser, peerID, serverEUID)); + } + } + ConfigureSocket(acceptedSocket, serverHandle, _direction, _inBufferSize, _outBufferSize, _inheritability); } catch diff --git a/src/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs b/src/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs index d22aea13c946..786d3a0bf60e 100644 --- a/src/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs +++ b/src/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs @@ -68,7 +68,7 @@ internal static string GetPipePath(string serverName, string pipeName, bool isCu throw CreateExceptionForLastError(); } - return Path.Combine(directory, s_pipePrefix + pipeName); + return Path.Combine(directory, pipeName); } return s_pipePrefix + pipeName; diff --git a/src/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.netcoreapp.cs b/src/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.netcoreapp.cs index 97cf82e2b0c7..f02976511e4b 100644 --- a/src/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.netcoreapp.cs +++ b/src/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.netcoreapp.cs @@ -30,7 +30,7 @@ public static void CreateServer_CurrentUserOnly() [Fact] public static void CreateServer_ConnectClient() { - var name = GetUniquePipeName(); + string name = GetUniquePipeName(); using (var server = new NamedPipeServerStream(name, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.CurrentUserOnly)) { using (var client = new NamedPipeClientStream(".", name, PipeDirection.InOut, PipeOptions.CurrentUserOnly)) @@ -41,12 +41,26 @@ public static void CreateServer_ConnectClient() } } + [Fact] + [PlatformSpecific(TestPlatforms.AnyUnix)] // On Unix domain socket should have different location in this case. + public static void CreateServerNotCurrentUserOnly_ClientCurrentUserOnly_ThrowsTimeout_OnUnix() + { + string name = GetUniquePipeName(); + using (var server = new NamedPipeServerStream(name, PipeDirection.InOut, 1, PipeTransmissionMode.Byte)) + { + using (var client = new NamedPipeClientStream(".", name, PipeDirection.InOut, PipeOptions.CurrentUserOnly)) + { + Assert.Throws(() => client.Connect(1)); + } + } + } + [Fact] public static void CreateMultipleServers_ConnectMultipleClients() { - var name1 = GetUniquePipeName(); - var name2 = GetUniquePipeName(); - var name3 = GetUniquePipeName(); + string name1 = GetUniquePipeName(); + string name2 = GetUniquePipeName(); + string name3 = GetUniquePipeName(); using (var server1 = new NamedPipeServerStream(name1, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.CurrentUserOnly)) using (var server2 = new NamedPipeServerStream(name2, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.CurrentUserOnly)) using (var server3 = new NamedPipeServerStream(name3, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.CurrentUserOnly))