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

When running a Kestrel HTTPS server on Windows, unneccessary active directory authentication is attempted when accepting a new connection from a client. #100774

Closed
ds185287 opened this issue Apr 8, 2024 · 5 comments · Fixed by #100833
Assignees
Labels
Milestone

Comments

@ds185287
Copy link

ds185287 commented Apr 8, 2024

Description

We have a .NET 8 web app based on Kestrel with self-signed certificates for HTTPS. When the app is running on Windows 10 and receiving requests, a pair of 4625 (Logon Failure) evets appear in the Windows Security event log, indicating unnecessary attempts to authenticate with Windows despite the certificate not correspoding to any Windows security principal. When debugging the code, I found out that the events are logged during the initial TLS handshake, within the call to SSPIWrapper.AcceptSecurityContext at https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs#L124 .

Configuration

.NET 8 (build 8.0.1.23580)
Windows 10 Pro, build 19045.2486
x64

Regression?

No

Data

The main impact of this issue for us is on Windows security event storage, as the app is used quite heavily. This also makes it difficult to inspect Windows Security logs, needing to filter all 4625 Logon Failure event and possibly missing actual malicious login attempts.

Analysis

I believe this issue has the same root cause as #78350 . This issue was already fixed for the old schannel API in #80886 by setting the SCH_CRED_NO_SYSTEM_MAPPER flag by default when calling AcquireCredentialsHandle, however for the new SCH API, the flag is only set when sending the full certificate chain: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs#L368 . As the affected system is running on a Windows 10 build new enough that the SCH API is used, I believe this issue could get resolved if the SCH_CRED_NO_SYSTEM_MAPPER is set by default also to the call to AcquireCredentialsHandle for SCH.

@ds185287 ds185287 added the tenet-performance Performance related issue label Apr 8, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Apr 8, 2024
@rzikm
Copy link
Member

rzikm commented Apr 9, 2024

Looks like something missed in #80886, I think we should be safe doing the same thing for the other API as well. CC @wfurt

@rzikm rzikm removed the untriaged New issue has not been triaged by the area owner label Apr 9, 2024
@rzikm rzikm added this to the 9.0.0 milestone Apr 9, 2024
@rzikm rzikm self-assigned this Apr 9, 2024
@rzikm
Copy link
Member

rzikm commented Apr 9, 2024

@ds185287 Do you have an estimate how severe the performance impact is? Thinking aloud if it makes sense to consider backporting to .NET 8 since it's LTS release

@ds185287
Copy link
Author

We don't have solid data on the performance impact, as our lab systems aren't connected to a domain controller and aren't used enough for the performance impact to be visible. However we do get reports from a customer claiming over 2 million failed logon events amassing over a few days and their SIEM solution failing to cope with the flood of audit events. They claim major impact on their operations, but I'm not certain that this is the only cause.

The main impact from my point of view is the possibility of actual malicious failed login attempts getting lost among the events logged from the .NET app, making this issue more of security than performance. The reason I logged it as performance was that the original issue #78350 to which this is related was loged as performance.

@rzikm
Copy link
Member

rzikm commented Apr 10, 2024

@karelz would this be viable justification for backporting the change to 8.0?

@karelz
Copy link
Member

karelz commented Apr 10, 2024

@ds185287 for a 8.0 backport, we would need more details about impact on the production - ideally directly from the customer.
What about workarounds: Is it possible to filter out these (useless) events and ignore them? If not, why?
What does it mean that CIEM solution cannot cope? Is it running out of memory, CPU, or something like that? ... More details here would help us to make the call.

@rzikm rzikm assigned rzikm and unassigned rzikm Apr 10, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants