Skip to content
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

Jump to bottom

[Mono.Android] Fix ServerCertificateCustomValidator #8594

Merged
merged 2 commits into from
Jan 3, 2024
Merged

[Mono.Android] Fix ServerCertificateCustomValidator #8594

merged 2 commits into from
Jan 3, 2024

Conversation

simonrozsival
Copy link
Member

@simonrozsival simonrozsival commented Dec 14, 2023 •
edited
Loading

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

/cc @grendello

@jonathanpeppers
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@grendello grendello merged commit dc9c978 into dotnet:main Jan 3, 2024
45 of 47 checks passed
grendello added a commit that referenced this pull request Jan 8, 2024
@grendello
Merge branch 'main' into dev/grendel/blobs-in-lib
80b9c64 
* main:
  [tests] fix `MAUI Integration` lane (#8614)
  [xaprepare] use `$(XAPackagesDir)` for NuGet folder (#8613)
  Add ApiScan to nightly build (#8605)
  Bump to dotnet/installer@29db8a1157 9.0.100-alpha.1.23628.5 (#8611)
  [Mono.Android] Fix ServerCertificateCustomValidator (#8594)
  Bump to dotnet/installer@e08874da58 9.0.100-alpha.1.23625.1
  Bump to dotnet/installer@fae23c6571 9.0.100-alpha.1.23618.2
  Localized file check-in by OneLocBuild Task (#8589)
  [tests] enable Xamarin.Android.LibraryProjectZip-LibBinding test (#8588)
@jpobst
Copy link
Contributor

jpobst commented Jan 10, 2024

This appears to have broken our nightly tests:
https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=8899873&view=ms.vss-test-web.build-test-results-tab

Mono.Android.NET_Tests, Xamarin.Android.NetTests.AndroidMessageHandlerTests.ServerCertificateCustomValidationCallback_ApproveRequest / Release

System.InvalidOperationException : Could not find IX509TrustManager in ITrustManager array.

   at Xamarin.Android.Net.ServerCertificateCustomValidator.FindX509TrustManager(ITrustManager[] )
   at Xamarin.Android.Net.ServerCertificateCustomValidator.ReplaceX509TrustManager(ITrustManager[] , HttpRequestMessage )
   at Xamarin.Android.Net.AndroidMessageHandler.SetupSSL(HttpsURLConnection , HttpRequestMessage )
   at Xamarin.Android.Net.AndroidMessageHandler.SetupRequestInternal(HttpRequestMessage , URLConnection )
   at Xamarin.Android.Net.AndroidMessageHandler.DoSendAsync(HttpRequestMessage , CancellationToken )
   at Xamarin.Android.Net.AndroidMessageHandler.SendWithNegotiateAuthenticationAsync(HttpRequestMessage , CancellationToken )
   at System.Net.Http.HttpClient.GetStringAsyncCore(HttpRequestMessage , CancellationToken )
   at NUnit.Framework.Internal.AsyncInvocationRegion.AsyncTaskInvocationRegion.WaitForPendingOperationsToComplete(Object invocationResult)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunAsyncTestMethod(TestExecutionContext context)

This test fails on API-21 and API-23, but not on API-24+, so it is likely something that is handled differently between the two?

@simonrozsival simonrozsival mentioned this pull request Jan 15, 2024
Merged
grendello pushed a commit that referenced this pull request Jan 16, 2024
@simonrozsival
Fix ServerCertificateCustomValidator on API 21-23 (#8637)
64eae1e 
Follow-up to #8594 to fix dotnet/runtime#95506 on Android API 21-23
grendello pushed a commit that referenced this pull request Jan 17, 2024
@simonrozsival @grendello
Fix ServerCertificateCustomValidator on API 21-23 (#8637)
1c9c698 
Follow-up to #8594 to fix dotnet/runtime#95506 on Android API 21-23
jonathanpeppers pushed a commit that referenced this pull request Jan 17, 2024
@simonrozsival @jonathanpeppers
[Mono.Android] Fix ServerCertificateCustomValidator (#8594)
940f059 
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
jonathanpeppers pushed a commit that referenced this pull request Jan 17, 2024
@simonrozsival @jonathanpeppers
Fix ServerCertificateCustomValidator on API 21-23 (#8637)
6667772 
Follow-up to #8594 to fix dotnet/runtime#95506 on Android API 21-23
jonpryor pushed a commit that referenced this pull request Feb 2, 2024
@jonathanpeppers
Bump $(AndroidNetPreviousVersion) to 34.0.79 (#8693)
bf73889 
Context: fd47b02
Context: https://dotnet.microsoft.com/platform/support/policy/maui

Changes: d12da3a...87e4a6d

  * 87e4a6d89: [release/8.0.1xx] localization changes for `XA1039` (#8674)
  * f61c76030: [workload] Allow installation on linux-arm64 (#8500)
  * 4cdf1e018: [Xamarin.Android.build.Tasks] `<CheckDuplicateJavaLibraries/>` ignores `repackaged.jar` (#8664)
  * 77ac82a11: [release/8.0.1xx] [Xamarin.Android.Build.Tasks] XA1039 warning for Android.Support (#8648)
  * c22c17fbe: [Xamarin.Android.Build.Tasks] fix warning when missing `proguard-android.txt` (#8641)
  * 31f031a67: Localized file check-in by OneLocBuild Task (#8643)
  * 3da479192: LEGO: Merge pull request 8642
  * dbf100d49: Update a number of l18n files (#8633)
  * d2da5bf4e: Bump com.android.tools:r8 from 8.1.72 to 8.2.33 (#8577)
  * 9f9114afc: Bump com.android.tools:r8 from 8.1.56 to 8.1.72 (#8532)
  * 666777248: Fix ServerCertificateCustomValidator on API 21-23 (#8637)
  * 940f059b8: [Mono.Android] Fix ServerCertificateCustomValidator (#8594)
  * 351bfa3f0: Bump to dotnet/runtime@a6e4834d53 8.0.1 (#8616)
  * 64363367d: [Microsoft.Android.Templates] New files should open editor (#8623)
  * 26752564d: Bump to dotnet/installer@461c776642 8.0.101-servicing.23614.6
  * 6972027f0: Localized file check-in by OneLocBuild Task (#8589)
  * 73efcfcad: [Xamarin.Android.Buid.Tasks] don't set $(PublishSelfContained) for .NET 9 (#8565)
  * 3333306b8: LEGO: Merge pull request 8587
  * 38e950b41: Localized file check-in by OneLocBuild Task (#8576)
  * 607115fd5: LEGO: Merge pull request 8575
  * a1fd1b2cb: Localized file check-in by OneLocBuild Task (#8570)
  * c020626b0: [Xamarin.Android.Build.Tasks] Update XA5207 for VSCode (#8494)
  * b0aab545a: Bump to dotnet/installer@822071c28a 8.0.101-servicing.23578.14 (#8558)


The .NET MAUI Support policy, which covers .NET Android, states:

> A major version of .NET MAUI receives support for a minimum of 6
> months after a successor (the next major release) ships.

meaning that a given version of .NET Android only needs to support
building `$(TargetFramework)` values for the current .NET Android
version and the previous .NET Android version.

Meanwhlie, from fd47b02:

> We currently have a hard dependency between:
> 
>   * `Xamarin.Android.Build.Tasks.dll`
>   * `libmonodroid.so`

…in order to safely build projects targeting the previous .NET Android
version with the current .NET Android version, the current .NET Android
version workload *includes the previous workload version contents*.

The `$(AndroidNetPreviousVersion)` MSBuild property specifies the
.NET Android SDK version, as obtained from `dotnet workllad list`,
which should be used to build previous `$(TargetFramework)` versions.

`$(AndroidNetPreviousVersion)` should be updated in
xamarin-android/main whenever a .NET servicing release for the previous
.NET version is shipped.

.NET Android 34.0.79 will be shipped as part of .NET 8.0.2.
Update `$(AndroidNetPreviousVersion)` to so that .NET 9 previews use
the latest .NET 8 workload contents.
jonathanpeppers added a commit that referenced this pull request Feb 2, 2024
@jonathanpeppers
Bump $(AndroidNetPreviousVersion) to 34.0.79 (#8693)
205600c 
Context: fd47b02
Context: https://dotnet.microsoft.com/platform/support/policy/maui

Changes: d12da3a...87e4a6d

  * 87e4a6d89: [release/8.0.1xx] localization changes for `XA1039` (#8674)
  * f61c76030: [workload] Allow installation on linux-arm64 (#8500)
  * 4cdf1e018: [Xamarin.Android.build.Tasks] `<CheckDuplicateJavaLibraries/>` ignores `repackaged.jar` (#8664)
  * 77ac82a11: [release/8.0.1xx] [Xamarin.Android.Build.Tasks] XA1039 warning for Android.Support (#8648)
  * c22c17fbe: [Xamarin.Android.Build.Tasks] fix warning when missing `proguard-android.txt` (#8641)
  * 31f031a67: Localized file check-in by OneLocBuild Task (#8643)
  * 3da479192: LEGO: Merge pull request 8642
  * dbf100d49: Update a number of l18n files (#8633)
  * d2da5bf4e: Bump com.android.tools:r8 from 8.1.72 to 8.2.33 (#8577)
  * 9f9114afc: Bump com.android.tools:r8 from 8.1.56 to 8.1.72 (#8532)
  * 666777248: Fix ServerCertificateCustomValidator on API 21-23 (#8637)
  * 940f059b8: [Mono.Android] Fix ServerCertificateCustomValidator (#8594)
  * 351bfa3f0: Bump to dotnet/runtime@a6e4834d53 8.0.1 (#8616)
  * 64363367d: [Microsoft.Android.Templates] New files should open editor (#8623)
  * 26752564d: Bump to dotnet/installer@461c776642 8.0.101-servicing.23614.6
  * 6972027f0: Localized file check-in by OneLocBuild Task (#8589)
  * 73efcfcad: [Xamarin.Android.Buid.Tasks] don't set $(PublishSelfContained) for .NET 9 (#8565)
  * 3333306b8: LEGO: Merge pull request 8587
  * 38e950b41: Localized file check-in by OneLocBuild Task (#8576)
  * 607115fd5: LEGO: Merge pull request 8575
  * a1fd1b2cb: Localized file check-in by OneLocBuild Task (#8570)
  * c020626b0: [Xamarin.Android.Build.Tasks] Update XA5207 for VSCode (#8494)
  * b0aab545a: Bump to dotnet/installer@822071c28a 8.0.101-servicing.23578.14 (#8558)


The .NET MAUI Support policy, which covers .NET Android, states:

> A major version of .NET MAUI receives support for a minimum of 6
> months after a successor (the next major release) ships.

meaning that a given version of .NET Android only needs to support
building `$(TargetFramework)` values for the current .NET Android
version and the previous .NET Android version.

Meanwhlie, from fd47b02:

> We currently have a hard dependency between:
> 
>   * `Xamarin.Android.Build.Tasks.dll`
>   * `libmonodroid.so`

…in order to safely build projects targeting the previous .NET Android
version with the current .NET Android version, the current .NET Android
version workload *includes the previous workload version contents*.

The `$(AndroidNetPreviousVersion)` MSBuild property specifies the
.NET Android SDK version, as obtained from `dotnet workllad list`,
which should be used to build previous `$(TargetFramework)` versions.

`$(AndroidNetPreviousVersion)` should be updated in
xamarin-android/main whenever a .NET servicing release for the previous
.NET version is shipped.

.NET Android 34.0.79 will be shipped as part of .NET 8.0.2.
Update `$(AndroidNetPreviousVersion)` to so that .NET 9 previews use
the latest .NET 8 workload contents.
@github-actions github-actions bot locked and limited conversation to collaborators Feb 10, 2024
vitek-karas
vitek-karas reviewed Jun 6, 2024
View reviewed changes
src/Mono.Android/Xamarin.Android.Net/ServerCertificateCustomValidator.cs
Comment on lines +162 to +163
[DynamicDependency(nameof(IX509TrustManager.CheckServerTrusted), typeof(IX509TrustManagerInvoker))]
[DynamicDependency(nameof(IX509TrustManager.CheckServerTrusted), typeof(X509ExtendedTrustManagerInvoker))]
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...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vitek-karas vitek-karas vitek-karas left review comments

@grendello grendello grendello approved these changes

@dellis1972 dellis1972 Awaiting requested review from dellis1972 dellis1972 is a code owner

@jonathanpeppers jonathanpeppers Awaiting requested review from jonathanpeppers jonathanpeppers is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

certificate validation not working properly in .Net Android
5 participants
@simonrozsival @jonathanpeppers @jpobst @grendello @vitek-karas