Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NamedPipeServerStream: Provide support for WRITE_DAC #23554

Open
ianhays opened this issue Sep 14, 2017 · 12 comments
Open

NamedPipeServerStream: Provide support for WRITE_DAC #23554

ianhays opened this issue Sep 14, 2017 · 12 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.IO
Milestone

Comments

@ianhays
Copy link
Contributor

ianhays commented Sep 14, 2017

When creating a NamedPipeServerStream on Windows, WRITE_DAC on CreateNamedPipe will always be unset. On netfx we had a constructor that took a PipeSecurity object with ACL info but we had to take that out for netcore/netstandard.

We should add a new entry on netcoreapp to the PipeOptions enum to set WRITE_DAC:

    [Flags]
    public enum PipeOptions
    {
        None = 0x0,
        WriteThrough = unchecked((int)0x80000000),  // corresponds to FILE_FLAG_WRITE_THROUGH
        Asynchronous = unchecked((int)0x40000000),  // corresponds to FILE_FLAG_OVERLAPPED,
        WriteDac = unchecked((int)0x00040000L),     // corresponds to WRITE_DAC
    }

cc: @bartonjs @stephentoub @jaredpar

@danmoseley
Copy link
Member

@ianhays we removed the ctors from AnonymousPipeServerStream as well as NamedPipeClientStream and NamedPipeServerStream. But AnonymousPipeServerStream does not have a ctor that takes PipeOptions.

I notice AnonymousPipeClientStream does not take PipeSecurity on desktop, only the server, so I'm not sure how those semantics work.

@danmoseley
Copy link
Member

danmoseley commented Sep 14, 2017

@jaredpar do you need this for code that runs on 2.0, ie., do we need to service for it?

cc @weshaggard its intersting this is the first case I've seen where removing members for net standard broke functionality

@bartonjs
Copy link
Member

Jared's in-office suggestion was to add a new ctor with a bool to indicate that PipeAccessRights.ChangePermissions (WRITE_DAC) was desired.

Since we already have the value on PipeAccessRights (NetFx) I don't think we'd want to duplicate it onto PipeOptions, because it would make the NetFx path weird.

We just can't let a new-bool ctor and a PipeAccessRights ctor ever touch.

@danmoseley
Copy link
Member

What about moving PipeAccessRights down so that we could consume just that one enum? That way we could bring the constructor back and code would not need to be written specifically for .NET Core. That wouldn't help anyone targeting .NET Standard but then neither would the new constructor proposed above.

@danmoseley
Copy link
Member

Ugh, that constructor also needs PipeSecurity. I guess in theory we could move that down also, potentially using reflection to not pull all of S.IO.Pipes.AccessControl with it.

@danmoseley
Copy link
Member

I hate to make a .NET Core specific API, as it basically only helps Jared.

@ianhays
Copy link
Contributor Author

ianhays commented Sep 15, 2017

My problem with the constructor (beyond it being little more than a limited version of a netfx constructor) is that it's highly visible despite it being unneeded for most scenarios.

Since we already have the value on PipeAccessRights (NetFx) I don't think we'd want to duplicate it onto PipeOptions, because it would make the NetFx path weird.

That's true, but either way we're going to be adding redundant API by a different name. IMO we should either bury it as deep as we can or revisit whether it's something we really need to expose.

@danmoseley
Copy link
Member

This is required for 2.1 for persistent Roslyn and MSBuild processes.

@pjanotti pjanotti assigned safern and unassigned pjanotti Jan 3, 2018
@danmoseley
Copy link
Member

Update, Roslyn/MSBuild will use https://app.zenhub.com/workspace/o/dotnet/corefx/issues/25427

@Jens-G
Copy link

Jens-G commented Feb 28, 2019

Is there a way this could be fixed? The whole PipeSecurity package is useless without this.

@stephentoub
Copy link
Member

@danmosemsft, is anyone working on this for 3.0?

@JeremyKuhne
Copy link
Member

@carlossanlop Was this taken care of in our ACL work that you did?

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@JeremyKuhne JeremyKuhne removed the untriaged New issue has not been triaged by the area owner label Mar 3, 2020
@carlossanlop carlossanlop modified the milestones: 5.0.0, Future Jun 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.IO
Projects
None yet
Development

No branches or pull requests