From 7171baaeca76bd26cc2fd2d5458951ef171b85eb Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 17 Oct 2022 10:14:39 +0200 Subject: [PATCH] [Foundation] Implement the server certificate custom validation callback usage in NSUrlSessionHandler (#15117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We recently implemented the `ServerCertificateCustomValidationCallback` in Xamarin.Android (https://github.com/xamarin/xamarin-android/pull/6665). It would be great to have feature parity and support the same callback in Xamarin.iOS and Xamarin.Mac. Related to https://github.com/dotnet/runtime/issues/68898. Partial fix for https://github.com/xamarin/xamarin-macios/issues/14632. Co-authored-by: Alexander Köplinger Co-authored-by: Rolf Bjarne Kvinge --- src/Foundation/NSUrlSessionHandler.cs | 143 +++++++++++++++--- .../System.Net.Http/MessageHandlers.cs | 93 ++++++++++++ 2 files changed, 218 insertions(+), 18 deletions(-) diff --git a/src/Foundation/NSUrlSessionHandler.cs b/src/Foundation/NSUrlSessionHandler.cs index 9dfc718b91..18864dc2bd 100644 --- a/src/Foundation/NSUrlSessionHandler.cs +++ b/src/Foundation/NSUrlSessionHandler.cs @@ -639,13 +639,114 @@ namespace Foundation { [UnsupportedOSPlatform ("macos")] public SslProtocols SslProtocols { get; set; } = SslProtocols.Tls12 | SslProtocols.Tls13; - // We're ignoring this property, just like Xamarin.Android does: - // https://github.com/xamarin/xamarin-android/blob/09e8cb5c07ea6c39383185a3f90e53186749b802/src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs#L160 - [UnsupportedOSPlatform ("ios")] - [UnsupportedOSPlatform ("maccatalyst")] - [UnsupportedOSPlatform ("tvos")] - [UnsupportedOSPlatform ("macos")] - public Func? ServerCertificateCustomValidationCallback { get; set; } + private ServerCertificateCustomValidationCallbackHelper? _serverCertificateCustomValidationCallbackHelper; + public Func? ServerCertificateCustomValidationCallback { + get => _serverCertificateCustomValidationCallbackHelper?.Callback; + set { + if (value is null) { + _serverCertificateCustomValidationCallbackHelper = null; + } else { + _serverCertificateCustomValidationCallbackHelper = new ServerCertificateCustomValidationCallbackHelper (value); + } + } + } + + // returns false if there's no callback + internal bool TryInvokeServerCertificateCustomValidationCallback (HttpRequestMessage request, SecTrust secTrust, out bool trusted) + { + trusted = false; + var helper = _serverCertificateCustomValidationCallbackHelper; + if (helper is null) + return false; + trusted = helper.Invoke (request, secTrust); + return true; + } + + sealed class ServerCertificateCustomValidationCallbackHelper + { + public Func Callback { get; private set; } + + public ServerCertificateCustomValidationCallbackHelper (Func callback) + { + Callback = callback; + } + + public bool Invoke (HttpRequestMessage request, SecTrust secTrust) + { + X509Certificate2[] certificates = ConvertCertificates (secTrust); + X509Certificate2? certificate = certificates.Length > 0 ? certificates [0] : null; + using X509Chain chain = CreateChain (certificates); + SslPolicyErrors sslPolicyErrors = EvaluateSslPolicyErrors (certificate, chain, secTrust); + + return Callback (request, certificate, chain, sslPolicyErrors); + } + + X509Certificate2[] ConvertCertificates (SecTrust secTrust) + { + var certificates = new X509Certificate2 [secTrust.Count]; + + if (IsSecTrustGetCertificateChainSupported) { + var originalChain = secTrust.GetCertificateChain (); + for (int i = 0; i < originalChain.Length; i++) + certificates [i] = originalChain [i].ToX509Certificate2 (); + } else { + for (int i = 0; i < secTrust.Count; i++) + certificates [i] = secTrust [i].ToX509Certificate2 (); + } + + return certificates; + } + + static bool? isSecTrustGetCertificateChainSupported = null; + static bool IsSecTrustGetCertificateChainSupported { + get { + if (!isSecTrustGetCertificateChainSupported.HasValue) { +#if MONOMAC + isSecTrustGetCertificateChainSupported = ObjCRuntime.SystemVersion.CheckmacOS (12, 0); +#elif WATCH + isSecTrustGetCertificateChainSupported = ObjCRuntime.SystemVersion.CheckWatchOS (8, 0); +#elif IOS || TVOS || MACCATALYST + isSecTrustGetCertificateChainSupported = ObjCRuntime.SystemVersion.CheckiOS (15, 0); +#else + #error Unknown platform +#endif + } + + return isSecTrustGetCertificateChainSupported.Value; + } + } + + X509Chain CreateChain (X509Certificate2[] certificates) + { + // inspired by https://github.com/dotnet/runtime/blob/99d21b9276ebe8f7bea7fb3ba74dca9fca625fe2/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs#L691-L696 + var chain = new X509Chain (); + chain.ChainPolicy.RevocationMode = X509RevocationMode.Online; + chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot; + chain.ChainPolicy.ExtraStore.AddRange (certificates); + return chain; + } + + SslPolicyErrors EvaluateSslPolicyErrors (X509Certificate2? certificate, X509Chain chain, SecTrust secTrust) + { + var sslPolicyErrors = SslPolicyErrors.None; + + try { + if (certificate is null) { + sslPolicyErrors |= SslPolicyErrors.RemoteCertificateNotAvailable; + } else if (!chain.Build (certificate)) { + sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors; + } + } catch (ArgumentException) { + sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors; + } + + if (!secTrust.Evaluate (out _)) { + sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors; + } + + return sslPolicyErrors; + } + } // There's no way to turn off automatic decompression, so yes, we support it public bool SupportsAutomaticDecompression { @@ -879,26 +980,32 @@ namespace Foundation { return; // ToCToU for the callback + var trustCallbackForUrl = sessionHandler.TrustOverrideForUrl; + var trustSec = false; + var usedCallback = false; #if !NET var trustCallback = sessionHandler.TrustOverride; -#endif - var trustCallbackForUrl = sessionHandler.TrustOverrideForUrl; -#if NET - var hasCallBack = trustCallbackForUrl is not null; -#else var hasCallBack = trustCallback is not null || trustCallbackForUrl is not null; -#endif if (hasCallBack && challenge.ProtectionSpace.AuthenticationMethod == NSUrlProtectionSpace.AuthenticationMethodServerTrust) { -#if NET - // if the trust delegate allows to ignore the cert, do it. Since we are using nullables, if the delegate is not present, by default is false - var trustSec = (trustCallbackForUrl?.Invoke (sessionHandler, inflight.RequestUrl, challenge.ProtectionSpace.ServerSecTrust) ?? false); -#else // if one of the delegates allows to ignore the cert, do it. We check first the one that takes the url because is more precisse, later the // more general one. Since we are using nullables, if the delegate is not present, by default is false - var trustSec = (trustCallbackForUrl?.Invoke (sessionHandler, inflight.RequestUrl, challenge.ProtectionSpace.ServerSecTrust) ?? false) || + trustSec = (trustCallbackForUrl?.Invoke (sessionHandler, inflight.RequestUrl, challenge.ProtectionSpace.ServerSecTrust) ?? false) || (trustCallback?.Invoke (sessionHandler, challenge.ProtectionSpace.ServerSecTrust) ?? false); + usedCallback = true; + } +#else + if (challenge.ProtectionSpace.AuthenticationMethod == NSUrlProtectionSpace.AuthenticationMethodServerTrust) { + // if the trust delegate allows to ignore the cert, do it. Since we are using nullables, if the delegate is not present, by default is false + if (trustCallbackForUrl is not null) { + trustSec = trustCallbackForUrl (sessionHandler, inflight.RequestUrl, challenge.ProtectionSpace.ServerSecTrust); + usedCallback = true; + } else if (sessionHandler.TryInvokeServerCertificateCustomValidationCallback (inflight.Request, challenge.ProtectionSpace.ServerSecTrust, out trustSec)) { + usedCallback = true; + } + } #endif + if (usedCallback) { if (trustSec) { var credential = new NSUrlCredential (challenge.ProtectionSpace.ServerSecTrust); completionHandler (NSUrlSessionAuthChallengeDisposition.UseCredential, credential); diff --git a/tests/monotouch-test/System.Net.Http/MessageHandlers.cs b/tests/monotouch-test/System.Net.Http/MessageHandlers.cs index 2090ea2d33..012e201544 100644 --- a/tests/monotouch-test/System.Net.Http/MessageHandlers.cs +++ b/tests/monotouch-test/System.Net.Http/MessageHandlers.cs @@ -11,6 +11,7 @@ using System.Net; using System.Net.Http; #if NET using System.Net.Security; +using System.Security.Cryptography.X509Certificates; #endif using System.Linq; using System.IO; @@ -546,6 +547,98 @@ namespace MonoTests.System.Net.Http } } +#if NET + [TestCase ("https://self-signed.badssl.com/")] + [TestCase ("https://wrong.host.badssl.com/")] + public void AcceptSslCertificatesWithCustomValidationCallbackNSUrlSessionHandler (string url) + { + TestRuntime.AssertSystemVersion (ApplePlatform.MacOSX, 10, 9, throwIfOtherPlatform: false); + TestRuntime.AssertSystemVersion (ApplePlatform.iOS, 7, 0, throwIfOtherPlatform: false); + + bool callbackWasExecuted = false; + bool done = false; + Exception ex = null; + HttpResponseMessage result = null; + X509Certificate2 serverCertificate = null; + SslPolicyErrors sslPolicyErrors = SslPolicyErrors.None; + + var handler = new NSUrlSessionHandler { + ServerCertificateCustomValidationCallback = (request, certificate, chain, errors) => { + callbackWasExecuted = true; + serverCertificate = certificate; + sslPolicyErrors = errors; + return true; + } + }; + + TestRuntime.RunAsync (DateTime.Now.AddSeconds (30), async () => + { + try { + var client = new HttpClient (handler); + result = await client.GetAsync (url); + } catch (Exception e) { + ex = e; + } finally { + done = true; + } + }, () => done); + + if (!done) { // timeouts happen in the bots due to dns issues, connection issues etc.. we do not want to fail + Assert.Inconclusive ("Request timedout."); + } else { + Assert.True (callbackWasExecuted, "Validation Callback called"); + Assert.AreNotEqual (SslPolicyErrors.None, sslPolicyErrors, "Callback was called with unexpected SslPolicyErrors"); + Assert.IsNotNull (serverCertificate, "Server certificate is null"); + Assert.IsNull (ex, "Exception wasn't expected."); + Assert.IsNotNull (result, "Result was null"); + Assert.IsTrue (result.IsSuccessStatusCode, "Status code was not success"); + } + } + + [TestCase ("https://www.microsoft.com/")] + public void RejectSslCertificatesWithCustomValidationCallbackNSUrlSessionHandler (string url) + { + TestRuntime.AssertSystemVersion (ApplePlatform.MacOSX, 10, 9, throwIfOtherPlatform: false); + TestRuntime.AssertSystemVersion (ApplePlatform.iOS, 7, 0, throwIfOtherPlatform: false); + + bool callbackWasExecuted = false; + bool done = false; + Exception ex = null; + HttpResponseMessage result = null; + + var handler = new NSUrlSessionHandler { + ServerCertificateCustomValidationCallback = (sender, certificate, chain, errors) => { + callbackWasExecuted = true; + Assert.IsNotNull (certificate); + Assert.AreEqual (SslPolicyErrors.None, errors); + return false; + } + }; + + TestRuntime.RunAsync (DateTime.Now.AddSeconds (30), async () => + { + try { + var client = new HttpClient (handler); + result = await client.GetAsync (url); + } catch (Exception e) { + ex = e; + } finally { + done = true; + } + }, () => done); + + if (!done) { // timeouts happen in the bots due to dns issues, connection issues etc.. we do not want to fail + Assert.Inconclusive ("Request timedout."); + } else { + Assert.True (callbackWasExecuted, "Validation Callback called."); + Assert.IsNotNull (ex, result == null ? "Expected exception is missing and got no result." : $"Expected exception but got {result.Content.ReadAsStringAsync ().Result}."); + Assert.IsInstanceOf (typeof (HttpRequestException), ex, "Exception type"); + Assert.IsNotNull (ex.InnerException, "InnerException"); + Assert.IsInstanceOf (typeof (WebException), ex.InnerException, "InnerException type"); + } + } +#endif + [Test] public void AssertDefaultValuesNSUrlSessionHandler () {