Skip to content

Commit 86f6fa9

Browse files
Copilotliveanskrwq
authored
Unify certificate chain building between SslStream and WinHttpHandler (#115761)
* Initial plan for issue * Update WinHttpRequestCallback to use CertificateValidation Co-authored-by: liveans <[email protected]> * Remove unused WinHttpCertificateHelper.cs file Co-authored-by: liveans <[email protected]> * Fix formatting issues in WinHttpRequestCallback.cs Co-authored-by: liveans <[email protected]> * Fix compatibility with .NET Framework by using GetLastWin32Error Co-authored-by: liveans <[email protected]> * Fix FakeX509Certificates.cs to clarify empty namespace Co-authored-by: krwq <[email protected]> * Remove unnecessary code per review comments Co-authored-by: liveans <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: liveans <[email protected]> Co-authored-by: krwq <[email protected]>
1 parent de76a8c commit 86f6fa9

File tree

5 files changed

+40
-120
lines changed

5 files changed

+40
-120
lines changed

src/libraries/Common/src/System/Net/Security/CertificateValidation.Windows.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ internal static SslPolicyErrors BuildChainAndVerifyProperties(X509Chain chain, X
2626
if (!chainBuildResult // Build failed on handle or on policy.
2727
&& chain.SafeHandle!.DangerousGetHandle() == IntPtr.Zero) // Build failed to generate a valid handle.
2828
{
29+
#if NETFRAMEWORK
30+
throw new CryptographicException(Marshal.GetLastWin32Error());
31+
#else
2932
throw new CryptographicException(Marshal.GetLastPInvokeError());
33+
#endif
3034
}
3135

3236
if (checkCertName)

src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ System.Net.Http.WinHttpHandler</PackageDescription>
7272
Link="Common\System\Net\Security\CertificateHelper.cs" />
7373
<Compile Include="$(CommonPath)\System\Net\Security\CertificateHelper.Windows.cs"
7474
Link="Common\System\Net\Security\CertificateHelper.Windows.cs" />
75+
<Compile Include="$(CommonPath)\System\Net\Security\CertificateValidation.Windows.cs"
76+
Link="Common\System\Net\Security\CertificateValidation.Windows.cs" />
7577
<Compile Include="$(CommonPath)\System\Runtime\ExceptionServices\ExceptionStackTrace.cs"
7678
Link="Common\System\Runtime\ExceptionServices\ExceptionStackTrace.cs" />
7779
<Compile Include="$(CommonPath)\System\Threading\Tasks\RendezvousAwaitable.cs"
@@ -80,7 +82,6 @@ System.Net.Http.WinHttpHandler</PackageDescription>
8082
<Compile Include="System\Net\Http\NetEventSource.WinHttpHandler.cs" />
8183
<Compile Include="System\Net\Http\NoWriteNoSeekStreamContent.cs" />
8284
<Compile Include="System\Net\Http\WinHttpAuthHelper.cs" />
83-
<Compile Include="System\Net\Http\WinHttpCertificateHelper.cs" />
8485
<Compile Include="System\Net\Http\WinHttpChannelBinding.cs" />
8586
<Compile Include="System\Net\Http\WinHttpChunkMode.cs" />
8687
<Compile Include="System\Net\Http\WinHttpCookieContainerAdapter.cs" />

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

Lines changed: 0 additions & 102 deletions
This file was deleted.

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

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using System.Net.Sockets;
1111
using System.Runtime.CompilerServices;
1212
using System.Runtime.InteropServices;
13+
using System.Security.Cryptography;
1314
using System.Security.Cryptography.X509Certificates;
1415
using System.Threading;
1516
using SafeWinHttpHandle = Interop.WinHttp.SafeWinHttpHandle;
@@ -21,6 +22,8 @@ namespace System.Net.Http
2122
/// </summary>
2223
internal static class WinHttpRequestCallback
2324
{
25+
private static readonly Oid ServerAuthOid = new Oid("1.3.6.1.5.5.7.3.1", "1.3.6.1.5.5.7.3.1");
26+
2427
public static Interop.WinHttp.WINHTTP_STATUS_CALLBACK StaticCallbackDelegate =
2528
new Interop.WinHttp.WINHTTP_STATUS_CALLBACK(WinHttpCallback);
2629

@@ -370,13 +373,34 @@ private static void OnRequestSendingRequest(WinHttpRequestState state)
370373

371374
try
372375
{
373-
WinHttpCertificateHelper.BuildChain(
376+
// Create and configure the X509Chain
377+
chain = new X509Chain();
378+
chain.ChainPolicy.RevocationMode = state.CheckCertificateRevocationList ? X509RevocationMode.Online : X509RevocationMode.NoCheck;
379+
chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot;
380+
// Authenticate the remote party: (e.g. when operating in client mode, authenticate the server).
381+
chain.ChainPolicy.ApplicationPolicy.Add(ServerAuthOid);
382+
383+
if (remoteCertificateStore.Count > 0)
384+
{
385+
if (NetEventSource.Log.IsEnabled())
386+
{
387+
foreach (X509Certificate cert in remoteCertificateStore)
388+
{
389+
NetEventSource.Info(remoteCertificateStore, $"Adding cert to ExtraStore: {cert.Subject}");
390+
}
391+
}
392+
393+
chain.ChainPolicy.ExtraStore.AddRange(remoteCertificateStore);
394+
}
395+
396+
// Call the shared BuildChainAndVerifyProperties method
397+
// isServer=false because WinHttpHandler is a client validating a server certificate
398+
sslPolicyErrors = System.Net.CertificateValidation.BuildChainAndVerifyProperties(
399+
chain,
374400
serverCertificate,
375-
remoteCertificateStore,
376-
state.RequestMessage.RequestUri.Host,
377-
state.CheckCertificateRevocationList,
378-
out chain,
379-
out sslPolicyErrors);
401+
checkCertName: true,
402+
isServer: false,
403+
hostName: state.RequestMessage.RequestUri.Host);
380404

381405
result = state.ServerCertificateValidationCallback(
382406
state.RequestMessage,

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

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,13 @@
66
using System.Net.Security;
77
using System.Security.Cryptography.X509Certificates;
88

9-
namespace System.Net.Http
9+
namespace System.Net
1010
{
11-
internal static class WinHttpCertificateHelper
11+
internal static partial class CertificateValidation
1212
{
13-
public static void BuildChain(
14-
X509Certificate2 certificate,
15-
X509Certificate2Collection remoteCertificateStore,
16-
string hostName,
17-
bool checkCertificateRevocationList,
18-
out X509Chain chain,
19-
out SslPolicyErrors sslPolicyErrors)
13+
internal static SslPolicyErrors BuildChainAndVerifyProperties(X509Chain chain, X509Certificate2 remoteCertificate, bool checkCertName, bool isServer, string? hostName)
2014
{
21-
chain = null;
22-
sslPolicyErrors = SslPolicyErrors.None;
15+
return SslPolicyErrors.None;
2316
}
2417
}
2518
}

0 commit comments

Comments
 (0)