“Attempted to perform an unauthorized operation” when calling NamedPipeServerStream.SetAccessControl

[TylerLeonhardt]

I'm having some trouble securing a NamedPipeServerStream from a .NET Standard lib. I opened a question on stack overflow here.

The gist is that when I try to call SetAccessControl using either of the listed ways, I get an "Attempted to perform an unauthorized operation":

• PipesAclExtensions.SetAccessControl(pipeServer, pipeSecurity);
• pipeServer.SetAccessControl(pipeSecurity);

Here's a gist of the problem.

PipeSecurity pipeSecurity = new PipeSecurity();

WindowsIdentity identity = WindowsIdentity.GetCurrent();
WindowsPrincipal principal = new WindowsPrincipal(identity);

if (principal.IsInRole(WindowsBuiltInRole.Administrator))
{
    // Allow the Administrators group full access to the pipe.
    pipeSecurity.AddAccessRule(new PipeAccessRule(
        new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null).Translate(typeof(NTAccount)),
        PipeAccessRights.FullControl, AccessControlType.Allow));
} else {
    // Allow AuthenticatedUser read and write access to the pipe.
    pipeSecurity.AddAccessRule(new PipeAccessRule(
        WindowsIdentity.GetCurrent().User,
        PipeAccessRights.ReadWrite, AccessControlType.Allow));
}

var pipeServer =
    new NamedPipeServerStream(
        "mypipe",
        PipeDirection.InOut,
        1,
        PipeTransmissionMode.Byte,
        PipeOptions.Asynchronous);

// Both of these throw
PipesAclExtensions.SetAccessControl(pipeServer, pipeSecurity);
// or
pipeServer.SetAccessControl(pipeSecurity);

The exception I'm getting is:

Attempted to perform an unauthorized operation.
at System.Security.AccessControl.Win32.SetSecurityInfo(ResourceType type, String name, SafeHandle handle, SecurityInfos securityInformation, SecurityIdentifier owner, SecurityIdentifier group, GenericAcl sacl, GenericAcl dacl)
at System.Security.AccessControl.NativeObjectSecurity.Persist(String name, SafeHandle handle, AccessControlSections includeSections, Object exceptionContext)
at System.Security.AccessControl.NativeObjectSecurity.Persist(SafeHandle handle, AccessControlSections includeSections, Object exceptionContext)
at System.IO.Pipes.PipeSecurity.Persist(SafeHandle handle)

NOTE: I am pulling in the System.IO.Pipes.AccessControl nuget package

Am I doing something wrong?

[TylerLeonhardt]

I noticed that the tests for the extension methods all use the empty PipeSecurity() constructor so I tried using an empty one in my code to see if this could be a bug that was overlooked since the tests never added an access rule.

Sure enough, if I pass in an empty PipeSecurity into SetAccessControl, my code works:

PipesAclExtensions.SetAccessControl(pipeServer, new PipeSecurity());

From what I know about Named Pipes, when you create them, you can pass in a Security object that will create all the ACLs you want but you can't update them once created. This means that SetAccessControl probably won't work for NamedPipes.

In the .NET Framework, there was a ctor for NamePipeServerStream that allowed you to pass in a PipeSecurity but that doesn't seem to exist in .NET Core (probably because everything is split up now).

I'm really hoping there's some sort of solution for this out in the wild as I need it in .NET Core 2.0 - otherwise I'll have to go down the pinvoke route to create a Pipe that way.

[feco93]

A think the main reason is that the ACLs feature currently not implemented in UNIX variant NamedPipeServer implementation.

This SO post also about this problem:
https://stackoverflow.com/questions/50500488/securing-namedpipeserverstream-on-unix

I also hope this feature will be implemented in the near future.

[TylerLeonhardt]

I appreciate the response but the limitation I pointed out is on Windows which I would expect to work with Pipe Security.

Unix is important as well, but that to me is an additional feature, not a bug like this issue is :)

[danmoseley]

