Skip to content

[Android] Enable TlsContext/TlsSession on Android - #131461

Open
wfurt wants to merge 12 commits into
dotnet:mainfrom
wfurt:TlsSession-android
Open

wfurt wants to merge 12 commits into
dotnet:mainfrom
wfurt:TlsSession-android

Conversation

@wfurt

@wfurt wfurt commented Jul 28, 2026

Copy link
Copy Markdown
Member

Phase 1 of the SslStream → TlsSession unification effort. Bring Android onto the real TlsSession implementation so it stops being the only platform stuck on TlsSession.Stub.cs (throwing PlatformNotSupportedException), and re-enable the System.Net.Security functional test suite on Android CI to actually exercise the new code path.

Background

TlsContext / TlsSession shipped as experimental API in #130366. On every platform except Android, the real implementation compiles and works (macOS/iOS/tvOS via SecureTransport; Linux/FreeBSD via OpenSSL; Windows via SChannel). Android was stubbed because Pal.Android/SafeDeleteSslContext required SslAuthenticationOptions.SslStreamProxy, and the SslStream.JavaProxy that populated it was hardwired to close over an SslStream instance for its JSSE trust-manager back-channel. TlsSession has no SslStream, so SafeDeleteSslContext(options) would throw immediately, and the source files were shut off entirely for Android via TlsSession.Stub.cs.

Changes

  • Decouple JavaProxy from SslStream (SslStream.Android.cs): the proxy now carries a Func<IntPtr, RemoteCertificateValidationResult> validator. SslStream keeps a convenience overload new JavaProxy(this) that closes over its private VerifyRemoteCertificate(IntPtr) method, so SslStream behavior is bit-for-bit identical.
  • New TlsSession.Android.cs: populates each per-session options bag with a session-owned JavaProxy whose validator mirrors SslStream.Android's trust-manager routing (ShouldRespectPlatformValidation + VerifyRemoteCertificateCore). Runs synchronously from the JSSE trust-manager callback, since JSSE needs a synchronous decision — same constraint that shapes SslStream on Android today.
  • TlsSession.cs: added a partial void InitializePlatformSpecificSessionState() hook fired from InitializeFromContext so per-platform partials (currently just the Android one) can wire session-local native bridge state without polluting the shared file.
  • System.Net.Security.csproj gating: real TlsSession.cs / TlsBufferSession.cs / TlsSocketSession.cs now compile on Android; TlsSession.Stub.cs is restricted to the "no target platform" netstandard build only; new TlsSession.Android.cs conditioned on UseAndroidCrypto.
  • Pal.Android/SafeDeleteSslContext moved to System.Net.Security namespace to match Windows/Linux (and the base SafeDeleteContext type). Purely a rename; the type is internal sealed. Lets us drop the #elif TARGET_ANDROID special case from TlsSession.cs's TlsSecurityContext alias block. The equivalent macOS mismatch (Pal.OSX/SafeDeleteSslContext in System.Net) is untouched — macOS's TlsSession uses the base class already.
  • SslStreamPal.Android.cs signature cleanup: AcceptSecurityContext / InitializeSecurityContext took ref SafeFreeCredentials credential (non-nullable) instead of the ref SafeFreeCredentials? used on Windows/Linux/OSX. Android's AcquireCredentialsHandle always returns null and HandshakeInternal never reads the parameter, so this is a no-op behaviourally and just aligns the four PAL signatures.
  • Tests: enabled TlsSessionTests.cs on Android in System.Net.Security.Tests.csproj, and un-excluded System.Net.Security.Tests from Android CI in tests.proj so the coverage actually runs.

Net diff: ~140 added / ~14 removed across 8 files (excluding tests.proj).

Coordination with #130755

@simonrozsival's #130755 ([Android] Support delayed client certificate selection in SslStream) is currently open and touches the same JavaProxy type. It changes _handle from GCHandle? → GCHandle<JavaProxy>, adds a second UnmanagedCallersOnly callback (SelectClientCertificate), renames RegisterRemoteCertificateValidationCallback() → RegisterCallbacks(), and swaps the Android interop entry point. There will be a real merge conflict on SslStream.Android.cs no matter which lands first.

