[Android] Respect platform trust manager in SslStream by simonrozsival · Pull Request #124173 · dotnet/runtime

simonrozsival · 2026-02-09T12:54:33Z

Problem

On Android, SslStream bypassed the platform's trust infrastructure entirely. The DotnetProxyTrustManager did not consult Android's X509TrustManager, so network_security_config.xml — used for certificate pinning and custom trust anchors — was ignored. RemoteCertificateValidationCallback always received SslPolicyErrors.None even when the platform would have rejected the certificate chain (e.g. due to a pin mismatch).

This is the exact scenario reported in #107695: an app configures SSL pinning via network_security_config.xml with an intentionally wrong pin, but SslPolicyErrors.None is still reported.

Fix

This PR wraps the platform's X509TrustManager (obtained via TrustManagerFactory) inside DotnetProxyTrustManager. During TLS handshakes the platform's trust infrastructure — including network_security_config.xml — is now consulted. The platform's verdict is combined with managed (.NET) validation to be more strict, never less:

Platform rejects → sslPolicyErrors is pre-seeded with RemoteCertificateChainErrors. Managed validation cannot clear this flag.
Platform accepts → managed validation (X509Chain.Build) still runs independently and can add its own errors.
RemoteCertificateValidationCallback always receives the union of both assessments.

This is consistent with iOS behavior, where the platform trust manager is always consulted.

Behavioral change

Apps using managed-only custom trust roots for CAs not in the Android system store will now see RemoteCertificateChainErrors in the callback (previously SslPolicyErrors.None). This is correct — the callback now reflects the platform's trust assessment. Apps can still accept the connection by returning true from RemoteCertificateValidationCallback.

Exception: When CertificateChainPolicy specifies CustomRootTrust with ExtraStore, the platform's verdict is bypassed because it lacks intermediate certs from ExtraStore and would produce false rejections. The managed chain builder is authoritative in this case.

Testing

Layer	Tests	Coverage
`tests/FunctionalTests/SslStreamPlatformTrustManagerTests.Android.cs`	3	Contract-level. Run on every `runtime-extra-platforms-android` leg. Cover the pre-seeded chain error, callback override, and callback rejection paths.
`tests/AndroidPlatformTrustTests/` (new APK)	10	End-to-end. Real `SslStream` handshakes against a `network_security_config.xml` that trusts the System.Net.TestData root CA for `testservereku.contoso.com`. Cover signed-by-trusted-CA, signed-by-untrusted-CA, domain-not-in-config, callback override / reject / absent, `CustomRootTrust` with/without `ExtraStore`, IP-literal TargetHost, and `TargetHost == null` (non-hostname-aware path).

Manual device verification: all 13 tests pass on physical Samsung SM-S911B (API 36, arm64-v8a) and Android emulator (API 36, arm64-v8a).

Earlier revisions of the PR also shipped three platform-introspection tests (NetworkSecurityConfig_*) that called private JNI helpers exposed from System.Security.Cryptography.Native.Android. Those tests and the corresponding test-only product surface have been removed — they only exercised the plumbing, while the 10 real-handshake tests verify the behaviour that matters for users.

Changes

Java (`DotnetProxyTrustManager`)

Wraps the platform X509TrustManager obtained via TrustManagerFactory
Uses X509TrustManagerExtensions.checkServerTrusted(chain, authType, hostname) for hostname-aware validation (required for network_security_config.xml per-domain dispatch)
Caches the X509TrustManagerExtensions instance in the ctor (avoids per-handshake allocation)
Passes chainTrustedByPlatform to the native callback
getAcceptedIssuers() returns an empty array to avoid restricting client-certificate selection

Native C (`pal_sslstream.c`, `pal_trust_manager.c`, `pal_jni.c`)

