Runtime: SystemNative_GetPeerID and SystemNative_GetEUid are not exposed

Created on 19 Oct 2017 · 14Comments · Source: dotnet/runtime

This is a blocking issue for Roslyn. We need to enforce that both ends of a unix domain socket are the same user (to disallow connecting to a compiler server that is not your own user, spitting junk at it, and potentially executing code as another (higher-privileged) user). These APIs allow us to do that.

area-System.IO question

Source

khyperia

All 14 comments

These are just P/Invokes to a small amount of native code using libc APIs:
https://github.com/dotnet/corefx/blob/c4d0529c24a8abc50009e561e96efed6b3e002b9/src/Native/Unix/System.Native/pal_io.cpp#L1256
https://github.com/dotnet/corefx/blob/a4f243fedb1298294587495d5fdea85dd3a7ab08/src/Native/Unix/System.Native/pal_uid.cpp#L59
If it's really blocking, Roslyn can call such POSIX methods itself. We shouldn't expose these, as they're just implementation details.

If there are missing APIs needed, they should be properly designed. Feel free to put up API proposals for such things.
https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/api-review-process.md

But, this sounds like a solution to a specific problem, e.g. Roslyn wants more security features on NamedPipe{Server/Client}Stream, and because such things don't exist, it's dropping down to using a Unix-specific mechanism in Unix Domain Sockets and is then looking for various libc APIs related to those to be exposed. Sounds like it'd be better for features to be proposed at the higher-level.

stephentoub on 19 Oct 2017

👍1

NamedPipe{Server/Client}Stream

The implementation of this type on Mac/Linux does not fit our functional needs. It does not open a persistent named pipe waiting for connections upon creation, but rather creates a new pipe for every single call to WaitForConnection, and then destroys it immediately. That causes a gap in availability as the caller is processing the new connection, something that is unacceptable.

If there are missing APIs needed, they should be properly designed.

