You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by je...@apache.org on 2019/02/28 07:40:57 UTC

[thrift] branch master updated: THRIFT-4813 NamedPipes may not work in all cases Client: netstd Patch: Jens Geyer

This is an automated email from the ASF dual-hosted git repository.

jensg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git


The following commit(s) were added to refs/heads/master by this push:
     new 68ff6cd  THRIFT-4813 NamedPipes may not work in all cases Client: netstd Patch: Jens Geyer
68ff6cd is described below

commit 68ff6cddc055acd9f5611c297c7721a573745678
Author: Jens Geyer <je...@apache.org>
AuthorDate: Tue Feb 26 21:38:04 2019 +0100

    THRIFT-4813 NamedPipes may not work in all cases
    Client: netstd
    Patch: Jens Geyer
---
 .../Thrift.IntegrationTests.csproj                 |  10 +-
 .../Thrift.PublicInterfaces.Compile.Tests.csproj   |   2 +-
 lib/netstd/Tests/Thrift.Tests/Thrift.Tests.csproj  |  10 +-
 lib/netstd/Thrift/Thrift.csproj                    |  19 ++-
 .../Transport/Server/TNamedPipeServerTransport.cs  | 141 ++++++++++++++++++---
 test/netstd/Client/Client.csproj                   |   4 +-
 test/netstd/Server/Server.csproj                   |   6 +-
 tutorial/netstd/Interfaces/Interfaces.csproj       |   2 +-
 tutorial/netstd/Server/Server.csproj               |   8 +-
 9 files changed, 160 insertions(+), 42 deletions(-)

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 @@ namespace Thrift.Transport.Server
             {
                 try
                 {
-                    //TODO: check for disconection 
-                    _stream.Disconnect();
+                    if (_stream.IsConnected)
+                        _stream.Disconnect();
                     _stream.Dispose();
                 }
                 finally
@@ -72,25 +77,34 @@ namespace Thrift.Transport.Server
         {
             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 @@ namespace Thrift.Transport.Server
             }
         }
 
+
+        #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 @@ namespace Thrift.Transport.Server
             }
         }
     }
-}
\ No newline at end of file
+}
diff --git a/test/netstd/Client/Client.csproj b/test/netstd/Client/Client.csproj
index 38e455d..ed30c30 100644
--- a/test/netstd/Client/Client.csproj
+++ b/test/netstd/Client/Client.csproj
@@ -31,9 +31,9 @@
     <GenerateAssemblyCopyrightAttribute>false</GenerateAssemblyCopyrightAttribute>
   </PropertyGroup>
   <ItemGroup>
-    <PackageReference Include="System.Net.Http.WinHttpHandler" Version="[4.4,)" />
+    <PackageReference Include="System.Net.Http.WinHttpHandler" Version="4.5.2" />
     <PackageReference Include="System.Runtime.Serialization.Primitives" Version="[4.3,)" />
-    <PackageReference Include="System.ServiceModel.Primitives" Version="[4.4,)" />
+    <PackageReference Include="System.ServiceModel.Primitives" Version="4.5.3" />
     <PackageReference Include="System.Threading" Version="[4.3,)" />
   </ItemGroup>
   <ItemGroup>
diff --git a/test/netstd/Server/Server.csproj b/test/netstd/Server/Server.csproj
index 60e1968..44f46c9 100644
--- a/test/netstd/Server/Server.csproj
+++ b/test/netstd/Server/Server.csproj
@@ -31,9 +31,11 @@
     <GenerateAssemblyCopyrightAttribute>false</GenerateAssemblyCopyrightAttribute>
   </PropertyGroup>
   <ItemGroup>
-    <PackageReference Include="System.Net.Http.WinHttpHandler" Version="[4.4,)" />
+    <PackageReference Include="System.IO.Pipes" Version="4.3.0" />
+    <PackageReference Include="System.IO.Pipes.AccessControl" Version="4.5.1" />
+    <PackageReference Include="System.Net.Http.WinHttpHandler" Version="4.5.2" />
     <PackageReference Include="System.Runtime.Serialization.Primitives" Version="[4.3,)" />
-    <PackageReference Include="System.ServiceModel.Primitives" Version="[4.4,)" />
+    <PackageReference Include="System.ServiceModel.Primitives" Version="4.5.3" />
     <PackageReference Include="System.Threading" Version="[4.3,)" />
   </ItemGroup>
   <ItemGroup>
diff --git a/tutorial/netstd/Interfaces/Interfaces.csproj b/tutorial/netstd/Interfaces/Interfaces.csproj
index 68d8747..4ebeb4f 100644
--- a/tutorial/netstd/Interfaces/Interfaces.csproj
+++ b/tutorial/netstd/Interfaces/Interfaces.csproj
@@ -33,7 +33,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="System.ServiceModel.Primitives" Version="[4.4,)" />
+    <PackageReference Include="System.ServiceModel.Primitives" Version="4.5.3" />
   </ItemGroup>
 
   <Target Name="PreBuild" BeforeTargets="_GenerateRestoreProjectSpec;Restore;Compile">
diff --git a/tutorial/netstd/Server/Server.csproj b/tutorial/netstd/Server/Server.csproj
index 4b20a3b..fbc2c03 100644
--- a/tutorial/netstd/Server/Server.csproj
+++ b/tutorial/netstd/Server/Server.csproj
@@ -35,10 +35,10 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.AspNetCore" Version="[2.0,)" />
-    <PackageReference Include="Microsoft.AspNetCore.Server.IISIntegration" Version="[2.0,)" />
-    <PackageReference Include="Microsoft.AspNetCore.Server.Kestrel" Version="[2.0,)" />
-    <PackageReference Include="Microsoft.Extensions.Configuration.FileExtensions" Version="[2.0,)" />
+    <PackageReference Include="Microsoft.AspNetCore" Version="2.2.0" />
+    <PackageReference Include="Microsoft.AspNetCore.Server.IISIntegration" Version="2.2.1" />
+    <PackageReference Include="Microsoft.AspNetCore.Server.Kestrel" Version="2.2.0" />
+    <PackageReference Include="Microsoft.Extensions.Configuration.FileExtensions" Version="2.2.0" />
   </ItemGroup>
 
 </Project>