Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Port System.IO.Pipes AccessControl to CoreFX #8930

Merged
merged 2 commits into from
Jun 9, 2016
Merged

Port System.IO.Pipes AccessControl to CoreFX #8930

merged 2 commits into from
Jun 9, 2016

Conversation

ianhays
Copy link
Contributor

@ianhays ianhays commented May 27, 2016

This commit ports the Pipes AccessControl source from the full framework and adds infrastructure to fit it into CoreFX as part of the regular build.

  • The PipeStream GetAccessControl and SetAccessControl functions are implemented as Extension methods similar to what was done for FileSystem AccessControl.
  • A few basic tests for behavior were added.
  • A net46 implementation of the extension methods is available as well.
  • This commit includes packaging
  • Behavior is the same as the .NET 4.6 version of PipeSecurity/PipeAuditRule/etc. with the exception that the SetAccessControl extension method does not retain the same exception throwing behavior for NamedPipeClientStream. Since it's an extension method, we don't have access to the internal PipeStream State that is used in the full version to decide which error to throw. The original will throw an InvalidOperation for a WaitingToConnect stream or a IOException for a Broken stream. We just throw an InvalidOperation for either.

resolves https:/dotnet/corefx/issues/7449

@stephentoub
Copy link
Member

Thanks for doing this, @ianhays.

@terrajobst, @KrzysztofCwalina, @danmosemsft, should we just add all of this into System.IO.Pipes.dll rather than creating a separate assembly for it?

@ianhays
Copy link
Contributor Author

ianhays commented Jun 6, 2016

should we just add all of this into System.IO.Pipes.dll rather than creating a separate assembly for it?

We could do that. I went with the separate assembly since that's what Threading and FileSystem did, but it could be in the Pipes assembly instead.

Merging them would require that Pipes (win) take a project.json dependency on System.Security.AccessControl and probably System.Security.Principal.Windows. We'd also need to PlatformNotSupport all of the Security stuff for non-windows.

@stephentoub
Copy link
Member

We'd also need to PlatformNotSupport all of the Security stuff for non-windows

Potentially. It would be interesting to see if we could make some of it work via some kind of mapping to RWX for user/group/everyone. We have had requests for controlling those permissions, and if there was a reasonable way to map at least some subset of it, that could be useful. Might not work, but would be worth exploring.

@ianhays
Copy link
Contributor Author

ianhays commented Jun 6, 2016

The Pipes accesscontrol uses AccessRights/AccessRules/AuditRules and all that jazz, so if we bought Pipes.AccessControl to Unix we would first need to bring System.Security.AcessControl over to Unix.

I think an implementation of System.Security.AccessControl specifically for Unix to control the basic permissions would be useful, but aligning it with the Windows AccessControl API would be undoubtedly clunky. Some googling tells me that Unix has ACLs which I was unaware of, so maybe we could use those to get behaviorally closer with Windows.

Do you know of any issue for a Unix System.Security.AccessControl? If not, let's open one.

@danmoseley
Copy link
Member

danmoseley commented Jun 6, 2016

should we just add all of this into System.IO.Pipes.dll rather than creating a separate assembly for it?

To me consistency is attractive, ie., following threading and file system in having a separate assembly.

@stephentoub
Copy link
Member

Why don't we get this PR merged, and then we can move the implementation to be part of System.IO.Pipes.dll if that makes sense.

@terrajobst
Copy link
Member

👍

@danmoseley
Copy link
Member

Agreed..

dotnet-bot and others added 2 commits June 9, 2016 09:04
This commit ports the Pipes AccessControl source from the full framework and adds infrastructure to fit it into CoreFX as part of the regular build.
- The PipeStream GetAccessControl and SetAccessControl functions are implemented as Extension methods similar to what was done for FileSystem AccessControl.
- A few basic tests for behavior were added.
- A net46 implementation of the extension methods is available as well.
- This commit includes packaging
- Behavior is the same as the .NET 4.6 version of PipeSecurity/PipeAuditRule/etc. with the exception that the SetAccessControl extension method does not retain the same exception throwing behavior for NamedPipeClientStream. Since it's an extension method, we don't have access to the internal PipeStream State that is used in the full version to decide which error to throw. The original will throw an InvalidOperation for a WaitingToConnect stream or a IOException for a Broken stream. We just throw an InvalidOperation for either.
@ianhays
Copy link
Contributor Author

ianhays commented Jun 9, 2016

Thanks all! I rebased to two commits; will merge when CI turns green.

@ianhays ianhays merged commit 6e05a66 into dotnet:master Jun 9, 2016
@ianhays ianhays deleted the iopipes_accesscontrol branch July 20, 2016 17:35
@karelz karelz added port-to-core Issue tracking port of .NET Framework code to .NET Core and removed Port to GitHub labels Nov 9, 2016
@karelz karelz modified the milestones: 1.2.0, 1.1.0 Dec 3, 2016
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Port System.IO.Pipes AccessControl to CoreFX

Commit migrated from dotnet/corefx@6e05a66
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO os-windows port-to-core Issue tracking port of .NET Framework code to .NET Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port AccessControl for System.IO.Pipes
7 participants