Use also extended policy for looking up auth context - #133195
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
There was a problem hiding this comment.
🟡 Changes recommended
The new extended-protection equivalence check compares/stores the raw per-request policy rather than the effective policy used to create the NegotiateAuthentication context, which can incorrectly force a new context mid-handshake and break authentication flows.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the Windows HttpListener authentication-session reuse logic to incorporate Extended Protection policy into the cached NegotiateAuthentication context, and adds a regression test covering an NTLM handshake where the ExtendedProtectionSelectorDelegate changes policy between handshake legs.
Changes:
- Track ExtendedProtectionPolicy alongside the cached NegotiateAuthentication session and include it in the reuse decision.
- Add a Windows-only functional test that exercises NTLM over a single connection with a per-request ExtendedProtectionSelectorDelegate that becomes stricter mid-handshake.
File summaries
| File | Description |
|---|---|
| src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs | Extends the cached auth-session keying to include extended protection policy equivalence when deciding whether to reuse a NegotiateAuthentication context. |
| src/libraries/System.Net.HttpListener/tests/HttpListenerAuthenticationTests.cs | Adds a regression test that performs an NTLM handshake over a single socket and asserts auth fails if extended protection becomes stricter between handshake legs. |
Review details
Suppressed comments (1)
src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs:1116
- When persisting the cached NegotiateAuthentication session, store the same extended protection policy that was used to create (or validate reuse of) the session context. After adjusting reuse to compare the effective authentication policy, this assignment should persist that effective policy rather than the raw per-request value so subsequent requests compare apples-to-apples.
disconnectResult.Session = sessionContext;
disconnectResult.SessionPackage = contextPackage;
disconnectResult.SessionExtendedProtectionPolicy = extendedProtectionPolicy;
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
| string package = headerScheme == AuthenticationSchemes.Ntlm ? NegotiationInfoClass.NTLM : NegotiationInfoClass.Negotiate; | ||
| if (sessionContext is null || sessionContext.IsAuthenticated || contextPackage != package) | ||
| if (sessionContext is null || sessionContext.IsAuthenticated || contextPackage != package || | ||
| !AreExtendedProtectionPoliciesEquivalent(contextExtendedProtectionPolicy, extendedProtectionPolicy)) | ||
| { |
This adds extended attributes to auth context so we check the expected level when custom delegate runs.
fixes #133180