Refactored SSLStreamCreate: a single function taking targetHost, trustCerts, and keyManagers (replacing three variants)
Added key-manager helpers (SSLStreamCreateKeyManagers, SSLStreamCreateKeyManagersFromKeyStoreEntry)
GetX509TrustManager() returns the platform trust manager, optionally initialised with a custom KeyStore for CustomRootTrust. The default (no custom KeyStore) instance is cached as a JNI global ref so TrustManagerFactory.init(null) no longer fires on every handshake.
RegisterNatives for verifyRemoteCertificate now captures the return value and abort_unless to fail fast on registration failures (NativeAOT)
Removed ApplyLegacyAndroidSNIWorkaround and the UNSUPPORTED_API_LEVEL plumbing — both unreachable after Bump Android minimum API level from 21 to 24 #126838 bumped the Android min API to 24
Fixed a potential SIGSEGV in FreeSSLStream when SSLStreamInitialize was never called, and a JNI local-ref leak in GetX509TrustManager

Managed C# (`SslStream.Android.cs`, `SslStream.IO.cs`, `SslStream.Protocol.cs`, `Interop.OpenSsl.cs`)

CreateSslContext refactored: key-manager creation extracted; keyManagers global ref released in finally
GetTrustCertHandles collects custom root certs from CertificateChainPolicy.CustomTrustStore or SslCertificateTrust
VerifyRemoteCertificate now takes ref SslPolicyErrors instead of out, so callers can pre-seed the platform's verdict. SslStream.IO.cs initialises it to SslPolicyErrors.None before invocation on PALs where the callback isn't called inside the TLS callback. The Linux OpenSSL callback (Interop.OpenSsl.cs) and the Android trust-manager callback both seed it appropriately before calling in.
VerifyRemoteCertificate(bool chainTrustedByPlatform) (Android-only) pre-seeds sslPolicyErrors based on the platform's verdict and the presence of user-supplied ExtraStore

Build infrastructure (`AndroidAppBuilder`)

Supports NetworkSecurityConfig and NetworkSecurityConfigResourcesDir for including network_security_config.xml in APKs

Fixes #107695

Note

The latest revision of this PR was prepared with assistance from GitHub Copilot.

Copilot

Pull request overview

Updates Android SslStream certificate validation so it consults the platform trust infrastructure (including network-security-config.xml) before delegating to managed validation, aligning Android behavior more closely with other platforms and correctly surfacing SslPolicyErrors in callbacks.

Changes:

Wrap Android’s default X509TrustManager in DotnetProxyTrustManager and pass platform-trust outcome into managed validation.
Plumb targetHost through native/managed layers (for hostname-aware checks on newer Android APIs) and move SNI setup into SSLStreamInitialize.
Add build/test infrastructure to include network_security_config.xml (and related resources) and introduce Android-focused tests.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/tasks/AndroidAppBuilder/Templates/AndroidManifest.xml	Adds a templated placeholder for `networkSecurityConfig` on the `<application>` element.
src/tasks/AndroidAppBuilder/ApkBuilder.cs	Copies `network_security_config.xml` + optional resources into `res/` and passes `-S res` to `aapt`.
src/tasks/AndroidAppBuilder/AndroidAppBuilder.cs	Exposes MSBuild task properties for network security config inputs.
src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager.h	Extends callback/get-trust-managers APIs to include platform-trust flag and `targetHost`.
src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager.c	Fetches default platform `X509TrustManager` via `TrustManagerFactory` and constructs the proxy trust manager with hostname.
src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.h	Threads `targetHost` into SSLStream create APIs; removes `SSLStreamSetTargetHost`.
src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c	Passes `targetHost` into trust manager creation; moves SNI handling into `SSLStreamInitialize`.
src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.h	Adds cached JNI handles for `TrustManagerFactory` and `X509TrustManager`.
src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c	Initializes JNI refs/method IDs for platform trust manager discovery and updated proxy TM ctor sig.
src/native/libs/System.Security.Cryptography.Native.Android/net/dot/android/crypto/DotnetProxyTrustManager.java	Wraps platform trust manager and performs platform trust checks (hostname-aware on API 24+).
src/mono/msbuild/android/build/AndroidBuild.targets	Plumbs `NetworkSecurityConfig*` MSBuild properties into `AndroidAppBuilder`.
src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj	Adds Android TFM and includes Android-only functional test source.
src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamPlatformTrustManagerTests.Android.cs	Adds Android functional tests validating platform-trust reporting/override behavior.
src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/network_security_config.xml	Adds a network security config used by Android-only tests to validate custom trust anchors.
src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/System.Net.Security.AndroidPlatformTrust.Tests.csproj	New Android-only test project that prepares/embeds network security config and CA resource.
src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/AndroidPlatformTrustTests.cs	New tests validating that platform trust anchors affect `SslPolicyErrors`.
src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs	Changes `VerifyRemoteCertificate` to take `SslPolicyErrors` by `ref` (caller-initialized).
src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs	Initializes `sslPolicyErrors` before calling the `ref`-based `VerifyRemoteCertificate`.
src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs	Incorporates platform-trust result into managed validation and avoids reintroducing chain errors when platform already trusted.
src/libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs	Passes `targetHost` into native SSLStream creation (for trust manager hostname-aware validation); removes explicit `SSLStreamSetTargetHost` call.
src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Ssl.cs	Updates P/Invokes for new `targetHost` parameters and removes `SSLStreamSetTargetHost` interop.

dotnet-policy-service · 2026-02-09T14:51:30Z

Tagging subscribers to 'arch-android': @vitek-karas, @simonrozsival, @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

On Android, SslStream's DotnetProxyTrustManager previously bypassed the platform's trust infrastructure entirely. This meant that Android's network-security-config.xml (certificate pinning, custom trust anchors) was ignored, and RemoteCertificateValidationCallback always received SslPolicyErrors.None even when the platform would have rejected the certificate chain. This change wraps the platform's default X509TrustManager inside DotnetProxyTrustManager so that Android's trust infrastructure is consulted before delegating to the managed SslStream validation code. Changes: - DotnetProxyTrustManager.java now wraps the platform X509TrustManager and calls checkServerTrusted/checkClientTrusted before invoking the managed callback. Uses X509TrustManagerExtensions for hostname-aware validation on API 24+. - The targetHost is passed through SSLStreamCreate* to GetTrustManagers so the trust manager can perform hostname-aware validation. - SSLStreamSetTargetHost is removed; SNI setup is now done in SSLStreamInitialize. - VerifyRemoteCertificate receives a chainTrustedByPlatform flag. When the platform rejects the chain, RemoteCertificateChainErrors is reported. When the platform trusts it, AllowUnknownCertificateAuthority is set to prevent the managed chain builder from re-introducing errors for CAs that are trusted by the platform but not in the managed store. - AndroidAppBuilder supports NetworkSecurityConfig to include network_security_config.xml in test APKs. Behavioral change: Apps using managed-only custom trust roots for CAs not in the Android system store will now see RemoteCertificateChainErrors in the callback (previously SslPolicyErrors.None due to platform bypass). This is correct — the callback accurately reflects the platform's trust assessment, and apps can still accept by returning true. Fixes dotnet#107695

X509TrustManagerExtensions.checkServerTrusted(chain, authType, host) is available since API 17, and our minimum supported API level is 21. The Build.VERSION.SDK_INT >= 24 guard was unnecessarily falling back to hostname-unaware validation on API 21-23.

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

- Extract SNI server name setup into SetSNIServerName() with proper INIT_LOCALS/RELEASE_LOCALS to prevent JNI local ref leaks on error - Replace abort_unless with graceful LOG_ERROR + goto cleanup when platform X509TrustManager is unavailable - Add ON_EXCEPTION_PRINT_AND_GOTO after make_java_object_array - Remove unnecessary comment about API level in DotnetProxyTrustManager

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.

stephentoub · 2026-02-09T19:44:30Z

🤖 Copilot Code Review — PR #124173

Holistic Assessment

Motivation: This PR addresses a real security and correctness bug where Android's network-security-config.xml (certificate pinning, custom trust anchors) was completely bypassed by SslStream. The fix correctly ensures Android's platform trust infrastructure is consulted before managed validation. This aligns with the existing iOS behavior and fixes issue #107695.

Approach: Wrapping the platform's default X509TrustManager inside DotnetProxyTrustManager, threading targetHost from managed through native to Java, and conditioning managed validation on chainTrustedByPlatform is the right layering. It preserves callback semantics while making the platform authoritative for chain trust.

Net positive: ✅ This is a significant correctness and security fix for Android apps. The design is clean and the tests are comprehensive.

Detailed Findings

✅ Correctness — Trust Integration & Callback Behavior

