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

Add CreateNamedPipeServerStream to named pipes options #56568

Closed
JamesNK opened this issue Jul 2, 2024 · 2 comments · Fixed by #56567
Closed

Add CreateNamedPipeServerStream to named pipes options #56568

JamesNK opened this issue Jul 2, 2024 · 2 comments · Fixed by #56567
Labels
api-approved API was approved in API review, it can be implemented area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Comments

@JamesNK
Copy link
Member

JamesNK commented Jul 2, 2024

Background and Motivation

Addresses #53306

Named pipes transport allows pipe security to be configured, but it's applied to all of server's named pipe endpoints. An API is needed to customize endpoint security when they're created.

Proposed API

Adds CreateNamedPipeServerStream func to options, allowing advanced users to customize creating endpoints. They can choose to specify different PipeSecurity options on a per-endpoint basis.

This feature is modeled after SocketTransportOptions.CreateBoundListenSocket.

namespace Microsoft.AspNetCore.Server.Kestrel.Transport.NamedPipes;

+ public sealed class CreateNamedPipeServerStreamContext
+ {
+     public required NamedPipeEndPoint NamedPipeEndPoint { get; init; }
+     public PipeOptions PipeOptions { get; init; }
+     public PipeSecurity? PipeSecurity { get; init; }
+ }

public sealed class NamedPipeTransportOptions
{
+     public Func<CreateNamedPipeServerStreamContext, NamedPipeServerStream> CreateNamedPipeServerStream { get; set; }
+     public static NamedPipeServerStream CreateDefaultNamedPipeServerStream(CreateNamedPipeServerStreamContext context);
}

CreateNamedPipeServerStreamContext is added because more information than just the endpoint is needed when creating a NamedPipeServerStream. An object provides flexibility for more data to be provided, if needed. Allocations aren't a concern because the context is only created when a stream is created, and streams are cached and reused.

Usage Examples

var builder = new HostBuilder()
    .ConfigureWebHost(webHostBuilder =>
    {
        webHostBuilder
            .UseKestrel(o =>
            {
                o.ListenNamedPipe(defaultSecurityPipeName, listenOptions =>
                {
                    listenOptions.Protocols = HttpProtocols.Http1;
                });
                o.ListenNamedPipe(customSecurityPipeName, listenOptions =>
                {
                    listenOptions.Protocols = HttpProtocols.Http1;
                });
            })
            .UseNamedPipes(options =>
            {
                var defaultSecurity = new PipeSecurity();
                defaultSecurity.AddAccessRule(new PipeAccessRule("Users", PipeAccessRights.ReadWrite | PipeAccessRights.CreateNewInstance, AccessControlType.Allow));
                options.PipeSecurity = defaultSecurity;
                options.CurrentUserOnly = false;
                options.CreateNamedPipeServerStream = (context) =>
                {
                    if (context.NamedPipeEndPoint.PipeName == defaultSecurityPipeName)
                    {
                        return NamedPipeTransportOptions.CreateDefaultNamedPipeServerStream(context);
                    }
                    var allowSecurity = new PipeSecurity();
                    allowSecurity.AddAccessRule(new PipeAccessRule("Users", PipeAccessRights.FullControl, AccessControlType.Allow));
                    return NamedPipeServerStreamAcl.Create(
                        context.NamedPipeEndPoint.PipeName,
                        PipeDirection.InOut,
                        NamedPipeServerStream.MaxAllowedServerInstances,
                        PipeTransmissionMode.Byte,
                        context.PipeOptions,
                        inBufferSize: 0, // Buffer in System.IO.Pipelines
                        outBufferSize: 0, // Buffer in System.IO.Pipelines
                        allowSecurity);
                };
            })
            .Configure(app =>
            {
                app.Run(async context =>
                {
                    await context.Response.WriteAsync("hello, world");
                });
            });
    });

Alternative Designs

Risks

@JamesNK JamesNK added api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions labels Jul 2, 2024
Copy link
Contributor

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@amcasey
Copy link
Member

amcasey commented Jul 3, 2024

[API Review]

  • Should CreateNamedPipeServerStreamContext.PipeOptions be required?
    • Nearly always want to set it
    • Not strictly necessary, since it's an enum
  • PipeSecurity is configured globally and is passed per-call as a convenience
  • This won't affect pooling of named pipe server streams
    • Pooling is per-endpoint
  • Func instantiates stream, rather than modifying context, for consistency with unix sockets
  • NamedPipeServerStream rather than PipeStream since we're already exposing it that way elsewhere

API Approved!

@amcasey amcasey added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants