Linux SslStream: custom BIO_METHOD over managed buffer windows#128245
Open
rzikm wants to merge 20 commits into
Open
Linux SslStream: custom BIO_METHOD over managed buffer windows#128245rzikm wants to merge 20 commits into
rzikm wants to merge 20 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces Linux/OpenSSL SslStream memory BIO staging with a custom managed-window BIO to reduce TLS record copies, and also includes a separate NegotiateStream stale-buffer bug fix.
Changes:
- Adds native managed-span BIO APIs, OpenSSL shim entries, and exports.
- Updates Unix
SslStreamhandshake/encrypt/decrypt paths to use managed read/write windows plus spill draining. - Fixes NegotiateStream read-buffer state after mid-frame read failure and adds a regression test.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/native/libs/System.Security.Cryptography.Native/pal_bio.h |
Declares managed-span BIO native APIs. |
src/native/libs/System.Security.Cryptography.Native/pal_bio.c |
Implements custom BIO_METHOD with read carry and write spill buffers. |
src/native/libs/System.Security.Cryptography.Native/opensslshim.h |
Adds OpenSSL BIO method/flag function shims. |
src/native/libs/System.Security.Cryptography.Native/entrypoints.c |
Exports the new native BIO entry points. |
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs |
Adds P/Invokes and switches Unix SSL handles to managed-span BIOs. |
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs |
Reworks Unix OpenSSL handshake/encrypt/decrypt to arm BIO windows and drain spill output. |
src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs |
Defers read-buffer state updates until reads/decryption succeed. |
src/libraries/System.Net.Security/tests/FunctionalTests/NegotiateStreamStreamToStreamTest.cs |
Adds a regression test for stale data after mid-frame read failure. |
Replace the pair of BIO_s_mem instances backing each SSL handle on Linux with a custom BIO_METHOD that reads/writes directly into caller-supplied managed buffer windows, with a heap-backed spill buffer for output overflow and a heap-backed carry buffer for unconsumed input bytes. This eliminates one memcpy per TLS record in both directions (encrypt and decrypt) by allowing OpenSSL to read plaintext from and write ciphertext into managed buffers in-place, instead of staging through BIO_s_mem. Native side (src/native/libs/System.Security.Cryptography.Native/): * pal_bio.c gains a ManagedSpanBio implementation (read/write/ctrl callbacks, lazy BIO_METHOD init via pthread_once) plus seven exports: BioNewManagedSpan, BioSetReadWindow, BioClearReadWindow, BioSetWriteWindow, BioGetWriteResult, BioDrainSpill, BioResetManagedSpan. * When BioClearReadWindow is called with unread bytes still in the window, the tail is copied into a per-BIO readCarry buffer so the next BIO_read drains it before any new window. This preserves the BIO_s_mem semantic that the SslStreamPal layer relies on. * opensslshim.h adds the BIO_meth_* / BIO_get_data / BIO_set_data / BIO_get_new_index / BIO_clear_flags / BIO_test_flags / BIO_set_init / BIO_set_flags shim entries (all required since OpenSSL 1.1.0). * entrypoints.c registers the new exports. Managed side: * Interop.Ssl.cs declares the seven new P/Invokes and switches SafeSslHandle.Create to allocate ManagedSpan BIOs instead of memory BIOs. * Interop.OpenSsl.cs rewrites Decrypt, Encrypt and DoSslHandshake to pin caller buffers, call BioSet*Window before the SSL_* operation, and BioClearReadWindow / BioGetWriteResult / BioDrainSpill afterwards. New helpers ComputeMaxTlsOutput and DrainOutputBioSpill centralise the output-bound logic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
a9e5bb6 to
a134137
Compare
- pal_bio: fail BIO_read on lost carry bytes instead of silently dropping - pal_bio: BIO_CTRL_RESET clears window pointers + error flag - pal_bio: drop unused BioResetManagedSpan entry point - DoSslHandshake/Encrypt/Decrypt: clear BIO windows in finally inside fixed - Encrypt: snapshot pre-write Size so drained spill bytes survive a failed SSL_write instead of being reset to 0 - Encrypt: pass only the per-record upper bound to EnsureAvailableSpace (not Size + upperBound, which over-allocates by Size) - ComputeMaxTlsOutput: use OpenSSL's SSL3_RT_MAX_ENCRYPTED_OVERHEAD (256) per record instead of the 128-byte estimate that could trigger the spill fallback for legitimate cipher suites Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Symmetric to the existing custom-BIO encrypt optimization, this change threads the caller-supplied Memory<byte> all the way through to SSL_read so that OpenSSL writes decrypted plaintext directly into the user destination, eliminating the intermediate copy from the internal encrypted buffer to the user buffer (CopyDecryptedData on the read path) for the common case where the user buffer has enough room. Approach: - Split Interop.OpenSsl.Decrypt into a (ReadOnlySpan<byte> input, Span<byte> output) form: the input span feeds the BIO read window for ciphertext, the output span is the SSL_read destination for plaintext. The legacy in-place call site (DecryptMessage) now passes the same span for both, preserving today behavior. - Add SSL_pending wrapper (CryptoNative_SslPending) so we can detect plaintext residual that OpenSSL buffered internally when the user span was smaller than a records plaintext. The next read drains it via DecryptMessageDirect(empty input, user buffer) before any network IO. - New SslStreamPal API (Unix only for now): DecryptMessageDirect plus IsDirectDecryptSupported. Other PALs (Windows/OSX/Android) expose IsDirectDecryptSupported=false and a throwing stub so the JIT eliminates the new branch on those platforms. - SslStream.IO.ReadAsyncInternal: gated on SslStreamPal.IsDirectDecryptSupported, non-empty user buffer and no in-flight rehandshake, uses the new direct path. Non-OK status copies the direct-written bytes into extraBuffer so the existing Renegotiate/ContextExpired handlers keep working. The net_ssl_renegotiate_buffer guard now also checks _palHasPendingPlaintext to keep the NegotiateClientCertificateAsync_PendingDecryptedData_Throws contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit 7371f2e. The direct-decrypt optimization caused a record-layer failure (error:0A000139:SSL routines::record layer failure) under HTTP/2 with concurrency >= 2, where the client receives large response payloads via direct decrypt. HTTP/1.1 keep-alive and HTTP/1.1 connection: close paths were not affected and the original PR validation did not exercise HTTP/2 multiplexed reads. Keeping the custom-BIO encrypt optimization (commit a134137), which remains correct under all protocols tested. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bartonjs
reviewed
May 15, 2026
Address PR review feedback by collapsing the per-operation BIO setup,
SSL_{do_handshake,read,write}, BIO result retrieval and ERR_clear_error
into a single P/Invoke per TLS operation (CryptoNative_Ssl{Handshake,
Encrypt,Decrypt}). This removes three GC suspend/resume transitions per
TLS read or write.
On the read path, the atomic SslDecrypt now takes separate input
(ciphertext) and output (user buffer) pointers. When the user buffer is
large enough to receive a full TLS record plaintext (>= 16 KB), the
decrypted bytes are written directly into the user-provided memory,
avoiding the intermediate copy from _buffer.DecryptedSpan via
CopyDecryptedData. Smaller reads continue to use the in-place path,
keeping the implementation free of partial-record/drain state.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
3 tasks
Contributor
|
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones |
…oGetWriteResult docs - SslDecrypt: previous comment described a (removed) plaintextPending output; rewrite to describe the leftoverOffset/leftoverLength contract (in-place drain back into the input buffer when destination is too small or empty). - BioGetWriteResult: previous comment claimed both counts reset on SetWriteWindow, but only writtenToWindow does. Clarify that spillLen accumulates across operations and is only drained by BioDrainSpill (so KeyUpdate/alerts emitted during SSL_read are preserved for the next outgoing call). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a managed-side test hook in Interop.OpenSsl that, when DOTNET_SSL_FORCE_BIO_SPILL=1 is set in the environment, passes a zero-length write window to the SslHandshake and SslEncrypt single-shot shims. With no window, every byte the SSL implementation writes ends up in the BIO spill buffer instead of the caller-supplied window. This lets functional tests cover the spill semantics that would otherwise only trigger for outputs larger than our window estimate. Adds SslStreamForceSpillTests (Linux-only, RemoteExecutor-based): - ForceSpill_PingPong_Succeeds - ForceSpill_LargeTransfer_Succeeds (1B, 64KiB, 256KiB, 1MiB) - ForceSpill_BidirectionalStress_Succeeds Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
74bdca9 to
649e1ad
Compare
…gs, whitespace - pal_ssl.c CryptoNative_SslDecrypt: derive errorCode from whichever SSL_read determined the outcome (first when it returned >0, otherwise the second), and only publish leftoverLength when the second SSL_read returned >0. Renamed an inner local to avoid shadowing. - pal_bio.c ManagedSpanBioCtrl: collapse to a single BIO_CTRL_FLUSH case. Empirical verification across the full System.Net.Security test suite shows only BIO_CTRL_FLUSH is ever issued against this BIO when it is plugged into the SSL state machine; BIO_CTRL_RESET / PENDING / WPENDING / EOF are never invoked. Returning 0 for unhandled commands correctly answers BIO_CTRL_PUSH /POP and the kTLS probes BIO_CTRL_GET_KTLS_SEND/RECV. Removing the RESET handler also sidesteps the unsafe choice of either preserving or dropping the spill buffer of out-of-band bytes (alerts/KeyUpdate) that have not yet been drained. - pal_bio.c ManagedSpanBioRead/Write: move BIO_clear_retry_flags(bio) after the bio != NULL check so the defensive null handling is consistent. - Interop.OpenSsl.cs Decrypt: expand the all-input-consumed assertion comment to document why consumed is not plumbed to the managed surface. - Interop.Ssl.cs / SslStream.IO.cs: collapse stray blank lines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SslStream.WriteAsyncChunked clamps each chunk to MaxDataSize before calling EncryptMessage, and on Unix MaxDataSize is at most StreamSizes.Default.MaximumMessage = 32 * 1024. The worst-case computation in ComputeMaxTlsOutput therefore yields ~33 KiB, orders of magnitude below int.MaxValue, so the multiplications and additions cannot overflow. Add a comment documenting the invariant and a Debug.Assert that catches future changes to the chunking layer that would invalidate it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The unified CryptoNative_SslHandshake shim (which sets up the input/output BIO windows and calls SSL_do_handshake in a single P/Invoke) has fully replaced the legacy CryptoNative_SslDoHandshake entry point. There are no remaining managed callers, so the declaration, definition, P/Invoke import, and DllImport table entry can be removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Opportunistically decrypt the first chunk of plaintext directly into the caller-provided destination span (when one is supplied), mirroring the behavior the Linux/OpenSSL PAL already implements. Remaining plaintext (if any) is captured in-place via the existing leftoverOffset/leftoverLength contract, so SslStream can pick it up without an extra memcpy on the hot path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
3 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Note
This pull request was prepared with AI assistance (GitHub Copilot CLI). The code, build, and test validation were performed by the assistant under my supervision.
Summary
Replace the two
BIO_s_meminstances backing each SSL handle on Linux with a customBIO_METHODthat operates on caller-supplied managed buffer windows, and refactor the Linux PAL so every SSL operation (SSL_do_handshake,SSL_write,SSL_read) is wrapped in a single P/Invoke that installs the windows, runs the SSL call, and tears them down. Combined, these changes:SSL_readwrite plaintext directly into the user'sMemory<byte>so the prior_buffer → user bufferCopyDecryptedDatacopy disappears on the common case,Before
BIO_write(copy 1) →BIO_s_memstorage →SSL_readreads from BIO (copy 2 inside OpenSSL record buffer) → decrypt in place intoSslStream._buffer→CopyDecryptedData(copy 3) into userMemory<byte>SSL_write→BIO_writetoBIO_s_mem(copy 1) →BIO_readfrom BIO tooutToken(copy 2)After
SSL_readwrites plaintext directly into the user'sMemory<byte>; if the destination is too small the remaining plaintext is drained in-place back into the input buffer (1 OpenSSL-internal copy total)SSL_write→BIO_writelands directly in the managedoutTokenwindow (1 OpenSSL-internal copy total)Pieces
1.
ManagedSpanBio(pal_bio.c)BIO_METHODwhose per-BIO context tracks a read window (caller's ciphertext span), a write window (caller's outgoing-token span) and a heap-backed spill buffer.BIO_writelands directly in the window; once it fills, the BIO falls back to the heap spill. The spill is not reset between SSL calls, it absorbs out-of-band bytes that OpenSSL emits during anSSL_read(alerts, TLS 1.3 KeyUpdate / post-handshake auth frames) and is drained on the next outgoing operation.BIO_readconsumes the window in place; any unread tail stays in the caller's buffer and is re-supplied on the next call (no BIO-internal carry buffer).2. Single-call SSL operations (
pal_ssl.{h,c})CryptoNative_SslHandshake,CryptoNative_SslEncrypt,CryptoNative_SslDecrypt. Each function: clears the error queue, installs the input/output windows, runsSSL_do_handshake/SSL_write/SSL_read, reportswritten/pending(write side) andconsumed/leftover(read side), then tears the windows down, all without leaving the native frame.CryptoNative_SslDecryptsupports the "destination too small" case by doing a follow-upSSL_readin-place into the input buffer whenSSL_pending > 0(or when the caller passes a zero-capacity destination, e.g. during NegotiateClientCertificate drains).3. Managed integration
Interop.OpenSsl.Decrypt/Encrypt/Handshakenow call the single-call shims and forward the newleftoverOffset/leftoverLength/outputPendingoutputs to theSslStreamupper layer.DecryptMessagePAL contract (Unix/OSX/Android) unified to a single signature:bytesWrittenbytes land directly indestination; remaining plaintext stays as a[leftoverOffset, leftoverLength)window inside the original encrypted buffer. Windows continues to setbytesWritten = 0and decrypt in-place (preserving today's behaviour); Linux opportunistically populatesbytesWrittenfrom the new direct path. OSX/Android keep today's in-place behaviour.SslStream.DecryptDataand the read-loop inSslStream.IO.csconsume the new outputs directly; the leftover bytes are recorded via_buffer.OnDecrypted(leftoverOffset, leftoverLength, frameSize).Concurrency
DecryptData/EncryptDatarun under_handshakeLockinSslStream, so the BIO state machine, write spill, andSSL_*operations see a single in-flight call at a time. Buffer pointers are stashed in the BIO context only for the duration of the native call (set on entry, cleared before return), thefixedblock on the managed side covers the same scope.Compatibility
All shim entries used (
BIO_meth_new,BIO_meth_set_*,BIO_set_data,BIO_get_data,BIO_get_new_index,BIO_set_init,BIO_set_flags/BIO_clear_flags/BIO_test_flags,SSL_pending) are available since OpenSSL 1.1.0, which is our minimum.Validation
libs.native+libsclean (0 warnings, 0 errors).System.Net.Security.Testsfunctional: full suite passes on Linux.aspnet-gold-lin, TLS 1.3) and an EgorBot microbenchmark are posted as follow-up comments on this PR with mean Γö¼ΓûÆ stddev and CPU deltas.