The new DotnetProxyTrustManager correctly delegates to the platform trust manager first (including hostname-aware validation via X509TrustManagerExtensions.checkServerTrusted(chain, authType, targetHost)). The managed side correctly distinguishes between "platform rejected" (RemoteCertificateChainErrors set) and "platform trusted" (strip spurious chain errors) so user callbacks see consistent errors.

✅ Correctness — JNI Lifecycle & Exception Handling

The GetDefaultX509TrustManager function has proper ON_EXCEPTION_PRINT_AND_GOTO(cleanup) checks after JNI method invocations. Local refs are correctly managed through INIT_LOCALS / RELEASE_LOCALS, and the JNI signature for DotnetProxyTrustManagerCtor was correctly updated. The make_java_object_array result is also checked with ON_EXCEPTION_PRINT_AND_GOTO.

✅ Correctness — Managed-Side API Consistency

Changing VerifyRemoteCertificate to use ref SslPolicyErrors and explicitly initializing sslPolicyErrors = SslPolicyErrors.None in CompleteHandshake (SslStream.IO.cs) is consistent and avoids stale error propagation. The pattern of initializing with the platform's verdict and passing by ref ensures the shared logic accumulates errors rather than overwriting the platform's trust decision.

✅ Test Coverage — Good

The tests validate key behaviors: untrusted self-signed certs report chain errors, callbacks can override platform failures, callbacks rejecting cause AuthenticationException, and network-security-config-pinned CA scenario does not surface chain errors.

💡 Performance — Consider Caching X509TrustManagerExtensions

File: DotnetProxyTrustManager.java

java X509TrustManagerExtensions extensions = new X509TrustManagerExtensions(platformTrustManager); extensions.checkServerTrusted(chain, authType, targetHost);

A new X509TrustManagerExtensions instance is created on every handshake (in isServerTrustedByPlatformTrustManager). Consider caching this in the constructor to avoid per-handshake allocations. This is a minor optimization but aligns with the repo's performance-conscious patterns.

(Flagged by multiple models)

💡 Performance — TrustManagerFactory Initialization

File: pal_trust_manager.c

GetDefaultX509TrustManager is called on every handshake via GetTrustManagers. Initializing TrustManagerFactory involves init(null), which loads the system KeyStore (potential disk I/O) and parses certificates. If benchmarking shows this is a bottleneck, consider caching the default X509TrustManager lazily during initialization rather than creating a new factory per connection.

(Flagged by multiple models)

💡 Behavioral Change — SNI Setup Exception Type

Files: pal_sslstream.c, SafeDeleteSslContext.cs

Previously, SSLStreamSetTargetHost mapped UNSUPPORTED_API_LEVEL to PlatformNotSupportedException with SR.net_android_ssl_api_level_unsupported. Now SNI setup is performed inside SSLStreamInitialize, and failures will cause a generic SslException.

The retained ApplyLegacyAndroidSNIWorkaround means this path should rarely fail on API 21-23, but if API-specific exception behavior was relied upon, it would differ. Consider documenting this or returning a distinct status code if preserving exception shape is important.

Summary

LGTM. The security fix is essential, the design is sound, and JNI wiring and exception handling are correct. Consider addressing the minor performance suggestions (caching X509TrustManagerExtensions and potentially the platform trust manager) as follow-up optimizations.

Review generated with input from multiple AI models (Gemini 3 Pro, Claude Opus 4.5). Claude Sonnet 4 (primary).

steveisok

LGTM - I would add more security people to review.

When the Android remote certificate validation callback throws, the exception was captured but never surfaced. Emit it through NetEventSource so the failure reason is observable via dotnet-trace during development. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…orm-trust-manager-107695

When the Android platform trust manager rejects the certificate chain, it only signals a boolean verdict; the underlying reason is lost. Capture the Java CertificateException text and pass it through JNI to the managed callback, which logs it via NetEventSource when a listener is attached. The redundant chainTrustedByPlatform boolean is dropped: a non-null error string now encodes rejection, and the string is only marshalled inside the NetEventSource.Log.IsEnabled() guard to avoid allocations when tracing is off. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot's findings

Files reviewed: 26/26 changed files
Comments generated: 3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival · 2026-07-07T21:48:18Z

/azp run runtime-android