@pjanotti @weshaggard do you know why NamedPipeServerSTream is missing the ACL overloads like NamedPipeServerStream(string pipeName, PipeDirection direction, int maxNumberOfServerInstances, PipeTransmissionMode transmissionMode, PipeOptions options, int inBufferSize, int outBufferSize, PipeSecurity pipeSecurity, HandleInheritability inheritability, PipeAccessRights additionalAccessRights) ? I assumed it was because ACL could not be referenced (cycle) but it is already referenced by this library.

[weshaggard]

@danmosemsft your correct that it wasn't included because of layering issues. NamedPipeServerStream is part of .NET Standard but the ACL API's are not so this member was excluded from the standard and here in the core.

[danmoseley]

Talked offline, part of the problem is that ACL's are not even part of the base shared package so having NPSS reference ACL's pulls a lot of code into the core install.

@pjanotti hopefully can help brainstorm possible solutions for what you want to do.

[pjanotti]

Hi @tylerl0706 - Windows docs state that should be possible, but I gave a few tries and couldn't do it too - I'll have to investigate it more. Is the new feature CurrentUserOnly a possible workaround for your scenario?

[pjanotti]

@tylerl0706 you won't be able to do without p/invoke since you need to specify something like WRITE_DAC when creating the named pipe (see here, search for WRITE_DAC).

@danmosemsft I am getting under the impression that System.IO.Pipes.AccessControl never worked with anything other than an empty PipeSecurity ... do you remember any history in this regard?

The feature request to support WRITE_DAC is already opened, see #23554.

[danmoseley]

@pjanotti I don't know. The original commit suggests PipeSecurity is expected to work. Hopefully you can figure out the history and missing pieces.

[pjanotti]

Looking at the history it should never worked since the rights flags were never set when calling CreateNamedPipe (since 1st version on the repo).

[pjanotti]

One possible way to make this work is to add to S.IO.Pipes.AccessControl some static factory methods that can create named pipes with PipeSecurity and PipeAccessRights equivalent to the constructors that are in the Fx but not in the standard - this will allow the same old functionality of the Fx. There should be some refactor to avoid/reduce code duplication but in principle I don't see a reason for it not to work.

OTOH S.IO.Pipes.AccessControl seems to never ever actually worked with anything other than the empty PipeSecurity and since this is the first time that we are realizing that I'm questioning if it is worth to fix... likely we can't just drop the package for compat reasons: there seems to be a number of projects referencing it but I can't see any useful usage of it as it is.

@tylerl0706 if you can provide some more info about your scenario and what you are trying to achieve? It can be a helpful data point.

/cc @danmosemsft @stephentoub @weshaggard @safern

[pjanotti]

The constructors that are related to AccessControl and could be covered in the package for matching functionality with Fx are:

namespace System.IO.Pipes
{
    public sealed class AnonymousPipeServerStream : PipeStream
    {
        public AnonymousPipeServerStream(PipeDirection direction, HandleInheritability inheritability, int bufferSize, PipeSecurity pipeSecurity);
    }

    public sealed class NamedPipeClientStream : PipeStream
    {
        public NamedPipeClientStream(string serverName, string pipeName, PipeAccessRights desiredAccessRights, PipeOptions options, TokenImpersonationLevel impersonationLevel, HandleInheritability inheritability);
    }

    public sealed class NamedPipeServerStream : PipeStream
    {
        public NamedPipeServerStream(string pipeName, PipeDirection direction, int maxNumberOfServerInstances, PipeTransmissionMode transmissionMode, PipeOptions options, int inBufferSize, int outBufferSize, PipeSecurity pipeSecurity);
        public NamedPipeServerStream(string pipeName, PipeDirection direction, int maxNumberOfServerInstances, PipeTransmissionMode transmissionMode, PipeOptions options, int inBufferSize, int outBufferSize, PipeSecurity pipeSecurity, HandleInheritability inheritability);
        public NamedPipeServerStream(string pipeName, PipeDirection direction, int maxNumberOfServerInstances, PipeTransmissionMode transmissionMode, PipeOptions options, int inBufferSize, int outBufferSize, PipeSecurity pipeSecurity, HandleInheritability inheritability, PipeAccessRights additionalAccessRights);
    }
}

Note that only one of them shows some usage in apisof.net: 0.10 of nuget packages for NamedPipeServerStream..ctor(String,PipeDirection,Int32,PipeTransmissionMode,PipeOptions,Int32,Int32,PipeSecurity,HandleInheritability).

