You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Work in progress, but at a coherent checkpoint: the full System.Net.Security suite now passes under System.Net.Security.UseNetworkFramework on macOS. Posting it now for early feedback on direction rather than as a finished change.
The main problem: over-reading past the handshake
The bug that motivated this is the one most likely to bite real callers, and it is not obvious from the symptom.
The Network.framework transport pump always read in bulk. Network.framework never asks for a specific number of bytes, so the pump speculatively reads whatever the inner stream has and feeds it in. That is fine while the connection carries nothing but TLS — but it is not true of every caller.
SqlClient tunnels the TLS handshake inside TDS packets and stops that encapsulation the moment AuthenticateAsClient returns. So the bytes immediately after the final handshake record are framed differently from the ones before it. A bulk read that ran past the end of the handshake consumed the first post-handshake record and interpreted a raw TLS record as a TDS header — surfacing as TLS error -9836, far from the actual mistake.
The fix is to make handshake reads frame precise: consume exactly one TLS record per iteration while the handshake is in flight, cancel the speculative read as soon as the handshake completes, and then leave the transport untouched until the application starts its own I/O — by which point it has finished re-framing the stream. Bulk reads resume afterwards, where they are safe.
Also fixed
SNI was never reported to servers. Network.framework parses the ClientHello internally, so unlike the other PALs the managed server cannot recover the host name from the transport, and SslStream.TargetHostName stayed empty. It is now read from connection metadata and decoded through the existing TlsFrameHelper path, so a server sees the same Unicode host the client passed. Client-side TargetHostName is deliberately left alone, since the wire form is punycode.
Truncated TLS records looked like a clean EOF. Cancelling the connection on transport EOF is what lets Dispose complete, but it also converted a mid-record truncation into a graceful close, so a caller read 0 instead of failing. The pump now tracks record boundaries and raises IOException only when the stream ends mid-record.
A use-after-free that aborted the process.Dispose completed the transport read source itself, letting the read loop finish while Network.framework still owned a framer delivery; the GCHandle was freed and the native completion then resolved a dead handle. This reproduced as a SIGABRT partway through a full test run, not as a test failure.
An unbounded strcpy for the negotiated ALPN was replaced with a length-checked copy, now shared with the server-name output.
Caveats and documented divergences
Two behaviours are documented rather than emulated, so they need a explicit decision before the switch is flipped:
ClientCertificateRequired is strict. .NET treats it as "request a certificate and let RemoteCertificateValidationCallback decide", so a callback may accept a client that sent none. Network.framework enforces the requirement itself and aborts the handshake before managed validation runs. sec_protocol_options_set_peer_authentication_optional expresses exactly the semantics .NET wants, but it is declared API_UNAVAILABLE(macos, ios, watchos, tvos) and, per its own header, is disregarded whenever peer_authentication_required is set. mTLS works normally whenever the client actually presents a certificate; only the accept-the-absence case diverges. Documented at the call site and pinned by NetworkFramework_MissingClientCertificate_FailsHandshake; the two affected rows of CertificateSelectionCallback_DelayedCertificate_OK are skipped.
Inner-stream reads happen on the transport pump, not the caller's thread, so tests asserting synchronous inner reads do not apply and are skipped for this PAL.
I considered mapping the first case to PlatformNotSupportedException, but it cannot be detected from configuration alone — only from the outcome — so it would have meant inferring intent by invoking the user's validation callback after the handshake had already failed. Documenting it is the smaller lie.
Validation
Full System.Net.Security functional suite on macOS arm64, both PALs:
PAL
Total
Failed
Skipped
SecureTransport (default)
5045
0
30
Network.framework
4782
0
89
The TLS-over-TDS scenario is additionally covered by a standalone repro that asserts no non-TLS framing is consumed; it passes on both PALs and over 15 consecutive runs.
Skips are higher under Network.framework because of the documented divergences above and pre-existing unsupported configurations that fall back to SecureTransport.
Still to do
Decide whether the ClientCertificateRequired divergence is acceptable or needs an API/behavioural answer before enabling by default.
A sweeping review pass is planned before flipping the default.
No breaking-change doc yet; that is only warranted if this becomes the default.
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.
The Network.framework PAL could lose inbound data, read past the end of the
handshake, and hang instead of reporting failures. Four related defects:
Inbound data was discarded when it arrived before the framer started.
WriteInboundWireDataAsync dropped the buffer whenever _framerHandle was still
null. The transport read loop starts with the connection, but the framer only
appears when Network.framework invokes the start handler, so a peer's first
flight could be thrown away - reliably so over an in-memory transport, where
the write is visible immediately. TLS never retransmits, so the handshake then
waited forever. Delivery now waits for the framer instead of dropping.
The read loop consumed bytes past the final handshake record. It always read in
bulk, so it could take application data that the peer had already re-framed -
SqlClient's TLS-over-TDS stream leaves TDS encapsulation only after
AuthenticateAsClient returns, and the trailing read consumed a raw TLS record as
a TDS header (reported as TLS error -9836). Handshake reads are now frame
precise, the speculative read is cancelled as soon as the handshake completes,
and the loop then idles until the application starts its own I/O.
Outbound writes used the transport's synchronous API from a Network.framework
queue, which fails on streams that reject synchronous operations, and the
failure only ever reached a pending application write. During the handshake
there is none, so the error was swallowed and the connection hung. Writes now
use the asynchronous API and failures fault the handshake.
Disposal released the application receive buffer while a native receive
completion could still write into it, and an exception escaping that callback
terminated the process. The buffer is now released only after the connection
reports cancellation, and the callback no longer lets exceptions escape.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 021e2d41-d78f-4c87-96ff-ce25bbe7be9c
SslServerAuthenticationOptions.ClientCertificateRequired was silently ignored:
nothing ever called sec_protocol_options_set_peer_authentication_required, so
the server never sent a CertificateRequest. The client therefore sent no
certificate, the verify block never produced a peer certificate, and the
callback saw a null certificate with a null chain. An application asking for
client certificates got an unauthenticated connection instead of an error.
The requirement is now plumbed from the managed options into the connection.
Network.framework has no API for advertising a custom CA list in the
CertificateRequest, so a server configured with SslCertificateTrust now stays on
SecureTransport instead of quietly dropping the caller's trust list, using the
fallback that already exists for configurations the new PAL cannot satisfy.
Also stop asserting that framer options are available. They are gone once the
framer is being torn down, which is exactly when the cleanup handler reads them,
so the assert fired during ordinary teardown in debug builds.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 021e2d41-d78f-4c87-96ff-ce25bbe7be9c
…nection
SslStream.ShutdownAsync builds a shutdown token by calling ApplyShutdownToken and
then GenerateToken. The first already handles a Network Framework context by
cancelling the connection, but the second reached the SecureTransport-only
handshake path, whose debug assertion that the context is a SafeDeleteSslContext
then terminated the process instead of failing a single test.
Network Framework emits close_notify itself once the connection is cancelled, so
there is no token for SslStream to send. Report success for that context type.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 021e2d41-d78f-4c87-96ff-ce25bbe7be9c
Three unrelated defects in the Network.framework PAL, plus the test changes
that pin the behaviour.
The server never saw the requested host name. Network.framework parses the
ClientHello itself, so the managed server cannot recover SNI from the transport
stream the way the other PALs do, and SslStream.TargetHostName stayed empty.
The negotiated name is now read back from the connection metadata and applied
to the server context. It arrives as ASCII/punycode and is decoded through the
existing TlsFrameHelper path, so a server observes the same Unicode host name
the client passed. Only the server side is updated; a client keeps the caller's
original string. AppleCryptoNative_GetConnectionInfo gained the extra output,
and the unbounded strcpy it used for the negotiated ALPN was replaced with a
length-checked copy shared by both strings.
A transport EOF in the middle of a TLS record looked like a clean shutdown.
Cancelling the connection on EOF is what lets Dispose complete, but it also
turned a truncated record into a graceful close, so a caller read 0 instead of
failing. The transport pump now tracks encrypted record boundaries and faults
the pending read with IOException when the stream ends mid-record, while an EOF
on a boundary keeps the existing cancellation path. Only the record length is
retained, so no buffering is added.
Disposal could free the callback context while Network.framework still owned a
framer delivery. Dispose completed the transport read source itself, which let
the read loop finish early; the GCHandle was then released and the native
completion resolved a freed handle, aborting the process. The delivery is left
to be completed by its callback, which is what keeps the handle alive.
Two behaviours are documented rather than emulated. Network.framework enforces
ClientCertificateRequired itself and fails the handshake when the client sends
no certificate, so RemoteCertificateValidationCallback never gets to accept the
absence: sec_protocol_options_set_peer_authentication_optional expresses what
.NET wants but is API_UNAVAILABLE on macOS, and is disregarded when
peer_authentication_required is set. Reads of the inner stream also happen on
the transport pump rather than the caller's thread, so tests asserting
synchronous inner reads do not apply to this PAL.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 80f9832a-76b3-4767-aa82-58ab20582da2
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.
… leak
Three issues found by automated review of the Network.framework PAL.
AppleCryptoNative_NwFramerDeliverInput returns -1 without invoking the
completion callback when nw_framer_message_create fails, which happens when the
connection is failing or being cancelled. The managed caller discarded that
return and awaited the completion, so the transport pump and a later Dispose
could hang on exactly the race this path exists to handle. The status is now
checked and the pending source faulted.
The transport pump treated any completed handshake as a successful one. The
handshake source completes with an Exception value on failure and is cancelled
when authentication is cancelled, so a delivery returning after either of those
moved the pump into the post-handshake state and parked it on application I/O
that would never start. The transition now requires successful completion and
the pump stops otherwise.
The framer cleanup handler looked the session queue up through the framer
options to balance the retain taken when the options were built. Options are
unavailable once teardown has started - the lookup is explicitly tolerant of
that - so the release was skipped exactly when cleanup ran, leaking a dispatch
queue per connection. The queue is now captured while the options are still
readable and released from the captured block.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 80f9832a-76b3-4767-aa82-58ab20582da2
ArrayPool<byte>.Shared.Rent(MaxTlsRecordSize) is allowed to return an array larger than MaxTlsRecordSize, and this Memory<byte> exposes the entire rented array. As a result, a header advertising a record larger than the RFC limit can pass the header.Length > buffer.Length check (for example, in a larger pool bucket) and consume more than the documented maximum. Limit the memory passed to ReadSingleTlsRecordAsync to MaxTlsRecordSize so the bound is enforced.
Session queue retain leaks when setup fails before framer startup
The explicit dispatch_retain(sessionQueue) is balanced only by the cleanup handler installed from framer_start. Any connection/listener setup failure before a framer starts (for example, listener creation/readiness or inbound-delivery failure) releases the local queue reference but never this retain, leaking the per-session queue. Tie the ownership to a cleanup path that also covers setup failures, or explicitly release it on every pre-start abort path.
Dispose released the GCHandle that native callbacks resolve once the transport
read loop had stopped, but an application send is not covered by that wait.
AppleCryptoNative_NwConnectionSend's completion block and the framer output
write both resolve the handle, so a write still in flight when the stream is
disposed can resolve a freed handle. Because GCHandle slots are reused, that
does not reliably fault: the callback can resolve to an unrelated connection
created afterwards and corrupt it.
An in-flight send is now published before the native call is issued and is
completed only after both callbacks have been observed, and Dispose waits for
it before releasing the handle, deferring the release when it does not settle
in the bounded wait.
The wait deliberately runs after Dispose faults the pending write completion.
A write parked on that source is unblocked by Dispose itself, so waiting any
earlier would deadlock the two against each other.
The accompanying test covers the shutdown ordering but does not reproduce the
use-after-free: on loopback the send completion has already run by the time
Dispose is reached, so it passes with and without the wait. It is kept as a
regression guard against Dispose hanging or crashing on a racing write.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 80f9832a-76b3-4767-aa82-58ab20582da2
The explicit dispatch_retain is only balanced by the cleanup handler installed in framer_start. If nw_connection_create or nw_listener_create fails, no framer starts and that handler is never installed; the existing dispatch_release(sessionQueue) in those failure paths only balances the queue created at lines 401/455, leaving this extra retain leaked. Add an error-path rollback for the options retain (and cover any connection-failure path before framer_start).
This file contains hidden or 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
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.
contributes to #1979
Work in progress, but at a coherent checkpoint: the full
System.Net.Securitysuite now passes underSystem.Net.Security.UseNetworkFrameworkon macOS. Posting it now for early feedback on direction rather than as a finished change.The main problem: over-reading past the handshake
The bug that motivated this is the one most likely to bite real callers, and it is not obvious from the symptom.
The Network.framework transport pump always read in bulk. Network.framework never asks for a specific number of bytes, so the pump speculatively reads whatever the inner stream has and feeds it in. That is fine while the connection carries nothing but TLS — but it is not true of every caller.
SqlClienttunnels the TLS handshake inside TDS packets and stops that encapsulation the momentAuthenticateAsClientreturns. So the bytes immediately after the final handshake record are framed differently from the ones before it. A bulk read that ran past the end of the handshake consumed the first post-handshake record and interpreted a raw TLS record as a TDS header — surfacing as TLS error-9836, far from the actual mistake.The fix is to make handshake reads frame precise: consume exactly one TLS record per iteration while the handshake is in flight, cancel the speculative read as soon as the handshake completes, and then leave the transport untouched until the application starts its own I/O — by which point it has finished re-framing the stream. Bulk reads resume afterwards, where they are safe.
Also fixed
SslStream.TargetHostNamestayed empty. It is now read from connection metadata and decoded through the existingTlsFrameHelperpath, so a server sees the same Unicode host the client passed. Client-sideTargetHostNameis deliberately left alone, since the wire form is punycode.Disposecomplete, but it also converted a mid-record truncation into a graceful close, so a caller read0instead of failing. The pump now tracks record boundaries and raisesIOExceptiononly when the stream ends mid-record.Disposecompleted the transport read source itself, letting the read loop finish while Network.framework still owned a framer delivery; theGCHandlewas freed and the native completion then resolved a dead handle. This reproduced as aSIGABRTpartway through a full test run, not as a test failure.strcpyfor the negotiated ALPN was replaced with a length-checked copy, now shared with the server-name output.Caveats and documented divergences
Two behaviours are documented rather than emulated, so they need a explicit decision before the switch is flipped:
ClientCertificateRequiredis strict. .NET treats it as "request a certificate and letRemoteCertificateValidationCallbackdecide", so a callback may accept a client that sent none. Network.framework enforces the requirement itself and aborts the handshake before managed validation runs.sec_protocol_options_set_peer_authentication_optionalexpresses exactly the semantics .NET wants, but it is declaredAPI_UNAVAILABLE(macos, ios, watchos, tvos)and, per its own header, is disregarded wheneverpeer_authentication_requiredis set. mTLS works normally whenever the client actually presents a certificate; only the accept-the-absence case diverges. Documented at the call site and pinned byNetworkFramework_MissingClientCertificate_FailsHandshake; the two affected rows ofCertificateSelectionCallback_DelayedCertificate_OKare skipped.I considered mapping the first case to
PlatformNotSupportedException, but it cannot be detected from configuration alone — only from the outcome — so it would have meant inferring intent by invoking the user's validation callback after the handshake had already failed. Documenting it is the smaller lie.Validation
Full
System.Net.Securityfunctional suite on macOS arm64, both PALs:The TLS-over-TDS scenario is additionally covered by a standalone repro that asserts no non-TLS framing is consumed; it passes on both PALs and over 15 consecutive runs.
Skips are higher under Network.framework because of the documented divergences above and pre-existing unsupported configurations that fall back to SecureTransport.
Still to do
ClientCertificateRequireddivergence is acceptable or needs an API/behavioural answer before enabling by default.