azure-pipelines · 2026-07-07T21:48:34Z

Azure Pipelines successfully started running 1 pipeline(s).

Keep the native callback symbol aligned with the Android NativeAOT test linker metadata while continuing to bind it through RegisterNatives. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival · 2026-07-08T11:17:26Z

/azp run runtime-android

azure-pipelines · 2026-07-08T11:17:36Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Copilot's findings

Comments suppressed due to low confidence (1)

src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs:65

TryGetRemoteCertificateValidationResult reads SslStreamProxy.ValidationResult but never clears it. That means future handshakes / renegotiation / post-handshake auth could incorrectly reuse a stale validation result and skip certificate validation, and the Debug.Assert(proxy.ValidationResult is null) in the Java callback will fire if validation runs more than once.

Consider consuming the result (read + clear) once it has been used by CompleteHandshake so each validation callback produces an independent result.

        private bool TryGetRemoteCertificateValidationResult(out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus, ref ProtocolToken alertToken, out bool isValid)
        {
            JavaProxy.RemoteCertificateValidationResult? validationResult = _securityContext?.SslStreamProxy.ValidationResult;
            sslPolicyErrors = validationResult?.SslPolicyErrors ?? default;
            chainStatus = validationResult?.ChainStatus ?? default;
            isValid = validationResult?.IsValid ?? default;
            alertToken = validationResult?.AlertToken ?? default;
            return validationResult is not null;
        }

Files reviewed: 26/26 changed files
Comments generated: 2

simonrozsival · 2026-07-09T07:17:21Z

Failing tests are unrelated to the changes in this PR:

Pipeline leg	Failed test(s)	Notes
`runtime-android (Build android-arm64 Release AllSubsets_CoreCLR)`	`System.Net.NameResolution.PalTests.NameResolutionPalTests.TryGetAddrInfo_ExternalHost(justAddresses: False)` `System.Net.NameResolution.PalTests.NameResolutionPalTests.TryGetAddrInfo_ExternalHost(justAddresses: True)`	Both failed with `Expected: Success`, `Actual: HostNotFound`. The Helix log also detected a network connectivity issue (`ping: unknown host www.microsoft.com`) and requested an infrastructure retry.
`runtime (Build browser-wasm windows Release CoreCLR_WasmBuildTests)`	`Wasm.Build.Tests.NativeLibraryTests.ProjectWithNativeReference(config: Release, aot: False)`	Failed with `Timed out after 10s waiting for 'WASM EXIT' message`; the log also reported `Browser has been disconnected`.

simonrozsival · 2026-07-09T07:32:14Z

Regarding Copilot's low-confidence note about TryGetRemoteCertificateValidationResult not clearing SslStreamProxy.ValidationResult: I don't think this is actionable for this PR unless there's an Android path that can invoke certificate validation more than once for the same JavaProxy / SafeDeleteSslContext.

On the current Android path, SslStreamPal.Renegotiate(...) throws PlatformNotSupportedException, so renegotiation/post-handshake validation should not reuse this value. The result is intended to bridge the Java TrustManager callback during the native handshake back into CompleteHandshake; if no Java callback happened, CompleteHandshake falls back to regular managed validation.

So unless someone can point to a supported Android path where the same proxy/context performs multiple certificate validations, I'd prefer not to churn the PR by adding consume-and-clear plumbing for a theoretical case.

Note

This comment was drafted with assistance from GitHub Copilot.

