Skip to content

[Android] Fix SslStream certificate rejection completion - #136525

Draft
simonrozsival wants to merge 1 commit into
simonrozsival-android-tls-handshakefrom
simonrozsival-android-tls-rejection-fix
Draft

simonrozsival wants to merge 1 commit into
simonrozsival-android-tls-handshakefrom
simonrozsival-android-tls-rejection-fix

Conversation

@simonrozsival

Copy link
Copy Markdown
Member

Summary and stack dependency

Fix Android SslStream certificate-rejection handling so a managed validation failure terminates authentication instead of waiting for more handshake data.

Stacked on #136517: this draft targets simonrozsival-android-tls-handshake, not main. The expanded tests depend on that PR's connection-info query correction. The diff here contains only the Android PAL rejection fix and the expanded Android regression theory; it does not duplicate the first fix.

Platforms and scenarios

These observations come from managed SslStream tests on stock Android ARM64 emulator images, not just Java probes. "Without this fix" below means the first PR is present, but neither rejection-handling change in this PR is applied.

Platform / configuration Scenario and observation What the evidence establishes
Android 7.0 / API 24, CoreCLR With a trusted chain, all four accepting controls passed, but all four callback-false cases timed out after 30 seconds instead of throwing AuthenticationException: TLS 1.2 / OS default, validating the server certificate on the client / the client certificate on the server. Existing hostname/policy rejection cases also timed out. A demonstrated rejection failure without this fix, not a general connection-info or successful-handshake failure.
Android 7.0 / API 24, Mono A separate fix-1-only survey reproduced both existing wrong-hostname and wrong-policy rejection timeouts. The full module subsequently hit the 600-second harness timeout, so there is no completed-suite count for that failing run. The original rejection problem is also demonstrated on Mono; this is not a CoreCLR-only correction.
Android 9 / API 28 Fix-1-only full trust suites passed 21/21 on each runtime. The complete fix also passes the expanded cases. A control platform, not evidence that every older Android version originally failed.
Android 10 / API 29 Fix-1-only existing rejection controls pass on both runtimes. After changing only CertValidationInCallback to true, the provider emitted a fatal certificate_unknown alert to the peer while the local PAL returned NeedData; local authentication continued waiting. The explicit failure-status mapping is coupled regression prevention for the callback-flag correction. This is not evidence that the untouched API 29 implementation fails those existing controls.
Newer Android controls Fix-1-only full existing trust suites passed on CoreCLR APIs 29, 34, 35, 36, 37 (115/115 total) and Mono APIs 29, 36 (46/46 total), including hostname/policy rejection cases. These controls do not pre-fix-test every newly added trusted-chain callback-false / throwing-callback combination. Passing with the patch does not prove a platform was affected before it.

The relevant triggers are a validation callback returning false, a callback throwing, or managed rejection of a hostname/certificate-policy error. Accepting callbacks are controls. The exact affected Android-version/provider range has not been established; in particular, the passing API 24-37 matrix is not a claim that all those versions require the fix.

Mechanics

Android's trust manager calls managed certificate validation from inside SSLEngine. Marking CertValidationInCallback = true prevents the managed rejection path from recursively entering GenerateToken / the handshake to construct a fatal alert.

After the native handshake returns, the PAL gives a saved callback exception priority and returns InternalError, preserving the original exception as the authentication exception's inner exception. A recorded negative validation result instead becomes CertValidationFailed with the existing certificate-validation exception. Neither can be mistaken for a request for more data.

CanGenerateCustomAlerts remains true; it does not enable recursive managed alert generation because that path also requires !CertValidationInCallback. SSLEngine generates its rejection alert, and the PAL still drains pending writes before returning the failure token so the outer handshake can send that output before throwing.

The theory covers acceptance, rejection, and throwing callbacks in both validation roles, checks callback counts, preserves thrown-exception identity, and verifies negotiated connection properties and bidirectional data exchange on successful connections.

Validation

Fresh for this restored two-file patch:

Check Result
Clean desktop baseline: ./build.sh clr+libs -rc release Passed
All product TFMs: ./dotnet.sh build src/libraries/System.Net.Security/src/System.Net.Security.csproj Passed, zero warnings/errors
Desktop functional suite: ./dotnet.sh build /t:Test src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj 5,048 passed, 31 existing skips, zero failures/errors
Android ARM64 CoreCLR full AndroidPlatformTrustTests suite, API 24 29/29 passed
Same full suite, API 28 29/29 passed
Same full suite, API 29 35/35 passed

The fresh Android total is 93/93, including all 42 accept/false/throw theory cases and the existing hostname/policy rejection tests on each image. TLS 1.2 and OS-default negotiation run on all three images; explicit TLS 1.3 adds cases on API 29.

Android build and packaging commands:

./build.sh clr+libs -os android -arch arm64 -rc Release -lc Release
./build.sh tasks -os android -arch arm64 -c Release
./build.sh libs.sfx+libs.pretest -os android -arch arm64 -rc Release -lc Release /p:RuntimeFlavor=CoreCLR
./dotnet.sh build src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/System.Net.Security.AndroidPlatformTrust.Tests.csproj -c Release /p:TargetOS=android /p:TargetArchitecture=arm64 /p:RuntimeFlavor=CoreCLR "/p:PublishDir=$PWD/artifacts/tmp/fix2-coreclr-publish/"

All completed successfully. The first Android baseline attempt failed with the SDK's MSB3883 / "Cannot release a lock that is not owned by the current thread" in System.Text.RegularExpressions.Generator. Repeating the same baseline command with only local binary-log capture added succeeded; no source, SDK, cache, shared-server, or build-setting workaround was used.

The freshly built APK was frozen and run without rebuilding through dotnet xharness android test on each API. Its nested assets/assets.zip contains byte-identical fresh System.Net.Security.dll, test assembly, and matching CoreCLR System.Private.CoreLib.dll; the native host contains coreclr_initialize, not mono_jit_init_version.

Historical, not fresh runs for this draft: the latest pre-split Mono full-suite runs passed 151/151 on APIs 24, 26, 27, 28, 29, including the expanded theory. Applying the saved combined patch to its original base and restoring these two fixes on the stacked base produces the same Git tree (46539bd1b679af227e3b4a6f9fdbf5aec7642b1c); the only change after those Mono runs was an explanatory custom-alert comment. Earlier combined verification passed 920/920 across every API 24-37 and both runtimes, including 444 expanded-regression cases. These historical figures are separate evidence, not additional fresh executions or proof of the original affected-version range.

Scope and limits

Only Android product code and Android-specific tests change. There are no API additions or native interop changes, and desktop behavior is not intentionally changed. Coverage is ARM64 CoreCLR/Mono, not NativeAOT, other Android architectures, physical devices, or all vendor TLS providers. No production incident, released-version bisection, or certificate-validation bypass was established.

This restores the expected failure contract: rejected authentication completes with AuthenticationException rather than hanging. Applications must still reject invalid certificates; there is no validation opt-out added by this change.

Note

This pull request and its description were prepared with GitHub Copilot.

Avoid recursive SSLEngine entry during certificate validation and map saved managed failures to terminal handshake status while draining pending alert output. Expand Android regression coverage for accepting, rejecting, and throwing callbacks.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d827a5f4-206f-4b85-8331-dfc0b820aa28
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 4 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant