THRIFT-4813 NamedPipes may not work in all cases
Client: netstd
Patch: Jens Geyer
diff --git a/lib/netstd/Tests/Thrift.IntegrationTests/Thrift.IntegrationTests.csproj b/lib/netstd/Tests/Thrift.IntegrationTests/Thrift.IntegrationTests.csproj
index 4a235ed..381d8aa 100644
--- a/lib/netstd/Tests/Thrift.IntegrationTests/Thrift.IntegrationTests.csproj
+++ b/lib/netstd/Tests/Thrift.IntegrationTests/Thrift.IntegrationTests.csproj
@@ -32,11 +32,11 @@
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="CompareNETObjects" Version="4.3.0" />
- <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.5.0" />
- <PackageReference Include="MSTest.TestAdapter" Version="1.2.0" />
- <PackageReference Include="MSTest.TestFramework" Version="1.2.0" />
- <PackageReference Include="System.ServiceModel.Primitives" Version="4.4.0" />
+ <PackageReference Include="CompareNETObjects" Version="4.58.0" />
+ <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.9.0" />
+ <PackageReference Include="MSTest.TestAdapter" Version="1.4.0" />
+ <PackageReference Include="MSTest.TestFramework" Version="1.4.0" />
+ <PackageReference Include="System.ServiceModel.Primitives" Version="4.5.3" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\..\Thrift\Thrift.csproj" />
diff --git a/lib/netstd/Tests/Thrift.PublicInterfaces.Compile.Tests/Thrift.PublicInterfaces.Compile.Tests.csproj b/lib/netstd/Tests/Thrift.PublicInterfaces.Compile.Tests/Thrift.PublicInterfaces.Compile.Tests.csproj
index 6687a9d..58f61a2 100644
--- a/lib/netstd/Tests/Thrift.PublicInterfaces.Compile.Tests/Thrift.PublicInterfaces.Compile.Tests.csproj
+++ b/lib/netstd/Tests/Thrift.PublicInterfaces.Compile.Tests/Thrift.PublicInterfaces.Compile.Tests.csproj
@@ -33,7 +33,7 @@
</ItemGroup>
<ItemGroup>
- <PackageReference Include="System.ServiceModel.Primitives" Version="[4.1.0,)" />
+ <PackageReference Include="System.ServiceModel.Primitives" Version="4.5.3" />
</ItemGroup>
<Target Name="PreBuild" BeforeTargets="_GenerateRestoreProjectSpec;Restore;Compile">
diff --git a/lib/netstd/Tests/Thrift.Tests/Thrift.Tests.csproj b/lib/netstd/Tests/Thrift.Tests/Thrift.Tests.csproj
index 3150910..434424d 100644
--- a/lib/netstd/Tests/Thrift.Tests/Thrift.Tests.csproj
+++ b/lib/netstd/Tests/Thrift.Tests/Thrift.Tests.csproj
@@ -21,11 +21,11 @@
<TargetFramework>netcoreapp2.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="CompareNETObjects" Version="4.3.0" />
- <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.5.0" />
- <PackageReference Include="MSTest.TestAdapter" Version="1.2.0" />
- <PackageReference Include="MSTest.TestFramework" Version="1.2.0" />
- <PackageReference Include="NSubstitute" Version="3.1.0" />
+ <PackageReference Include="CompareNETObjects" Version="4.58.0" />
+ <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.9.0" />
+ <PackageReference Include="MSTest.TestAdapter" Version="1.4.0" />
+ <PackageReference Include="MSTest.TestFramework" Version="1.4.0" />
+ <PackageReference Include="NSubstitute" Version="4.0.0" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\..\Thrift\Thrift.csproj" />
diff --git a/lib/netstd/Thrift/Thrift.csproj b/lib/netstd/Thrift/Thrift.csproj
index e2f1725..86de1f8 100644
--- a/lib/netstd/Thrift/Thrift.csproj
+++ b/lib/netstd/Thrift/Thrift.csproj
@@ -34,17 +34,22 @@
<GenerateAssemblyFileVersionAttribute>false</GenerateAssemblyFileVersionAttribute>
</PropertyGroup>
+ <PropertyGroup>
+ <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+ </PropertyGroup>
+
<ItemGroup>
- <PackageReference Include="Microsoft.AspNetCore" Version="[2.0,)" />
- <PackageReference Include="Microsoft.AspNetCore.Http.Abstractions" Version="[2.0,)" />
- <PackageReference Include="Microsoft.Extensions.Logging" Version="[2.0,)" />
- <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="[2.0,)" />
- <PackageReference Include="Microsoft.Extensions.Logging.Debug" Version="[2.0,)" />
+ <PackageReference Include="Microsoft.AspNetCore" Version="2.2.0" />
+ <PackageReference Include="Microsoft.AspNetCore.Http.Abstractions" Version="2.2.0" />
+ <PackageReference Include="Microsoft.Extensions.Logging" Version="2.2.0" />
+ <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="2.2.0" />
+ <PackageReference Include="Microsoft.Extensions.Logging.Debug" Version="2.2.0" />
<PackageReference Include="System.IO.Pipes" Version="[4.3,)" />
- <PackageReference Include="System.Net.Http.WinHttpHandler" Version="[4.4,)" />
+ <PackageReference Include="System.IO.Pipes.AccessControl" Version="4.5.1" />
+ <PackageReference Include="System.Net.Http.WinHttpHandler" Version="4.5.2" />
<PackageReference Include="System.Net.NameResolution" Version="[4.3,)" />
<PackageReference Include="System.Net.Requests" Version="[4.3,)" />
- <PackageReference Include="System.Net.Security" Version="[4.3,)" />
+ <PackageReference Include="System.Net.Security" Version="4.3.2" />
</ItemGroup>
</Project>
diff --git a/lib/netstd/Thrift/Transport/Server/TNamedPipeServerTransport.cs b/lib/netstd/Thrift/Transport/Server/TNamedPipeServerTransport.cs
index be6d0fc..cd5683b 100644
--- a/lib/netstd/Thrift/Transport/Server/TNamedPipeServerTransport.cs
+++ b/lib/netstd/Thrift/Transport/Server/TNamedPipeServerTransport.cs
@@ -1,13 +1,13 @@
-// Licensed to the Apache Software Foundation(ASF) under one
+// Licensed to the Apache Software Foundation(ASF) under one
// or more contributor license agreements.See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership.The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
-//
+//
// http://www.apache.org/licenses/LICENSE-2.0
-//
+//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -15,10 +15,15 @@
// specific language governing permissions and limitations
// under the License.
+using Microsoft.Win32.SafeHandles;
using System;
using System.IO.Pipes;
+using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;
+using System.ComponentModel;
+using System.Security.AccessControl;
+using System.Security.Principal;
namespace Thrift.Transport.Server
{
@@ -51,8 +56,8 @@
{
try
{
- //TODO: check for disconection
- _stream.Disconnect();
+ if (_stream.IsConnected)
+ _stream.Disconnect();
_stream.Dispose();
}
finally
@@ -72,25 +77,34 @@
{
if (_stream == null)
{
- var direction = PipeDirection.InOut;
- var maxconn = 254;
- var mode = PipeTransmissionMode.Byte;
+ const PipeDirection direction = PipeDirection.InOut;
+ const int maxconn = NamedPipeServerStream.MaxAllowedServerInstances;
+ const PipeTransmissionMode mode = PipeTransmissionMode.Byte;
+ const int inbuf = 4096;
+ const int outbuf = 4096;
var options = _asyncMode ? PipeOptions.Asynchronous : PipeOptions.None;
- var inbuf = 4096;
- var outbuf = 4096;
- // TODO: security
+
+
+ // TODO: "CreatePipeNative" ist only a workaround, and there are have basically two possible outcomes:
+ // - once NamedPipeServerStream() gets a CTOR that supports pipesec, remove CreatePipeNative()
+ // - if 31190 gets resolved before, use _stream.SetAccessControl(pipesec) instead of CreatePipeNative()
+ // EITHER WAY,
+ // - if CreatePipeNative() finally gets removed, also remove "allow unsafe code" from the project settings
try
{
- _stream = new NamedPipeServerStream(_pipeAddress, direction, maxconn, mode, options, inbuf, outbuf);
+ var handle = CreatePipeNative(_pipeAddress, inbuf, outbuf);
+ if( (handle != null) && (!handle.IsInvalid))
+ _stream = new NamedPipeServerStream(PipeDirection.InOut, _asyncMode, false, handle);
+ else
+ _stream = new NamedPipeServerStream(_pipeAddress, direction, maxconn, mode, options, inbuf, outbuf/*, pipesec*/);
}
catch (NotImplementedException) // Mono still does not support async, fallback to sync
{
if (_asyncMode)
{
options &= (~PipeOptions.Asynchronous);
- _stream = new NamedPipeServerStream(_pipeAddress, direction, maxconn, mode, options, inbuf,
- outbuf);
+ _stream = new NamedPipeServerStream(_pipeAddress, direction, maxconn, mode, options, inbuf, outbuf);
_asyncMode = false;
}
else
@@ -101,6 +115,103 @@
}
}
+
+ #region CreatePipeNative workaround
+
+
+ [StructLayout(LayoutKind.Sequential)]
+ internal class SECURITY_ATTRIBUTES
+ {
+ internal int nLength = 0;
+ internal IntPtr lpSecurityDescriptor = IntPtr.Zero;
+ internal int bInheritHandle = 0;
+ }
+
+
+ private const string Kernel32 = "kernel32.dll";
+
+ [DllImport(Kernel32, SetLastError = true)]
+ internal static extern IntPtr CreateNamedPipe(
+ string lpName, uint dwOpenMode, uint dwPipeMode,
+ uint nMaxInstances, uint nOutBufferSize, uint nInBufferSize, uint nDefaultTimeOut,
+ SECURITY_ATTRIBUTES pipeSecurityDescriptor
+ );
+
+
+
+ // Workaround: create the pipe via API call
+ // we have to do it this way, since NamedPipeServerStream() for netstd still lacks a few CTORs
+ // and _stream.SetAccessControl(pipesec); only keeps throwing ACCESS_DENIED errors at us
+ // References:
+ // - https://github.com/dotnet/corefx/issues/30170 (closed, continued in 31190)
+ // - https://github.com/dotnet/corefx/issues/31190 System.IO.Pipes.AccessControl package does not work
+ // - https://github.com/dotnet/corefx/issues/24040 NamedPipeServerStream: Provide support for WRITE_DAC
+ // - https://github.com/dotnet/corefx/issues/34400 Have a mechanism for lower privileged user to connect to a privileged user's pipe
+ private SafePipeHandle CreatePipeNative(string name, int inbuf, int outbuf)
+ {
+ if (Environment.OSVersion.Platform != PlatformID.Win32NT)
+ return null; // Windows only
+
+ var pinningHandle = new GCHandle();
+ try
+ {
+ // owner gets full access, everyone else read/write
+ var pipesec = new PipeSecurity();
+ using (var currentIdentity = WindowsIdentity.GetCurrent())
+ {
+ var sidOwner = currentIdentity.Owner;
+ var sidWorld = new SecurityIdentifier(WellKnownSidType.WorldSid, null);
+
+ pipesec.SetOwner(sidOwner);
+ pipesec.AddAccessRule(new PipeAccessRule(sidOwner, PipeAccessRights.FullControl, AccessControlType.Allow));
+ pipesec.AddAccessRule(new PipeAccessRule(sidWorld, PipeAccessRights.ReadWrite, AccessControlType.Allow));
+ }
+
+ // create a security descriptor and assign it to the security attribs
+ var secAttrs = new SECURITY_ATTRIBUTES();
+ byte[] sdBytes = pipesec.GetSecurityDescriptorBinaryForm();
+ pinningHandle = GCHandle.Alloc(sdBytes, GCHandleType.Pinned);
+ unsafe {
+ fixed (byte* pSD = sdBytes) {
+ secAttrs.lpSecurityDescriptor = (IntPtr)pSD;
+ }
+ }
+
+ // a bunch of constants we will need shortly
+ const int PIPE_ACCESS_DUPLEX = 0x00000003;
+ const int FILE_FLAG_OVERLAPPED = 0x40000000;
+ const int WRITE_DAC = 0x00040000;
+ const int PIPE_TYPE_BYTE = 0x00000000;
+ const int PIPE_READMODE_BYTE = 0x00000000;
+ const int PIPE_UNLIMITED_INSTANCES = 255;
+
+ // create the pipe via API call
+ var rawHandle = CreateNamedPipe(
+ @"\\.\pipe\" + name,
+ PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED | WRITE_DAC,
+ PIPE_TYPE_BYTE | PIPE_READMODE_BYTE,
+ PIPE_UNLIMITED_INSTANCES, (uint)inbuf, (uint)outbuf,
+ 5 * 1000,
+ secAttrs
+ );
+
+ // make a SafePipeHandle() from it
+ var handle = new SafePipeHandle(rawHandle, true);
+ if (handle.IsInvalid)
+ throw new Win32Exception(Marshal.GetLastWin32Error());
+
+ // return it (to be packaged)
+ return handle;
+ }
+ finally
+ {
+ if (pinningHandle.IsAllocated)
+ pinningHandle.Free();
+ }
+ }
+
+ #endregion
+
protected override async Task<TTransport> AcceptImplementationAsync(CancellationToken cancellationToken)
{
try
@@ -188,4 +299,4 @@
}
}
}
-}
\ No newline at end of file
+}