> [!NOTE] > This PR description was drafted by an AI assistant (GitHub Copilot CLI) based on the actual code changes in this branch. All code in this PR was authored interactively with Simon driving design decisions through multiple review rounds. ## Summary This PR adds missing JNI exception checks across several files in the Android cryptography PAL. The work is based on a static audit of all `Call*Method`/`NewObject`/`GetArrayLength`-style JNI calls and targets sites where a pending Java exception could silently leak back into managed code. Beyond adding the exception checks, the changed functions are tightened to follow three patterns already established elsewhere in the PAL: 1. **`ON_EXCEPTION_PRINT_AND_GOTO(label)` immediately after each fallible JNI call.** The macro both prints and clears the exception, so subsequent JNI calls along the `cleanup`/`error` path are safe. 2. **`INIT_LOCALS(loc, …)` + single `cleanup:`/`RELEASE_LOCALS(loc, env)` tail** for local-ref hygiene, replacing ad-hoc `jobject foo = NULL;` declarations and scattered `DeleteLocalRef` calls. 3. **No partial output state on failure** — functions either defer caller-visible writes until success or clean up/reset partially promoted global refs before returning failure, so the caller never observes a half-populated result. A few smaller correctness fixes surfaced during review ride along with the audit: the shared `GetEnumAsInt` helper no longer deletes the local ref passed to it (each caller now owns and releases its own ref), and `SSLStreamWrite` now null-checks the buffer returned by `EnsureRemaining` before use. Both are described under **Behavior changes worth flagging** below. ## Files changed (11) All under `src/native/libs/System.Security.Cryptography.Native.Android/`: | File | Functions | |---|---| | `pal_cipher.c` | `ReinitializeCipher` | | `pal_dsa.c` | `DsaSizeP`, `GetDsaParameters`, `GetQParameter`, `DsaKeyCreateByExplicitParameters` | | `pal_ecc_import_export.c` | `GetECKeyParameters`, `GetECCurveParameters`, `CreateKeyPairFromCurveParameters`, `ConvertBigIntegerToPositiveInt32`, `EcKeyCreateByExplicitParameters` | | `pal_ecdh.c` | `EcdhDeriveKey` | | `pal_eckey.c` | `EcKeyGetSize` | | `pal_hmac.c` | `HmacCreate` | | `pal_jni.c` | `GetEnumAsInt` | | `pal_ssl.c` | `SSLGetSupportedProtocols` | | `pal_sslstream.c` | `GetHandshakeStatus`, `WrapAndProcessResult`, `Close`, `DoUnwrap`, `FreeSSLStream`, `SSLStreamInitialize`, `AddCertChainToStore`, `SSLStreamWrite` | | `pal_x509.c` | `X509DecodeCollection`, `X509ExportPkcs7` | | `pal_x509store.c` | `ContainsEntryForAlias`, `ContainsMatchingCertificateForAlias`, `X509StoreAddCertificate`, `X509StoreAddCertificateWithPrivateKey`, `X509StoreContainsCertificate`, `X509StoreRemoveCertificate`, `EnumerateCertificates`, `SystemAliasFilter` | ## Behavior changes worth flagging - **`GetEnumAsInt`** (`pal_jni.c`) no longer calls `DeleteLocalRef` on its `enumObj` argument — it is now a pure getter. Each of its five call sites already holds the enum result in a named local that is released through the function's own `cleanup:` path (`RELEASE_LOCALS` / `ReleaseLRef`), so every ref is freed exactly once by its owner. This removes a latent double-`DeleteLocalRef` in `GetHandshakeStatus` and the same footgun for the other callers. - **`AndroidCryptoNative_GetDsaParameters` / `AndroidCryptoNative_GetECKeyParameters`** now zero out their `out` length parameters (`*pLength`, `*qLength`, `*cbQx`, etc.) at function entry. Previously these were set on success or left untouched on failure. No managed caller relies on the prior partial-write behavior. - **`ContainsEntryForAlias` / `ContainsMatchingCertificateForAlias`** (`pal_x509store.c`) — return type changed from `bool` to `int32_t` (SUCCESS/FAIL) with a separate `*contains`/`*matches` out-parameter, so callers can distinguish "lookup failed due to JNI exception" from "lookup succeeded; not contained". Callers are updated accordingly. - **`FreeSSLStream`** now NULL-checks `sslStream->managedContextCleanup` before invoking it. This guards the case where `SSLStreamCreate` succeeded but `SSLStreamInitialize` was never called. - **`SSLStreamInitialize`** now transfers ownership of `managedContextHandle` / `streamReader` / `streamWriter` / `managedContextCleanup` to the native `SSLStream` as soon as initialization begins. Combined with the `FreeSSLStream` NULL-check above, native cleanup skips the callback only when initialization was never called, while partial JNI setup failures still release the managed context handle. - **`GetHandshakeStatus`** now splits `SSLEngine.getHandshakeStatus()` from the enum-ordinal conversion, checks for JNI exceptions/NULL before calling `GetEnumAsInt`, stores the ordinal in a local and only promotes it to the return value once retrieval fully succeeds (otherwise `ret` stays `-1`), and releases the handshake-status local through the standard `INIT_LOCALS` + `cleanup:` + `RELEASE_LOCALS` pattern. - **`AndroidCryptoNative_SSLStreamWrite`** now captures the buffer returned by `EnsureRemaining` in a local and bails out through `cleanup` if it is `NULL`, instead of assigning it straight to `sslStream->appOutBuffer`. On a buffer-expansion failure this avoids both a NULL-object JNI call (`ByteBuffer.put`) and a leaked global ref; the write returns `SSLStreamStatus_Error` (surfaced as `InternalError` by the managed caller) instead of crashing. Mirrors the existing handling in `DoUnwrap`. ## Out of scope A few pre-existing bugs were surfaced during review but intentionally left out of scope to keep this PR focused on the exception-check work: - `pal_x509store.c:115,141,186` — `EntryFlags_HasCertificate & EntryFlags_MatchesCertificate` should be `|` (dead code since 2021). - `pal_ecc_import_export.c:574` — `AddGRef` should be `ToGRef` (intermediate lref leak). - `pal_ecc_import_export.c:577-581` — `CheckJNIExceptions` only runs when `keyPair` is NULL. - `pal_x509.c` `X509DecodeCollection` — per-iteration gref leak on failure. These will be addressed as separate targeted fixes. ## Related Other open PRs in the same area (Android crypto PAL / JNI error handling): - #128747 — [Android] Clear pending JNI exceptions in `pal_rsa.c` to avoid CheckJNI aborts. Same JNI-exception-hygiene theme, applied to `pal_rsa.c` (which this PR does not touch). - #128651 — [Android] Handle `X509ChainContext` creation failures. Related JNI error-handling work in `pal_x509chain.c`. - #124173 — [Android] Respect platform trust manager in `SslStream`. Touches `pal_sslstream.c` and includes an equivalent NULL-check fix to `FreeSSLStream`; the two PRs will need a rebase against whichever lands second. ## Testing | Suite | Result | |---|---| | Android native libraries build (`./src/native/libs/build-native.sh arm64 Debug -os android ninja`) | Passed | | Android native libraries build (`./build.sh libs.native -os android -arch arm64 -c Release`) | Passed | | `System.Security.Cryptography.Tests` (Android arm64 Debug device) | 11715 run / 10738 passed / 0 failed / 977 skipped | | `System.Net.Security.Tests` (Android arm64 Debug device) | 4890 run / 4854 passed / 4 failed / 32 skipped (matches pre-PR baseline — unrelated `DelayedCertificate`/ALPN tests) | CI will provide cross-architecture validation (arm, x86, x64) and additional API levels not covered locally. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot

