Skip to content

Commit 940f059

Browse files
simonrozsivaljonathanpeppers
authored andcommitted
[Mono.Android] Fix ServerCertificateCustomValidator (#8594)
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
1 parent 351bfa3 commit 940f059

File tree

2 files changed

+47
-16
lines changed

2 files changed

+47
-16
lines changed

src/Mono.Android/Xamarin.Android.Net/ServerCertificateCustomValidator.cs

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Diagnostics.CodeAnalysis;
34
using System.Net.Http;
45
using System.Net.Security;
56
using System.Security.Cryptography.X509Certificates;
@@ -31,12 +32,12 @@ public ITrustManager[] ReplaceX509TrustManager (ITrustManager[]? trustManagers,
3132

3233
private sealed class TrustManager : Java.Lang.Object, IX509TrustManager
3334
{
34-
private readonly IX509TrustManager? _internalTrustManager;
35+
private readonly IX509TrustManager _internalTrustManager;
3536
private readonly HttpRequestMessage _request;
3637
private readonly Func<HttpRequestMessage, X509Certificate2?, X509Chain?, SslPolicyErrors, bool> _serverCertificateCustomValidationCallback;
3738

3839
public TrustManager (
39-
IX509TrustManager? internalTrustManager,
40+
IX509TrustManager internalTrustManager,
4041
HttpRequestMessage request,
4142
Func<HttpRequestMessage, X509Certificate2?, X509Chain?, SslPolicyErrors, bool> serverCertificateCustomValidationCallback)
4243
{
@@ -50,7 +51,7 @@ public void CheckServerTrusted (JavaX509Certificate[] javaChain, string authType
5051
var sslPolicyErrors = SslPolicyErrors.None;
5152

5253
try {
53-
_internalTrustManager?.CheckServerTrusted (javaChain, authType);
54+
_internalTrustManager.CheckServerTrusted (javaChain, authType);
5455
} catch (JavaCertificateException) {
5556
sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors;
5657
}
@@ -158,33 +159,29 @@ private sealed class AlwaysAcceptingHostnameVerifier : Java.Lang.Object, IHostna
158159
public bool Verify (string? hostname, ISSLSession? session) => true;
159160
}
160161

161-
private static IX509TrustManager? FindX509TrustManager(ITrustManager[]? trustManagers)
162+
[DynamicDependency(nameof(IX509TrustManager.CheckServerTrusted), typeof(IX509TrustManagerInvoker))]
163+
[DynamicDependency(nameof(IX509TrustManager.CheckServerTrusted), typeof(X509ExtendedTrustManagerInvoker))]
164+
private static IX509TrustManager FindX509TrustManager(ITrustManager[] trustManagers)
162165
{
163-
if (trustManagers is null)
164-
return null;
165-
166166
foreach (var trustManager in trustManagers) {
167167
if (trustManager is IX509TrustManager tm)
168168
return tm;
169169
}
170170

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

174-
private static ITrustManager[] ModifyTrustManagersArray (ITrustManager[] trustManagers, IX509TrustManager? original, IX509TrustManager replacement)
174+
private static ITrustManager[] ModifyTrustManagersArray (ITrustManager[] trustManagers, IX509TrustManager original, IX509TrustManager replacement)
175175
{
176-
var modifiedTrustManagersCount = original is null ? trustManagers.Length + 1 : trustManagers.Length;
177-
var modifiedTrustManagersArray = new ITrustManager [modifiedTrustManagersCount];
178-
179-
modifiedTrustManagersArray [0] = replacement;
180-
int nextIndex = 1;
176+
var modifiedTrustManagersArray = new ITrustManager [trustManagers.Length];
181177

182178
for (int i = 0; i < trustManagers.Length; i++) {
183179
if (trustManagers [i] == original) {
184-
continue;
180+
modifiedTrustManagersArray [i] = replacement;
181+
} else {
182+
modifiedTrustManagersArray [i] = trustManagers [i];
185183
}
186184

187-
modifiedTrustManagersArray [nextIndex++] = trustManagers [i];
188185
}
189186

190187
return modifiedTrustManagersArray;

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,40 @@ public void AndroidUseNegotiateAuthentication ([Values (true, false, null)] bool
516516
}
517517
}
518518

519+
[Test]
520+
public void PreserveIX509TrustManagerSubclasses ([Values(true, false)] bool hasServerCertificateCustomValidationCallback)
521+
{
522+
var proj = new XamarinAndroidApplicationProject { IsRelease = true };
523+
proj.AddReferences ("System.Net.Http");
524+
proj.MainActivity = proj.DefaultMainActivity.Replace (
525+
"base.OnCreate (bundle);",
526+
"base.OnCreate (bundle);\n" +
527+
(hasServerCertificateCustomValidationCallback
528+
? "var handler = new Xamarin.Android.Net.AndroidMessageHandler { ServerCertificateCustomValidationCallback = (message, certificate, chain, errors) => true };\n"
529+
: "var handler = new Xamarin.Android.Net.AndroidMessageHandler();\n") +
530+
"var client = new System.Net.Http.HttpClient (handler);\n" +
531+
"client.GetAsync (\"https://microsoft.com\").GetAwaiter ().GetResult ();");
532+
533+
using (var b = CreateApkBuilder ()) {
534+
Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
535+
var assemblyPath = BuildTest.GetLinkedPath (b, true, "Mono.Android.dll");
536+
537+
using (var assembly = AssemblyDefinition.ReadAssembly (assemblyPath)) {
538+
Assert.IsTrue (assembly != null);
539+
540+
var types = new[] { "Javax.Net.Ssl.X509ExtendedTrustManager", "Javax.Net.Ssl.IX509TrustManagerInvoker" };
541+
foreach (var typeName in types) {
542+
var td = assembly.MainModule.GetType (typeName);
543+
if (hasServerCertificateCustomValidationCallback) {
544+
Assert.IsNotNull (td, $"{typeName} shouldn't have been linked out");
545+
} else {
546+
Assert.IsNull (td, $"{typeName} should have been linked out");
547+
}
548+
}
549+
}
550+
}
551+
}
552+
519553
[Test]
520554
public void DoNotErrorOnPerArchJavaTypeDuplicates ([Values(true, false)] bool enableMarshalMethods)
521555
{

0 commit comments

Comments
 (0)