[TylerLeonhardt]

I ended up going the p/invoke route and borrowed most of the code from PowerShell...
https://github.com/PowerShell/PowerShellEditorServices/blob/1031e2296449ab30bb4968e0285566a33e4bf9f4/src/PowerShellEditorServices.Protocol/MessageProtocol/Channel/NamedPipeServerListener.cs#L136-L274

It's ugly... But at least our pipes are secure. -CurrentUser would do the trick... But I have to support back to .NET Core 2.0.

[pjanotti]

Thanks for reporting back. I am under the impression that CurrentUserOnly should satisfy most needs in this regard... but we may get more people asking for this as they migrate from Fx to CoreFx.

[danmoseley]

@tylerl0706 as you probably know 2.0 will go out of support in 2 1/2 months - Oct 1st so perhaps you can move to CurrentUserOnly at that point?

[TylerLeonhardt]

@danmosemsft PowerShell Editor Services (which powers things like the vscode PowerShell extension) is dependent on whatever version of PowerShell the user is using. Since PowerShell Core 6.0.0 shipped with .NET Core 2.0.X, we will have to support .NET Core 2.0.X until we're ready to drop PowerShell 6.0.0 support in the vscode extension

(maybee after 6.1.0 is released which is on Core 2.1... Need to talk to @Steve_MSFT some more about though).

[TylerLeonhardt]

Thanks all for the quick responses. I really appreciate it!

[pjanotti]

Notice that CurrentUserOnly is only on netcoreapp, so no way to target .NET Standard and use it at this moment. BTW, it seems reasonable to add CurrentUserOnly to .NET Standard vNext.

[TylerLeonhardt]

Yeah. I'm looking to target .NET Standard because this library gets loaded into Windows PowerShell (.NET Framework based) and PowerShell Core (.NET Core based)

[l-o-l]

Just a general comment, and please ignore if foolish as I am a complete newbie to core and standard. Lack of ACL support (or a consistent replacement) in .netstandard seems problematic to me. I've just had a try at porting a medium sized communications library written in .net framework and it all went fairly painlessly until security. Named mutexs, memory mapped files, and named pipes, all need an ACL on windows to prevent userland privilege escalation, and that issue is going to be there on any OS. It just seems a little perverse that the only way I could get things to work right now is to turn off security.

I'd like to target .netstandard through most of our components, but IPC mechanisms are at the base of the stack, and if they can't be secured, they can't be used, and the whole house of cards falls down.

That's a long winded way of saying, please add CurrentUserOnly to standard .next. Ideally all these constructors that went missing with the security Param would come back. In magical unicorn land there would be an object security abstraction that would work across platforms. I'm very very ignorant, but surely the concept of users, roles and permissions crosses platforms. The ACL just seems so pivotal to security on windows that it came as a shock that the replacement is either missing, or not obvious, in .net standard.

CurrentUserOnly is much better than nothing. At least this prevents a trust boundary violation.

[pjanotti]

Thanks for the feedback @l-o-l and @tylerl0706. I will start the steps to get CurrentUserOnly added to .NET Standard vNext. Regarding general support for ACL (or something similar) I will have to investigate and understand better the scope and what are the communality/differences that we can user to build and an abstraction on top of it - at this stage is to early for me to say anything concrete about it (perhaps somebody else has better grasp/plans in this regard).

[TylerLeonhardt]

@pjanotti I have a couple questions about CurrentUserOnly.

• Does it work cross plat (Win, macOS, linux)?
• Does CurrentUserOnly prevent the following scenario (privilege escalation) :

Process1 started by myUser was run elevated (as Admin). Process2 started by myUser was run without elevation. If Process1 (aka the elevated process) creates the pipe, will Process2 be able to access it?

[safern]

Does it work cross plat (Win, macOS, linux)?

Yes it does work cross plat. In Windows we use ACL and in Unix we use PeerID to get the socket's owner ID and check if it is the same user trying to connect:

https://github.com/dotnet/corefx/blob/master/src/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs#L91-L105

Does CurrentUserOnly prevent the following scenario (privilege escalation) :

