Skip to content

Add support for setting security attributes on Http.Sys RequestQueue #61325

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Servers/HttpSys/src/HttpSysListener.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ public HttpSysListener(HttpSysOptions options, ILoggerFactory loggerFactory)
try
{
_serverSession = new ServerSession();
_requestQueue = new RequestQueue(options.RequestQueueName, options.RequestQueueMode, Logger);
_requestQueue = new RequestQueue(options.RequestQueueName, options.RequestQueueMode,
options.RequestQueueSecurityDescriptor, Logger);
_urlGroup = new UrlGroup(_serverSession, _requestQueue, Logger);

_disconnectListener = new DisconnectListener(_requestQueue, Logger);
Expand Down
9 changes: 9 additions & 0 deletions src/Servers/HttpSys/src/HttpSysOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Globalization;
using System.Security.AccessControl;
using System.Text;
using Microsoft.AspNetCore.Http.Features;

Expand Down Expand Up @@ -167,6 +168,14 @@ public long RequestQueueLimit
}
}

/// <summary>
/// Gets or sets the security descriptor for the request queue.
/// </summary>
/// <remarks>
/// Only applies when creating a new request queue, see <see cref="RequestQueueMode" />.
/// </remarks>
public GenericSecurityDescriptor? RequestQueueSecurityDescriptor { get; set; }

/// <summary>
/// Gets or sets the maximum allowed size of any request body in bytes.
/// When set to null, the maximum request body size is unlimited.
Expand Down
46 changes: 40 additions & 6 deletions src/Servers/HttpSys/src/NativeInterop/RequestQueue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Security.AccessControl;
using Microsoft.Extensions.Logging;
using Windows.Win32;
using Windows.Win32.Networking.HttpServer;
using Windows.Win32.Security;

namespace Microsoft.AspNetCore.Server.HttpSys;

Expand All @@ -16,22 +18,25 @@ internal sealed partial class RequestQueue
private bool _disposed;

internal RequestQueue(string requestQueueName, ILogger logger)
: this(requestQueueName, RequestQueueMode.Attach, logger, receiver: true)
: this(requestQueueName, RequestQueueMode.Attach, securityDescriptor: null, logger, receiver: true)
{
}

internal RequestQueue(string? requestQueueName, RequestQueueMode mode, ILogger logger)
: this(requestQueueName, mode, logger, false)
internal RequestQueue(string? requestQueueName, RequestQueueMode mode, GenericSecurityDescriptor? securityDescriptor, ILogger logger)
: this(requestQueueName, mode, securityDescriptor, logger, false)
{ }

