Skip to content

Commit

Permalink
[Mono.Android] Fix ServerCertificateCustomValidator (#8594)
Browse files Browse the repository at this point in the history
Fixes: dotnet/runtime#95506

In Release configuration the `X509ExtendedTrustManagerInvoker` class is trimmed and so 
the `trustManager is IX509TrustManager tm` pattern matching doesn't work. 

This PR addresses the problem in two ways:

    * an internal X509 trust manager is now required - it can't silently work with a null 
       internal trust manager anymore
    * `[DynamicDependency]` attribute to prevent trimming of the invoker classes for 
       the `IX509TrustManager` interface and for the `X509ExtendedTrustManager` 
       abstract class
  • Loading branch information
simonrozsival authored and jonathanpeppers committed Jan 17, 2024
1 parent 351bfa3 commit 940f059
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Net.Http;
using System.Net.Security;
using System.Security.Cryptography.X509Certificates;
Expand Down Expand Up @@ -31,12 +32,12 @@ public ITrustManager[] ReplaceX509TrustManager (ITrustManager[]? trustManagers,

private sealed class TrustManager : Java.Lang.Object, IX509TrustManager
{
private readonly IX509TrustManager? _internalTrustManager;
private readonly IX509TrustManager _internalTrustManager;
private readonly HttpRequestMessage _request;
private readonly Func<HttpRequestMessage, X509Certificate2?, X509Chain?, SslPolicyErrors, bool> _serverCertificateCustomValidationCallback;

public TrustManager (
IX509TrustManager? internalTrustManager,
IX509TrustManager internalTrustManager,
HttpRequestMessage request,
Func<HttpRequestMessage, X509Certificate2?, X509Chain?, SslPolicyErrors, bool> serverCertificateCustomValidationCallback)
{
Expand All @@ -50,7 +51,7 @@ public void CheckServerTrusted (JavaX509Certificate[] javaChain, string authType
var sslPolicyErrors = SslPolicyErrors.None;

try {
_internalTrustManager?.CheckServerTrusted (javaChain, authType);
_internalTrustManager.CheckServerTrusted (javaChain, authType);
} catch (JavaCertificateException) {
sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors;
}
Expand Down Expand Up @@ -158,33 +159,29 @@ private sealed class AlwaysAcceptingHostnameVerifier : Java.Lang.Object, IHostna
public bool Verify (string? hostname, ISSLSession? session) => true;
}

private static IX509TrustManager? FindX509TrustManager(ITrustManager[]? trustManagers)
[DynamicDependency(nameof(IX509TrustManager.CheckServerTrusted), typeof(IX509TrustManagerInvoker))]
[DynamicDependency(nameof(IX509TrustManager.CheckServerTrusted), typeof(X509ExtendedTrustManagerInvoker))]
private static IX509TrustManager FindX509TrustManager(ITrustManager[] trustManagers)
{
if (trustManagers is null)
return null;

foreach (var trustManager in trustManagers) {
if (trustManager is IX509TrustManager tm)
return tm;
}

return null;
throw new InvalidOperationException($"Could not find {nameof(IX509TrustManager)} in {nameof(ITrustManager)} array.");
}

private static ITrustManager[] ModifyTrustManagersArray (ITrustManager[] trustManagers, IX509TrustManager? original, IX509TrustManager replacement)
private static ITrustManager[] ModifyTrustManagersArray (ITrustManager[] trustManagers, IX509TrustManager original, IX509TrustManager replacement)
{
var modifiedTrustManagersCount = original is null ? trustManagers.Length + 1 : trustManagers.Length;
var modifiedTrustManagersArray = new ITrustManager [modifiedTrustManagersCount];

modifiedTrustManagersArray [0] = replacement;
int nextIndex = 1;
var modifiedTrustManagersArray = new ITrustManager [trustManagers.Length];

for (int i = 0; i < trustManagers.Length; i++) {
if (trustManagers [i] == original) {
continue;
modifiedTrustManagersArray [i] = replacement;
} else {
modifiedTrustManagersArray [i] = trustManagers [i];
}

modifiedTrustManagersArray [nextIndex++] = trustManagers [i];
}

return modifiedTrustManagersArray;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,40 @@ public void AndroidUseNegotiateAuthentication ([Values (true, false, null)] bool
}
}

[Test]
public void PreserveIX509TrustManagerSubclasses ([Values(true, false)] bool hasServerCertificateCustomValidationCallback)
{
var proj = new XamarinAndroidApplicationProject { IsRelease = true };
proj.AddReferences ("System.Net.Http");
proj.MainActivity = proj.DefaultMainActivity.Replace (
"base.OnCreate (bundle);",
"base.OnCreate (bundle);\n" +
(hasServerCertificateCustomValidationCallback
? "var handler = new Xamarin.Android.Net.AndroidMessageHandler { ServerCertificateCustomValidationCallback = (message, certificate, chain, errors) => true };\n"
: "var handler = new Xamarin.Android.Net.AndroidMessageHandler();\n") +
"var client = new System.Net.Http.HttpClient (handler);\n" +
"client.GetAsync (\"https://microsoft.com\").GetAwaiter ().GetResult ();");

using (var b = CreateApkBuilder ()) {
Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
var assemblyPath = BuildTest.GetLinkedPath (b, true, "Mono.Android.dll");

using (var assembly = AssemblyDefinition.ReadAssembly (assemblyPath)) {
Assert.IsTrue (assembly != null);

var types = new[] { "Javax.Net.Ssl.X509ExtendedTrustManager", "Javax.Net.Ssl.IX509TrustManagerInvoker" };
foreach (var typeName in types) {
var td = assembly.MainModule.GetType (typeName);
if (hasServerCertificateCustomValidationCallback) {
Assert.IsNotNull (td, $"{typeName} shouldn't have been linked out");
} else {
Assert.IsNull (td, $"{typeName} should have been linked out");
}
}
}
}
}

[Test]
public void DoNotErrorOnPerArchJavaTypeDuplicates ([Values(true, false)] bool enableMarshalMethods)
{
Expand Down

0 comments on commit 940f059

Please sign in to comment.