Suggestions:

  1. Land [Android] Support delayed client certificate selection in SslStream #130755 first (it's older, more feature-complete, and the delayed cert-selection story is user-visible). Once merged I rebase this PR: apply the delegate-based JavaProxy ctor on top of the strongly-typed GCHandle<JavaProxy> shape.
  2. Or land these simultaneously and let whichever merges second handle a small rebase — the conflicting region is small (the ctor and the _handle field).

Either way is fine; happy to coordinate.

Follow-ups (not in this PR)

  • Pal.OSX/SafeDeleteSslContext still lives under System.Net — cosmetic-only, can move to a separate cleanup PR.
  • If the un-exclusion of System.Net.Security.Tests.csproj on Android CI causes the emulator to time out (which is why it was excluded originally), we'll narrow the scope — either arch-specific or a slim standalone Android test project along the lines of the existing AndroidPlatformTrustTests.
  • Phase 2 of the unification (introducing SslPlatformContext and consolidating the three per-platform caching layers) is a separate PR.
  • Phase 3 (SslStream internally driven by TlsBufferSession) is another separate PR after that.

Sequencing

Follows #130366 (merged) and #131457 (small doc/assert follow-up, open). Unblocks the SslStream → TlsSession adapter work — that adapter can't ship until TlsSession is universally supported.

Note

This PR description was drafted with GitHub Copilot assistance.

Copilot AI lite review requested due to automatic review settings July 28, 2026 12:37
@wfurt

wfurt commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

Triggering Android CI to validate the new TlsSession Android implementation:

/azp run runtime-extra-platforms

Note

This comment was posted with GitHub Copilot assistance.

@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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR enables the real TlsSession/TlsContext implementation on Android by decoupling Android’s JSSE bridge (SslStream.JavaProxy) from SslStream, wiring a session-owned proxy for TlsSession, and updating project file gating so the non-stub TLS session code compiles for Android and is exercised by the Android CI test matrix.

Changes:

  • Refactored SslStream.JavaProxy to be delegate-based so both SslStream and TlsSession can own/use it on Android.
  • Added an Android-specific TlsSession partial to attach the proxy and perform synchronous certificate validation through the existing shared validation core.
  • Updated build/test project gating to compile TlsSession on Android and to stop excluding System.Net.Security functional tests from Android CI.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/libraries/tests.proj Removes the exclusion that prevented System.Net.Security functional tests from running on Android CI.
src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj Includes TlsSessionTests.cs for Android builds.
src/libraries/System.Net.Security/src/System/Net/Security/TlsSession.cs Adds a platform hook to allow platform-specific per-session initialization (Android proxy wiring).
src/libraries/System.Net.Security/src/System/Net/Security/TlsSession.Android.cs New Android partial that attaches a session-owned JavaProxy and routes JSSE trust-manager validation into VerifyRemoteCertificateCore.
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs Aligns PAL signatures with other platforms by using nullable SafeFreeCredentials?.
src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs Refactors JavaProxy to hold a validator delegate instead of a hard SslStream reference.
src/libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs Moves SafeDeleteSslContext into the System.Net.Security namespace for consistency with other platforms.
src/libraries/System.Net.Security/src/System.Net.Security.csproj Enables real TlsSession compilation on Android, gates the stub to netstandard-only, and adds TlsSession.Android.cs under UseAndroidCrypto.

Comment thread src/libraries/System.Net.Security/src/System/Net/Security/TlsSession.Android.cs Outdated
@wfurt

wfurt commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

/azp run runtime-android

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

wfurt added a commit to wfurt/runtime that referenced this pull request Jul 29, 2026
- Add TestPlatforms.Android to the class-level [PlatformSpecific]
  attribute on TlsSessionTests so the tests actually execute on
  Android CI, not just compile.
- Update TlsSession.Android.cs comment to refer to the type by its
  proper namespace (System.Net.Security.SafeDeleteSslContext) instead
  of the folder-based Pal.Android pseudo-namespace, which was stale
  after the Android SafeDeleteSslContext moved into System.Net.Security
  in the previous commit.
Copilot AI review requested due to automatic review settings July 29, 2026 09:08
@wfurt

wfurt commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

/azp run runtime-android

Note

This comment was posted with GitHub Copilot assistance.

@azure-pipelines