Yes, that is what I was suggesting - I will look into doing so. For now, Roslyn is manually copying the pinvoke definition to those methods, and pinvoking into System.Native. (SystemNative_GetPeerID has a nontrivial native implementation that is keyed off of #defines at compile-time of the runtime, so it would be impractical to re-implement)

khyperia on 19 Oct 2017

For now, Roslyn is manually copying the pinvoke definition to those methods, and pinvoking into System.Native.

System.Native.so/dylib is private implementation detail. P/Invoking into them is akin to using private reflection to access the internals of some managed library you depend on. While I don't at this moment know of a plan to change them, they could be changed at any point in the future and it would not be considered a breaking change.

The implementation of this type on Mac/Linux does not fit our functional needs.

Is there a way it can be made to fit your functional needs? Is there a bug you can fix? Is there a feature that can be implemented? Worst case are there new APIs that could be added to them? Let's start from the position of assuming there's a bug that can be fixed, and if that's not true figure out what features could be added to the types to help them to meet your functional needs, rather than jumping right in to asking for specific libc-method wrappers to be exposed.

stephentoub on 19 Oct 2017

Note as well that we / the Mono folks released a NuGet package containing libc wrappers. You could also use that. It works on .NET Core.
https://www.nuget.org/packages/Mono.Posix.NETStandard/1.0.0-beta2
cc: @eerhardt

stephentoub on 19 Oct 2017

If it's really blocking, Roslyn can call such POSIX methods itself. We shouldn't expose these, as they're just implementation details.

True but Windows also offers up similar implementation details in named pipes via the RunAsClient API. Getting the peerid is the morale equivalent for non-windows platforms. Not having this means there is functionality difference between the platforms. That is blocking us from adopting them. We considered using UnixDomainSockets directly but they are not exposed as an API. Hence we are pretty firmly blocked here.

Is there a way it can be made to fit your functional needs? Is there a bug you can fix?

@khyperia described the problem above

The implementation of this type on Mac/Linux does not fit our functional needs. It does not open a persistent named pipe waiting for connections upon creation, but rather creates a new pipe for every single call to WaitForConnection, and then destroys it immediately. That causes a gap in availability as the caller is processing the new connection, something that is unacceptable.

jaredpar on 19 Oct 2017

True but Windows also offers up similar implementation details in named pipes via the RunAsClient API.

I'm confused; RunAsClient is exposed and implemented on Unix. It's sufficient on Windows but not on Unix? Is there a bug there that needs to be fixed?
https://github.com/dotnet/corefx/blob/7f5b838709fb4726b6227922cfaca1ea6d99d779/src/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs#L205-L238

@khyperia described the problem above

I know, and I asked whether there's something that can be done in the implementation to address that, and if not, what APIs would need to be added to address it.

stephentoub on 19 Oct 2017

Also, NamedPipeServerStream exposes GetImpersonationUserName, which just uses SystemNative_GetPeerId to get the client ID and from that the associated user name:
https://github.com/dotnet/corefx/blob/7f5b838709fb4726b6227922cfaca1ea6d99d779/src/Native/Unix/System.Native/pal_networking.cpp#L2712-L2718
Can GetImpersonationUserName be used to achieve a similar goal? e.g.
C# if (serverStream.GetImpersonationUserName() != Environment.UserName) { Console.WriteLine("go away!"); serverStream.Dispose(); }

stephentoub on 19 Oct 2017

I'm confused; RunAsClient is exposed and implemented on Unix. It's sufficient on Windows but not on Unix?

When did this API become available? We'd never considered it before because it's not in the version of named pipes we are using (netstandard 1.3).

Based on the implementation it should suit our needs though. Assuming it's in a version of named pipes we can depend on.

I know, and I asked whether there's something that can be done in the implementation to address that, and if not, what APIs would need to be added to address it.

Not sure an API can help us here. It's pretty core to how the type is implemented presently. We can file a separate but to track that problem.

jaredpar on 19 Oct 2017

When did this API become available?

2.0
https://github.com/dotnet/corefx/pull/12769

stephentoub on 19 Oct 2017

Anyway, my uber point is: let's try to use what's available, and if it doesn't work because of a bug or some other implementation detail that we can fix, let's fix it, and if that's insufficient, let's look at what APIs could be added to address needs. If it comes down to new APIs, and if you're actually using Socket and unix domain sockets, I'd prefer to look and see what APIs would make sense in System.Net.Sockets. It may very well be that we'd want some kind of "connected ID" on Socket, along with some kind of Environment.CurrentUserId, or whatever, though it would need to be designed to make sense across all platforms. And if it comes down to you really just needing to call to libc APIs, that's what the https://www.nuget.org/packages/Mono.Posix.NETStandard/1.0.0-beta2 lib is for.

stephentoub on 19 Oct 2017

👍1

@khyperia Ashley Hauck FTE described the problem above

I know, and I asked whether there's something that can be done in the implementation to address that.

This is what I meant by addressing the problem in the implementation:
https://github.com/dotnet/corefx/pull/24798
As far as I can tell, the issue isn't as described, in that the current implementation looks to work fine when dealing with a single client at a time; the client will simply find that there isn't a server yet and will wait. The problem I see is that if multiple clients try to connect, they will do so successfully when the server waits for connections, and then all but one will have their connections fail when the server closes the listening socket after accepting the one. That's what the PR above addresses. If there's another problem, please open a separate issue for it.

stephentoub on 22 Oct 2017

Fantastic, that PR looks great!

Just some nitpicking since I want to make sure we're on the same page:

the client will simply find that there isn't a server yet and will wait

In my testing, this isn't the case. I made a little example repo to demonstrate this - the client fails immediately when trying to connect to a domain socket that doesn't exist. Note, however, this is _desired_ behavior in Roslyn - explanation below - so I want to make sure that behavior is reproduced in NamedPipeClientStream.

Example:

The areas surrounding compiler server startup/shutdown are inherently extremely race-y. We generally implement a behavior of "make a reasonable effort to use the compiler server, but it's not the end of the world if we have to fall back to a local compilation (i.e. normal csc)". Consider the following: We get a request by MSBuild to run a compilation. We check if the server is running: cool, it is! We try to connect to it: oh no, oops, the server shut down in the time it took for us to connect! The connection fails immediately, and we catch the error, and say "okay, fine, we'll just use local csc".

If the connection didn't fail immediately, we would be left hanging either forever, or until a timeout happened, or whatever. That isn't good behavior!

khyperia on 23 Oct 2017

In my testing, this isn't the case.

You're testing Unix Domain Sockets directly, not NamedPipeClientStream, which explicitly deals with this case. Do you have a repro of this using NamedPipeClientStream?