Skip to content

[Mono.Android] Fix ServerCertificateCustomValidator #8594

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 3, 2024
Merged

[Mono.Android] Fix ServerCertificateCustomValidator #8594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d46ba6
Prevent IX509TrustManager classes from being trimmed
simonrozsival Dec 14, 2023
d4823a9
Add linker test
simonrozsival Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 13 additions & 16 deletions src/Mono.Android/Xamarin.Android.Net/ServerCertificateCustomValidator.cs
View file
Open in desktop
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))]
Comment on lines +162 to +163
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the use of these dependencies - I assume we have a suppression for it.
I'm just curious why we can't solve this any other way...

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
34 changes: 34 additions & 0 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,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