Copy link
Copy Markdown
No pipelines are associated with this pull request.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

src/libraries/System.Net.Security/src/System/Net/Security/TlsSession.Android.cs:86

  • The finally block only disposes the chain when there is no user cert-validation callback, and it doesn’t dispose certificates added to ChainPolicy.ExtraStore by GetRemoteCertificate. Dispose the chain unconditionally, and when there is no user callback, also dispose ExtraStore entries added after the preexisting count and dispose ChainElements certificates (mirrors SslStream.VerifyRemoteCertificate cleanup).
            finally
            {
                // Mirror SslStream's chain-cleanup: dispose the chain elements that
                // VerifyRemoteCertificateCore populated (unless the caller has a user
                // callback that may retain them), matching the behavior of SslStream's
                // VerifyRemoteCertificate wrapper path.
                if (chain is not null && _options.CertValidationDelegate is null)
                {
                    for (int i = 0; i < chain.ChainElements.Count; i++)
                    {
                        chain.ChainElements[i].Certificate.Dispose();
                    }
                    chain.Dispose();
                }
            }

src/libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs:10

  • After moving SafeDeleteSslContext into the System.Net.Security namespace, unqualified references to IPAddress no longer resolve (IPAddress is in System.Net). Add a System.Net using (or fully qualify the type) while keeping the existing using directives intact.
using System.Diagnostics;
using System.Collections.Generic;
using System.Runtime.InteropServices;
using System.Security.Authentication;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Threading;

src/libraries/System.Net.Security/src/System/Net/Security/TlsSession.Android.cs:45

  • CertificateValidationPal.GetRemoteCertificate can append certificates into CertificateChainPolicy.ExtraStore; to avoid leaking/discarding user-provided entries, capture the preexisting ExtraStore.Count before calling into the PAL so the cleanup code can dispose only the added certs (matching SslStream.VerifyRemoteCertificate).

This issue also appears on line 72 of the same file.

            ProtocolToken alertToken = default;
            X509Chain? chain = null;

@wfurt

wfurt commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

can you take a look at this change @simonrozsival @kotlarmilos ? This closes Android gap for API we added to .NET 11. My goal would be to consolidate SslStream and make this bottom part e.g. this would be the pair where platform magic happens and SslStream would consume it providing the Stream interface. That may or may not happen for in 11 but we cannot make it without Android implementation.

@wfurt

wfurt commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

/azp run runtime-android

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI review requested due to automatic review settings July 29, 2026 15:22
@wfurt

wfurt commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

/azp run runtime-android

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

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

  • The comment above _validator says TlsSession routes the platform trust result into session state, but the Android TlsSession implementation currently uses an accept-and-defer validator that does not do that. This comment is misleading; either update it to describe the deferral behavior, or adjust TlsSession.Android.cs to actually route/store platform validation details.
            // Session-side validator. SslStream supplies one that closes over its own
            // VerifyRemoteCertificate(IntPtr) method; TlsSession supplies one that routes
            // the platform trust result into the session's own state. Neither implementation
            // is aware of the other.

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

  • JavaProxy(SslStream sslStream) forwards to the delegate-based ctor without validating sslStream. If this ever gets called with null (e.g., from future refactoring), the delegate will be created with a null target and fail later in a harder-to-diagnose way. It’s safer to throw ArgumentNullException up-front.
            public JavaProxy(SslStream sslStream)
                : this(sslStream.VerifyRemoteCertificate)
            {
            }

src/libraries/System.Net.Security/tests/FunctionalTests/TlsSessionTests.cs:23

  • Adding TestPlatforms.Android at the class level will also run tests that are explicitly described as OpenSSL-only (e.g. ClientSession_WantCredentials_SetClientCertificateContext_ResumesHandshake and SetClientCertificateContext_ConcurrentSessionsOnSharedContext_DoNotRace). Android’s SslStreamPal handshake path never returns CredentialsNeeded, so these are very likely to fail/hang on Android unless they’re additionally skipped/conditioned for Android.
    [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.Windows | TestPlatforms.OSX | TestPlatforms.Android)]
    public class TlsSessionTests