private RequestQueue(string? requestQueueName, RequestQueueMode mode, ILogger logger, bool receiver)
private RequestQueue(string? requestQueueName, RequestQueueMode mode, GenericSecurityDescriptor? securityDescriptor, ILogger logger, bool receiver)
{
_mode = mode;
_logger = logger;

var flags = 0u;
Created = true;

SECURITY_ATTRIBUTES? securityAttributes = null;
nint? pSecurityDescriptor = null;

if (_mode == RequestQueueMode.Attach)
{
flags = PInvoke.HTTP_CREATE_REQUEST_QUEUE_FLAG_OPEN_EXISTING;
Expand All @@ -41,11 +46,31 @@ private RequestQueue(string? requestQueueName, RequestQueueMode mode, ILogger lo
flags |= PInvoke.HTTP_CREATE_REQUEST_QUEUE_FLAG_DELEGATION;
}
}
else if (securityDescriptor is not null) // Create or CreateOrAttach
{
// Convert the security descriptor to a byte array
byte[] securityDescriptorBytes = new byte[securityDescriptor.BinaryLength];
securityDescriptor.GetBinaryForm(securityDescriptorBytes, 0);

// Allocate native memory for the security descriptor
pSecurityDescriptor = Marshal.AllocHGlobal(securityDescriptorBytes.Length);
Marshal.Copy(securityDescriptorBytes, 0, pSecurityDescriptor.Value, securityDescriptorBytes.Length);

unsafe
{
securityAttributes = new SECURITY_ATTRIBUTES
{
nLength = (uint)Marshal.SizeOf<SECURITY_ATTRIBUTES>(),
lpSecurityDescriptor = pSecurityDescriptor.Value.ToPointer(),
bInheritHandle = false
};
}
}

var statusCode = PInvoke.HttpCreateRequestQueue(
HttpApi.Version,
requestQueueName,
default,
securityAttributes,
flags,
out var requestQueueHandle);

Expand All @@ -57,11 +82,17 @@ private RequestQueue(string? requestQueueName, RequestQueueMode mode, ILogger lo
statusCode = PInvoke.HttpCreateRequestQueue(
HttpApi.Version,
requestQueueName,
default,
SecurityAttributes: default, // Attaching should not pass any security attributes
flags,
out requestQueueHandle);
}

if (pSecurityDescriptor is not null)
{
// Free the allocated memory for the security descriptor
Marshal.FreeHGlobal(pSecurityDescriptor.Value);
}

if ((flags & PInvoke.HTTP_CREATE_REQUEST_QUEUE_FLAG_OPEN_EXISTING) != 0 && statusCode == ErrorCodes.ERROR_FILE_NOT_FOUND)
{
throw new HttpSysException((int)statusCode, $"Failed to attach to the given request queue '{requestQueueName}', the queue could not be found.");
Expand Down Expand Up @@ -143,6 +174,9 @@ public void Dispose()
}

_disposed = true;

PInvoke.HttpCloseRequestQueue(Handle);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a reasonable change but unrelated to this PR. Drive-by bug fix or is it related somehow?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by, was just reading the docs for HttpCreateRequestQueue to make sure I was doing the correct thing with the security attribute and noticed it said you should close the queue handle with HttpCloseRequestQueue.


BoundHandle.Dispose();
Handle.Dispose();
}
Expand Down
4 changes: 4 additions & 0 deletions src/Servers/HttpSys/src/NativeMethods.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ HTTPAPI_VERSION
HttpCancelHttpRequest
HttpCloseServerSession
HttpCloseUrlGroup
HttpCloseRequestQueue
HttpCreateRequestQueue
HttpCreateServerSession
HttpCreateUrlGroup
Expand All @@ -59,3 +60,6 @@ HttpSetUrlGroupProperty
SetFileCompletionNotificationModes
SOCKADDR_IN
SOCKADDR_IN6
GetSecurityInfo
GetSecurityDescriptorLength
LocalFree
2 changes: 2 additions & 0 deletions src/Servers/HttpSys/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#nullable enable
Microsoft.AspNetCore.Server.HttpSys.HttpSysOptions.TlsClientHelloBytesCallback.get -> System.Action<Microsoft.AspNetCore.Http.Features.IFeatureCollection!, System.ReadOnlySpan<byte>>?
Microsoft.AspNetCore.Server.HttpSys.HttpSysOptions.TlsClientHelloBytesCallback.set -> void
Microsoft.AspNetCore.Server.HttpSys.HttpSysOptions.RequestQueueSecurityDescriptor.get -> System.Security.AccessControl.GenericSecurityDescriptor?
Microsoft.AspNetCore.Server.HttpSys.HttpSysOptions.RequestQueueSecurityDescriptor.set -> void
95 changes: 94 additions & 1 deletion src/Servers/HttpSys/test/FunctionalTests/DelegateTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,17 @@

using System.Net.Http;
using System.Runtime.InteropServices;
using System.Security.AccessControl;
using System.Security.Principal;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Hosting.Server;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.HttpSys.Internal;
using Microsoft.AspNetCore.InternalTesting;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Windows.Win32;
using Windows.Win32.Foundation;
using Windows.Win32.Security;

namespace Microsoft.AspNetCore.Server.HttpSys.FunctionalTests;

Expand Down Expand Up @@ -266,6 +270,95 @@ public async Task DelegateAfterReceiverRestart()
destination?.Dispose();
}

[ConditionalFact]
[DelegateSupportedCondition(true)]
public async Task DelegateRequestTestCanSetSecurityDescriptor()
{
// Create a new security descriptor
CommonSecurityDescriptor securityDescriptor = new CommonSecurityDescriptor(false, false, string.Empty);

// Create a discretionary access control list (DACL)
DiscretionaryAcl dacl = new DiscretionaryAcl(false, false, 2);
dacl.AddAccess(AccessControlType.Allow, new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null), -1, InheritanceFlags.None, PropagationFlags.None);
dacl.AddAccess(AccessControlType.Deny, new SecurityIdentifier(WellKnownSidType.BuiltinGuestsSid, null), -1, InheritanceFlags.None, PropagationFlags.None);

