Unify certificate chain building between SslStream and WinHttpHandler #115761

Copilot · 2025-05-20T06:36:34Z

This PR unifies the certificate chain building logic between SslStream and WinHttpHandler by replacing the duplicate implementation in WinHttpCertificateHelper.cs with the shared implementation from CertificateValidation.Windows.cs.

Changes

Removed WinHttpCertificateHelper.cs which contained duplicate certificate chain building logic
Modified WinHttpRequestCallback.cs to use the shared implementation in CertificateValidation.BuildChainAndVerifyProperties
Updated project files to include a reference to CertificateValidation.Windows.cs
Added appropriate parameter mapping between the two implementations:
- Created and configured X509Chain with the same settings as before
- Set appropriate parameters for certificate validation (isServer=false since WinHttpHandler is a client validating server certificates)
Updated test files to work with the new implementation

This change makes the codebase more consistent, reduces duplication, and simplifies future maintenance of certificate validation logic.

Fixes #113468.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

dc.services.visualstudio.com
- Triggering command: /home/REDACTED/work/runtime/runtime/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/runtime/runtime/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/runtime/runtime/artifacts/toolset/10.0.0-beta.25260.104.txt (dns block)
pkgs.dev.azure.com
- Triggering command: /home/REDACTED/work/runtime/runtime/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/runtime/runtime/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/runtime/runtime/artifacts/toolset/10.0.0-beta.25260.104.txt (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to my firewall allow list

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: liveans <45091041+liveans@users.noreply.github.com>

liveans · 2025-05-20T06:48:24Z

/azp run runtime-libraries-coreclr outerloop-windows

azure-pipelines · 2025-05-20T06:48:33Z

Azure Pipelines successfully started running 1 pipeline(s).

dotnet-policy-service · 2025-05-20T07:29:58Z

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

liveans

@copilot Seems like code has unnecessary trailing/leading whitespaces, which causes build failures.
Also, you should be aware that WinHttpHandler is shared code between .NET and .NET Framework, and some of the APIs we're currently using Marshal.GetLastPInvokeError in Common\System\Net\Security\CertificateValidation.Windows.cs which doesn't exist on .NET Framework, we might need to do some changes here.

src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs

Co-authored-by: liveans <45091041+liveans@users.noreply.github.com>

Copilot · 2025-05-20T07:43:02Z

@copilot Seems like code has unnecessary trailing/leading whitespaces, which causes build failures.
Also, you should be aware that WinHttpHandler is shared code between .NET and .NET Framework, and...

Fixed the whitespace issues in be4a4e4. Removed trailing spaces and reformatted the code to maintain consistent style. Regarding the .NET Framework compatibility, I understand that we may need additional changes to handle the Marshal.GetLastPInvokeError API which doesn't exist in .NET Framework. Please let me know if you'd like me to address this issue as well.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

dc.services.visualstudio.com
- Triggering command: /home/REDACTED/work/runtime/runtime/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/runtime/runtime/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/runtime/runtime/artifacts/toolset/10.0.0-beta.25260.104.txt (dns block)
pkgs.dev.azure.com
- Triggering command: /home/REDACTED/work/runtime/runtime/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/runtime/runtime/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/runtime/runtime/artifacts/toolset/10.0.0-beta.25260.104.txt (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to my firewall allow list

liveans · 2025-05-20T07:45:23Z

@copilot Seems like code has unnecessary trailing/leading whitespaces, which causes build failures.
Also, you should be aware that WinHttpHandler is shared code between .NET and .NET Framework, and...

Fixed the whitespace issues in be4a4e4. Removed trailing spaces and reformatted the code to maintain consistent style. Regarding the .NET Framework compatibility, I understand that we may need additional changes to handle the Marshal.GetLastPInvokeError API which doesn't exist in .NET Framework. Please let me know if you'd like me to address this issue as well.

Warning

Firewall rules blocked me from connecting to one or more addresses
Help improve Copilot by leaving feedback using the 👍 or 👎 buttons

Please address this issue as well, be aware that current usage should not change for .NET, we should only handle this case for .NET Framework.

Co-authored-by: liveans <45091041+liveans@users.noreply.github.com>

Copilot · 2025-05-20T07:49:05Z

@copilot Seems like code has unnecessary trailing/leading whitespaces, which causes build failures.
Also, you should be aware that WinHttpHandler is shared code between .NET and .NET Framew...

Added conditional compilation in c6a5f9d to handle the Marshal.GetLastPInvokeError() incompatibility with .NET Framework. The fix uses Marshal.GetLastWin32Error() when targeting .NET Framework while preserving the existing behavior for .NET.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

dc.services.visualstudio.com
- Triggering command: /home/REDACTED/work/runtime/runtime/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/runtime/runtime/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/runtime/runtime/artifacts/toolset/10.0.0-beta.25260.104.txt (dns block)
pkgs.dev.azure.com
- Triggering command: /home/REDACTED/work/runtime/runtime/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/runtime/runtime/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/runtime/runtime/artifacts/toolset/10.0.0-beta.25260.104.txt (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to my firewall allow list

src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/FakeX509Certificates.cs

Co-authored-by: krwq <660048+krwq@users.noreply.github.com>

...tem.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj

src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/FakeX509Certificates.cs

Co-authored-by: liveans <45091041+liveans@users.noreply.github.com>

liveans · 2025-05-20T13:34:04Z

/azp run runtime-libraries-coreclr outerloop-windows

azure-pipelines · 2025-05-20T13:34:17Z

Azure Pipelines successfully started running 1 pipeline(s).

rzikm

LGTM if CI is green

liveans · 2025-05-20T15:13:38Z

/ba-g Cancelled jobs (failed according to build analysis) are linux, although this change is windows only. Unrelated failures.

liveans · 2025-05-20T16:12:08Z

Outerloop failures are also unrelated.

…#115761) * Initial plan for issue * Update WinHttpRequestCallback to use CertificateValidation Co-authored-by: liveans <45091041+liveans@users.noreply.github.com> * Remove unused WinHttpCertificateHelper.cs file Co-authored-by: liveans <45091041+liveans@users.noreply.github.com> * Fix formatting issues in WinHttpRequestCallback.cs Co-authored-by: liveans <45091041+liveans@users.noreply.github.com> * Fix compatibility with .NET Framework by using GetLastWin32Error Co-authored-by: liveans <45091041+liveans@users.noreply.github.com> * Fix FakeX509Certificates.cs to clarify empty namespace Co-authored-by: krwq <660048+krwq@users.noreply.github.com> * Remove unnecessary code per review comments Co-authored-by: liveans <45091041+liveans@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: liveans <45091041+liveans@users.noreply.github.com> Co-authored-by: krwq <660048+krwq@users.noreply.github.com>

Initial plan for issue

f7456b2

Copilot AI assigned Copilot and liveans May 20, 2025

Copilot started work on behalf of liveans May 20, 2025 06:36 View session

dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label May 20, 2025

Copilot AI and others added 2 commits May 20, 2025 06:45

Update WinHttpRequestCallback to use CertificateValidation

a8df85f

Co-authored-by: liveans <45091041+liveans@users.noreply.github.com>

Remove unused WinHttpCertificateHelper.cs file

72d517d

Co-authored-by: liveans <45091041+liveans@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Unify certificate chain building between SslStream and WinHttpHandler~~ Unify certificate chain building between SslStream and WinHttpHandler May 20, 2025

Copilot AI requested a review from liveans May 20, 2025 06:46

Copilot finished work on behalf of liveans May 20, 2025 06:46

jkotas added area-System.Net.Http and removed linkable-framework Issues associated with delivering a linker friendly framework labels May 20, 2025

liveans reviewed May 20, 2025

View reviewed changes

src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs Outdated Show resolved Hide resolved

src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs Outdated Show resolved Hide resolved

Copilot started work on behalf of liveans May 20, 2025 07:38 View session

Fix formatting issues in WinHttpRequestCallback.cs

be4a4e4

Co-authored-by: liveans <45091041+liveans@users.noreply.github.com>

Copilot finished work on behalf of liveans May 20, 2025 07:43

Copilot started work on behalf of liveans May 20, 2025 07:45 View session

Fix compatibility with .NET Framework by using GetLastWin32Error

c6a5f9d

Co-authored-by: liveans <45091041+liveans@users.noreply.github.com>

Copilot finished work on behalf of liveans May 20, 2025 07:49

krwq reviewed May 20, 2025

View reviewed changes

src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/FakeX509Certificates.cs Outdated Show resolved Hide resolved

Copilot started work on behalf of krwq May 20, 2025 07:58 View session

build-analysis bot mentioned this pull request May 20, 2025

Intermittent build failure in AfterSourceBuild: "Could not write state file" #76488

Open

Fix FakeX509Certificates.cs to clarify empty namespace

06fed96

Co-authored-by: krwq <660048+krwq@users.noreply.github.com>

Copilot finished work on behalf of krwq May 20, 2025 08:02

liveans reviewed May 20, 2025

View reviewed changes

...tem.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj Outdated Show resolved Hide resolved

src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/FakeX509Certificates.cs Outdated Show resolved Hide resolved

Copilot started work on behalf of liveans May 20, 2025 10:29 View session

Remove unnecessary code per review comments

33c9398

Co-authored-by: liveans <45091041+liveans@users.noreply.github.com>

Copilot finished work on behalf of liveans May 20, 2025 10:34

rzikm self-requested a review May 20, 2025 11:55

liveans marked this pull request as ready for review May 20, 2025 13:34

rzikm approved these changes May 20, 2025

View reviewed changes

build-analysis bot mentioned this pull request May 20, 2025

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

3 tasks

liveans merged commit 86f6fa9 into main May 20, 2025
82 of 89 checks passed

danmoseley deleted the copilot/fix-113468 branch May 22, 2025 16:21

Unify certificate chain building between SslStream and WinHttpHandler #115761

Unify certificate chain building between SslStream and WinHttpHandler #115761

Conversation

Copilot AI commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

liveans commented May 20, 2025

Uh oh!

azure-pipelines bot commented May 20, 2025

Uh oh!

dotnet-policy-service bot commented May 20, 2025

Uh oh!

liveans left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

liveans commented May 20, 2025

Uh oh!

Copilot AI commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liveans commented May 20, 2025

Uh oh!

azure-pipelines bot commented May 20, 2025

Uh oh!

rzikm left a comment

Choose a reason for hiding this comment

Uh oh!

liveans commented May 20, 2025

Uh oh!

liveans commented May 20, 2025

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented May 20, 2025 •

edited

Loading

Copilot AI commented May 20, 2025 •

edited

Loading

Copilot AI commented May 20, 2025 •

edited

Loading