Copilot's findings

Files reviewed: 26/26 changed files
Comments generated: 3

rzikm

LGTM, thanks!

simonrozsival requested a review from steveisok as a code owner February 9, 2026 12:54

Copilot AI review requested due to automatic review settings February 9, 2026 12:54

simonrozsival requested a review from vitek-karas as a code owner February 9, 2026 12:54

github-actions Bot added the area-System.Net.Security label Feb 9, 2026

dotnet-policy-service Bot assigned simonrozsival Feb 9, 2026

Copilot started reviewing on behalf of simonrozsival February 9, 2026 12:55 View session

simonrozsival force-pushed the feature/android-platform-trust-manager-107695 branch from 8cf3eb9 to 7f1bf1f Compare February 9, 2026 13:03

Copilot AI reviewed Feb 9, 2026

View reviewed changes

Comment thread src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/AndroidPlatformTrustTests.cs

Comment thread src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c Outdated

Comment thread src/tasks/AndroidAppBuilder/ApkBuilder.cs Outdated

simonrozsival force-pushed the feature/android-platform-trust-manager-107695 branch from 7f1bf1f to 6243d5b Compare February 9, 2026 14:35

simonrozsival added the os-android label Feb 9, 2026

Copilot AI review requested due to automatic review settings February 9, 2026 14:56

simonrozsival force-pushed the feature/android-platform-trust-manager-107695 branch from 6243d5b to 06ea804 Compare February 9, 2026 14:56

