Skip to content

Commit 97d260a

Browse files
authored
[Foundation] Make it possible to customize the X509ChainPolicy when validating certificates in NSUrlSessionHandler. Fixes #23764. (#23767)
* Add a `CertificateChainPolicy` property to `NSUrlSessionHandler` to make it possible for developers to customize the the policy that is used when validating certificate chains when using a custom server certificate validation. * Also implement `NSUrlSessionHandler.CheckCertificateRevocationList` using the new `CertificateChainPolicy` property. Fixes #23764.
1 parent 9c01ee4 commit 97d260a

File tree

2 files changed

+100
-12
lines changed

2 files changed

+100
-12
lines changed

src/Foundation/NSUrlSessionHandler.cs

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ public partial class NSUrlSessionHandler : HttpMessageHandler {
131131
NSObject? notificationToken; // needed to make sure we do not hang if not using a background session
132132
readonly object notificationTokenLock = new object (); // need to make sure that threads do no step on each other with a dispose and a remove inflight data
133133
#endif
134+
X509ChainPolicy? policy;
134135

135136
static NSUrlSessionConfiguration CreateConfig ()
136137
{
@@ -586,15 +587,47 @@ public DecompressionMethods AutomaticDecompression {
586587
set { }
587588
}
588589

589-
// We're ignoring this property, just like Xamarin.Android does:
590-
// https://github.com/xamarin/xamarin-android/blob/09e8cb5c07ea6c39383185a3f90e53186749b802/src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs#L158
591-
[UnsupportedOSPlatform ("ios")]
592-
[UnsupportedOSPlatform ("maccatalyst")]
593-
[UnsupportedOSPlatform ("tvos")]
594-
[UnsupportedOSPlatform ("macos")]
590+
/// <summary>Gets or sets a value that indicates whether the certificate is checked against the certificate authority revocation list.</summary>
591+
/// <remarks>
592+
/// <para>This is the same as setting CertificateChainPolicy.RevocationMode = X509RevocationMode.Online (if enabling the check) or X509RevocationMode.NoCheck (if disabling the check).</para>
593+
/// <para>This only has an effect if a custom server certificate validation callback is being used ('ServerCertificateCustomValidationCallback' is set).</para>
594+
/// </remarks>
595595
[EditorBrowsable (EditorBrowsableState.Never)]
596-
public bool CheckCertificateRevocationList { get; set; } = false;
596+
public bool CheckCertificateRevocationList {
597+
// This implementation was mostly copied from https://github.com/dotnet/runtime/blob/0e3562e97c6db531f26a2ffe3e8084cf67ba8a93/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs#L326-L335
598+
get => CertificateChainPolicy!.RevocationMode == X509RevocationMode.Online;
599+
set {
600+
EnsureModifiability ();
597601

602+
CertificateChainPolicy!.RevocationMode = value ? X509RevocationMode.Online : X509RevocationMode.NoCheck;
603+
}
604+
}
605+
606+
/// <summary>Gets or sets the custom chain policy to use when validating certificate chains.</summary>
607+
/// <remarks>
608+
/// <para>The getter will never return a <see langword="null" /> policy, it will return a policy configured with the default behavior.</para>
609+
/// <para>To select the default policy, call the setter with <see langword="null" /> value.</para>
610+
/// <para>This only has an effect if a custom server certificate validation callback is being used ('ServerCertificateCustomValidationCallback' is set).</para>
611+
/// </remarks>
612+
public X509ChainPolicy? CertificateChainPolicy {
613+
get {
614+
if (policy is null) {
615+
policy = new X509ChainPolicy () {
616+
RevocationMode = X509RevocationMode.Online,
617+
RevocationFlag = X509RevocationFlag.ExcludeRoot,
618+
// Ignore unknown revocation status, because Apple has a bug where revocation checks fail if the certificate(s)
619+
// in question don't support revocation checking via OCSP.
620+
// References:
621+
// * https://learn.microsoft.com/en-us/dotnet/core/compatibility/networking/10.0/ssl-certificate-revocation-check-default
622+
// * https://github.com/dotnet/macios/issues/23764#issuecomment-3264999234
623+
// * https://github.com/dotnet/runtime/issues/117195
624+
VerificationFlags = X509VerificationFlags.IgnoreEndRevocationUnknown,
625+
};
626+
}
627+
return policy;
628+
}
629+
set => policy = value;
630+
}
598631

599632
X509CertificateCollection? _clientCertificates;
600633

@@ -703,7 +736,7 @@ public IWebProxy? Proxy {
703736
if (value is null) {
704737
_serverCertificateCustomValidationCallbackHelper = null;
705738
} else {
706-
_serverCertificateCustomValidationCallbackHelper = new ServerCertificateCustomValidationCallbackHelper (value);
739+
_serverCertificateCustomValidationCallbackHelper = new ServerCertificateCustomValidationCallbackHelper (value, CertificateChainPolicy!);
707740
}
708741
}
709742
}
@@ -720,11 +753,13 @@ internal bool TryInvokeServerCertificateCustomValidationCallback (HttpRequestMes
720753
}
721754

722755
sealed class ServerCertificateCustomValidationCallbackHelper {
756+
X509ChainPolicy policy;
723757
public Func<HttpRequestMessage, X509Certificate2?, X509Chain?, SslPolicyErrors, bool> Callback { get; private set; }
724758

725-
public ServerCertificateCustomValidationCallbackHelper (Func<HttpRequestMessage, X509Certificate2?, X509Chain?, SslPolicyErrors, bool> callback)
759+
public ServerCertificateCustomValidationCallbackHelper (Func<HttpRequestMessage, X509Certificate2?, X509Chain?, SslPolicyErrors, bool> callback, X509ChainPolicy policy)
726760
{
727761
Callback = callback;
762+
this.policy = policy;
728763
}
729764

730765
public bool Invoke (HttpRequestMessage request, SecTrust secTrust)
@@ -759,10 +794,9 @@ X509Certificate2 [] ConvertCertificates (SecTrust secTrust)
759794

760795
X509Chain CreateChain (X509Certificate2 [] certificates)
761796
{
762-
// inspired by https://github.com/dotnet/runtime/blob/99d21b9276ebe8f7bea7fb3ba74dca9fca625fe2/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs#L691-L696
797+
// See https://github.com/dotnet/macios/issues/23764 for more information.
763798
var chain = new X509Chain ();
764-
chain.ChainPolicy.RevocationMode = X509RevocationMode.Online;
765-
chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot;
799+
chain.ChainPolicy = policy.Clone ();
766800
chain.ChainPolicy.ExtraStore.AddRange (certificates);
767801
return chain;
768802
}

tests/monotouch-test/System.Net.Http/MessageHandlers.cs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -900,5 +900,59 @@ public void RequestUriNotUpdatedIfNotRedirect (Type handlerType)
900900
Assert.IsNull (ex, "Exception");
901901
}
902902
}
903+
904+
// https://github.com/dotnet/macios/issues/23764
905+
[TestCase ("https://sha256.badssl.com/")]
906+
public void SslCertificatesWithoutOCSPEndPointsNSUrlSessionHandler_AllowByDefault (string url)
907+
{
908+
SslCertificatesWithoutOCSPEndPointsNSUrlSessionHandler (url, null, SslPolicyErrors.None);
909+
}
910+
911+
// https://github.com/dotnet/macios/issues/23764
912+
[TestCase ("https://sha256.badssl.com/")]
913+
public void SslCertificatesWithoutOCSPEndPointsNSUrlSessionHandler_Disallow (string url)
914+
{
915+
SslCertificatesWithoutOCSPEndPointsNSUrlSessionHandler (url, (X509VerificationFlags) 0, SslPolicyErrors.RemoteCertificateChainErrors);
916+
}
917+
918+
void SslCertificatesWithoutOCSPEndPointsNSUrlSessionHandler (string url, X509VerificationFlags? verificationFlags, SslPolicyErrors expectedError)
919+
{
920+
bool callbackWasExecuted = false;
921+
HttpResponseMessage result = null;
922+
X509Certificate2 serverCertificate = null;
923+
SslPolicyErrors sslPolicyErrors = SslPolicyErrors.None;
924+
925+
var handler = new NSUrlSessionHandler {
926+
ServerCertificateCustomValidationCallback = (request, certificate, chain, errors) => {
927+
callbackWasExecuted = true;
928+
serverCertificate = certificate;
929+
sslPolicyErrors = errors;
930+
return true;
931+
},
932+
};
933+
if (verificationFlags.HasValue)
934+
handler.CertificateChainPolicy.VerificationFlags = verificationFlags.Value;
935+
936+
Assert.IsTrue (handler.CheckCertificateRevocationList, "CheckCertificateRevocationList");
937+
938+
var done = TestRuntime.TryRunAsync (TimeSpan.FromSeconds (30), async () => {
939+
var client = new HttpClient (handler);
940+
// Disable keep-alive and cache to force reconnection for each request
941+
client.DefaultRequestHeaders.ConnectionClose = true;
942+
client.DefaultRequestHeaders.CacheControl = new CacheControlHeaderValue { NoCache = true, NoStore = true };
943+
result = await client.GetAsync (url);
944+
}, out var ex);
945+
946+
if (!done) { // timeouts happen in the bots due to dns issues, connection issues etc., we do not want to fail
947+
Assert.Inconclusive ("Request timedout.");
948+
} else {
949+
Assert.True (callbackWasExecuted, "Validation Callback called");
950+
Assert.AreEqual (expectedError, sslPolicyErrors, "Callback was called with unexpected SslPolicyErrors");
951+
Assert.IsNotNull (serverCertificate, "Server certificate is null");
952+
Assert.IsNull (ex, "Exception wasn't expected.");
953+
Assert.IsNotNull (result, "Result was null");
954+
Assert.IsTrue (result.IsSuccessStatusCode, $"Status code was not success: {result.StatusCode}");
955+
}
956+
}
903957
}
904958
}

0 commit comments

Comments
 (0)