// Assign the DACL to the security descriptor
securityDescriptor.DiscretionaryAcl = dacl;

var queueName = Guid.NewGuid().ToString();
using var receiver = Utilities.CreateHttpServer(out var receiverAddress, async httpContext =>
{
await httpContext.Response.WriteAsync(_expectedResponseString);
},
options =>
{
options.RequestQueueName = queueName;
options.RequestQueueSecurityDescriptor = securityDescriptor;
}, LoggerFactory);

DelegationRule destination = default;

using var delegator = Utilities.CreateHttpServer(out var delegatorAddress, httpContext =>
{
var delegateFeature = httpContext.Features.Get<IHttpSysRequestDelegationFeature>();
delegateFeature.DelegateRequest(destination);
return Task.CompletedTask;
}, LoggerFactory);

var delegationProperty = delegator.Features.Get<IServerDelegationFeature>();
destination = delegationProperty.CreateDelegationRule(queueName, receiverAddress);

AssertPermissions(destination.Queue.Handle);
unsafe void AssertPermissions(SafeHandle handle)
{
PSECURITY_DESCRIPTOR pSecurityDescriptor = new();

WIN32_ERROR result = PInvoke.GetSecurityInfo(
handle,
Windows.Win32.Security.Authorization.SE_OBJECT_TYPE.SE_KERNEL_OBJECT,
4, // DACL_SECURITY_INFORMATION
null,
null,
null,
null,
&pSecurityDescriptor);

var length = (int)PInvoke.GetSecurityDescriptorLength(pSecurityDescriptor);

// Copy the security descriptor to a managed byte array
byte[] securityDescriptorBytes = new byte[length];
Marshal.Copy(new IntPtr(pSecurityDescriptor.Value), securityDescriptorBytes, 0, length);

// Convert the byte array to a RawSecurityDescriptor
var securityDescriptor = new RawSecurityDescriptor(securityDescriptorBytes, 0);

var checkedAllowUser = false;
var checkedDenyGuest = false;

foreach (CommonAce ace in securityDescriptor.DiscretionaryAcl)
{
if (ace.SecurityIdentifier.IsWellKnown(WellKnownSidType.BuiltinGuestsSid))
{
Assert.Equal(AceType.AccessDenied, ace.AceType);
checkedDenyGuest = true;
}
else if (ace.SecurityIdentifier.IsWellKnown(WellKnownSidType.BuiltinUsersSid))
{
Assert.Equal(AceType.AccessAllowed, ace.AceType);
checkedAllowUser = true;
}
}

PInvoke.LocalFree((HLOCAL)pSecurityDescriptor.Value);

Assert.True(checkedDenyGuest && checkedAllowUser, "DACL does not contain the expected ACEs");
}

var responseString = await SendRequestAsync(delegatorAddress);
Assert.Equal(_expectedResponseString, responseString);
destination?.Dispose();
}

private async Task<string> SendRequestAsync(string uri)
{
using var client = new HttpClient();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
Expand All @@ -20,7 +20,7 @@
<Compile Include="$(SharedSourceRoot)ValueTaskExtensions\**\*.cs" LinkBase="Shared\" />
<Compile Remove="$(SharedSourceRoot)ServerInfrastructure\DuplexPipe.cs" />
<Compile Remove="$(SharedSourceRoot)ServerInfrastructure\StringUtilities.cs" />
<Compile Include="$(SharedSourceRoot)InternalHeaderNames.cs" Linkbase="shared"/>
<Compile Include="$(SharedSourceRoot)InternalHeaderNames.cs" Linkbase="shared" />
<Compile Include="$(SharedSourceRoot)TestResources.cs" LinkBase="shared" />
<Content Include="$(SharedSourceRoot)TestCertificates\*.pfx" LinkBase="shared\TestCertificates" CopyToOutputDirectory="PreserveNewest" />
<Compile Include="$(SharedSourceRoot)TransportTestHelpers\MsQuicSupportedAttribute.cs" LinkBase="shared\" />
Expand Down
Loading