Copilot started reviewing on behalf of simonrozsival February 9, 2026 14:57 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

Comment thread src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c Outdated

Comment thread src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager.c

Comment thread src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager.c Outdated

Create X509TrustManagerExtensions inline instead of storing as field

0088428

simonrozsival commented Feb 9, 2026

View reviewed changes

Comment thread ...tem.Security.Cryptography.Native.Android/net/dot/android/crypto/DotnetProxyTrustManager.java Outdated

Copilot AI review requested due to automatic review settings February 9, 2026 17:17

Copilot AI reviewed Feb 9, 2026

View reviewed changes

Use const char* for targetHost, improve ApkBuilder validation

fdfe650

steveisok approved these changes Feb 11, 2026

View reviewed changes

Comment thread src/libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs Outdated

simonrozsival and others added 3 commits July 3, 2026 13:54

Merge remote-tracking branch 'origin/main' into feature/android-platf…

ecaff8b

…orm-trust-manager-107695

Copilot AI review requested due to automatic review settings July 3, 2026 13:08

Copilot started reviewing on behalf of simonrozsival July 3, 2026 13:08 View session

Copilot AI reviewed Jul 3, 2026

View reviewed changes

Comment thread src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs

Comment thread src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager.c Outdated

Comment thread src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs

Address Android trust validation review feedback

e33122c

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Restore Android trust manager JNI export

440b2ea

Keep the native callback symbol aligned with the Android NativeAOT test linker metadata while continuing to bind it through RegisterNatives. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 8, 2026 11:11

Copilot started reviewing on behalf of simonrozsival July 8, 2026 11:12 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

Comment thread src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs

Comment thread src/tasks/AndroidAppBuilder/ApkBuilder.cs

This was referenced Jul 8, 2026

[browser] Wasm.Build.Tests timeout - Timed out after 10s waiting for 'WASM EXIT' #116697

Open

browser-wasm linux Release LibraryTests queues timing out #117974

Open

Merge branch 'main' into feature/android-platform-trust-manager-107695

243f7cd

Copilot AI review requested due to automatic review settings July 9, 2026 08:17

Copilot started reviewing on behalf of simonrozsival July 9, 2026 08:18 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

rzikm approved these changes Jul 9, 2026

View reviewed changes

simonrozsival merged commit 7203f39 into dotnet:main Jul 11, 2026
170 checks passed

dotnet-maestro Bot mentioned this pull request Jul 12, 2026

[main] Source code updates from dotnet/runtime dotnet/dotnet#7679

Open

Uh oh!

Conversation

simonrozsival commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Behavioral change

Testing

Changes

Java (DotnetProxyTrustManager)

Native C (pal_sslstream.c, pal_trust_manager.c, pal_jni.c)

Managed C# (SslStream.Android.cs, SslStream.IO.cs, SslStream.Protocol.cs, Interop.OpenSsl.cs)

Build infrastructure (AndroidAppBuilder)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dotnet-policy-service Bot commented Feb 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stephentoub commented Feb 9, 2026

🤖 Copilot Code Review — PR #124173

Holistic Assessment

Detailed Findings

✅ Correctness — Trust Integration & Callback Behavior

✅ Correctness — JNI Lifecycle & Exception Handling

✅ Correctness — Managed-Side API Consistency

✅ Test Coverage — Good

💡 Performance — Consider Caching X509TrustManagerExtensions

💡 Performance — TrustManagerFactory Initialization

💡 Behavioral Change — SNI Setup Exception Type

Summary

Uh oh!

steveisok left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonrozsival commented Jul 7, 2026

Uh oh!

azure-pipelines Bot commented Jul 7, 2026

Uh oh!

simonrozsival commented Jul 8, 2026

Uh oh!

azure-pipelines Bot commented Jul 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

simonrozsival commented Feb 9, 2026 •

edited

Loading

Java (`DotnetProxyTrustManager`)

Native C (`pal_sslstream.c`, `pal_trust_manager.c`, `pal_jni.c`)

Managed C# (`SslStream.Android.cs`, `SslStream.IO.cs`, `SslStream.Protocol.cs`, `Interop.OpenSsl.cs`)

Build infrastructure (`AndroidAppBuilder`)