Yes it does, in Windows ACL will handle this by itself, and in Unix whenever you elevate you're now a different user (root) so the PeerID and current user ID should differ.

Just as a guide this are the tests for this feature:
https://github.com/dotnet/corefx/blob/master/src/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.netcoreapp.cs
https://github.com/dotnet/corefx/blob/master/src/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.netcoreapp.Unix.cs
https://github.com/dotnet/corefx/blob/master/src/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.netcoreapp.Windows.cs

[TylerLeonhardt]

Fantastic. I'd use that in a heartbeat if it was available in .NET Standard.

[pjanotti]

Straight to the relevant comment in the PR on dotnet/standard https://github.com/dotnet/standard/pull/826/files#r202885532

[filipnavara]

Sorry for hijacking this thread a little bit.

There's a pending issue in Mono to import System.IO.Pipes from CoreFX. I've update the pull request from last year and this resulted in the following API diff: https://jenkins.mono-project.com/job/test-mono-pull-request-amd64/16144//Public_20API_20Diff/

In essence we are missing the ACL constructors mentioned above. They can't be effectively added using the existing API surface since it would not behave the same on Windows, where the PipeSecurity is used when creating the pipe handle.

What would be the correct way to proceed with a proposal to add these APIs back? Should I open an issue for it?

Since System.IO.Pipes.AccessControl was already merged into System.IO.Pipes there would be no additional dependency.

[pjanotti]

@filipnavara for my understanding: do you need these constructors back to keep the API surface unchanged? What is Mono behavior with these constructors outside of Windows? (Ignore not supported parameters? Throw PNSE? Something else?).

[filipnavara]

@pjanotti At the moment Mono completely lacks the pipe implementation on non-Windows platforms and throws NotImplementedException for most calls. My current plan is to throw PNSE on non-Windows platforms for these additional constructors on Unix and restore NetFX behavior on Windows.

[filipnavara]

More specifically, I am currently fiddling with approach that throws PNSE only if non-default values are used in the parameters.

[pjanotti]

@filipnavara the issue is that the types needed for those constructors are not exposed from the ref, if we bring back those constructors we will have to make the types public again. However, at this moment, they only make sense for Windows and are not part of .NET Standard so I don't think we should bring them back, since they already do nothing on Mono there shouldn't be anybody using them. If someone is porting code from Fx the main use for these constructors should be replaced by the CurrentUserOnly which is supported on Unix and Windows and should be added to .NET Standard vNext.

Is it a deal breaker, for importing CoreFx, removing these constructors from Mono? If yes, why?

[filipnavara]

I assumed the types are exposed in ref (https://github.com/dotnet/corefx/blob/master/src/System.IO.Pipes.AccessControl/ref/System.IO.Pipes.AccessControl.cs), although in different assembly. It seems the CoreFX code already references the types in System.IO.Pipes though only in an extension class.

The current Mono code implements the constructors, but they only work on Windows (the whole System.IO.Pipes code does). The desktop API profile of Mono is tracking the NetFX API. Thus removing the constructors would be a regression and it would also break the API compatibility (which is checked against reference assemblies).

I came up with a solution to use the CoreFX code and at the same time provide the constructors on all Mono supported platforms. I just need a little bit of the internals exposed (pull request dotnet/corefx#31118). Then I can implement the rest as partial classes on Mono side. For Unix I can throw PNSE and for Windows I can provide a legacy implementation. The only downside is that it is fragile dependency that someone can accidentally break.

[pjanotti]

System.IO.Pipes.AccessControl is a package that is not shipped with CoreFx, so those types are only exposed if one explicit imports the package, that's why we don't want to expose those constructors directly in CoreFx.

I took a very quick glance at your changes and it seems fine since you are not exposing any net public constructor or type - as you said the constructors will be done on Mono partial classes. The overall thing is good since it will bring CurrentUserOnly to Mono and gets it ready for .NET Standard vNext. I will take a more careful look at the changes later today (should not take long).

/cc @marek-safar

[pjanotti]

Closing this one and tracking issue regarding System.IO.Pipes.AccessControl package at https://github.com/dotnet/corefx/issues/31190