[Android] Improvements to remote certificate verification in SslStream #77386
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tagging subscribers to this area: @dotnet/ncl, @vcsjones Issue DetailsThis is WIP. This PR changes how we're verifying server certificates on Android. Instead of performing the verification after the TLS handshake in C#, we create a custom Java X509TrustManager which calls into the C# code during the TLS handshake. The Java code is compiled and bundled into a .dex file ( work in progress:
|
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
simonrozsival
changed the title
…-remote-certificate-verification-improvements
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…-remote-certificate-verification-improvements
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…-remote-certificate-verification-improvements
|
@elinor-fung no, it doesn't need any documentation changes. Thanks for the feedback. |
|
/azp run runtime-android |
|
Azure Pipelines successfully started running 1 pipeline(s). |
akoeplinger
approved these changes
Jan 9, 2023
src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs
Show resolved
Hide resolved
…-remote-certificate-verification-improvements
…-remote-certificate-verification-improvements
akoeplinger
approved these changes
Jan 13, 2023
Co-authored-by: Alexander Köplinger <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
I went through the failing tests and they are all unrelated to this PR so I'll merge it. |
simonrozsival
deleted the
android-remote-certificate-verification-improvements
branch
jonathanpeppers
pushed a commit
to dotnet/android
that referenced
this pull request
Jan 26, 2023
Since dotnet/runtime#77386 has been merged, .NET will require a certain class from `libSystem.Security.Cryptography.Native.Android.jar` that will be located in the runtime pack files. I'm not sure if this is the best way to pull the .jar from the runtimepack so feedback is welcome. * Collect jar files from the runtime pack * Remove invalid dependency
jonathanpeppers
added a commit
to dotnet/android
that referenced
this pull request
Jan 28, 2023
Changes: dotnet/installer@9962c6a...779a644 Changes: dotnet/linker@4b3f78c...c790896 Changes: dotnet/runtime@5da4a9e...ddb6988 Changes: dotnet/emsdk@66b9845...5b46122 Updates: * Microsoft.Dotnet.Sdk.Internal: from 8.0.100-alpha.1.23063.11 to 8.0.100-alpha.1.23070.23 * Microsoft.NET.ILLink.Tasks: from 8.0.100-1.23055.2 to 8.0.100-1.23067.1 * Microsoft.NETCore.App.Ref: from 8.0.0-alpha.1.23058.2 to 8.0.0-alpha.1.23070.1 * Microsoft.NET.Workload.Emscripten.net7.Manifest-8.0.100: from 8.0.0-alpha.1.22620.1 to 8.0.0-alpha.1.23066.1 ~~ Other Changes ~~ * Update `.apkdesc` files for app size changes. * Use .jar files from the .NET runtime pack (#7665) Since [dotnet/runtime#77386][0] has been merged, .NET will require a certain class from `libSystem.Security.Cryptography.Native.Android.jar` that will be located in the runtime pack files. [0]: dotnet/runtime#77386 * Disambiguate `.jar` files from Mono runtime packs. We were getting the build error: error JAVA0000: Caused by: com.android.tools.r8.internal.f: Type net.dot.android.crypto.DotnetProxyTrustManager is defined multiple times This `.jar` file is contained in each runtime pack (4 architectures) gives us 4 `.jar` files! We can pass in these files to the `<ProcessAssemblies/>` MSBuild task. We also filter them based on `%(NuGetPackageId)`, so that any random `.jar` file doesn't get added to `@(AndroidJavaLibrary)`. I renamed the `IsFrameworkAssembly()` method to `IsFromAKnownRuntimePack()` to make this more clear in the existing code. * Update `proguard_xamarin.cfg` for .NET 8. Apps using `$(AndroidLinkTool)` of r8, now need to preserve: -keep class net.dot.android.crypto.DotnetProxyTrustManager { *; <init>(...); } Otherwise we run into a crash when this type isn't present, such as: 01-26 23:59:19.855 8684 8684 F DEBUG : #2 pc 00000000000191d6 /data/app/Mono.Android.NET_Tests-cpTzt8Q9KwgS-znzkuAdNQ==/split_config.x86_64.apk!libSystem.Security.Cryptography.Native.Android.so (offset 0xe7000) (JNI_OnLoad+31302) (BuildId: 7d9e4013a9dd99810070587ab42956703fef69f9) Co-authored-by: Jonathan Peppers <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]>
jonathanpeppers
pushed a commit
to jonathanpeppers/xamarin-android
that referenced
this pull request
Jan 31, 2023
Context: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=7264347&view=ms.vss-test-web.build-test-results-tab&runId=67617676&resultId=100033&paneView=debug A single test was failing on Windows-only: BuildReleaseArm64(True) apkdiff regression test failed with exit code: 3 stdErr: Error: apkdiff: PackageSize decrease 8,192 is 3,072 bytes more than the threshold 5,120. apk1 size: 9,521,310 bytes, apk2 size: 9,513,118 bytes. Error: apkdiff: Size regression occured, 1 check(s) failed. Most notably a single file changed: 14,556 classes.dex I believe this started failing in 35db527, due to a combination of changes: * [dotnet/runtime#77386][0] introduced `libSystem.Security.Cryptography.Native.Android.jar` in the Mono runtime packs. * We updated `.apkdesc` files *before* including the new `.jar`. * We added appropriate logic to include the new `.jar`. The result was we have `.apkdesc` files that don't match the full set of changes in 35db527. Then in 8872d04 the app size changed beyond the threshold for a test to fail. [0]: dotnet/runtime#77386
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In .NET 6 and .NET 7 on Android, it's not possible to bypass Android's verification of server certificate. This makes it difficult to develop android apps which communicate with servers with self-signed certificates. One common use case is running a server with self-signed development certificate on localhost. At the moment, the only option is to bundle the self-signed certificate in the .apk use Android's
network_security_config.xmlto trust it.This PR changes how we're verifying server certificates on Android by replacing the default Java X509TrustManager with a custom trust manager that calls into the C#
SslStreamcode that performs the usual validation. This makes it possible for our customers to perform their custom validation inRemoteCertificateValidationCallbackwhenever they useSslStreamorSocketsHttpHandler.One of the major changes in this PR is the presence of Java code. The Java code is compiled and bundled into a .dex and .jar files (
libSystem.Security.Cryptography.Native.Android.dexand.jar). Any .NET Android app will have to include this Java library or the app will crash at startup. Xamarin.Android will have to adjust their build process once this change is merged. There is roughly 4 kB size increase of the resulting .apk file compared to currentmain. The Java code isn't trimmable at all at the moment but I think this can be improved in cooperation with the Xamarin team in follow up PRs.Thanks to this fix we can re-enable many tests disabled in System.Net.Http.Functional.Tests and System.Net.Security.Functional.Tests on Android and remove an existing network_security_config.xml workaround.
Ref #45741
Ref dotnet/android#7278
Closes dotnet/android#7641