src/libraries/tests.proj:192

  • This change removes the Android/Bionic exclusion for System.Net.Security.Tests, but the project is still excluded earlier for TargetsLinuxBionic == true with TargetArchitecture == x64 (commented as a Helix timeout). If the Android CI lane you’re trying to re-enable corresponds to that Bionic x64 condition, the suite will remain disabled there.
  <ItemGroup Condition="('$(TargetOS)' == 'android' or '$(TargetsLinuxBionic)' == 'true') and '$(RunDisabledAndroidTests)' != 'true'">
    <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Console\tests\System.Console.Tests.csproj" />
    <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime\tests\System.IO.FileSystem.Tests\System.IO.FileSystem.Tests.csproj" />
    <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.IO.Ports\tests\System.IO.Ports.Tests.csproj" />
  </ItemGroup>

Comment thread src/libraries/System.Net.Security/src/System/Net/Security/TlsSession.Android.cs Outdated
Copilot AI review requested due to automatic review settings July 30, 2026 09:41
@wfurt

wfurt commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

/azp run runtime-android

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Enabling System.Net.Security.Tests on Android surfaced 12 deterministic
failures (identical on android-arm and android-arm64).

Seven are TlsSession tests hitting JSSE limitations: no deferred
client-credential prompt, no server-side session cache / ticket issuance,
and no retry-verify equivalent in the trust manager. Skip these on Android
alongside the equivalent OSX exclusions.

Four are pre-existing SslStream gaps on Android (oversized ALPN list
surfacing AuthenticationException, and a hang on a zero-payload TLS frame).

The last two are CertificateSelectionCallback_DelayedCertificate_OK, which
already threw SkipTestException for Android but was declared [Theory].
SkipTestException is only translated into a skip for tests discovered via
ConditionalFact/ConditionalTheory, so it was reported as a failure instead.
Switch it to [ConditionalTheory].

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 9, 2026 04:15
@wfurt

wfurt commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

/azp run runtime-android

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Android test re-enablement appears incomplete because System.Net.Security.Tests is still excluded for at least one Android-related test configuration in src/libraries/tests.proj.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/libraries/tests.proj:192

  • In addition to removing the general Android exclusion, System.Net.Security.Tests is still excluded for the TargetsLinuxBionic == true / TargetArchitecture == x64 Android test runs earlier in this file. If the intent is to re-enable the functional test suite on Android CI, this remaining exclusion likely prevents the suite from running in that configuration.
  • Files reviewed: 13/13 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

…auses

Review fixes:
- InitializePlatformSpecificSessionState now installs the JavaProxy only
  when the options bag does not already have one. Not reachable today
  (the wedge is not compiled for Android and Clone() does not copy
  SslStreamProxy), but consolidating SslStream onto TlsSession would
  enable the wedge there and make the overwrite and GCHandle leak live.
- The platform trust verdict is assigned rather than latched, so a later
  validation is not tainted by an earlier rejection.

Skip reasons now cite mechanisms confirmed against the source rather than
inferred from symptoms:
- Deferred client credentials are OpenSSL-only. CredentialsNeeded is
  produced by the Windows, OpenSSL, Unix and OSX PALs but not by
  SslStreamPal.Android; JSSE takes the KeyManagers up-front at
  SSLContext.init, so no mid-handshake CertificateRequest is surfaced.
- AndroidCryptoNative_SSLStreamCreate builds a fresh SSLContext per
  session, so the JSSE server session cache is never shared between
  connections and the second handshake cannot resume.
- The protocol-mismatch reason now states only the observed behavior.
  The earlier text blamed unapplied EnabledSslProtocols, which is wrong:
  SafeDeleteSslContext applies them correctly. Root cause is not yet
  established and the reason says so.

Verified on an android-arm64 emulator: 4892 passed, 0 failed, 38 skipped,
matching the 8-round stability baseline.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 10, 2026 00:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Android-specific test skips are currently ineffective on some tests (non-conditional xUnit attributes), and the Android TlsSession proxy wiring has correctness/ownership issues that should be fixed before enabling CI coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/libraries/System.Net.Security/tests/FunctionalTests/TlsSessionTests.cs:2418

  • SkipOnPlatform only takes effect with ConditionalFact/ConditionalTheory. This test is using [Fact], so the new Android skip will be ignored and the test will still run on Android (despite the hang note).
        [Fact]
        [SkipOnPlatform(TestPlatforms.Android, "JSSE does not fail the handshake on a protocol mismatch through the socket-replay BIO path; the peer hangs instead.")]
        public async Task SocketBoundSession_DeferredOptions_ProtocolMismatch_Fails()
  • Files reviewed: 13/13 changed files
  • Comments generated: 4
  • Review effort level: Lite

Comment thread src/libraries/System.Net.Security/src/System/Net/Security/TlsSession.Android.cs Outdated
Comment thread src/libraries/tests.proj
@wfurt

wfurt commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

/azp run runtime-android

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

wfurt and others added 2 commits September 10, 2026 10:22
Merging main brought in four new RequestClientCertificate tests that skip
macOS but not Android. Enabling System.Net.Security.Tests on Android makes
them run there, where the PAL throws PlatformNotSupportedException:
Android has no post-handshake client authentication, the same gap
SecureTransport has.

This matches how the rest of the file already treats Android:
TestConfiguration.SupportsRenegotiation excludes it, and the older sibling
ServerSession_RequestClientCertificate_Tls12_ProducesHandshakeBytes is
gated on that property and skips correctly.

android-arm64 emulator, after the merge: 4906 passed, 0 failed, 38 skipped.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 10, 2026 19:38
@wfurt

wfurt commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

/azp run runtime-android

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It changes Android TLS session/certificate-validation wiring and CI test enablement in security-sensitive code paths, which warrants final human review despite no concrete issues found in the diff.

Review details
  • Files reviewed: 13/13 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@wfurt

wfurt commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

this should be ready for review @simonrozsival @kotlarmilos @matouskozak. The goal for .NET 12 is to make the low-level primitives the heart of the SslStream and leave SslStream be the legacy Stream interface. That should allow to consume the functionality without binding to the streaming contract. Lack of the implementation currently blocks the progress. I did install the emulators and verify everything is passing locally as well as started the Android pipeline and I did not see related failures.

Comment on lines +28 to +33
// Invoked synchronously from Android's DotnetProxyTrustManager. Always accepts so
// the handshake progresses; the platform verdict (if respected) is recorded and
// surfaced later through AcceptWithDefaultValidation. The verdict is assigned rather
// than latched so a later validation (e.g. renegotiation with a different chain) is
// not tainted by an earlier rejection.
private SslStream.JavaProxy.RemoteCertificateValidationResult AcceptAndDeferPlatformValidation(IntPtr platformValidationError)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I remember that always accepting during handshake and only surfacing the result later did not work on Android and was causing all sorts of problem. That's the reason why we even needed the JavaProxy and hacking a way to integrate it in SslStream. I would prefer if we would not do it this way in TlsSession. Is that possible? Is that something that could be done later? Wouldn't it be better to do the actual validation before the handshake completes on other platforms too?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

do you have specific examples? Maybe you can ping me privately. I'm open to what ever this needs. As I mentioned , the goal is to isolate TLS functionality here and make SslStream consumer of it. So in general it needs to do everything SslStream needs. I used my local setup with emulator and the CI Android pipeline for verification. But I can broaden that as needed. Also we may do follow-up fixes if/when need to. This is primarily for the completeness so we can start on the SslStream internal cleanup.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I did little bit more digging @simonrozsival

The fundamental problem is that the trust manager runs inside the PAL call on the caller's thread. To block it until the caller posts a verdict, the caller can't be the blocked thread, or it can never return from Handshake() to supply one. And since we can't know which frame carries the peer Certificate, we can't offload just that call — the session would have to drive all PAL interaction on a dedicated thread from the start.

So it did work for SslStream with synchronous callback. But that is problematic in general as the validation may need to fetch intermediates or do other IO (like revocation check) so to do it right would should probably add some ways hot to support asynchronous validation in SslStream.

@rzikm did some work for OpenSSL but that still have some caveats. And this goes possibly beyond handshake: with TLS 1.3 post-handshake auth the peer certificate arrives during application data, so the trust manager can fire inside Read/Write/RequestClientCertificate as well. So every PAL entry point becomes a cross-thread handoff, with the worker JNI-attached for the session's lifetime.

I'm not sure what would be good way out of this and I'm open to suggestions.

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.

6 participants