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 ef0cb01 THRIFT-5391 Named pipes transport hardening Client: netstd
Patch: Jens Geyer
ef0cb01 is described below
commit ef0cb01abed2558a2a15ed6675e9156d765ff81e
Author: Jens Geyer <[email protected]>
AuthorDate: Fri Apr 2 12:18:15 2021 +0200
THRIFT-5391 Named pipes transport hardening
Client: netstd
Patch: Jens Geyer
This closes #2367
---
.../Thrift/Transport/Client/TNamedPipeTransport.cs | 3 +-
.../Transport/Server/TNamedPipeServerTransport.cs | 43 +++++++++++++++++-----
test/netstd/Server/TestServer.cs | 7 +++-
3 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/lib/netstd/Thrift/Transport/Client/TNamedPipeTransport.cs
b/lib/netstd/Thrift/Transport/Client/TNamedPipeTransport.cs
index 47d1ccb..e6c79c4 100644
--- a/lib/netstd/Thrift/Transport/Client/TNamedPipeTransport.cs
+++ b/lib/netstd/Thrift/Transport/Client/TNamedPipeTransport.cs
@@ -17,6 +17,7 @@
using System;
using System.IO.Pipes;
+using System.Security.Principal;
using System.Threading;
using System.Threading.Tasks;
@@ -39,7 +40,7 @@ namespace Thrift.Transport.Client
var serverName = string.IsNullOrWhiteSpace(server) ? server : ".";
ConnectTimeout = (timeout > 0) ? timeout : Timeout.Infinite;
- PipeStream = new NamedPipeClientStream(serverName, pipe,
PipeDirection.InOut, PipeOptions.None);
+ PipeStream = new NamedPipeClientStream(serverName, pipe,
PipeDirection.InOut, PipeOptions.None, TokenImpersonationLevel.Anonymous);
}
public override bool IsOpen => PipeStream != null &&
PipeStream.IsConnected;
diff --git a/lib/netstd/Thrift/Transport/Server/TNamedPipeServerTransport.cs
b/lib/netstd/Thrift/Transport/Server/TNamedPipeServerTransport.cs
index 67ba29f..307b7f8 100644
--- a/lib/netstd/Thrift/Transport/Server/TNamedPipeServerTransport.cs
+++ b/lib/netstd/Thrift/Transport/Server/TNamedPipeServerTransport.cs
@@ -27,6 +27,12 @@ using System.Security.Principal;
namespace Thrift.Transport.Server
{
+ [Flags]
+ public enum NamedPipeClientFlags {
+ None = 0x00,
+ OnlyLocalClients = 0x01
+ };
+
// ReSharper disable once InconsistentNaming
public class TNamedPipeServerTransport : TServerTransport
{
@@ -37,11 +43,21 @@ namespace Thrift.Transport.Server
private bool _asyncMode = true;
private volatile bool _isPending = true;
private NamedPipeServerStream _stream = null;
+ private readonly bool _onlyLocalClients = false; // compatibility
default
+
+ public TNamedPipeServerTransport(string pipeAddress, TConfiguration
config, NamedPipeClientFlags flags)
+ : base(config)
+ {
+ _pipeAddress = pipeAddress;
+ _onlyLocalClients =
flags.HasFlag(NamedPipeClientFlags.OnlyLocalClients);
+ }
+ [Obsolete("This CTOR is deprecated, please use the other one
instead.")]
public TNamedPipeServerTransport(string pipeAddress, TConfiguration
config)
: base(config)
{
_pipeAddress = pipeAddress;
+ _onlyLocalClients = false;
}
public override bool IsOpen() {
@@ -96,7 +112,7 @@ namespace Thrift.Transport.Server
try
{
- var handle = CreatePipeNative(_pipeAddress, inbuf, outbuf);
+ var handle = CreatePipeNative(_pipeAddress, inbuf, outbuf,
_onlyLocalClients);
if ((handle != null) && (!handle.IsInvalid))
{
_stream = new
NamedPipeServerStream(PipeDirection.InOut, _asyncMode, false, handle);
@@ -149,14 +165,14 @@ namespace Thrift.Transport.Server
// Workaround: create the pipe via API call
- // we have to do it this way, since NamedPipeServerStream() for netstd
still lacks a few CTORs
+ // we have to do it this way, since NamedPipeServerStream() for netstd
still lacks a few CTORs and/or arguments
// 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 static SafePipeHandle CreatePipeNative(string name, int inbuf,
int outbuf)
+ private static SafePipeHandle CreatePipeNative(string name, int inbuf,
int outbuf, bool OnlyLocalClients)
{
if (! RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
return null; // Windows only
@@ -187,18 +203,25 @@ namespace Thrift.Transport.Server
}
// 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;
+ const uint PIPE_ACCESS_DUPLEX = 0x00000003;
+ const uint FILE_FLAG_OVERLAPPED = 0x40000000;
+ const uint WRITE_DAC = 0x00040000;
+ const uint PIPE_TYPE_BYTE = 0x00000000;
+ const uint PIPE_READMODE_BYTE = 0x00000000;
+ const uint PIPE_UNLIMITED_INSTANCES = 255;
+ const uint PIPE_ACCEPT_REMOTE_CLIENTS = 0x00000000; //
Connections from remote clients can be accepted and checked against the
security descriptor for the pipe.
+ const uint PIPE_REJECT_REMOTE_CLIENTS = 0x00000008; //
Connections from remote clients are automatically rejected.
+
+ // any extra flags we want to add
+ uint dwPipeModeXtra
+ = (OnlyLocalClients ? PIPE_REJECT_REMOTE_CLIENTS :
PIPE_ACCEPT_REMOTE_CLIENTS)
+ ;
// 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_TYPE_BYTE | PIPE_READMODE_BYTE | dwPipeModeXtra,
PIPE_UNLIMITED_INSTANCES, (uint)inbuf, (uint)outbuf,
5 * 1000,
secAttrs
diff --git a/test/netstd/Server/TestServer.cs b/test/netstd/Server/TestServer.cs
index 5c99099..bec9fae 100644
--- a/test/netstd/Server/TestServer.cs
+++ b/test/netstd/Server/TestServer.cs
@@ -149,7 +149,10 @@ namespace ThriftTest
public class TestServer
{
- public static int _clientID = -1;
+ #pragma warning disable CA2211
+ public static int _clientID = -1; // use with Interlocked only!
+ #pragma warning restore CA2211
+
private static readonly TConfiguration Configuration = null; // or
new TConfiguration() if needed
public delegate void TestLogDelegate(string msg, params object[]
values);
@@ -556,7 +559,7 @@ namespace ThriftTest
{
case TransportChoice.NamedPipe:
Debug.Assert(param.pipe != null);
- trans = new TNamedPipeServerTransport(param.pipe,
Configuration);
+ trans = new TNamedPipeServerTransport(param.pipe,
Configuration, NamedPipeClientFlags.OnlyLocalClients);
break;
