From a13413717bb6413110195eb3917f0b2fa45768e2 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 14 May 2026 19:10:06 +0200 Subject: [PATCH 01/20] Linux SslStream: custom BIO over managed buffer windows 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> --- .../Interop.OpenSsl.cs | 148 ++++-- .../Interop.Ssl.cs | 25 +- .../entrypoints.c | 7 + .../opensslshim.h | 28 ++ .../pal_bio.c | 463 ++++++++++++++++++ .../pal_bio.h | 42 ++ 6 files changed, 667 insertions(+), 46 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index a7fa684d3ceda1..7fcb5b176039bc 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -666,21 +666,46 @@ internal static SecurityStatusPal SslRenegotiate(SafeSslHandle sslContext, out b return new SecurityStatusPal(SecurityStatusPalErrorCode.OK); } - internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, ReadOnlySpan input, ref ProtocolToken token) + internal static unsafe SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, ReadOnlySpan input, ref ProtocolToken token) { token.Size = 0; Exception? handshakeException = null; - if (input.Length > 0) + // Reserve a reasonable initial window in the outgoing token; the spill buffer + // catches anything that doesn't fit. + const int InitialHandshakeWindow = 4096; + token.EnsureAvailableSpace(InitialHandshakeWindow); + + // Drain any bytes accumulated in the OutputBio's spill from a prior call + // (e.g. SSL_read emitting alerts before this handshake step). + DrainOutputBioSpill(context, ref token); + token.EnsureAvailableSpace(InitialHandshakeWindow); + + int retVal; + int writtenToWindow; + int spillLen; + Ssl.SslErrorCode errorCode; + + Span windowSpan = token.AvailableSpan; + fixed (byte* inputPtr = input) + fixed (byte* windowPtr = windowSpan) { - if (Ssl.BioWrite(context.InputBio!, ref MemoryMarshal.GetReference(input), input.Length) != input.Length) + if (input.Length > 0) { - // Make sure we clear out the error that is stored in the queue - throw Crypto.CreateOpenSslCryptographicException(); + Ssl.BioSetReadWindow(context.InputBio!, inputPtr, input.Length); } + + Ssl.BioSetWriteWindow(context.OutputBio!, windowPtr, windowSpan.Length); + + retVal = Ssl.SslDoHandshake(context, out errorCode); + + Ssl.BioGetWriteResult(context.OutputBio!, out writtenToWindow, out spillLen); + Ssl.BioSetWriteWindow(context.OutputBio!, null, 0); + Ssl.BioClearReadWindow(context.InputBio!); } - int retVal = Ssl.SslDoHandshake(context, out Ssl.SslErrorCode errorCode); + token.Size += writtenToWindow; + if (retVal != 1) { if (errorCode == Ssl.SslErrorCode.SSL_ERROR_WANT_X509_LOOKUP) @@ -706,31 +731,17 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, } } - int sendCount = Crypto.BioCtrlPending(context.OutputBio!); - if (sendCount > 0) + if (spillLen > 0) { - token.EnsureAvailableSpace(sendCount); - try - { - sendCount = BioRead(context.OutputBio!, token.AvailableSpan, sendCount); - } - catch (Exception) when (handshakeException != null) - { - // If we already have handshake exception, ignore any exception from BioRead(). - } - finally + token.EnsureAvailableSpace(spillLen); + Span spillDst = token.AvailableSpan; + fixed (byte* spillPtr = spillDst) { - if (sendCount <= 0) - { - // Make sure we clear out the error that is stored in the queue - Crypto.ErrClearError(); - sendCount = 0; - } + int drained = Ssl.BioDrainSpill(context.OutputBio!, spillPtr, spillDst.Length); + token.Size += drained; } } - token.Size = sendCount; - if (handshakeException != null) { ExceptionDispatchInfo.Throw(handshakeException); @@ -755,9 +766,29 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, return stateOk ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.ContinueNeeded; } - internal static Ssl.SslErrorCode Encrypt(SafeSslHandle context, ReadOnlySpan input, ref ProtocolToken outToken) + internal static unsafe Ssl.SslErrorCode Encrypt(SafeSslHandle context, ReadOnlySpan input, ref ProtocolToken outToken) { - int retVal = Ssl.SslWrite(context, ref MemoryMarshal.GetReference(input), input.Length, out Ssl.SslErrorCode errorCode); + // Drain any bytes that the OutputBio may have accumulated outside of an explicit + // write window (e.g. from a prior SSL_read that emitted alerts / KeyUpdate / etc.). + DrainOutputBioSpill(context, ref outToken); + + // Worst-case TLS output: input bytes + per-record overhead (~128 B) across all records. + int upperBound = ComputeMaxTlsOutput(input.Length); + outToken.EnsureAvailableSpace(outToken.Size + upperBound); + + int retVal; + int writtenToWindow; + int spillLen; + Ssl.SslErrorCode errorCode; + + Span windowSpan = outToken.AvailableSpan; + fixed (byte* windowPtr = windowSpan) + { + Ssl.BioSetWriteWindow(context.OutputBio!, windowPtr, windowSpan.Length); + retVal = Ssl.SslWrite(context, ref MemoryMarshal.GetReference(input), input.Length, out errorCode); + Ssl.BioGetWriteResult(context.OutputBio!, out writtenToWindow, out spillLen); + Ssl.BioSetWriteWindow(context.OutputBio!, null, 0); + } if (retVal != input.Length) { @@ -772,33 +803,62 @@ internal static Ssl.SslErrorCode Encrypt(SafeSslHandle context, ReadOnlySpan 0) + { + outToken.EnsureAvailableSpace(spillLen); + Span spillDst = outToken.AvailableSpan; + fixed (byte* spillPtr = spillDst) { - outToken.Size = retVal; + int drained = Ssl.BioDrainSpill(context.OutputBio!, spillPtr, spillDst.Length); + outToken.Size += drained; } } return errorCode; } - internal static int Decrypt(SafeSslHandle context, Span buffer, out Ssl.SslErrorCode errorCode) + private static int ComputeMaxTlsOutput(int inputLength) { - BioWrite(context.InputBio!, buffer); + // TLS 1.3 record max plaintext = 16384 bytes. Per-record overhead is bounded by + // ~128 bytes (record header + AEAD tag + padding + inner content-type byte). + // Always add slack for at least one record's overhead even when inputLength == 0, + // since SSL_write of an empty buffer can still emit handshake/alert bytes. + int records = (inputLength >> 14) + 2; + return inputLength + (records * 128); + } - int retVal = Ssl.SslRead(context, ref MemoryMarshal.GetReference(buffer), buffer.Length, out errorCode); + private static unsafe void DrainOutputBioSpill(SafeSslHandle context, ref ProtocolToken outToken) + { + Ssl.BioGetWriteResult(context.OutputBio!, out _, out int spillLen); + if (spillLen <= 0) + { + return; + } + + outToken.EnsureAvailableSpace(spillLen); + Span dst = outToken.AvailableSpan; + fixed (byte* dstPtr = dst) + { + int drained = Ssl.BioDrainSpill(context.OutputBio!, dstPtr, dst.Length); + outToken.Size += drained; + } + } + + internal static unsafe int Decrypt(SafeSslHandle context, Span buffer, out Ssl.SslErrorCode errorCode) + { + int retVal; + fixed (byte* bufPtr = buffer) + { + Ssl.BioSetReadWindow(context.InputBio!, bufPtr, buffer.Length); + retVal = Ssl.SslRead(context, ref MemoryMarshal.GetReference(buffer), buffer.Length, out errorCode); + Ssl.BioClearReadWindow(context.InputBio!); + } if (retVal > 0) { return retVal; diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index af4eb0f78a8b01..24a7d15cf4e057 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -131,6 +131,27 @@ internal static ushort[] GetDefaultSignatureAlgorithms() [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioWrite")] internal static partial int BioWrite(SafeBioHandle b, ref byte data, int len); + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioNewManagedSpan")] + internal static partial SafeBioHandle BioNewManagedSpan(); + + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioSetReadWindow")] + internal static unsafe partial void BioSetReadWindow(SafeBioHandle bio, byte* ptr, int len); + + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioClearReadWindow")] + internal static partial void BioClearReadWindow(SafeBioHandle bio); + + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioSetWriteWindow")] + internal static unsafe partial void BioSetWriteWindow(SafeBioHandle bio, byte* ptr, int capacity); + + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioGetWriteResult")] + internal static partial void BioGetWriteResult(SafeBioHandle bio, out int writtenToWindow, out int spillLen); + + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioDrainSpill")] + internal static unsafe partial int BioDrainSpill(SafeBioHandle bio, byte* dst, int dstLen); + + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioResetManagedSpan")] + internal static partial void BioResetManagedSpan(SafeBioHandle bio); + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetPeerCertificate")] internal static partial IntPtr SslGetPeerCertificate(SafeSslHandle ssl); @@ -437,8 +458,8 @@ internal void MarkHandshakeCompleted() public static SafeSslHandle Create(SafeSslContextHandle context, SslAuthenticationOptions options) { - SafeBioHandle readBio = Interop.Crypto.CreateMemoryBio(); - SafeBioHandle writeBio = Interop.Crypto.CreateMemoryBio(); + SafeBioHandle readBio = Interop.Ssl.BioNewManagedSpan(); + SafeBioHandle writeBio = Interop.Ssl.BioNewManagedSpan(); SafeSslHandle handle = Interop.Ssl.SslCreate(context); if (readBio.IsInvalid || writeBio.IsInvalid || handle.IsInvalid) { diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index 389504b3265e3f..d210f76edddb6f 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -44,12 +44,19 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_BigNumDestroy) DllImportEntry(CryptoNative_BigNumFromBinary) DllImportEntry(CryptoNative_BigNumToBinary) + DllImportEntry(CryptoNative_BioClearReadWindow) DllImportEntry(CryptoNative_BioCtrlPending) DllImportEntry(CryptoNative_BioDestroy) + DllImportEntry(CryptoNative_BioDrainSpill) + DllImportEntry(CryptoNative_BioGetWriteResult) DllImportEntry(CryptoNative_BioGets) DllImportEntry(CryptoNative_BioNewFile) + DllImportEntry(CryptoNative_BioNewManagedSpan) DllImportEntry(CryptoNative_BioRead) + DllImportEntry(CryptoNative_BioResetManagedSpan) DllImportEntry(CryptoNative_BioSeek) + DllImportEntry(CryptoNative_BioSetReadWindow) + DllImportEntry(CryptoNative_BioSetWriteWindow) DllImportEntry(CryptoNative_BioTell) DllImportEntry(CryptoNative_BioWrite) DllImportEntry(CryptoNative_CheckX509Hostname) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 22425beb8a4126..2622e83e6535e1 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -331,13 +331,27 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(ASN1_TIME_set) \ REQUIRED_FUNCTION(ASN1_TIME_to_tm) \ REQUIRED_FUNCTION(ASN1_TIME_free) \ + REQUIRED_FUNCTION(BIO_clear_flags) \ REQUIRED_FUNCTION(BIO_ctrl) \ REQUIRED_FUNCTION(BIO_ctrl_pending) \ REQUIRED_FUNCTION(BIO_free) \ + REQUIRED_FUNCTION(BIO_get_data) \ + REQUIRED_FUNCTION(BIO_get_new_index) \ REQUIRED_FUNCTION(BIO_gets) \ + REQUIRED_FUNCTION(BIO_meth_free) \ + REQUIRED_FUNCTION(BIO_meth_new) \ + REQUIRED_FUNCTION(BIO_meth_set_create) \ + REQUIRED_FUNCTION(BIO_meth_set_ctrl) \ + REQUIRED_FUNCTION(BIO_meth_set_destroy) \ + REQUIRED_FUNCTION(BIO_meth_set_read) \ + REQUIRED_FUNCTION(BIO_meth_set_write) \ REQUIRED_FUNCTION(BIO_new) \ REQUIRED_FUNCTION(BIO_new_file) \ REQUIRED_FUNCTION(BIO_read) \ + REQUIRED_FUNCTION(BIO_set_data) \ + REQUIRED_FUNCTION(BIO_set_flags) \ + REQUIRED_FUNCTION(BIO_set_init) \ + REQUIRED_FUNCTION(BIO_test_flags) \ REQUIRED_FUNCTION(BIO_up_ref) \ REQUIRED_FUNCTION(BIO_s_mem) \ REQUIRED_FUNCTION(BIO_write) \ @@ -890,13 +904,27 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define ASN1_TIME_new ASN1_TIME_new_ptr #define ASN1_TIME_set ASN1_TIME_set_ptr #define ASN1_TIME_to_tm ASN1_TIME_to_tm_ptr +#define BIO_clear_flags BIO_clear_flags_ptr #define BIO_ctrl BIO_ctrl_ptr #define BIO_ctrl_pending BIO_ctrl_pending_ptr #define BIO_free BIO_free_ptr +#define BIO_get_data BIO_get_data_ptr +#define BIO_get_new_index BIO_get_new_index_ptr #define BIO_gets BIO_gets_ptr +#define BIO_meth_free BIO_meth_free_ptr +#define BIO_meth_new BIO_meth_new_ptr +#define BIO_meth_set_create BIO_meth_set_create_ptr +#define BIO_meth_set_ctrl BIO_meth_set_ctrl_ptr +#define BIO_meth_set_destroy BIO_meth_set_destroy_ptr +#define BIO_meth_set_read BIO_meth_set_read_ptr +#define BIO_meth_set_write BIO_meth_set_write_ptr #define BIO_new BIO_new_ptr #define BIO_new_file BIO_new_file_ptr #define BIO_read BIO_read_ptr +#define BIO_set_data BIO_set_data_ptr +#define BIO_set_flags BIO_set_flags_ptr +#define BIO_set_init BIO_set_init_ptr +#define BIO_test_flags BIO_test_flags_ptr #define BIO_up_ref BIO_up_ref_ptr #define BIO_s_mem BIO_s_mem_ptr #define BIO_write BIO_write_ptr diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c index f0a4ce45d06bf7..7663f51f3b9e3a 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c @@ -58,3 +58,466 @@ int32_t CryptoNative_BioCtrlPending(BIO* bio) assert(result <= INT32_MAX); return (int32_t)result; } + +#include +#include +#include + +typedef struct +{ + /* Carry-over buffer: holds bytes from a prior read window that SSL did + not consume before the window was cleared. */ + uint8_t* readCarry; + int32_t readCarryCapacity; + int32_t readCarryLen; + int32_t readCarryPos; + + const uint8_t* readPtr; + int32_t readLen; + int32_t readPos; + + uint8_t* writePtr; + int32_t writeCapacity; + int32_t writePos; + + uint8_t* spillBuf; + int32_t spillCapacity; + int32_t spillLen; +} ManagedSpanBioCtx; + +#define MANAGED_SPAN_SPILL_INITIAL 4096 + +static BIO_METHOD* g_managedSpanBioMethod = NULL; +static pthread_once_t g_managedSpanBioOnce = PTHREAD_ONCE_INIT; + +static int ManagedSpanBioRead(BIO* bio, char* buf, int len) +{ + BIO_clear_retry_flags(bio); + + if (bio == NULL || buf == NULL || len <= 0) + { + return 0; + } + + ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + if (ctx == NULL) + { + return -1; + } + + int32_t carryAvail = ctx->readCarryLen - ctx->readCarryPos; + if (carryAvail > 0) + { + int32_t toCopy = len < carryAvail ? len : carryAvail; + memcpy(buf, ctx->readCarry + ctx->readCarryPos, (size_t)toCopy); + ctx->readCarryPos += toCopy; + if (ctx->readCarryPos == ctx->readCarryLen) + { + ctx->readCarryPos = 0; + ctx->readCarryLen = 0; + } + return toCopy; + } + + int32_t available = ctx->readLen - ctx->readPos; + if (available <= 0 || ctx->readPtr == NULL) + { + BIO_set_retry_read(bio); + return -1; + } + + int32_t toCopy = len < available ? len : available; + memcpy(buf, ctx->readPtr + ctx->readPos, (size_t)toCopy); + ctx->readPos += toCopy; + return toCopy; +} + +static int ManagedSpanBioGrowCarry(ManagedSpanBioCtx* ctx, int32_t needed) +{ + if (ctx->readCarryCapacity >= needed) + { + return 1; + } + + int32_t newCap = ctx->readCarryCapacity > 0 ? ctx->readCarryCapacity : 4096; + while (newCap < needed) + { + if (newCap > INT32_MAX / 2) + { + newCap = needed; + break; + } + newCap *= 2; + } + + uint8_t* newBuf = (uint8_t*)realloc(ctx->readCarry, (size_t)newCap); + if (newBuf == NULL) + { + return 0; + } + + ctx->readCarry = newBuf; + ctx->readCarryCapacity = newCap; + return 1; +} + +static int ManagedSpanBioGrowSpill(ManagedSpanBioCtx* ctx, int32_t needed) +{ + if (ctx->spillCapacity >= needed) + { + return 1; + } + + int32_t newCap = ctx->spillCapacity > 0 ? ctx->spillCapacity : MANAGED_SPAN_SPILL_INITIAL; + while (newCap < needed) + { + if (newCap > INT32_MAX / 2) + { + newCap = needed; + break; + } + newCap *= 2; + } + + uint8_t* newBuf = (uint8_t*)realloc(ctx->spillBuf, (size_t)newCap); + if (newBuf == NULL) + { + return 0; + } + + ctx->spillBuf = newBuf; + ctx->spillCapacity = newCap; + return 1; +} + +static int ManagedSpanBioWrite(BIO* bio, const char* buf, int len) +{ + BIO_clear_retry_flags(bio); + + if (bio == NULL || buf == NULL || len < 0) + { + return 0; + } + + if (len == 0) + { + return 0; + } + + ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + if (ctx == NULL) + { + return -1; + } + + int32_t remaining = len; + const uint8_t* src = (const uint8_t*)buf; + + if (ctx->writePtr != NULL) + { + int32_t windowAvail = ctx->writeCapacity - ctx->writePos; + if (windowAvail > 0) + { + int32_t toCopy = remaining < windowAvail ? remaining : windowAvail; + memcpy(ctx->writePtr + ctx->writePos, src, (size_t)toCopy); + ctx->writePos += toCopy; + src += toCopy; + remaining -= toCopy; + } + } + + if (remaining > 0) + { + int32_t needed = ctx->spillLen + remaining; + if (!ManagedSpanBioGrowSpill(ctx, needed)) + { + return -1; + } + memcpy(ctx->spillBuf + ctx->spillLen, src, (size_t)remaining); + ctx->spillLen += remaining; + } + + return len; +} + +static long ManagedSpanBioCtrl(BIO* bio, int cmd, long num, void* ptr) +{ + (void)num; + (void)ptr; + + ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + + switch (cmd) + { + case BIO_CTRL_FLUSH: + return 1; + + case BIO_CTRL_PENDING: + if (ctx == NULL) + { + return 0; + } + return (long)((ctx->readCarryLen - ctx->readCarryPos) + (ctx->readLen - ctx->readPos)); + + case BIO_CTRL_WPENDING: + if (ctx == NULL) + { + return 0; + } + return (long)(ctx->writePos + ctx->spillLen); + + case BIO_CTRL_RESET: + if (ctx != NULL) + { + ctx->readCarryLen = 0; + ctx->readCarryPos = 0; + ctx->readPos = 0; + ctx->writePos = 0; + ctx->spillLen = 0; + BIO_clear_retry_flags(bio); + } + return 1; + + case BIO_CTRL_EOF: + return 0; + + default: + return 0; + } +} + +static int ManagedSpanBioCreate(BIO* bio) +{ + ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)calloc(1, sizeof(ManagedSpanBioCtx)); + if (ctx == NULL) + { + return 0; + } + + BIO_set_data(bio, ctx); + BIO_set_init(bio, 1); + return 1; +} + +static int ManagedSpanBioDestroy(BIO* bio) +{ + if (bio == NULL) + { + return 0; + } + + ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + if (ctx != NULL) + { + free(ctx->readCarry); + free(ctx->spillBuf); + free(ctx); + BIO_set_data(bio, NULL); + } + BIO_set_init(bio, 0); + return 1; +} + +static void ManagedSpanBioMethodInit(void) +{ + int index = BIO_get_new_index(); + if (index == -1) + { + return; + } + + BIO_METHOD* method = BIO_meth_new(index | BIO_TYPE_SOURCE_SINK, "dotnet-managed-span"); + if (method == NULL) + { + return; + } + + if (!BIO_meth_set_write(method, ManagedSpanBioWrite) || + !BIO_meth_set_read(method, ManagedSpanBioRead) || + !BIO_meth_set_ctrl(method, ManagedSpanBioCtrl) || + !BIO_meth_set_create(method, ManagedSpanBioCreate) || + !BIO_meth_set_destroy(method, ManagedSpanBioDestroy)) + { + BIO_meth_free(method); + return; + } + + g_managedSpanBioMethod = method; +} + +static BIO_METHOD* GetManagedSpanBioMethod(void) +{ + pthread_once(&g_managedSpanBioOnce, ManagedSpanBioMethodInit); + return g_managedSpanBioMethod; +} + +BIO* CryptoNative_BioNewManagedSpan(void) +{ + ERR_clear_error(); + + BIO_METHOD* method = GetManagedSpanBioMethod(); + if (method == NULL) + { + return NULL; + } + + return BIO_new(method); +} + +void CryptoNative_BioSetReadWindow(BIO* bio, const void* ptr, int32_t len) +{ + if (bio == NULL) + { + return; + } + + ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + if (ctx == NULL) + { + return; + } + + ctx->readPtr = (const uint8_t*)ptr; + ctx->readLen = ptr != NULL ? len : 0; + ctx->readPos = 0; +} + +void CryptoNative_BioClearReadWindow(BIO* bio) +{ + if (bio == NULL) + { + return; + } + + ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + if (ctx == NULL) + { + return; + } + + int32_t unread = ctx->readLen - ctx->readPos; + if (unread > 0 && ctx->readPtr != NULL) + { + /* Move existing carry tail down to position 0 first. */ + int32_t carryTail = ctx->readCarryLen - ctx->readCarryPos; + if (carryTail > 0 && ctx->readCarryPos > 0) + { + memmove(ctx->readCarry, ctx->readCarry + ctx->readCarryPos, (size_t)carryTail); + } + ctx->readCarryLen = carryTail; + ctx->readCarryPos = 0; + + int32_t needed = ctx->readCarryLen + unread; + if (ManagedSpanBioGrowCarry(ctx, needed)) + { + memcpy(ctx->readCarry + ctx->readCarryLen, ctx->readPtr + ctx->readPos, (size_t)unread); + ctx->readCarryLen += unread; + } + /* If allocation failed, the unread bytes are dropped. SSL will fail + on the next read with a protocol error, which is the safest fallback. */ + } + + ctx->readPtr = NULL; + ctx->readLen = 0; + ctx->readPos = 0; +} + +void CryptoNative_BioSetWriteWindow(BIO* bio, void* ptr, int32_t capacity) +{ + if (bio == NULL) + { + return; + } + + ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + if (ctx == NULL) + { + return; + } + + ctx->writePtr = (uint8_t*)ptr; + ctx->writeCapacity = ptr != NULL ? capacity : 0; + ctx->writePos = 0; +} + +void CryptoNative_BioGetWriteResult(BIO* bio, int32_t* writtenToWindow, int32_t* spillLen) +{ + if (writtenToWindow != NULL) + { + *writtenToWindow = 0; + } + if (spillLen != NULL) + { + *spillLen = 0; + } + + if (bio == NULL) + { + return; + } + + ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + if (ctx == NULL) + { + return; + } + + if (writtenToWindow != NULL) + { + *writtenToWindow = ctx->writePos; + } + if (spillLen != NULL) + { + *spillLen = ctx->spillLen; + } +} + +int32_t CryptoNative_BioDrainSpill(BIO* bio, void* dst, int32_t dstLen) +{ + if (bio == NULL || dst == NULL || dstLen <= 0) + { + return 0; + } + + ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + if (ctx == NULL || ctx->spillLen == 0) + { + return 0; + } + + int32_t toCopy = dstLen < ctx->spillLen ? dstLen : ctx->spillLen; + memcpy(dst, ctx->spillBuf, (size_t)toCopy); + + int32_t remaining = ctx->spillLen - toCopy; + if (remaining > 0) + { + memmove(ctx->spillBuf, ctx->spillBuf + toCopy, (size_t)remaining); + } + ctx->spillLen = remaining; + return toCopy; +} + +void CryptoNative_BioResetManagedSpan(BIO* bio) +{ + if (bio == NULL) + { + return; + } + + ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + if (ctx == NULL) + { + return; + } + + ctx->readCarryLen = 0; + ctx->readCarryPos = 0; + ctx->readPtr = NULL; + ctx->readLen = 0; + ctx->readPos = 0; + ctx->writePtr = NULL; + ctx->writeCapacity = 0; + ctx->writePos = 0; + ctx->spillLen = 0; + BIO_clear_retry_flags(bio); +} diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.h b/src/native/libs/System.Security.Cryptography.Native/pal_bio.h index 7f1a395419e4f7..05ed3193604383 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.h @@ -54,3 +54,45 @@ Shims the BIO_ctrl_pending method. Returns the number of pending characters in the BIOs read and write buffers. */ PALEXPORT int32_t CryptoNative_BioCtrlPending(BIO* bio); + +/* +Creates a new BIO using the managed-span BIO_METHOD that operates on +caller-supplied buffer windows (with a heap spill on write overflow). +*/ +PALEXPORT BIO* CryptoNative_BioNewManagedSpan(void); + +/* +Sets the read window on a managed-span BIO. Subsequent BIO_read calls +consume from ptr[0..len). Passing ptr=NULL/len=0 clears the window. +*/ +PALEXPORT void CryptoNative_BioSetReadWindow(BIO* bio, const void* ptr, int32_t len); + +/* +Clears the read window on a managed-span BIO. +*/ +PALEXPORT void CryptoNative_BioClearReadWindow(BIO* bio); + +/* +Sets the write window on a managed-span BIO. BIO_write fills this window +first, then spills the remainder to a heap buffer. Passing ptr=NULL/cap=0 +clears the window so all writes go to the spill buffer. +*/ +PALEXPORT void CryptoNative_BioSetWriteWindow(BIO* bio, void* ptr, int32_t capacity); + +/* +Returns the number of bytes written into the window and into the spill +buffer respectively since the last reset/window-set. +*/ +PALEXPORT void CryptoNative_BioGetWriteResult(BIO* bio, int32_t* writtenToWindow, int32_t* spillLen); + +/* +Drains up to dstLen bytes from the start of the spill buffer into dst, +shifting the rest down. Returns the number of bytes drained. +*/ +PALEXPORT int32_t CryptoNative_BioDrainSpill(BIO* bio, void* dst, int32_t dstLen); + +/* +Resets the managed-span BIO: clears windows, zeroes the spill length, +and keeps the spill allocation. +*/ +PALEXPORT void CryptoNative_BioResetManagedSpan(BIO* bio); From 68743ca4d5536915278a11cb4f9e688d3b119fc7 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Fri, 15 May 2026 10:39:05 +0200 Subject: [PATCH 02/20] Address PR review feedback - 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> --- .../Interop.OpenSsl.cs | 55 ++++++++++++++----- .../Interop.Ssl.cs | 3 - .../entrypoints.c | 1 - .../pal_bio.c | 48 ++++++++-------- .../pal_bio.h | 5 -- 5 files changed, 62 insertions(+), 50 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 7fcb5b176039bc..7536e29a6764ee 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -697,11 +697,16 @@ internal static unsafe SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle c Ssl.BioSetWriteWindow(context.OutputBio!, windowPtr, windowSpan.Length); - retVal = Ssl.SslDoHandshake(context, out errorCode); - - Ssl.BioGetWriteResult(context.OutputBio!, out writtenToWindow, out spillLen); - Ssl.BioSetWriteWindow(context.OutputBio!, null, 0); - Ssl.BioClearReadWindow(context.InputBio!); + try + { + retVal = Ssl.SslDoHandshake(context, out errorCode); + Ssl.BioGetWriteResult(context.OutputBio!, out writtenToWindow, out spillLen); + } + finally + { + Ssl.BioSetWriteWindow(context.OutputBio!, null, 0); + Ssl.BioClearReadWindow(context.InputBio!); + } } token.Size += writtenToWindow; @@ -772,9 +777,14 @@ internal static unsafe Ssl.SslErrorCode Encrypt(SafeSslHandle context, ReadOnlyS // write window (e.g. from a prior SSL_read that emitted alerts / KeyUpdate / etc.). DrainOutputBioSpill(context, ref outToken); - // Worst-case TLS output: input bytes + per-record overhead (~128 B) across all records. + // Preserve any bytes already in outToken (including those just drained from a prior SSL_read's + // alerts / KeyUpdate output). On error we restore Size to this snapshot so those bytes are + // still sent rather than overwritten with the partial output of a failed SSL_write. + int preWriteSize = outToken.Size; + + // Worst-case TLS output for the user's plaintext. int upperBound = ComputeMaxTlsOutput(input.Length); - outToken.EnsureAvailableSpace(outToken.Size + upperBound); + outToken.EnsureAvailableSpace(upperBound); int retVal; int writtenToWindow; @@ -785,14 +795,21 @@ internal static unsafe Ssl.SslErrorCode Encrypt(SafeSslHandle context, ReadOnlyS fixed (byte* windowPtr = windowSpan) { Ssl.BioSetWriteWindow(context.OutputBio!, windowPtr, windowSpan.Length); - retVal = Ssl.SslWrite(context, ref MemoryMarshal.GetReference(input), input.Length, out errorCode); - Ssl.BioGetWriteResult(context.OutputBio!, out writtenToWindow, out spillLen); - Ssl.BioSetWriteWindow(context.OutputBio!, null, 0); + try + { + retVal = Ssl.SslWrite(context, ref MemoryMarshal.GetReference(input), input.Length, out errorCode); + Ssl.BioGetWriteResult(context.OutputBio!, out writtenToWindow, out spillLen); + } + finally + { + Ssl.BioSetWriteWindow(context.OutputBio!, null, 0); + } } if (retVal != input.Length) { - outToken.Size = 0; + // Drop any partial output written by the failed SSL_write but keep the drained spill bytes. + outToken.Size = preWriteSize; switch (errorCode) { // indicate end-of-file @@ -826,11 +843,13 @@ internal static unsafe Ssl.SslErrorCode Encrypt(SafeSslHandle context, ReadOnlyS private static int ComputeMaxTlsOutput(int inputLength) { // TLS 1.3 record max plaintext = 16384 bytes. Per-record overhead is bounded by - // ~128 bytes (record header + AEAD tag + padding + inner content-type byte). + // OpenSSL's SSL3_RT_MAX_ENCRYPTED_OVERHEAD (256 bytes, covering record header, AEAD + // tag, optional MAC, padding, and the inner content-type byte for TLS 1.3). // Always add slack for at least one record's overhead even when inputLength == 0, // since SSL_write of an empty buffer can still emit handshake/alert bytes. + const int MaxRecordOverhead = 256; int records = (inputLength >> 14) + 2; - return inputLength + (records * 128); + return inputLength + (records * MaxRecordOverhead); } private static unsafe void DrainOutputBioSpill(SafeSslHandle context, ref ProtocolToken outToken) @@ -856,8 +875,14 @@ internal static unsafe int Decrypt(SafeSslHandle context, Span buffer, out fixed (byte* bufPtr = buffer) { Ssl.BioSetReadWindow(context.InputBio!, bufPtr, buffer.Length); - retVal = Ssl.SslRead(context, ref MemoryMarshal.GetReference(buffer), buffer.Length, out errorCode); - Ssl.BioClearReadWindow(context.InputBio!); + try + { + retVal = Ssl.SslRead(context, ref MemoryMarshal.GetReference(buffer), buffer.Length, out errorCode); + } + finally + { + Ssl.BioClearReadWindow(context.InputBio!); + } } if (retVal > 0) { diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 24a7d15cf4e057..8038aba9c95062 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -149,9 +149,6 @@ internal static ushort[] GetDefaultSignatureAlgorithms() [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioDrainSpill")] internal static unsafe partial int BioDrainSpill(SafeBioHandle bio, byte* dst, int dstLen); - [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioResetManagedSpan")] - internal static partial void BioResetManagedSpan(SafeBioHandle bio); - [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetPeerCertificate")] internal static partial IntPtr SslGetPeerCertificate(SafeSslHandle ssl); diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index d210f76edddb6f..fa5051d9061d82 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -53,7 +53,6 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_BioNewFile) DllImportEntry(CryptoNative_BioNewManagedSpan) DllImportEntry(CryptoNative_BioRead) - DllImportEntry(CryptoNative_BioResetManagedSpan) DllImportEntry(CryptoNative_BioSeek) DllImportEntry(CryptoNative_BioSetReadWindow) DllImportEntry(CryptoNative_BioSetWriteWindow) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c index 7663f51f3b9e3a..32cc5eee5066cb 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c @@ -83,6 +83,8 @@ typedef struct uint8_t* spillBuf; int32_t spillCapacity; int32_t spillLen; + + int32_t readError; /* set to 1 if a carry allocation failed and bytes were lost */ } ManagedSpanBioCtx; #define MANAGED_SPAN_SPILL_INITIAL 4096 @@ -119,6 +121,14 @@ static int ManagedSpanBioRead(BIO* bio, char* buf, int len) return toCopy; } + if (ctx->readError) + { + /* A prior BioClearReadWindow could not allocate carry space and dropped + unread bytes. Surface this as a hard read failure so it does not look + like a transient EAGAIN to the SSL engine. */ + return -1; + } + int32_t available = ctx->readLen - ctx->readPos; if (available <= 0 || ctx->readPtr == NULL) { @@ -271,9 +281,14 @@ static long ManagedSpanBioCtrl(BIO* bio, int cmd, long num, void* ptr) { ctx->readCarryLen = 0; ctx->readCarryPos = 0; + ctx->readPtr = NULL; + ctx->readLen = 0; ctx->readPos = 0; + ctx->writePtr = NULL; + ctx->writeCapacity = 0; ctx->writePos = 0; ctx->spillLen = 0; + ctx->readError = 0; BIO_clear_retry_flags(bio); } return 1; @@ -413,8 +428,13 @@ void CryptoNative_BioClearReadWindow(BIO* bio) memcpy(ctx->readCarry + ctx->readCarryLen, ctx->readPtr + ctx->readPos, (size_t)unread); ctx->readCarryLen += unread; } - /* If allocation failed, the unread bytes are dropped. SSL will fail - on the next read with a protocol error, which is the safest fallback. */ + else + { + /* Carry allocation failed; bytes are lost. Mark the BIO as + permanently broken so the next BIO_read surfaces the failure + rather than masking it as a protocol error. */ + ctx->readError = 1; + } } ctx->readPtr = NULL; @@ -497,27 +517,3 @@ int32_t CryptoNative_BioDrainSpill(BIO* bio, void* dst, int32_t dstLen) return toCopy; } -void CryptoNative_BioResetManagedSpan(BIO* bio) -{ - if (bio == NULL) - { - return; - } - - ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); - if (ctx == NULL) - { - return; - } - - ctx->readCarryLen = 0; - ctx->readCarryPos = 0; - ctx->readPtr = NULL; - ctx->readLen = 0; - ctx->readPos = 0; - ctx->writePtr = NULL; - ctx->writeCapacity = 0; - ctx->writePos = 0; - ctx->spillLen = 0; - BIO_clear_retry_flags(bio); -} diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.h b/src/native/libs/System.Security.Cryptography.Native/pal_bio.h index 05ed3193604383..8bd94200f91f0f 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.h @@ -91,8 +91,3 @@ shifting the rest down. Returns the number of bytes drained. */ PALEXPORT int32_t CryptoNative_BioDrainSpill(BIO* bio, void* dst, int32_t dstLen); -/* -Resets the managed-span BIO: clears windows, zeroes the spill length, -and keeps the spill allocation. -*/ -PALEXPORT void CryptoNative_BioResetManagedSpan(BIO* bio); From 7371f2e2a1b796666c7c18852cf75dbb9567c2b7 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Fri, 15 May 2026 14:57:16 +0200 Subject: [PATCH 03/20] SslStream/Linux: decrypt directly into user buffer Symmetric to the existing custom-BIO encrypt optimization, this change threads the caller-supplied Memory 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 input, Span 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> --- .../Interop.OpenSsl.cs | 22 ++- .../Interop.Ssl.cs | 3 + .../src/System/Net/Security/SslStream.IO.cs | 131 +++++++++++++++++- .../Net/Security/SslStreamPal.Android.cs | 18 +++ .../System/Net/Security/SslStreamPal.OSX.cs | 18 +++ .../System/Net/Security/SslStreamPal.Unix.cs | 35 ++++- .../Net/Security/SslStreamPal.Windows.cs | 18 +++ .../entrypoints.c | 1 + .../opensslshim.h | 2 + .../pal_ssl.c | 5 + .../pal_ssl.h | 1 + 11 files changed, 241 insertions(+), 13 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 7536e29a6764ee..49b8ddb724ae35 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -19,6 +19,7 @@ using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Text; +using System.Threading; using Microsoft.Win32.SafeHandles; internal static partial class Interop @@ -836,7 +837,6 @@ internal static unsafe Ssl.SslErrorCode Encrypt(SafeSslHandle context, ReadOnlyS outToken.Size += drained; } } - return errorCode; } @@ -859,7 +859,6 @@ private static unsafe void DrainOutputBioSpill(SafeSslHandle context, ref Protoc { return; } - outToken.EnsureAvailableSpace(spillLen); Span dst = outToken.AvailableSpan; fixed (byte* dstPtr = dst) @@ -869,21 +868,30 @@ private static unsafe void DrainOutputBioSpill(SafeSslHandle context, ref Protoc } } - internal static unsafe int Decrypt(SafeSslHandle context, Span buffer, out Ssl.SslErrorCode errorCode) + internal static unsafe int Decrypt(SafeSslHandle context, ReadOnlySpan input, Span output, out Ssl.SslErrorCode errorCode) { + Debug.Assert(output.Length > 0, "Decrypt output buffer must be non-empty"); + int retVal; - fixed (byte* bufPtr = buffer) + fixed (byte* inputPtr = input) { - Ssl.BioSetReadWindow(context.InputBio!, bufPtr, buffer.Length); + if (input.Length > 0) + { + Ssl.BioSetReadWindow(context.InputBio!, inputPtr, input.Length); + } try { - retVal = Ssl.SslRead(context, ref MemoryMarshal.GetReference(buffer), buffer.Length, out errorCode); + retVal = Ssl.SslRead(context, ref MemoryMarshal.GetReference(output), output.Length, out errorCode); } finally { - Ssl.BioClearReadWindow(context.InputBio!); + if (input.Length > 0) + { + Ssl.BioClearReadWindow(context.InputBio!); + } } } + if (retVal > 0) { return retVal; diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 8038aba9c95062..5a379cc62d8486 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -80,6 +80,9 @@ internal static unsafe ReadOnlySpan SslGetAlpnSelected(SafeSslHandle ssl) [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslWrite", SetLastError = true)] internal static partial int SslWrite(SafeSslHandle ssl, ref byte buf, int num, out SslErrorCode error); + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslPending")] + internal static partial int SslPending(SafeSslHandle ssl); + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslRead", SetLastError = true)] internal static partial int SslRead(SafeSslHandle ssl, ref byte buf, int num, out SslErrorCode error); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index a8faaae4fa9cbb..a769bcc8ab8fb5 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -32,6 +32,12 @@ public partial class SslStream private bool _receivedEOF; + // True when the SSL PAL has plaintext buffered internally (residual from a direct-decrypt SSL_read + // whose destination span was smaller than the record's plaintext). When set, the next read must + // drain the residual via DecryptMessageDirect with empty input before issuing more network IO. + // Guarded by _handshakeLock. + private bool _palHasPendingPlaintext; + // Used by Telemetry to ensure we log connection close exactly once private enum ConnectionStatus { @@ -215,7 +221,7 @@ private async Task RenegotiateAsync(CancellationToken cancellationTo token.RentBuffer = true; try { - if (_buffer.ActiveLength > 0) + if (_buffer.ActiveLength > 0 || _palHasPendingPlaintext) { throw new InvalidOperationException(SR.net_ssl_renegotiate_buffer); } @@ -845,6 +851,75 @@ private async ValueTask EnsureFullTlsFrameAsync(CancellationTok return frameSize; } + // Direct-decrypt variant: when the PAL supports it and we have a non-empty user destination, + // decrypt plaintext directly into the user buffer instead of into _buffer. Returns bytes written + // to `output` plus any extra reauth bytes via `extraBuffer`. + private SecurityStatusPal DecryptDataDirect(int frameSize, Span output, out int outputWritten, out byte[]? extraBuffer) + { + Debug.Assert(SslStreamPal.IsDirectDecryptSupported); + Debug.Assert(output.Length > 0); + outputWritten = 0; + extraBuffer = null; + + SecurityStatusPal status; + lock (_handshakeLock) + { + ThrowIfExceptionalOrNotAuthenticated(); + + status = SslStreamPal.DecryptMessageDirect(_securityContext!, _buffer.EncryptedReadOnlySpan.Slice(0, frameSize), output, out int written, out bool morePending); + + // OpenSSL consumed the full frame regardless of status; discard from _buffer. + _buffer.OnDecrypted(0, 0, frameSize); + + if (status.ErrorCode == SecurityStatusPalErrorCode.OK) + { + outputWritten = written; + _palHasPendingPlaintext = morePending; + } + else + { + // Non-OK status: do NOT expose direct-written bytes to the caller via outputWritten. + // The existing error handler in ReadAsyncInternal expects any plaintext produced by a + // failed decrypt to live in `extraBuffer` (for reauth) rather than the user span. + if (written > 0) + { + extraBuffer = new byte[written]; + output.Slice(0, written).CopyTo(extraBuffer); + } + // Clear any pending plaintext - on non-OK the morePending semantics are not meaningful. + _palHasPendingPlaintext = false; + } + + if (status.ErrorCode == SecurityStatusPalErrorCode.Renegotiate) + { + if (_sslAuthenticationOptions.AllowRenegotiation || SslProtocol == SslProtocols.Tls13 || _nestedAuth != NestedState.StreamNotInUse) + { + _handshakeWaiter = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + } + } + } + + return status; + } + + // Drain residual plaintext buffered inside the PAL (Unix only). Returns bytes written to `output`. + private SecurityStatusPal DrainPendingPlaintextDirect(Span output, out int outputWritten) + { + Debug.Assert(SslStreamPal.IsDirectDecryptSupported); + Debug.Assert(output.Length > 0); + + SecurityStatusPal status; + lock (_handshakeLock) + { + ThrowIfExceptionalOrNotAuthenticated(); + + status = SslStreamPal.DecryptMessageDirect(_securityContext!, ReadOnlySpan.Empty, output, out outputWritten, out bool morePending); + _palHasPendingPlaintext = (status.ErrorCode == SecurityStatusPalErrorCode.OK) && morePending; + } + + return status; + } + private SecurityStatusPal DecryptData(int frameSize) { SecurityStatusPal status; @@ -925,6 +1000,23 @@ private async ValueTask ReadAsyncInternal(Memory buffer, buffer = buffer.Slice(processedLength); } + // Drain any plaintext the PAL has buffered internally from a prior direct decrypt + // whose user-buffer was smaller than the record's plaintext. + if (SslStreamPal.IsDirectDecryptSupported && _palHasPendingPlaintext && !buffer.IsEmpty) + { + SecurityStatusPal drainStatus = DrainPendingPlaintextDirect(buffer.Span, out int drained); + if (drainStatus.ErrorCode != SecurityStatusPalErrorCode.OK) + { + throw new IOException(SR.net_io_decrypt, SslStreamPal.GetException(drainStatus)); + } + processedLength += drained; + buffer = buffer.Slice(drained); + if (buffer.IsEmpty) + { + return processedLength; + } + } + if (_receivedEOF && nextTlsFrameLength == UnknownTlsFrameLength) { // there should be no frames waiting for processing @@ -944,11 +1036,28 @@ private async ValueTask ReadAsyncInternal(Memory buffer, break; } - SecurityStatusPal status = DecryptData(payloadBytes); + SecurityStatusPal status; + byte[]? extraBuffer = null; + int directWritten = 0; + + // Use direct-decrypt only when the PAL supports it, we have user buffer space, and + // no concurrent re-handshake is in progress (which would change the SSL state machine). + bool useDirect = SslStreamPal.IsDirectDecryptSupported + && !buffer.IsEmpty + && _handshakeWaiter == null; + + if (useDirect) + { + status = DecryptDataDirect(payloadBytes, buffer.Span, out directWritten, out extraBuffer); + } + else + { + status = DecryptData(payloadBytes); + } + if (status.ErrorCode != SecurityStatusPalErrorCode.OK) { - byte[]? extraBuffer = null; - if (_buffer.DecryptedLength != 0) + if (!useDirect && _buffer.DecryptedLength != 0) { extraBuffer = new byte[_buffer.DecryptedLength]; _buffer.DecryptedSpan.CopyTo(extraBuffer); @@ -979,7 +1088,19 @@ private async ValueTask ReadAsyncInternal(Memory buffer, } } - if (_buffer.DecryptedLength > 0) + if (useDirect) + { + if (directWritten > 0) + { + processedLength += directWritten; + buffer = buffer.Slice(directWritten); + if (buffer.IsEmpty) + { + break; + } + } + } + else if (_buffer.DecryptedLength > 0) { // This will either copy data from rented buffer or adjust final buffer as needed. // In both cases _decryptedBytesOffset and _decryptedBytesCount will be updated as needed. diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs index 68df93d01217cd..d84c07aad0e95e 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs @@ -27,6 +27,7 @@ public static Exception GetException(SecurityStatusPal status) // depends on SslStream calling HandshakeInternal one last time when tearing down the connection // due to handshake failures. internal const bool CanGenerateCustomAlerts = true; + internal const bool IsDirectDecryptSupported = false; public static void VerifyPackageInfo() { @@ -146,6 +147,23 @@ public static SecurityStatusPal DecryptMessage( return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, e); } } + public static SecurityStatusPal DecryptMessageDirect( + SafeDeleteSslContext securityContext, + ReadOnlySpan input, + Span output, + out int outputWritten, + out bool morePending) + { + // Direct decrypt is not supported on this platform. + // Callers must gate on SslStreamPal.IsDirectDecryptSupported before invoking this method. + _ = securityContext; + _ = input; + _ = output; + outputWritten = 0; + morePending = false; + throw new NotSupportedException(); + } + public static ChannelBinding? QueryContextChannelBinding( SafeDeleteContext securityContext, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index 7aff801dab8260..04e8140f1b9f06 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -33,6 +33,7 @@ public static Exception GetException(SecurityStatusPal status) // special case of an empty array being passed to the `fixed` statement. internal const bool CanEncryptEmptyMessage = false; internal const bool CanGenerateCustomAlerts = false; + internal const bool IsDirectDecryptSupported = false; public static void VerifyPackageInfo() { @@ -257,6 +258,23 @@ public static SecurityStatusPal DecryptMessage( return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, e); } } + public static SecurityStatusPal DecryptMessageDirect( + SafeDeleteContext securityContext, + ReadOnlySpan input, + Span output, + out int outputWritten, + out bool morePending) + { + // Direct decrypt is not supported on this platform. + // Callers must gate on SslStreamPal.IsDirectDecryptSupported before invoking this method. + _ = securityContext; + _ = input; + _ = output; + outputWritten = 0; + morePending = false; + throw new NotSupportedException(); + } + public static ChannelBinding? QueryContextChannelBinding( SafeDeleteContext securityContext, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index 77c06a6dac022b..a3929df7e6d97f 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -80,13 +80,41 @@ public static ProtocolToken EncryptMessage(SafeDeleteSslContext securityContext, } public static SecurityStatusPal DecryptMessage(SafeDeleteSslContext securityContext, Span buffer, out int offset, out int count) + { + // In-place decrypt: source ciphertext span doubles as plaintext destination. + return DecryptMessageDirectCore((SafeSslHandle)securityContext, buffer, buffer, out offset, out count, out _); + } + + // True on this platform: Unix supports decrypting directly into a caller-provided output buffer, + // avoiding a copy from the internal SslStream encrypted buffer to the user's Memory. + internal const bool IsDirectDecryptSupported = true; + + public static SecurityStatusPal DecryptMessageDirect( + SafeDeleteSslContext securityContext, + ReadOnlySpan input, + Span output, + out int outputWritten, + out bool morePending) + { + Debug.Assert(output.Length > 0); + return DecryptMessageDirectCore((SafeSslHandle)securityContext, input, output, out _, out outputWritten, out morePending); + } + + private static SecurityStatusPal DecryptMessageDirectCore( + SafeSslHandle sslHandle, + ReadOnlySpan input, + Span output, + out int offset, + out int count, + out bool morePending) { offset = 0; count = 0; + morePending = false; try { - int resultSize = Interop.OpenSsl.Decrypt((SafeSslHandle)securityContext, buffer, out Interop.Ssl.SslErrorCode errorCode); + int resultSize = Interop.OpenSsl.Decrypt(sslHandle, input, output, out Interop.Ssl.SslErrorCode errorCode); SecurityStatusPal retVal = MapNativeErrorCode(errorCode); @@ -96,6 +124,11 @@ public static SecurityStatusPal DecryptMessage(SafeDeleteSslContext securityCont count = resultSize; } + if (resultSize > 0) + { + morePending = Interop.Ssl.SslPending(sslHandle) > 0; + } + return retVal; } catch (Exception ex) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index 0059c83b1f8fb3..3f5969d7f1ad05 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -56,6 +56,7 @@ public static Exception GetException(SecurityStatusPal status) internal const bool CertValidationInCallback = false; internal const bool CanEncryptEmptyMessage = true; internal const bool CanGenerateCustomAlerts = true; + internal const bool IsDirectDecryptSupported = false; private static readonly byte[] s_sessionTokenBuffer = InitSessionTokenBuffer(); @@ -652,6 +653,23 @@ public static unsafe SecurityStatusPal DecryptMessage(SafeDeleteSslContext? secu return SecurityStatusAdapterPal.GetSecurityStatusPalFromInterop(errorCode); } } + public static SecurityStatusPal DecryptMessageDirect( + SafeDeleteSslContext securityContext, + ReadOnlySpan input, + Span output, + out int outputWritten, + out bool morePending) + { + // Direct decrypt is not supported on this platform. + // Callers must gate on SslStreamPal.IsDirectDecryptSupported before invoking this method. + _ = securityContext; + _ = input; + _ = output; + outputWritten = 0; + morePending = false; + throw new NotSupportedException(); + } + public static SecurityStatusPal ApplyAlertToken(SafeDeleteSslContext? securityContext, TlsAlertType alertType, TlsAlertMessage alertMessage) { diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index fa5051d9061d82..9a39bfe3ac7a1e 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -399,6 +399,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslGetServerName) DllImportEntry(CryptoNative_SslGetSession) DllImportEntry(CryptoNative_SslGetVersion) + DllImportEntry(CryptoNative_SslPending) DllImportEntry(CryptoNative_SslRead) DllImportEntry(CryptoNative_SslSessionFree) DllImportEntry(CryptoNative_SslSessionGetHostname) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 2622e83e6535e1..014d1da4b774ce 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -751,6 +751,7 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(SSL_get_certificate) \ REQUIRED_FUNCTION(SSL_new) \ REQUIRED_FUNCTION(SSL_peek) \ + REQUIRED_FUNCTION(SSL_pending) \ REQUIRED_FUNCTION(SSL_read) \ REQUIRED_FUNCTION(SSL_renegotiate) \ REQUIRED_FUNCTION(SSL_renegotiate_pending) \ @@ -1329,6 +1330,7 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define SSL_is_init_finished SSL_is_init_finished_ptr #define SSL_new SSL_new_ptr #define SSL_peek SSL_peek_ptr +#define SSL_pending SSL_pending_ptr #define SSL_read SSL_read_ptr #define SSL_renegotiate SSL_renegotiate_ptr #define SSL_renegotiate_pending SSL_renegotiate_pending_ptr diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 0648508b513f16..08697fbcd51f24 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -409,6 +409,11 @@ int32_t CryptoNative_SslWrite(SSL* ssl, const void* buf, int32_t num, int32_t* e return result; } +int32_t CryptoNative_SslPending(SSL* ssl) +{ + return SSL_pending(ssl); +} + int32_t CryptoNative_SslRead(SSL* ssl, void* buf, int32_t num, int32_t* error) { ERR_clear_error(); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index 7ea042b960cd70..a63f9a551a354a 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -283,6 +283,7 @@ Shims the SSL_read method. Returns the positive number of bytes read when successful, 0 or a negative number when an error is encountered. */ +PALEXPORT int32_t CryptoNative_SslPending(SSL* ssl); PALEXPORT int32_t CryptoNative_SslRead(SSL* ssl, void* buf, int32_t num, int32_t* error); /* From dc16ab65bbacbd1ac5279a917bb894c39eb382c2 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Fri, 15 May 2026 18:32:39 +0200 Subject: [PATCH 04/20] Revert SslStream/Linux: decrypt directly into user buffer This reverts commit 7371f2e2a1b. 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 a13413717bb), which remains correct under all protocols tested. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Interop.OpenSsl.cs | 22 +-- .../Interop.Ssl.cs | 3 - .../src/System/Net/Security/SslStream.IO.cs | 131 +----------------- .../Net/Security/SslStreamPal.Android.cs | 18 --- .../System/Net/Security/SslStreamPal.OSX.cs | 18 --- .../System/Net/Security/SslStreamPal.Unix.cs | 35 +---- .../Net/Security/SslStreamPal.Windows.cs | 18 --- .../entrypoints.c | 1 - .../opensslshim.h | 2 - .../pal_ssl.c | 5 - .../pal_ssl.h | 1 - 11 files changed, 13 insertions(+), 241 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 49b8ddb724ae35..7536e29a6764ee 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -19,7 +19,6 @@ using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Text; -using System.Threading; using Microsoft.Win32.SafeHandles; internal static partial class Interop @@ -837,6 +836,7 @@ internal static unsafe Ssl.SslErrorCode Encrypt(SafeSslHandle context, ReadOnlyS outToken.Size += drained; } } + return errorCode; } @@ -859,6 +859,7 @@ private static unsafe void DrainOutputBioSpill(SafeSslHandle context, ref Protoc { return; } + outToken.EnsureAvailableSpace(spillLen); Span dst = outToken.AvailableSpan; fixed (byte* dstPtr = dst) @@ -868,30 +869,21 @@ private static unsafe void DrainOutputBioSpill(SafeSslHandle context, ref Protoc } } - internal static unsafe int Decrypt(SafeSslHandle context, ReadOnlySpan input, Span output, out Ssl.SslErrorCode errorCode) + internal static unsafe int Decrypt(SafeSslHandle context, Span buffer, out Ssl.SslErrorCode errorCode) { - Debug.Assert(output.Length > 0, "Decrypt output buffer must be non-empty"); - int retVal; - fixed (byte* inputPtr = input) + fixed (byte* bufPtr = buffer) { - if (input.Length > 0) - { - Ssl.BioSetReadWindow(context.InputBio!, inputPtr, input.Length); - } + Ssl.BioSetReadWindow(context.InputBio!, bufPtr, buffer.Length); try { - retVal = Ssl.SslRead(context, ref MemoryMarshal.GetReference(output), output.Length, out errorCode); + retVal = Ssl.SslRead(context, ref MemoryMarshal.GetReference(buffer), buffer.Length, out errorCode); } finally { - if (input.Length > 0) - { - Ssl.BioClearReadWindow(context.InputBio!); - } + Ssl.BioClearReadWindow(context.InputBio!); } } - if (retVal > 0) { return retVal; diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 5a379cc62d8486..8038aba9c95062 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -80,9 +80,6 @@ internal static unsafe ReadOnlySpan SslGetAlpnSelected(SafeSslHandle ssl) [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslWrite", SetLastError = true)] internal static partial int SslWrite(SafeSslHandle ssl, ref byte buf, int num, out SslErrorCode error); - [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslPending")] - internal static partial int SslPending(SafeSslHandle ssl); - [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslRead", SetLastError = true)] internal static partial int SslRead(SafeSslHandle ssl, ref byte buf, int num, out SslErrorCode error); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index a769bcc8ab8fb5..a8faaae4fa9cbb 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -32,12 +32,6 @@ public partial class SslStream private bool _receivedEOF; - // True when the SSL PAL has plaintext buffered internally (residual from a direct-decrypt SSL_read - // whose destination span was smaller than the record's plaintext). When set, the next read must - // drain the residual via DecryptMessageDirect with empty input before issuing more network IO. - // Guarded by _handshakeLock. - private bool _palHasPendingPlaintext; - // Used by Telemetry to ensure we log connection close exactly once private enum ConnectionStatus { @@ -221,7 +215,7 @@ private async Task RenegotiateAsync(CancellationToken cancellationTo token.RentBuffer = true; try { - if (_buffer.ActiveLength > 0 || _palHasPendingPlaintext) + if (_buffer.ActiveLength > 0) { throw new InvalidOperationException(SR.net_ssl_renegotiate_buffer); } @@ -851,75 +845,6 @@ private async ValueTask EnsureFullTlsFrameAsync(CancellationTok return frameSize; } - // Direct-decrypt variant: when the PAL supports it and we have a non-empty user destination, - // decrypt plaintext directly into the user buffer instead of into _buffer. Returns bytes written - // to `output` plus any extra reauth bytes via `extraBuffer`. - private SecurityStatusPal DecryptDataDirect(int frameSize, Span output, out int outputWritten, out byte[]? extraBuffer) - { - Debug.Assert(SslStreamPal.IsDirectDecryptSupported); - Debug.Assert(output.Length > 0); - outputWritten = 0; - extraBuffer = null; - - SecurityStatusPal status; - lock (_handshakeLock) - { - ThrowIfExceptionalOrNotAuthenticated(); - - status = SslStreamPal.DecryptMessageDirect(_securityContext!, _buffer.EncryptedReadOnlySpan.Slice(0, frameSize), output, out int written, out bool morePending); - - // OpenSSL consumed the full frame regardless of status; discard from _buffer. - _buffer.OnDecrypted(0, 0, frameSize); - - if (status.ErrorCode == SecurityStatusPalErrorCode.OK) - { - outputWritten = written; - _palHasPendingPlaintext = morePending; - } - else - { - // Non-OK status: do NOT expose direct-written bytes to the caller via outputWritten. - // The existing error handler in ReadAsyncInternal expects any plaintext produced by a - // failed decrypt to live in `extraBuffer` (for reauth) rather than the user span. - if (written > 0) - { - extraBuffer = new byte[written]; - output.Slice(0, written).CopyTo(extraBuffer); - } - // Clear any pending plaintext - on non-OK the morePending semantics are not meaningful. - _palHasPendingPlaintext = false; - } - - if (status.ErrorCode == SecurityStatusPalErrorCode.Renegotiate) - { - if (_sslAuthenticationOptions.AllowRenegotiation || SslProtocol == SslProtocols.Tls13 || _nestedAuth != NestedState.StreamNotInUse) - { - _handshakeWaiter = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - } - } - } - - return status; - } - - // Drain residual plaintext buffered inside the PAL (Unix only). Returns bytes written to `output`. - private SecurityStatusPal DrainPendingPlaintextDirect(Span output, out int outputWritten) - { - Debug.Assert(SslStreamPal.IsDirectDecryptSupported); - Debug.Assert(output.Length > 0); - - SecurityStatusPal status; - lock (_handshakeLock) - { - ThrowIfExceptionalOrNotAuthenticated(); - - status = SslStreamPal.DecryptMessageDirect(_securityContext!, ReadOnlySpan.Empty, output, out outputWritten, out bool morePending); - _palHasPendingPlaintext = (status.ErrorCode == SecurityStatusPalErrorCode.OK) && morePending; - } - - return status; - } - private SecurityStatusPal DecryptData(int frameSize) { SecurityStatusPal status; @@ -1000,23 +925,6 @@ private async ValueTask ReadAsyncInternal(Memory buffer, buffer = buffer.Slice(processedLength); } - // Drain any plaintext the PAL has buffered internally from a prior direct decrypt - // whose user-buffer was smaller than the record's plaintext. - if (SslStreamPal.IsDirectDecryptSupported && _palHasPendingPlaintext && !buffer.IsEmpty) - { - SecurityStatusPal drainStatus = DrainPendingPlaintextDirect(buffer.Span, out int drained); - if (drainStatus.ErrorCode != SecurityStatusPalErrorCode.OK) - { - throw new IOException(SR.net_io_decrypt, SslStreamPal.GetException(drainStatus)); - } - processedLength += drained; - buffer = buffer.Slice(drained); - if (buffer.IsEmpty) - { - return processedLength; - } - } - if (_receivedEOF && nextTlsFrameLength == UnknownTlsFrameLength) { // there should be no frames waiting for processing @@ -1036,28 +944,11 @@ private async ValueTask ReadAsyncInternal(Memory buffer, break; } - SecurityStatusPal status; - byte[]? extraBuffer = null; - int directWritten = 0; - - // Use direct-decrypt only when the PAL supports it, we have user buffer space, and - // no concurrent re-handshake is in progress (which would change the SSL state machine). - bool useDirect = SslStreamPal.IsDirectDecryptSupported - && !buffer.IsEmpty - && _handshakeWaiter == null; - - if (useDirect) - { - status = DecryptDataDirect(payloadBytes, buffer.Span, out directWritten, out extraBuffer); - } - else - { - status = DecryptData(payloadBytes); - } - + SecurityStatusPal status = DecryptData(payloadBytes); if (status.ErrorCode != SecurityStatusPalErrorCode.OK) { - if (!useDirect && _buffer.DecryptedLength != 0) + byte[]? extraBuffer = null; + if (_buffer.DecryptedLength != 0) { extraBuffer = new byte[_buffer.DecryptedLength]; _buffer.DecryptedSpan.CopyTo(extraBuffer); @@ -1088,19 +979,7 @@ private async ValueTask ReadAsyncInternal(Memory buffer, } } - if (useDirect) - { - if (directWritten > 0) - { - processedLength += directWritten; - buffer = buffer.Slice(directWritten); - if (buffer.IsEmpty) - { - break; - } - } - } - else if (_buffer.DecryptedLength > 0) + if (_buffer.DecryptedLength > 0) { // This will either copy data from rented buffer or adjust final buffer as needed. // In both cases _decryptedBytesOffset and _decryptedBytesCount will be updated as needed. diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs index d84c07aad0e95e..68df93d01217cd 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs @@ -27,7 +27,6 @@ public static Exception GetException(SecurityStatusPal status) // depends on SslStream calling HandshakeInternal one last time when tearing down the connection // due to handshake failures. internal const bool CanGenerateCustomAlerts = true; - internal const bool IsDirectDecryptSupported = false; public static void VerifyPackageInfo() { @@ -147,23 +146,6 @@ public static SecurityStatusPal DecryptMessage( return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, e); } } - public static SecurityStatusPal DecryptMessageDirect( - SafeDeleteSslContext securityContext, - ReadOnlySpan input, - Span output, - out int outputWritten, - out bool morePending) - { - // Direct decrypt is not supported on this platform. - // Callers must gate on SslStreamPal.IsDirectDecryptSupported before invoking this method. - _ = securityContext; - _ = input; - _ = output; - outputWritten = 0; - morePending = false; - throw new NotSupportedException(); - } - public static ChannelBinding? QueryContextChannelBinding( SafeDeleteContext securityContext, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index 04e8140f1b9f06..7aff801dab8260 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -33,7 +33,6 @@ public static Exception GetException(SecurityStatusPal status) // special case of an empty array being passed to the `fixed` statement. internal const bool CanEncryptEmptyMessage = false; internal const bool CanGenerateCustomAlerts = false; - internal const bool IsDirectDecryptSupported = false; public static void VerifyPackageInfo() { @@ -258,23 +257,6 @@ public static SecurityStatusPal DecryptMessage( return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, e); } } - public static SecurityStatusPal DecryptMessageDirect( - SafeDeleteContext securityContext, - ReadOnlySpan input, - Span output, - out int outputWritten, - out bool morePending) - { - // Direct decrypt is not supported on this platform. - // Callers must gate on SslStreamPal.IsDirectDecryptSupported before invoking this method. - _ = securityContext; - _ = input; - _ = output; - outputWritten = 0; - morePending = false; - throw new NotSupportedException(); - } - public static ChannelBinding? QueryContextChannelBinding( SafeDeleteContext securityContext, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index a3929df7e6d97f..77c06a6dac022b 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -80,41 +80,13 @@ public static ProtocolToken EncryptMessage(SafeDeleteSslContext securityContext, } public static SecurityStatusPal DecryptMessage(SafeDeleteSslContext securityContext, Span buffer, out int offset, out int count) - { - // In-place decrypt: source ciphertext span doubles as plaintext destination. - return DecryptMessageDirectCore((SafeSslHandle)securityContext, buffer, buffer, out offset, out count, out _); - } - - // True on this platform: Unix supports decrypting directly into a caller-provided output buffer, - // avoiding a copy from the internal SslStream encrypted buffer to the user's Memory. - internal const bool IsDirectDecryptSupported = true; - - public static SecurityStatusPal DecryptMessageDirect( - SafeDeleteSslContext securityContext, - ReadOnlySpan input, - Span output, - out int outputWritten, - out bool morePending) - { - Debug.Assert(output.Length > 0); - return DecryptMessageDirectCore((SafeSslHandle)securityContext, input, output, out _, out outputWritten, out morePending); - } - - private static SecurityStatusPal DecryptMessageDirectCore( - SafeSslHandle sslHandle, - ReadOnlySpan input, - Span output, - out int offset, - out int count, - out bool morePending) { offset = 0; count = 0; - morePending = false; try { - int resultSize = Interop.OpenSsl.Decrypt(sslHandle, input, output, out Interop.Ssl.SslErrorCode errorCode); + int resultSize = Interop.OpenSsl.Decrypt((SafeSslHandle)securityContext, buffer, out Interop.Ssl.SslErrorCode errorCode); SecurityStatusPal retVal = MapNativeErrorCode(errorCode); @@ -124,11 +96,6 @@ private static SecurityStatusPal DecryptMessageDirectCore( count = resultSize; } - if (resultSize > 0) - { - morePending = Interop.Ssl.SslPending(sslHandle) > 0; - } - return retVal; } catch (Exception ex) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index 3f5969d7f1ad05..0059c83b1f8fb3 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -56,7 +56,6 @@ public static Exception GetException(SecurityStatusPal status) internal const bool CertValidationInCallback = false; internal const bool CanEncryptEmptyMessage = true; internal const bool CanGenerateCustomAlerts = true; - internal const bool IsDirectDecryptSupported = false; private static readonly byte[] s_sessionTokenBuffer = InitSessionTokenBuffer(); @@ -653,23 +652,6 @@ public static unsafe SecurityStatusPal DecryptMessage(SafeDeleteSslContext? secu return SecurityStatusAdapterPal.GetSecurityStatusPalFromInterop(errorCode); } } - public static SecurityStatusPal DecryptMessageDirect( - SafeDeleteSslContext securityContext, - ReadOnlySpan input, - Span output, - out int outputWritten, - out bool morePending) - { - // Direct decrypt is not supported on this platform. - // Callers must gate on SslStreamPal.IsDirectDecryptSupported before invoking this method. - _ = securityContext; - _ = input; - _ = output; - outputWritten = 0; - morePending = false; - throw new NotSupportedException(); - } - public static SecurityStatusPal ApplyAlertToken(SafeDeleteSslContext? securityContext, TlsAlertType alertType, TlsAlertMessage alertMessage) { diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index 9a39bfe3ac7a1e..fa5051d9061d82 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -399,7 +399,6 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslGetServerName) DllImportEntry(CryptoNative_SslGetSession) DllImportEntry(CryptoNative_SslGetVersion) - DllImportEntry(CryptoNative_SslPending) DllImportEntry(CryptoNative_SslRead) DllImportEntry(CryptoNative_SslSessionFree) DllImportEntry(CryptoNative_SslSessionGetHostname) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 014d1da4b774ce..2622e83e6535e1 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -751,7 +751,6 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(SSL_get_certificate) \ REQUIRED_FUNCTION(SSL_new) \ REQUIRED_FUNCTION(SSL_peek) \ - REQUIRED_FUNCTION(SSL_pending) \ REQUIRED_FUNCTION(SSL_read) \ REQUIRED_FUNCTION(SSL_renegotiate) \ REQUIRED_FUNCTION(SSL_renegotiate_pending) \ @@ -1330,7 +1329,6 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define SSL_is_init_finished SSL_is_init_finished_ptr #define SSL_new SSL_new_ptr #define SSL_peek SSL_peek_ptr -#define SSL_pending SSL_pending_ptr #define SSL_read SSL_read_ptr #define SSL_renegotiate SSL_renegotiate_ptr #define SSL_renegotiate_pending SSL_renegotiate_pending_ptr diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 08697fbcd51f24..0648508b513f16 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -409,11 +409,6 @@ int32_t CryptoNative_SslWrite(SSL* ssl, const void* buf, int32_t num, int32_t* e return result; } -int32_t CryptoNative_SslPending(SSL* ssl) -{ - return SSL_pending(ssl); -} - int32_t CryptoNative_SslRead(SSL* ssl, void* buf, int32_t num, int32_t* error) { ERR_clear_error(); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index a63f9a551a354a..7ea042b960cd70 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -283,7 +283,6 @@ Shims the SSL_read method. Returns the positive number of bytes read when successful, 0 or a negative number when an error is encountered. */ -PALEXPORT int32_t CryptoNative_SslPending(SSL* ssl); PALEXPORT int32_t CryptoNative_SslRead(SSL* ssl, void* buf, int32_t num, int32_t* error); /* From 7d3fc4382fe29ce991096f7b89164a04348727b0 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Fri, 15 May 2026 20:37:32 +0200 Subject: [PATCH 05/20] SslStream/Linux: atomic SSL ops and direct-decrypt into user buffer 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> --- .../Interop.OpenSsl.cs | 75 ++++++------ .../Interop.Ssl.cs | 32 +++++ .../src/System/Net/Security/SslStream.IO.cs | 103 +++++++++++++++- .../System/Net/Security/SslStream.Protocol.cs | 11 ++ .../Net/Security/SslStreamPal.Android.cs | 13 ++ .../System/Net/Security/SslStreamPal.OSX.cs | 13 ++ .../System/Net/Security/SslStreamPal.Unix.cs | 46 ++++++- .../Net/Security/SslStreamPal.Windows.cs | 13 ++ .../entrypoints.c | 3 + .../opensslshim.h | 6 + .../pal_ssl.c | 112 ++++++++++++++++++ .../pal_ssl.h | 59 +++++++++ 12 files changed, 441 insertions(+), 45 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 7536e29a6764ee..ecd2db7c16b63a 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -690,23 +690,15 @@ internal static unsafe SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle c fixed (byte* inputPtr = input) fixed (byte* windowPtr = windowSpan) { - if (input.Length > 0) - { - Ssl.BioSetReadWindow(context.InputBio!, inputPtr, input.Length); - } - - Ssl.BioSetWriteWindow(context.OutputBio!, windowPtr, windowSpan.Length); - - try - { - retVal = Ssl.SslDoHandshake(context, out errorCode); - Ssl.BioGetWriteResult(context.OutputBio!, out writtenToWindow, out spillLen); - } - finally - { - Ssl.BioSetWriteWindow(context.OutputBio!, null, 0); - Ssl.BioClearReadWindow(context.InputBio!); - } + retVal = Ssl.SslHandshake( + context, + inputPtr, + input.Length, + windowPtr, + windowSpan.Length, + out writtenToWindow, + out spillLen, + out errorCode); } token.Size += writtenToWindow; @@ -792,18 +784,18 @@ internal static unsafe Ssl.SslErrorCode Encrypt(SafeSslHandle context, ReadOnlyS Ssl.SslErrorCode errorCode; Span windowSpan = outToken.AvailableSpan; + fixed (byte* plaintextPtr = input) fixed (byte* windowPtr = windowSpan) { - Ssl.BioSetWriteWindow(context.OutputBio!, windowPtr, windowSpan.Length); - try - { - retVal = Ssl.SslWrite(context, ref MemoryMarshal.GetReference(input), input.Length, out errorCode); - Ssl.BioGetWriteResult(context.OutputBio!, out writtenToWindow, out spillLen); - } - finally - { - Ssl.BioSetWriteWindow(context.OutputBio!, null, 0); - } + retVal = Ssl.SslEncrypt( + context, + plaintextPtr, + input.Length, + windowPtr, + windowSpan.Length, + out writtenToWindow, + out spillLen, + out errorCode); } if (retVal != input.Length) @@ -869,20 +861,25 @@ private static unsafe void DrainOutputBioSpill(SafeSslHandle context, ref Protoc } } - internal static unsafe int Decrypt(SafeSslHandle context, Span buffer, out Ssl.SslErrorCode errorCode) + internal static unsafe int Decrypt( + SafeSslHandle context, + ReadOnlySpan input, + Span output, + out int plaintextPending, + out Ssl.SslErrorCode errorCode) { int retVal; - fixed (byte* bufPtr = buffer) - { - Ssl.BioSetReadWindow(context.InputBio!, bufPtr, buffer.Length); - try - { - retVal = Ssl.SslRead(context, ref MemoryMarshal.GetReference(buffer), buffer.Length, out errorCode); - } - finally - { - Ssl.BioClearReadWindow(context.InputBio!); - } + fixed (byte* inputPtr = input) + fixed (byte* outputPtr = output) + { + retVal = Ssl.SslDecrypt( + context, + inputPtr, + input.Length, + outputPtr, + output.Length, + out plaintextPending, + out errorCode); } if (retVal > 0) { diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 8038aba9c95062..deb81daa49d5ed 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -120,6 +120,38 @@ internal static ushort[] GetDefaultSignatureAlgorithms() [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslDoHandshake", SetLastError = true)] internal static partial int SslDoHandshake(SafeSslHandle ssl, out SslErrorCode error); + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslHandshake", SetLastError = true)] + internal static unsafe partial int SslHandshake( + SafeSslHandle ssl, + byte* inputPtr, + int inputLen, + byte* outputPtr, + int outputCap, + out int outputWritten, + out int outputPending, + out SslErrorCode errorCode); + + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslEncrypt", SetLastError = true)] + internal static unsafe partial int SslEncrypt( + SafeSslHandle ssl, + byte* plaintextPtr, + int plaintextLen, + byte* outputPtr, + int outputCap, + out int outputWritten, + out int outputPending, + out SslErrorCode errorCode); + + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslDecrypt", SetLastError = true)] + internal static unsafe partial int SslDecrypt( + SafeSslHandle ssl, + byte* inputPtr, + int inputLen, + byte* outputPtr, + int outputCap, + out int plaintextPending, + out SslErrorCode errorCode); + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_IsSslStateOK")] [return: MarshalAs(UnmanagedType.Bool)] internal static partial bool IsSslStateOK(SafeSslHandle ssl); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index a8faaae4fa9cbb..0a4e03f84879e2 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -24,12 +24,21 @@ public partial class SslStream private object _handshakeLock => _sslAuthenticationOptions; private volatile TaskCompletionSource? _handshakeWaiter; + // Set when a direct-decrypt left plaintext buffered inside OpenSSL (SSL_pending > 0). + // Guarded by _handshakeLock for writes; advisory for unlocked reads. + private bool _palHasPendingPlaintext; private const int HandshakeTypeOffsetSsl2 = 2; // Offset of HelloType in Sslv2 and Unified frames private const int HandshakeTypeOffsetTls = 5; // Offset of HelloType in Sslv3 and TLS frames private const int UnknownTlsFrameLength = int.MaxValue; // frame too short to determine length + // Maximum plaintext length of a single TLS record (2^14 per RFC 5246/8446). + // Direct decrypt requires the user buffer to be at least this large so that + // a single SSL_read fully drains the decrypted record (no residual SSL_pending), + // keeping the read path free of partial-record / drain state. + private const int MaxTlsRecordPlaintextLength = 16384; + private bool _receivedEOF; // Used by Telemetry to ensure we log connection close exactly once @@ -215,7 +224,7 @@ private async Task RenegotiateAsync(CancellationToken cancellationTo token.RentBuffer = true; try { - if (_buffer.ActiveLength > 0) + if (_buffer.ActiveLength > 0 || _palHasPendingPlaintext) { throw new InvalidOperationException(SR.net_ssl_renegotiate_buffer); } @@ -885,6 +894,46 @@ private SecurityStatusPal DecryptData(int frameSize) return status; } + private SecurityStatusPal DecryptDataDirect(int frameSize, Span output, out int outputWritten) + { + SecurityStatusPal status; + + lock (_handshakeLock) + { + ThrowIfExceptionalOrNotAuthenticated(); + + status = DecryptDirect(_buffer.EncryptedSpanSliced(frameSize), output, out outputWritten, out int plaintextPending); + // Consume the whole ciphertext frame; no plaintext was deposited into _buffer. + _buffer.OnDecrypted(0, 0, frameSize); + _palHasPendingPlaintext = plaintextPending > 0; + + if (status.ErrorCode == SecurityStatusPalErrorCode.Renegotiate) + { + if (_sslAuthenticationOptions.AllowRenegotiation || SslProtocol == SslProtocols.Tls13 || _nestedAuth != NestedState.StreamNotInUse) + { + _handshakeWaiter = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + } + } + } + + return status; + } + + private SecurityStatusPal DrainPendingPlaintextDirect(Span output, out int outputWritten) + { + SecurityStatusPal status; + + lock (_handshakeLock) + { + ThrowIfExceptionalOrNotAuthenticated(); + + status = DecryptDirect(ReadOnlySpan.Empty, output, out outputWritten, out int plaintextPending); + _palHasPendingPlaintext = plaintextPending > 0; + } + + return status; + } + [AsyncMethodBuilder(typeof(PoolingAsyncValueTaskMethodBuilder<>))] private async ValueTask ReadAsyncInternal(Memory buffer, CancellationToken cancellationToken) where TIOAdapter : IReadWriteAdapter @@ -925,6 +974,8 @@ private async ValueTask ReadAsyncInternal(Memory buffer, buffer = buffer.Slice(processedLength); } + + if (_receivedEOF && nextTlsFrameLength == UnknownTlsFrameLength) { // there should be no frames waiting for processing @@ -937,6 +988,28 @@ private async ValueTask ReadAsyncInternal(Memory buffer, while (true) { + // Drain any plaintext OpenSSL retained internally from a prior partial direct-decrypt + // before fetching or decrypting any new ciphertext. If we proceeded to decrypt a fresh + // frame while SSL still holds buffered plaintext, SSL_read would return the buffered + // plaintext (ignoring the new frame's ciphertext) and we would incorrectly discard the + // unprocessed frame from _buffer. + while (_palHasPendingPlaintext && !buffer.IsEmpty && _handshakeWaiter == null) + { + SecurityStatusPal drainStatus = DrainPendingPlaintextDirect(buffer.Span, out int drained); + if (drainStatus.ErrorCode != SecurityStatusPalErrorCode.OK || drained == 0) + { + // Either an SSL error happened (will resurface on next decrypt) or SSL has no more + // pending plaintext available right now; stop draining and proceed. + break; + } + processedLength += drained; + buffer = buffer.Slice(drained); + } + if (buffer.IsEmpty && processedLength > 0) + { + // User buffer is full; any remaining pending plaintext will be drained on the next ReadAsync. + break; + } int payloadBytes = await EnsureFullTlsFrameAsync(cancellationToken, ReadBufferSize).ConfigureAwait(false); if (payloadBytes == 0) { @@ -944,7 +1017,17 @@ private async ValueTask ReadAsyncInternal(Memory buffer, break; } - SecurityStatusPal status = DecryptData(payloadBytes); + SecurityStatusPal status; + int directWritten = 0; + bool useDirect = SslStreamPal.IsDirectDecryptSupported && buffer.Length >= MaxTlsRecordPlaintextLength && _handshakeWaiter == null; + if (useDirect) + { + status = DecryptDataDirect(payloadBytes, buffer.Span, out directWritten); + } + else + { + status = DecryptData(payloadBytes); + } if (status.ErrorCode != SecurityStatusPalErrorCode.OK) { byte[]? extraBuffer = null; @@ -979,7 +1062,21 @@ private async ValueTask ReadAsyncInternal(Memory buffer, } } - if (_buffer.DecryptedLength > 0) + if (useDirect) + { + if (directWritten > 0) + { + processedLength += directWritten; + buffer = buffer.Slice(directWritten); + if (buffer.IsEmpty) + { + // User buffer is full. Any residual plaintext is captured by SSL_pending + // and will be drained on the next ReadAsync via _palHasPendingPlaintext. + break; + } + } + } + else if (_buffer.DecryptedLength > 0) { // This will either copy data from rented buffer or adjust final buffer as needed. // In both cases _decryptedBytesOffset and _decryptedBytesCount will be updated as needed. diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 51f673f2111edc..5e3062313bdf91 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -999,6 +999,17 @@ internal SecurityStatusPal Decrypt(Span buffer, out int outputOffset, out return status; } + internal SecurityStatusPal DecryptDirect(ReadOnlySpan input, Span output, out int outputWritten, out int plaintextPending) + { + SecurityStatusPal status = SslStreamPal.DecryptMessageDirect(_securityContext!, input, output, out outputWritten, out plaintextPending); + if (NetEventSource.Log.IsEnabled() && status.ErrorCode == SecurityStatusPalErrorCode.OK) + { + NetEventSource.DumpBuffer(this, output.Slice(0, outputWritten)); + } + + return status; + } + /*++ VerifyRemoteCertificate - Validates the content of a Remote Certificate diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs index 68df93d01217cd..50ccdd7bfffa67 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs @@ -109,6 +109,19 @@ public static ProtocolToken EncryptMessage( return token; } + // Direct-decrypt to user buffer is supported only on the OpenSSL-backed Unix PAL. + internal const bool IsDirectDecryptSupported = false; + + public static SecurityStatusPal DecryptMessageDirect( + SafeDeleteSslContext securityContext, + ReadOnlySpan input, + Span output, + out int outputWritten, + out int plaintextPending) + { + throw new System.PlatformNotSupportedException(); + } + public static SecurityStatusPal DecryptMessage( SafeDeleteSslContext securityContext, Span buffer, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index 7aff801dab8260..552733963202fa 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -203,6 +203,19 @@ public static ProtocolToken EncryptMessage( return token; } + // Direct-decrypt to user buffer is supported only on the OpenSSL-backed Unix PAL. + internal const bool IsDirectDecryptSupported = false; + + public static SecurityStatusPal DecryptMessageDirect( + SafeDeleteSslContext securityContext, + ReadOnlySpan input, + Span output, + out int outputWritten, + out int plaintextPending) + { + throw new System.PlatformNotSupportedException(); + } + public static SecurityStatusPal DecryptMessage( SafeDeleteContext securityContext, Span buffer, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index 77c06a6dac022b..71f3d2e95b6f5c 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -80,20 +80,60 @@ public static ProtocolToken EncryptMessage(SafeDeleteSslContext securityContext, } public static SecurityStatusPal DecryptMessage(SafeDeleteSslContext securityContext, Span buffer, out int offset, out int count) + { + // In-place decrypt: pass the same span as both input ciphertext and output plaintext. + // OpenSSL fully consumes the ciphertext via BIO_read before writing plaintext through + // SSL_read, so this aliasing is safe. + return DecryptMessageCore((SafeSslHandle)securityContext, buffer, buffer, out offset, out count, out _); + } + + // True on Unix: the OpenSSL-backed PAL supports decrypting directly into a caller-provided + // output buffer that is disjoint from the source ciphertext, avoiding a copy from + // SslStream's internal encrypted buffer to the user's Memory. + internal const bool IsDirectDecryptSupported = true; + + // Direct-decrypt variant. `input` is the ciphertext frame and `output` is the user's + // plaintext destination. `outputWritten` is the number of bytes written to `output`. + // `plaintextPending` is non-zero when SSL retains additional plaintext that the next call + // can drain by passing `input` of length 0. + public static SecurityStatusPal DecryptMessageDirect( + SafeDeleteSslContext securityContext, + ReadOnlySpan input, + Span output, + out int outputWritten, + out int plaintextPending) + { + Debug.Assert(output.Length > 0); + return DecryptMessageCore((SafeSslHandle)securityContext, input, output, out _, out outputWritten, out plaintextPending); + } + + private static SecurityStatusPal DecryptMessageCore( + SafeSslHandle sslHandle, + ReadOnlySpan input, + Span output, + out int offset, + out int count, + out int plaintextPending) { offset = 0; count = 0; + plaintextPending = 0; try { - int resultSize = Interop.OpenSsl.Decrypt((SafeSslHandle)securityContext, buffer, out Interop.Ssl.SslErrorCode errorCode); + int resultSize = Interop.OpenSsl.Decrypt( + sslHandle, + input, + output, + out int pending, + out Interop.Ssl.SslErrorCode errorCode); SecurityStatusPal retVal = MapNativeErrorCode(errorCode); - if (retVal.ErrorCode == SecurityStatusPalErrorCode.OK || - retVal.ErrorCode == SecurityStatusPalErrorCode.Renegotiate) + if (retVal.ErrorCode is SecurityStatusPalErrorCode.OK or SecurityStatusPalErrorCode.Renegotiate) { count = resultSize; + plaintextPending = pending; } return retVal; diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index 0059c83b1f8fb3..d4e4c237a541f6 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -601,6 +601,19 @@ public static unsafe ProtocolToken EncryptMessage(SafeDeleteSslContext securityC return token; } + // Direct-decrypt to user buffer is supported only on the OpenSSL-backed Unix PAL. + internal const bool IsDirectDecryptSupported = false; + + public static SecurityStatusPal DecryptMessageDirect( + SafeDeleteSslContext securityContext, + ReadOnlySpan input, + Span output, + out int outputWritten, + out int plaintextPending) + { + throw new System.PlatformNotSupportedException(); + } + public static unsafe SecurityStatusPal DecryptMessage(SafeDeleteSslContext? securityContext, Span buffer, out int offset, out int count) { const int NumSecBuffers = 4; // data + empty + empty + empty diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index fa5051d9061d82..d29d597f88c6f9 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -385,6 +385,9 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslAddClientCAs) DllImportEntry(CryptoNative_SslDestroy) DllImportEntry(CryptoNative_SslDoHandshake) + DllImportEntry(CryptoNative_SslHandshake) + DllImportEntry(CryptoNative_SslEncrypt) + DllImportEntry(CryptoNative_SslDecrypt) DllImportEntry(CryptoNative_SslGetClientCAList) DllImportEntry(CryptoNative_SslGetCurrentCipherId) DllImportEntry(CryptoNative_SslGetData) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 2622e83e6535e1..d789b6ab3dc857 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -751,6 +751,9 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(SSL_get_certificate) \ REQUIRED_FUNCTION(SSL_new) \ REQUIRED_FUNCTION(SSL_peek) \ + REQUIRED_FUNCTION(SSL_pending) \ + REQUIRED_FUNCTION(SSL_get_rbio) \ + REQUIRED_FUNCTION(SSL_get_wbio) \ REQUIRED_FUNCTION(SSL_read) \ REQUIRED_FUNCTION(SSL_renegotiate) \ REQUIRED_FUNCTION(SSL_renegotiate_pending) \ @@ -1329,6 +1332,9 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define SSL_is_init_finished SSL_is_init_finished_ptr #define SSL_new SSL_new_ptr #define SSL_peek SSL_peek_ptr +#define SSL_pending SSL_pending_ptr +#define SSL_get_rbio SSL_get_rbio_ptr +#define SSL_get_wbio SSL_get_wbio_ptr #define SSL_read SSL_read_ptr #define SSL_renegotiate SSL_renegotiate_ptr #define SSL_renegotiate_pending SSL_renegotiate_pending_ptr diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 0648508b513f16..69f2bca1a44b0d 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. #include "pal_ssl.h" +#include "pal_bio.h" #include "openssl.h" #include "pal_evp_pkey.h" #include "pal_evp_pkey_rsa.h" @@ -501,6 +502,117 @@ int32_t CryptoNative_SslDoHandshake(SSL* ssl, int32_t* error) return result; } +int32_t CryptoNative_SslHandshake( + SSL* ssl, + const uint8_t* inputPtr, + int32_t inputLen, + uint8_t* outputPtr, + int32_t outputCap, + int32_t* outputWritten, + int32_t* outputPending, + int32_t* errorCode) +{ + if (outputWritten != NULL) *outputWritten = 0; + if (outputPending != NULL) *outputPending = 0; + + ERR_clear_error(); + + BIO* inputBio = SSL_get_rbio(ssl); + BIO* outputBio = SSL_get_wbio(ssl); + + if (inputBio != NULL) + { + CryptoNative_BioSetReadWindow(inputBio, inputPtr, inputLen); + } + if (outputBio != NULL) + { + CryptoNative_BioSetWriteWindow(outputBio, outputPtr, outputCap); + } + + int32_t result = SSL_do_handshake(ssl); + *errorCode = (result == 1) ? SSL_ERROR_NONE : CryptoNative_SslGetError(ssl, result); + + if (outputBio != NULL) + { + CryptoNative_BioGetWriteResult(outputBio, outputWritten, outputPending); + CryptoNative_BioSetWriteWindow(outputBio, NULL, 0); + } + if (inputBio != NULL) + { + CryptoNative_BioClearReadWindow(inputBio); + } + + return result; +} + +int32_t CryptoNative_SslEncrypt( + SSL* ssl, + const uint8_t* plaintextPtr, + int32_t plaintextLen, + uint8_t* outputPtr, + int32_t outputCap, + int32_t* outputWritten, + int32_t* outputPending, + int32_t* errorCode) +{ + if (outputWritten != NULL) *outputWritten = 0; + if (outputPending != NULL) *outputPending = 0; + + ERR_clear_error(); + + BIO* outputBio = SSL_get_wbio(ssl); + if (outputBio != NULL) + { + CryptoNative_BioSetWriteWindow(outputBio, outputPtr, outputCap); + } + + int32_t result = SSL_write(ssl, plaintextPtr, plaintextLen); + *errorCode = (result > 0) ? SSL_ERROR_NONE : CryptoNative_SslGetError(ssl, result); + + if (outputBio != NULL) + { + CryptoNative_BioGetWriteResult(outputBio, outputWritten, outputPending); + CryptoNative_BioSetWriteWindow(outputBio, NULL, 0); + } + + return result; +} + +int32_t CryptoNative_SslDecrypt( + SSL* ssl, + const uint8_t* inputPtr, + int32_t inputLen, + uint8_t* outputPtr, + int32_t outputCap, + int32_t* plaintextPending, + int32_t* errorCode) +{ + if (plaintextPending != NULL) *plaintextPending = 0; + + ERR_clear_error(); + + BIO* inputBio = SSL_get_rbio(ssl); + if (inputBio != NULL) + { + CryptoNative_BioSetReadWindow(inputBio, inputPtr, inputLen); + } + + int32_t result = SSL_read(ssl, outputPtr, outputCap); + *errorCode = (result > 0) ? SSL_ERROR_NONE : CryptoNative_SslGetError(ssl, result); + + if (plaintextPending != NULL) + { + *plaintextPending = SSL_pending(ssl); + } + + if (inputBio != NULL) + { + CryptoNative_BioClearReadWindow(inputBio); + } + + return result; +} + int32_t CryptoNative_IsSslStateOK(SSL* ssl) { // No error queue impact. diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index 7ea042b960cd70..8b767e168503bf 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -325,6 +325,65 @@ and by the specifications of the TLS/SSL protocol; */ PALEXPORT int32_t CryptoNative_SslDoHandshake(SSL* ssl, int32_t* error); +/* +Atomically performs SSL_do_handshake with the input/output BIO windows set up +and torn down in a single P/Invoke. The input BIO window points at inputPtr +(ciphertext from peer, may be NULL/0). The output BIO window receives outgoing +handshake bytes into outputPtr/outputCap; outputWritten reports the count +placed there. If the handshake emitted more bytes than outputCap, those +overflow into the BIO spill and outputPending reports their size (caller drains +via CryptoNative_BioDrainSpill). + +Returns the SSL_do_handshake return value; errorCode receives SSL_get_error. +*/ +PALEXPORT int32_t CryptoNative_SslHandshake( + SSL* ssl, + const uint8_t* inputPtr, + int32_t inputLen, + uint8_t* outputPtr, + int32_t outputCap, + int32_t* outputWritten, + int32_t* outputPending, + int32_t* errorCode); + +/* +Atomically performs SSL_write with the output BIO window set up and torn down +in a single P/Invoke. plaintextPtr/plaintextLen is the plaintext. outputPtr/ +outputCap is the ciphertext destination window; outputWritten reports bytes +written, outputPending reports any overflow now in the BIO spill (drain via +CryptoNative_BioDrainSpill). + +Returns the SSL_write return value; errorCode receives SSL_get_error. +*/ +PALEXPORT int32_t CryptoNative_SslEncrypt( + SSL* ssl, + const uint8_t* plaintextPtr, + int32_t plaintextLen, + uint8_t* outputPtr, + int32_t outputCap, + int32_t* outputWritten, + int32_t* outputPending, + int32_t* errorCode); + +/* +Atomically performs SSL_read with the input BIO window set up and torn down +in a single P/Invoke. inputPtr/inputLen is incoming ciphertext (may be 0 to +drain plaintext already buffered inside SSL from a prior partial read). +outputPtr/outputCap is the plaintext destination. plaintextPending receives +SSL_pending after the read (bytes still buffered inside the SSL object). + +Returns the SSL_read return value (>0 = bytes written to outputPtr); errorCode +receives SSL_get_error. +*/ +PALEXPORT int32_t CryptoNative_SslDecrypt( + SSL* ssl, + const uint8_t* inputPtr, + int32_t inputLen, + uint8_t* outputPtr, + int32_t outputCap, + int32_t* plaintextPending, + int32_t* errorCode); + /* Gets a value indicating whether the SSL_state is SSL_ST_OK. From e925098fc1c3cddcaf9248b319cc4e432a35b6ba Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 18 May 2026 12:34:47 +0200 Subject: [PATCH 06/20] Simplify Direct Decrypt --- .../Interop.OpenSsl.cs | 8 +- .../Interop.Ssl.cs | 3 +- .../src/System/Net/Security/SslStream.IO.cs | 121 ++++-------------- .../System/Net/Security/SslStream.Protocol.cs | 35 +++-- .../Net/Security/SslStreamPal.Android.cs | 34 ++--- .../System/Net/Security/SslStreamPal.OSX.cs | 39 +++--- .../System/Net/Security/SslStreamPal.Unix.cs | 71 ++++------ .../Net/Security/SslStreamPal.Windows.cs | 41 +++--- .../pal_ssl.c | 23 +++- .../pal_ssl.h | 5 +- 10 files changed, 145 insertions(+), 235 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index ecd2db7c16b63a..0fe83b08300941 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -865,7 +865,8 @@ internal static unsafe int Decrypt( SafeSslHandle context, ReadOnlySpan input, Span output, - out int plaintextPending, + out int leftoverOffset, + out int leftoverLength, out Ssl.SslErrorCode errorCode) { int retVal; @@ -878,10 +879,11 @@ internal static unsafe int Decrypt( input.Length, outputPtr, output.Length, - out plaintextPending, + out leftoverOffset, + out leftoverLength, out errorCode); } - if (retVal > 0) + if (retVal + leftoverLength > 0) { return retVal; } diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index deb81daa49d5ed..21c70131fc5b79 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -149,7 +149,8 @@ internal static unsafe partial int SslDecrypt( int inputLen, byte* outputPtr, int outputCap, - out int plaintextPending, + out int leftoverOffset, + out int leftoverLength, out SslErrorCode errorCode); [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_IsSslStateOK")] diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index 0a4e03f84879e2..b22e54e6d7522d 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -24,21 +24,12 @@ public partial class SslStream private object _handshakeLock => _sslAuthenticationOptions; private volatile TaskCompletionSource? _handshakeWaiter; - // Set when a direct-decrypt left plaintext buffered inside OpenSSL (SSL_pending > 0). - // Guarded by _handshakeLock for writes; advisory for unlocked reads. - private bool _palHasPendingPlaintext; private const int HandshakeTypeOffsetSsl2 = 2; // Offset of HelloType in Sslv2 and Unified frames private const int HandshakeTypeOffsetTls = 5; // Offset of HelloType in Sslv3 and TLS frames private const int UnknownTlsFrameLength = int.MaxValue; // frame too short to determine length - // Maximum plaintext length of a single TLS record (2^14 per RFC 5246/8446). - // Direct decrypt requires the user buffer to be at least this large so that - // a single SSL_read fully drains the decrypted record (no residual SSL_pending), - // keeping the read path free of partial-record / drain state. - private const int MaxTlsRecordPlaintextLength = 16384; - private bool _receivedEOF; // Used by Telemetry to ensure we log connection close exactly once @@ -224,7 +215,7 @@ private async Task RenegotiateAsync(CancellationToken cancellationTo token.RentBuffer = true; try { - if (_buffer.ActiveLength > 0 || _palHasPendingPlaintext) + if (_buffer.ActiveLength > 0) { throw new InvalidOperationException(SR.net_ssl_renegotiate_buffer); } @@ -854,7 +845,7 @@ private async ValueTask EnsureFullTlsFrameAsync(CancellationTok return frameSize; } - private SecurityStatusPal DecryptData(int frameSize) + private SecurityStatusPal DecryptData(int frameSize, Span destination, out int bytesWritten) { SecurityStatusPal status; @@ -862,9 +853,17 @@ private SecurityStatusPal DecryptData(int frameSize) { ThrowIfExceptionalOrNotAuthenticated(); - // Decrypt will decrypt in-place and modify these to point to the actual decrypted data, which may be smaller. - status = Decrypt(_buffer.EncryptedSpanSliced(frameSize), out int decryptedOffset, out int decryptedCount); - _buffer.OnDecrypted(decryptedOffset, decryptedCount, frameSize); + // The PAL may either: + // * write plaintext directly into `destination` (bytesWritten > 0, no in-place leftover), or + // * decrypt in-place into `_buffer` (bytesWritten == 0, leftoverOffset/Length describe the region). + status = Decrypt( + _buffer.EncryptedSpanSliced(frameSize), + destination, + out bytesWritten, + out int leftoverOffset, + out int leftoverLength); + + _buffer.OnDecrypted(leftoverOffset, leftoverLength, frameSize); if (status.ErrorCode == SecurityStatusPalErrorCode.Renegotiate) { @@ -894,45 +893,6 @@ private SecurityStatusPal DecryptData(int frameSize) return status; } - private SecurityStatusPal DecryptDataDirect(int frameSize, Span output, out int outputWritten) - { - SecurityStatusPal status; - - lock (_handshakeLock) - { - ThrowIfExceptionalOrNotAuthenticated(); - - status = DecryptDirect(_buffer.EncryptedSpanSliced(frameSize), output, out outputWritten, out int plaintextPending); - // Consume the whole ciphertext frame; no plaintext was deposited into _buffer. - _buffer.OnDecrypted(0, 0, frameSize); - _palHasPendingPlaintext = plaintextPending > 0; - - if (status.ErrorCode == SecurityStatusPalErrorCode.Renegotiate) - { - if (_sslAuthenticationOptions.AllowRenegotiation || SslProtocol == SslProtocols.Tls13 || _nestedAuth != NestedState.StreamNotInUse) - { - _handshakeWaiter = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - } - } - } - - return status; - } - - private SecurityStatusPal DrainPendingPlaintextDirect(Span output, out int outputWritten) - { - SecurityStatusPal status; - - lock (_handshakeLock) - { - ThrowIfExceptionalOrNotAuthenticated(); - - status = DecryptDirect(ReadOnlySpan.Empty, output, out outputWritten, out int plaintextPending); - _palHasPendingPlaintext = plaintextPending > 0; - } - - return status; - } [AsyncMethodBuilder(typeof(PoolingAsyncValueTaskMethodBuilder<>))] private async ValueTask ReadAsyncInternal(Memory buffer, CancellationToken cancellationToken) @@ -988,28 +948,6 @@ private async ValueTask ReadAsyncInternal(Memory buffer, while (true) { - // Drain any plaintext OpenSSL retained internally from a prior partial direct-decrypt - // before fetching or decrypting any new ciphertext. If we proceeded to decrypt a fresh - // frame while SSL still holds buffered plaintext, SSL_read would return the buffered - // plaintext (ignoring the new frame's ciphertext) and we would incorrectly discard the - // unprocessed frame from _buffer. - while (_palHasPendingPlaintext && !buffer.IsEmpty && _handshakeWaiter == null) - { - SecurityStatusPal drainStatus = DrainPendingPlaintextDirect(buffer.Span, out int drained); - if (drainStatus.ErrorCode != SecurityStatusPalErrorCode.OK || drained == 0) - { - // Either an SSL error happened (will resurface on next decrypt) or SSL has no more - // pending plaintext available right now; stop draining and proceed. - break; - } - processedLength += drained; - buffer = buffer.Slice(drained); - } - if (buffer.IsEmpty && processedLength > 0) - { - // User buffer is full; any remaining pending plaintext will be drained on the next ReadAsync. - break; - } int payloadBytes = await EnsureFullTlsFrameAsync(cancellationToken, ReadBufferSize).ConfigureAwait(false); if (payloadBytes == 0) { @@ -1017,17 +955,13 @@ private async ValueTask ReadAsyncInternal(Memory buffer, break; } - SecurityStatusPal status; - int directWritten = 0; - bool useDirect = SslStreamPal.IsDirectDecryptSupported && buffer.Length >= MaxTlsRecordPlaintextLength && _handshakeWaiter == null; - if (useDirect) - { - status = DecryptDataDirect(payloadBytes, buffer.Span, out directWritten); - } - else - { - status = DecryptData(payloadBytes); - } + // Pass the user buffer to DecryptData so PALs that support it can decrypt directly + // into the destination, avoiding a CopyDecryptedData memcpy. When the renegotiate + // path is in flight we suppress the direct path by passing an empty span - the PAL + // will fall back to in-place decrypt and the existing CopyDecryptedData path runs. + Span destination = _handshakeWaiter == null ? buffer.Span : default; + SecurityStatusPal status = DecryptData(payloadBytes, destination, out int directWritten); + if (status.ErrorCode != SecurityStatusPalErrorCode.OK) { byte[]? extraBuffer = null; @@ -1062,18 +996,13 @@ private async ValueTask ReadAsyncInternal(Memory buffer, } } - if (useDirect) + if (directWritten > 0) { - if (directWritten > 0) + processedLength += directWritten; + buffer = buffer.Slice(directWritten); + if (buffer.IsEmpty) { - processedLength += directWritten; - buffer = buffer.Slice(directWritten); - if (buffer.IsEmpty) - { - // User buffer is full. Any residual plaintext is captured by SSL_pending - // and will be drained on the next ReadAsync via _palHasPendingPlaintext. - break; - } + break; } } else if (_buffer.DecryptedLength > 0) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 5e3062313bdf91..2dafbf2e2baa02 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -988,23 +988,32 @@ internal ProtocolToken Encrypt(ReadOnlyMemory buffer) return token; } - internal SecurityStatusPal Decrypt(Span buffer, out int outputOffset, out int outputCount) + internal SecurityStatusPal Decrypt( + Span encrypted, + Span destination, + out int bytesWritten, + out int leftoverOffset, + out int leftoverLength) { - SecurityStatusPal status = SslStreamPal.DecryptMessage(_securityContext!, buffer, out outputOffset, out outputCount); - if (NetEventSource.Log.IsEnabled() && status.ErrorCode == SecurityStatusPalErrorCode.OK) - { - NetEventSource.DumpBuffer(this, buffer.Slice(outputOffset, outputCount)); - } - - return status; - } + SecurityStatusPal status = SslStreamPal.DecryptMessage( + _securityContext!, + encrypted, + destination, + out bytesWritten, + out leftoverOffset, + out leftoverLength); - internal SecurityStatusPal DecryptDirect(ReadOnlySpan input, Span output, out int outputWritten, out int plaintextPending) - { - SecurityStatusPal status = SslStreamPal.DecryptMessageDirect(_securityContext!, input, output, out outputWritten, out plaintextPending); if (NetEventSource.Log.IsEnabled() && status.ErrorCode == SecurityStatusPalErrorCode.OK) { - NetEventSource.DumpBuffer(this, output.Slice(0, outputWritten)); + if (bytesWritten > 0) + { + NetEventSource.DumpBuffer(this, destination.Slice(0, bytesWritten)); + } + + if (leftoverLength > 0) + { + NetEventSource.DumpBuffer(this, encrypted.Slice(leftoverOffset, leftoverLength)); + } } return status; diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs index 50ccdd7bfffa67..931c9f41c6c646 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs @@ -109,39 +109,31 @@ public static ProtocolToken EncryptMessage( return token; } - // Direct-decrypt to user buffer is supported only on the OpenSSL-backed Unix PAL. - internal const bool IsDirectDecryptSupported = false; - - public static SecurityStatusPal DecryptMessageDirect( - SafeDeleteSslContext securityContext, - ReadOnlySpan input, - Span output, - out int outputWritten, - out int plaintextPending) - { - throw new System.PlatformNotSupportedException(); - } - public static SecurityStatusPal DecryptMessage( SafeDeleteSslContext securityContext, - Span buffer, - out int offset, - out int count) + Span encrypted, + Span destination, + out int bytesWritten, + out int leftoverOffset, + out int leftoverLength) { - offset = 0; - count = 0; + // The Android SSLStream PAL always decrypts in-place; `destination` is unused. + _ = destination; + bytesWritten = 0; + leftoverOffset = 0; + leftoverLength = 0; try { SafeSslHandle sslHandle = securityContext.SslContext; - securityContext.Write(buffer); + securityContext.Write(encrypted); - PAL_SSLStreamStatus ret = Interop.AndroidCrypto.SSLStreamRead(sslHandle, buffer, out int read); + PAL_SSLStreamStatus ret = Interop.AndroidCrypto.SSLStreamRead(sslHandle, encrypted, out int read); if (ret == PAL_SSLStreamStatus.Error) return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError); - count = read; + leftoverLength = read; SecurityStatusPalErrorCode statusCode = ret switch { diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index 552733963202fa..2dd6d0fd261a3b 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -203,42 +203,35 @@ public static ProtocolToken EncryptMessage( return token; } - // Direct-decrypt to user buffer is supported only on the OpenSSL-backed Unix PAL. - internal const bool IsDirectDecryptSupported = false; - - public static SecurityStatusPal DecryptMessageDirect( - SafeDeleteSslContext securityContext, - ReadOnlySpan input, - Span output, - out int outputWritten, - out int plaintextPending) - { - throw new System.PlatformNotSupportedException(); - } - public static SecurityStatusPal DecryptMessage( SafeDeleteContext securityContext, - Span buffer, - out int offset, - out int count) + Span encrypted, + Span destination, + out int bytesWritten, + out int leftoverOffset, + out int leftoverLength) { + // SecureTransport always decrypts in-place; `destination` is unused. + _ = destination; + bytesWritten = 0; + Debug.Assert(securityContext is SafeDeleteSslContext, "SafeDeleteSslContext expected"); SafeDeleteSslContext sslContext = (SafeDeleteSslContext)securityContext; - offset = 0; - count = 0; + leftoverOffset = 0; + leftoverLength = 0; try { SafeSslHandle sslHandle = sslContext.SslContext; - sslContext.Write(buffer); + sslContext.Write(encrypted); unsafe { - fixed (byte* ptr = buffer) + fixed (byte* ptr = encrypted) { - PAL_TlsIo status = Interop.AppleCrypto.SslRead(sslHandle, ptr, buffer.Length, out int written); + PAL_TlsIo status = Interop.AppleCrypto.SslRead(sslHandle, ptr, encrypted.Length, out int written); if (status < 0) { return new SecurityStatusPal( @@ -246,8 +239,8 @@ public static SecurityStatusPal DecryptMessage( Interop.AppleCrypto.CreateExceptionForOSStatus((int)status)); } - count = written; - offset = 0; + leftoverLength = written; + leftoverOffset = 0; switch (status) { diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index 71f3d2e95b6f5c..3210ac54207725 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -79,63 +79,40 @@ public static ProtocolToken EncryptMessage(SafeDeleteSslContext securityContext, return token; } - public static SecurityStatusPal DecryptMessage(SafeDeleteSslContext securityContext, Span buffer, out int offset, out int count) - { - // In-place decrypt: pass the same span as both input ciphertext and output plaintext. - // OpenSSL fully consumes the ciphertext via BIO_read before writing plaintext through - // SSL_read, so this aliasing is safe. - return DecryptMessageCore((SafeSslHandle)securityContext, buffer, buffer, out offset, out count, out _); - } - - // True on Unix: the OpenSSL-backed PAL supports decrypting directly into a caller-provided - // output buffer that is disjoint from the source ciphertext, avoiding a copy from - // SslStream's internal encrypted buffer to the user's Memory. - internal const bool IsDirectDecryptSupported = true; - // Direct-decrypt variant. `input` is the ciphertext frame and `output` is the user's - // plaintext destination. `outputWritten` is the number of bytes written to `output`. - // `plaintextPending` is non-zero when SSL retains additional plaintext that the next call - // can drain by passing `input` of length 0. - public static SecurityStatusPal DecryptMessageDirect( + // Decrypt entry point. + // + // encrypted - the ciphertext frame; may also serve as the in-place plaintext target. + // destination - optional user-provided plaintext buffer. When non-empty and large enough + // to hold a full TLS record plaintext, decrypt writes there directly and + // reports the count via `bytesWritten`. + // bytesWritten - bytes written into `destination`. + // leftoverOffset / leftoverLength - location of the in-place decrypted region inside + // `encrypted`. + public static SecurityStatusPal DecryptMessage( SafeDeleteSslContext securityContext, - ReadOnlySpan input, - Span output, - out int outputWritten, - out int plaintextPending) - { - Debug.Assert(output.Length > 0); - return DecryptMessageCore((SafeSslHandle)securityContext, input, output, out _, out outputWritten, out plaintextPending); - } - - private static SecurityStatusPal DecryptMessageCore( - SafeSslHandle sslHandle, - ReadOnlySpan input, - Span output, - out int offset, - out int count, - out int plaintextPending) + Span encrypted, + Span destination, + out int bytesWritten, + out int leftoverOffset, + out int leftoverLength) { - offset = 0; - count = 0; - plaintextPending = 0; + bytesWritten = 0; + leftoverOffset = 0; + leftoverLength = 0; try { - int resultSize = Interop.OpenSsl.Decrypt( - sslHandle, - input, - output, - out int pending, + bytesWritten = Interop.OpenSsl.Decrypt( + (SafeSslHandle)securityContext, + encrypted, + destination, + out leftoverOffset, + out leftoverLength, out Interop.Ssl.SslErrorCode errorCode); SecurityStatusPal retVal = MapNativeErrorCode(errorCode); - if (retVal.ErrorCode is SecurityStatusPalErrorCode.OK or SecurityStatusPalErrorCode.Renegotiate) - { - count = resultSize; - plaintextPending = pending; - } - return retVal; } catch (Exception ex) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index d4e4c237a541f6..6189b6fb15b9ff 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -602,20 +602,17 @@ public static unsafe ProtocolToken EncryptMessage(SafeDeleteSslContext securityC } // Direct-decrypt to user buffer is supported only on the OpenSSL-backed Unix PAL. - internal const bool IsDirectDecryptSupported = false; - - public static SecurityStatusPal DecryptMessageDirect( - SafeDeleteSslContext securityContext, - ReadOnlySpan input, - Span output, - out int outputWritten, - out int plaintextPending) - { - throw new System.PlatformNotSupportedException(); - } - - public static unsafe SecurityStatusPal DecryptMessage(SafeDeleteSslContext? securityContext, Span buffer, out int offset, out int count) + public static unsafe SecurityStatusPal DecryptMessage( + SafeDeleteSslContext? securityContext, + Span encrypted, + Span destination, + out int bytesWritten, + out int leftoverOffset, + out int leftoverLength) { + // SChannel always decrypts in-place; the caller-provided `destination` is unused. + _ = destination; + bytesWritten = 0; const int NumSecBuffers = 4; // data + empty + empty + empty Span unmanagedBuffers = stackalloc Interop.SspiCli.SecBuffer[NumSecBuffers]; @@ -627,12 +624,12 @@ public static unsafe SecurityStatusPal DecryptMessage(SafeDeleteSslContext? secu emptyBuffer.cbBuffer = 0; } - fixed (byte* bufferPtr = buffer) + fixed (byte* bufferPtr = encrypted) { ref Interop.SspiCli.SecBuffer dataBuffer = ref unmanagedBuffers[0]; dataBuffer.BufferType = SecurityBufferType.SECBUFFER_DATA; dataBuffer.pvBuffer = (IntPtr)bufferPtr; - dataBuffer.cbBuffer = buffer.Length; + dataBuffer.cbBuffer = encrypted.Length; Interop.SspiCli.SecBufferDesc sdcInOut = new Interop.SspiCli.SecBufferDesc(NumSecBuffers) { @@ -642,8 +639,8 @@ public static unsafe SecurityStatusPal DecryptMessage(SafeDeleteSslContext? secu // Decrypt may repopulate the sec buffers, likely with header + data + trailer + empty. // We need to find the data. - count = 0; - offset = 0; + leftoverLength = 0; + leftoverOffset = 0; for (int i = 0; i < NumSecBuffers; i++) { // Successfully decoded data and placed it at the following position in the buffer, @@ -651,12 +648,12 @@ public static unsafe SecurityStatusPal DecryptMessage(SafeDeleteSslContext? secu // or we failed to decode the data, here is the encoded data. || (errorCode != Interop.SECURITY_STATUS.OK && unmanagedBuffers[i].BufferType == SecurityBufferType.SECBUFFER_EXTRA)) { - offset = (int)((byte*)unmanagedBuffers[i].pvBuffer - bufferPtr); - count = unmanagedBuffers[i].cbBuffer; + leftoverOffset = (int)((byte*)unmanagedBuffers[i].pvBuffer - bufferPtr); + leftoverLength = unmanagedBuffers[i].cbBuffer; - // output is ignored on Windows. We always decrypt in place and we set outputOffset to indicate where the data start. - Debug.Assert(offset >= 0 && count >= 0, $"Expected offset and count greater than 0, got {offset} and {count}"); - Debug.Assert(checked(offset + count) <= buffer.Length, $"Expected offset+count <= buffer.Length, got {offset}+{count}>={buffer.Length}"); + // destination is ignored on Windows. We always decrypt in place and we set leftoverOffset to indicate where the data start. + Debug.Assert(leftoverOffset >= 0 && leftoverLength >= 0, $"Expected offset and count greater than 0, got {leftoverOffset} and {leftoverLength}"); + Debug.Assert(checked(leftoverOffset + leftoverLength) <= encrypted.Length, $"Expected offset+count <= buffer.Length, got {leftoverOffset}+{leftoverLength}>={encrypted.Length}"); break; } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 69f2bca1a44b0d..18cbb50734f10d 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -580,14 +580,16 @@ int32_t CryptoNative_SslEncrypt( int32_t CryptoNative_SslDecrypt( SSL* ssl, - const uint8_t* inputPtr, + uint8_t* inputPtr, int32_t inputLen, uint8_t* outputPtr, int32_t outputCap, - int32_t* plaintextPending, + int32_t* leftoverOffset, + int32_t* leftoverLength, int32_t* errorCode) { - if (plaintextPending != NULL) *plaintextPending = 0; + if (leftoverOffset != NULL) *leftoverOffset = 0; + if (leftoverLength != NULL) *leftoverLength = 0; ERR_clear_error(); @@ -597,14 +599,21 @@ int32_t CryptoNative_SslDecrypt( CryptoNative_BioSetReadWindow(inputBio, inputPtr, inputLen); } - int32_t result = SSL_read(ssl, outputPtr, outputCap); - *errorCode = (result > 0) ? SSL_ERROR_NONE : CryptoNative_SslGetError(ssl, result); + int32_t result = 0; + + if (outputCap > 0) + { + result = SSL_read(ssl, outputPtr, outputCap); + } - if (plaintextPending != NULL) + // if provided destination buffer was too small, leave the remaining plaintext in-place in the input buffer. + if (outputCap == 0 || SSL_pending(ssl) > 0) { - *plaintextPending = SSL_pending(ssl); + *leftoverLength = SSL_read(ssl, inputPtr, inputLen); } + *errorCode = (result + *leftoverLength > 0) ? SSL_ERROR_NONE : CryptoNative_SslGetError(ssl, result); + if (inputBio != NULL) { CryptoNative_BioClearReadWindow(inputBio); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index 8b767e168503bf..b8a36bc95b4a5b 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -377,11 +377,12 @@ receives SSL_get_error. */ PALEXPORT int32_t CryptoNative_SslDecrypt( SSL* ssl, - const uint8_t* inputPtr, + uint8_t* inputPtr, int32_t inputLen, uint8_t* outputPtr, int32_t outputCap, - int32_t* plaintextPending, + int32_t* leftoverOffset, + int32_t* leftoverLength, int32_t* errorCode); /* From 34b40930908429ceb0dc072de1e78cb9f8119b28 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 18 May 2026 12:55:52 +0200 Subject: [PATCH 07/20] Remove the need for read carry-over. --- .../Interop.OpenSsl.cs | 24 +++--- .../Interop.Ssl.cs | 2 + .../System/Net/Security/SslStreamPal.Unix.cs | 9 +- .../pal_bio.c | 83 +------------------ .../pal_bio.h | 2 +- .../pal_ssl.c | 27 ++++-- .../pal_ssl.h | 2 + 7 files changed, 49 insertions(+), 100 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 0fe83b08300941..2ce9aeff73ea59 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -666,19 +666,19 @@ internal static SecurityStatusPal SslRenegotiate(SafeSslHandle sslContext, out b return new SecurityStatusPal(SecurityStatusPalErrorCode.OK); } - internal static unsafe SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, ReadOnlySpan input, ref ProtocolToken token) + internal static unsafe SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, ReadOnlySpan input, out int consumed, ref ProtocolToken token) { token.Size = 0; + consumed = 0; Exception? handshakeException = null; - // Reserve a reasonable initial window in the outgoing token; the spill buffer - // catches anything that doesn't fit. - const int InitialHandshakeWindow = 4096; - token.EnsureAvailableSpace(InitialHandshakeWindow); - // Drain any bytes accumulated in the OutputBio's spill from a prior call // (e.g. SSL_read emitting alerts before this handshake step). DrainOutputBioSpill(context, ref token); + + // Reserve a reasonable initial window in the outgoing token; the spill buffer + // catches anything that doesn't fit. + const int InitialHandshakeWindow = 4096; token.EnsureAvailableSpace(InitialHandshakeWindow); int retVal; @@ -686,16 +686,17 @@ internal static unsafe SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle c int spillLen; Ssl.SslErrorCode errorCode; - Span windowSpan = token.AvailableSpan; + Span outputSpan = token.AvailableSpan; fixed (byte* inputPtr = input) - fixed (byte* windowPtr = windowSpan) + fixed (byte* outputPtr = outputSpan) { retVal = Ssl.SslHandshake( context, inputPtr, input.Length, - windowPtr, - windowSpan.Length, + out consumed, + outputPtr, + outputSpan.Length, out writtenToWindow, out spillLen, out errorCode); @@ -870,6 +871,7 @@ internal static unsafe int Decrypt( out Ssl.SslErrorCode errorCode) { int retVal; + int consumed; fixed (byte* inputPtr = input) fixed (byte* outputPtr = output) { @@ -877,6 +879,7 @@ internal static unsafe int Decrypt( context, inputPtr, input.Length, + out consumed, outputPtr, output.Length, out leftoverOffset, @@ -885,6 +888,7 @@ internal static unsafe int Decrypt( } if (retVal + leftoverLength > 0) { + Debug.Assert(consumed == input.Length, "Expected all input to be consumed."); return retVal; } diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 21c70131fc5b79..b665594275eea6 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -125,6 +125,7 @@ internal static unsafe partial int SslHandshake( SafeSslHandle ssl, byte* inputPtr, int inputLen, + out int consumed, byte* outputPtr, int outputCap, out int outputWritten, @@ -147,6 +148,7 @@ internal static unsafe partial int SslDecrypt( SafeSslHandle ssl, byte* inputPtr, int inputLen, + out int consumed, byte* outputPtr, int outputCap, out int leftoverOffset, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index 3210ac54207725..8c87256f8d71c5 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -203,8 +203,7 @@ private static ProtocolToken HandshakeInternal(ref SafeDeleteSslContext? context context = Interop.OpenSsl.AllocateSslHandle(sslAuthenticationOptions); } - SecurityStatusPalErrorCode errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, inputBuffer, ref token); - consumed = inputBuffer.Length; + SecurityStatusPalErrorCode errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, inputBuffer, out consumed, ref token); if (errorCode == SecurityStatusPalErrorCode.CredentialsNeeded) { @@ -223,14 +222,16 @@ private static ProtocolToken HandshakeInternal(ref SafeDeleteSslContext? context // set the cert and continue TryUpdateClintCertificate(null, context, sslAuthenticationOptions); - errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, ReadOnlySpan.Empty, ref token); + errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, inputBuffer.Slice(consumed), out int c, ref token); + consumed += c; } // sometimes during renegotiation processing message does not yield new output. // That seems to be flaw in OpenSSL state machine and we have workaround to peek it and try it again. if (token.Size == 0 && Interop.Ssl.IsSslRenegotiatePending((SafeSslHandle)context)) { - errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, ReadOnlySpan.Empty, ref token); + errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, inputBuffer.Slice(consumed), out int c, ref token); + consumed += c; } token.Status = new SecurityStatusPal(errorCode); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c index 32cc5eee5066cb..fa64d552ee2c6f 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c @@ -65,13 +65,6 @@ int32_t CryptoNative_BioCtrlPending(BIO* bio) typedef struct { - /* Carry-over buffer: holds bytes from a prior read window that SSL did - not consume before the window was cleared. */ - uint8_t* readCarry; - int32_t readCarryCapacity; - int32_t readCarryLen; - int32_t readCarryPos; - const uint8_t* readPtr; int32_t readLen; int32_t readPos; @@ -107,20 +100,6 @@ static int ManagedSpanBioRead(BIO* bio, char* buf, int len) return -1; } - int32_t carryAvail = ctx->readCarryLen - ctx->readCarryPos; - if (carryAvail > 0) - { - int32_t toCopy = len < carryAvail ? len : carryAvail; - memcpy(buf, ctx->readCarry + ctx->readCarryPos, (size_t)toCopy); - ctx->readCarryPos += toCopy; - if (ctx->readCarryPos == ctx->readCarryLen) - { - ctx->readCarryPos = 0; - ctx->readCarryLen = 0; - } - return toCopy; - } - if (ctx->readError) { /* A prior BioClearReadWindow could not allocate carry space and dropped @@ -142,35 +121,6 @@ static int ManagedSpanBioRead(BIO* bio, char* buf, int len) return toCopy; } -static int ManagedSpanBioGrowCarry(ManagedSpanBioCtx* ctx, int32_t needed) -{ - if (ctx->readCarryCapacity >= needed) - { - return 1; - } - - int32_t newCap = ctx->readCarryCapacity > 0 ? ctx->readCarryCapacity : 4096; - while (newCap < needed) - { - if (newCap > INT32_MAX / 2) - { - newCap = needed; - break; - } - newCap *= 2; - } - - uint8_t* newBuf = (uint8_t*)realloc(ctx->readCarry, (size_t)newCap); - if (newBuf == NULL) - { - return 0; - } - - ctx->readCarry = newBuf; - ctx->readCarryCapacity = newCap; - return 1; -} - static int ManagedSpanBioGrowSpill(ManagedSpanBioCtx* ctx, int32_t needed) { if (ctx->spillCapacity >= needed) @@ -267,7 +217,7 @@ static long ManagedSpanBioCtrl(BIO* bio, int cmd, long num, void* ptr) { return 0; } - return (long)((ctx->readCarryLen - ctx->readCarryPos) + (ctx->readLen - ctx->readPos)); + return (long)(ctx->readLen - ctx->readPos); case BIO_CTRL_WPENDING: if (ctx == NULL) @@ -279,8 +229,6 @@ static long ManagedSpanBioCtrl(BIO* bio, int cmd, long num, void* ptr) case BIO_CTRL_RESET: if (ctx != NULL) { - ctx->readCarryLen = 0; - ctx->readCarryPos = 0; ctx->readPtr = NULL; ctx->readLen = 0; ctx->readPos = 0; @@ -324,7 +272,6 @@ static int ManagedSpanBioDestroy(BIO* bio) ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); if (ctx != NULL) { - free(ctx->readCarry); free(ctx->spillBuf); free(ctx); BIO_set_data(bio, NULL); @@ -397,7 +344,7 @@ void CryptoNative_BioSetReadWindow(BIO* bio, const void* ptr, int32_t len) ctx->readPos = 0; } -void CryptoNative_BioClearReadWindow(BIO* bio) +void CryptoNative_BioClearReadWindow(BIO* bio, int32_t* leftoverLength) { if (bio == NULL) { @@ -410,31 +357,9 @@ void CryptoNative_BioClearReadWindow(BIO* bio) return; } - int32_t unread = ctx->readLen - ctx->readPos; - if (unread > 0 && ctx->readPtr != NULL) + if (leftoverLength != NULL) { - /* Move existing carry tail down to position 0 first. */ - int32_t carryTail = ctx->readCarryLen - ctx->readCarryPos; - if (carryTail > 0 && ctx->readCarryPos > 0) - { - memmove(ctx->readCarry, ctx->readCarry + ctx->readCarryPos, (size_t)carryTail); - } - ctx->readCarryLen = carryTail; - ctx->readCarryPos = 0; - - int32_t needed = ctx->readCarryLen + unread; - if (ManagedSpanBioGrowCarry(ctx, needed)) - { - memcpy(ctx->readCarry + ctx->readCarryLen, ctx->readPtr + ctx->readPos, (size_t)unread); - ctx->readCarryLen += unread; - } - else - { - /* Carry allocation failed; bytes are lost. Mark the BIO as - permanently broken so the next BIO_read surfaces the failure - rather than masking it as a protocol error. */ - ctx->readError = 1; - } + *leftoverLength = ctx->readLen - ctx->readPos; } ctx->readPtr = NULL; diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.h b/src/native/libs/System.Security.Cryptography.Native/pal_bio.h index 8bd94200f91f0f..66ced07a4083f3 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.h @@ -70,7 +70,7 @@ PALEXPORT void CryptoNative_BioSetReadWindow(BIO* bio, const void* ptr, int32_t /* Clears the read window on a managed-span BIO. */ -PALEXPORT void CryptoNative_BioClearReadWindow(BIO* bio); +PALEXPORT void CryptoNative_BioClearReadWindow(BIO* bio, int32_t* leftoverLength); /* Sets the write window on a managed-span BIO. BIO_write fills this window diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 18cbb50734f10d..43b726bc609acd 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -9,6 +9,8 @@ #include "pal_utilities.h" #include "pal_x509.h" +#include + #include #include #include @@ -456,10 +458,8 @@ int32_t CryptoNative_SslRenegotiate(SSL* ssl, int32_t* error) if(ret != 1) { *error = CryptoNative_SslGetError(ssl, ret); - return ret; } - - return CryptoNative_SslDoHandshake(ssl, error); + return ret; } *error = SSL_ERROR_NONE; @@ -470,7 +470,6 @@ int32_t CryptoNative_IsSslRenegotiatePending(SSL* ssl) { ERR_clear_error(); - SSL_peek(ssl, NULL, 0); return SSL_renegotiate_pending(ssl) != 0; } @@ -506,6 +505,7 @@ int32_t CryptoNative_SslHandshake( SSL* ssl, const uint8_t* inputPtr, int32_t inputLen, + int32_t* consumed, uint8_t* outputPtr, int32_t outputCap, int32_t* outputWritten, @@ -529,6 +529,10 @@ int32_t CryptoNative_SslHandshake( CryptoNative_BioSetWriteWindow(outputBio, outputPtr, outputCap); } + // this peek ensures that the SSL handshake state machine starts processing + // renegotiation and post-handshake client cert requests + SSL_peek(ssl, NULL, 0); + int32_t result = SSL_do_handshake(ssl); *errorCode = (result == 1) ? SSL_ERROR_NONE : CryptoNative_SslGetError(ssl, result); @@ -539,7 +543,12 @@ int32_t CryptoNative_SslHandshake( } if (inputBio != NULL) { - CryptoNative_BioClearReadWindow(inputBio); + int32_t leftover = 0; + CryptoNative_BioClearReadWindow(inputBio, &leftover); + if (consumed != NULL) + { + *consumed = inputLen - leftover; + } } return result; @@ -582,6 +591,7 @@ int32_t CryptoNative_SslDecrypt( SSL* ssl, uint8_t* inputPtr, int32_t inputLen, + int32_t* consumed, uint8_t* outputPtr, int32_t outputCap, int32_t* leftoverOffset, @@ -616,7 +626,12 @@ int32_t CryptoNative_SslDecrypt( if (inputBio != NULL) { - CryptoNative_BioClearReadWindow(inputBio); + int32_t leftover = 0; + CryptoNative_BioClearReadWindow(inputBio, &leftover); + if (consumed != NULL) + { + *consumed = inputLen - leftover; + } } return result; diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index b8a36bc95b4a5b..2de9d7b17cc052 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -340,6 +340,7 @@ PALEXPORT int32_t CryptoNative_SslHandshake( SSL* ssl, const uint8_t* inputPtr, int32_t inputLen, + int32_t* consumed, uint8_t* outputPtr, int32_t outputCap, int32_t* outputWritten, @@ -379,6 +380,7 @@ PALEXPORT int32_t CryptoNative_SslDecrypt( SSL* ssl, uint8_t* inputPtr, int32_t inputLen, + int32_t* consumed, uint8_t* outputPtr, int32_t outputCap, int32_t* leftoverOffset, From 698e3bb1d3206f67fc87be6ac25f7a8c73496bea Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 18 May 2026 18:16:23 +0200 Subject: [PATCH 08/20] Remove unneeded code --- .../Interop.Ssl.cs | 6 ------ .../entrypoints.c | 3 --- .../pal_bio.c | 11 ---------- .../pal_bio.h | 20 +++++-------------- .../pal_ssl.c | 2 -- 5 files changed, 5 insertions(+), 37 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index b665594275eea6..187feb53df5136 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -169,14 +169,8 @@ internal static unsafe partial int SslDecrypt( [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioNewManagedSpan")] internal static partial SafeBioHandle BioNewManagedSpan(); - [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioSetReadWindow")] - internal static unsafe partial void BioSetReadWindow(SafeBioHandle bio, byte* ptr, int len); - [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioClearReadWindow")] - internal static partial void BioClearReadWindow(SafeBioHandle bio); - [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioSetWriteWindow")] - internal static unsafe partial void BioSetWriteWindow(SafeBioHandle bio, byte* ptr, int capacity); [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioGetWriteResult")] internal static partial void BioGetWriteResult(SafeBioHandle bio, out int writtenToWindow, out int spillLen); diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index d29d597f88c6f9..18d66ae776a720 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -44,7 +44,6 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_BigNumDestroy) DllImportEntry(CryptoNative_BigNumFromBinary) DllImportEntry(CryptoNative_BigNumToBinary) - DllImportEntry(CryptoNative_BioClearReadWindow) DllImportEntry(CryptoNative_BioCtrlPending) DllImportEntry(CryptoNative_BioDestroy) DllImportEntry(CryptoNative_BioDrainSpill) @@ -54,8 +53,6 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_BioNewManagedSpan) DllImportEntry(CryptoNative_BioRead) DllImportEntry(CryptoNative_BioSeek) - DllImportEntry(CryptoNative_BioSetReadWindow) - DllImportEntry(CryptoNative_BioSetWriteWindow) DllImportEntry(CryptoNative_BioTell) DllImportEntry(CryptoNative_BioWrite) DllImportEntry(CryptoNative_CheckX509Hostname) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c index fa64d552ee2c6f..79685a6eb6da93 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c @@ -76,8 +76,6 @@ typedef struct uint8_t* spillBuf; int32_t spillCapacity; int32_t spillLen; - - int32_t readError; /* set to 1 if a carry allocation failed and bytes were lost */ } ManagedSpanBioCtx; #define MANAGED_SPAN_SPILL_INITIAL 4096 @@ -100,14 +98,6 @@ static int ManagedSpanBioRead(BIO* bio, char* buf, int len) return -1; } - if (ctx->readError) - { - /* A prior BioClearReadWindow could not allocate carry space and dropped - unread bytes. Surface this as a hard read failure so it does not look - like a transient EAGAIN to the SSL engine. */ - return -1; - } - int32_t available = ctx->readLen - ctx->readPos; if (available <= 0 || ctx->readPtr == NULL) { @@ -236,7 +226,6 @@ static long ManagedSpanBioCtrl(BIO* bio, int cmd, long num, void* ptr) ctx->writeCapacity = 0; ctx->writePos = 0; ctx->spillLen = 0; - ctx->readError = 0; BIO_clear_retry_flags(bio); } return 1; diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.h b/src/native/libs/System.Security.Cryptography.Native/pal_bio.h index 66ced07a4083f3..af6d9378a4d4b9 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.h @@ -62,22 +62,12 @@ caller-supplied buffer windows (with a heap spill on write overflow). PALEXPORT BIO* CryptoNative_BioNewManagedSpan(void); /* -Sets the read window on a managed-span BIO. Subsequent BIO_read calls -consume from ptr[0..len). Passing ptr=NULL/len=0 clears the window. +Internal helpers used by pal_ssl.c to drive the managed-span BIO during +atomic SSL handshake/encrypt/decrypt operations. Not exported. */ -PALEXPORT void CryptoNative_BioSetReadWindow(BIO* bio, const void* ptr, int32_t len); - -/* -Clears the read window on a managed-span BIO. -*/ -PALEXPORT void CryptoNative_BioClearReadWindow(BIO* bio, int32_t* leftoverLength); - -/* -Sets the write window on a managed-span BIO. BIO_write fills this window -first, then spills the remainder to a heap buffer. Passing ptr=NULL/cap=0 -clears the window so all writes go to the spill buffer. -*/ -PALEXPORT void CryptoNative_BioSetWriteWindow(BIO* bio, void* ptr, int32_t capacity); +void CryptoNative_BioSetReadWindow(BIO* bio, const void* ptr, int32_t len); +void CryptoNative_BioClearReadWindow(BIO* bio, int32_t* leftoverLength); +void CryptoNative_BioSetWriteWindow(BIO* bio, void* ptr, int32_t capacity); /* Returns the number of bytes written into the window and into the spill diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 43b726bc609acd..7da99b8c6994f8 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -9,8 +9,6 @@ #include "pal_utilities.h" #include "pal_x509.h" -#include - #include #include #include From 2d30f4e26537d3e4674802e920d0a262275a2755 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 18 May 2026 18:50:33 +0200 Subject: [PATCH 09/20] Improve documentation --- .../src/System/Net/Security/SslStream.IO.cs | 67 ----------- .../System/Net/Security/SslStream.Protocol.cs | 111 +++++++++++++----- .../System/Net/Security/SslStreamPal.Unix.cs | 10 -- .../Net/Security/SslStreamPal.Windows.cs | 1 - .../pal_bio.c | 91 ++++++++++++-- 5 files changed, 158 insertions(+), 122 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index b22e54e6d7522d..b94e58f6972a3c 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -85,24 +85,6 @@ private void CloseInternal() } } - private ProtocolToken EncryptData(ReadOnlyMemory buffer) - { - ThrowIfExceptionalOrNotAuthenticated(); - - lock (_handshakeLock) - { - if (_handshakeWaiter != null) - { - ProtocolToken token = default; - // avoid waiting under lock. - token.Status = new SecurityStatusPal(SecurityStatusPalErrorCode.TryAgain); - return token; - } - - return Encrypt(buffer); - } - } - // // This method assumes that a SSPI context is already in a good shape. // For example it is either a fresh context or already authenticated context that needs renegotiation. @@ -845,55 +827,6 @@ private async ValueTask EnsureFullTlsFrameAsync(CancellationTok return frameSize; } - private SecurityStatusPal DecryptData(int frameSize, Span destination, out int bytesWritten) - { - SecurityStatusPal status; - - lock (_handshakeLock) - { - ThrowIfExceptionalOrNotAuthenticated(); - - // The PAL may either: - // * write plaintext directly into `destination` (bytesWritten > 0, no in-place leftover), or - // * decrypt in-place into `_buffer` (bytesWritten == 0, leftoverOffset/Length describe the region). - status = Decrypt( - _buffer.EncryptedSpanSliced(frameSize), - destination, - out bytesWritten, - out int leftoverOffset, - out int leftoverLength); - - _buffer.OnDecrypted(leftoverOffset, leftoverLength, frameSize); - - if (status.ErrorCode == SecurityStatusPalErrorCode.Renegotiate) - { - // The status indicates that peer wants to renegotiate. (Windows only) - // In practice, there can be some other reasons too - like TLS1.3 session creation - // of alert handling. We need to pass the data to lsass and it is not safe to do parallel - // write any more as that can change TLS state and the EncryptData() can fail in strange ways. - - // To handle this we call DecryptData() under lock and we create TCS waiter. - // EncryptData() checks that under same lock and if it exist it will not call low-level crypto. - // Instead it will wait synchronously or asynchronously and it will try again after the wait. - // The result will be set when ReplyOnReAuthenticationAsync() is finished e.g. lsass business is over. - // If that happen before EncryptData() runs, _handshakeWaiter will be set to null - // and EncryptData() will work normally e.g. no waiting, just exclusion with DecryptData() - - if (_sslAuthenticationOptions.AllowRenegotiation || SslProtocol == SslProtocols.Tls13 || _nestedAuth != NestedState.StreamNotInUse) - { - // create TCS only if we plan to proceed. If not, we will throw later outside of the lock. - // Tls1.3 does not have renegotiation. However on Windows this error code is used - // for session management e.g. anything lsass needs to see. - // We also allow it when explicitly requested using RenegotiateAsync(). - _handshakeWaiter = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - } - } - } - - return status; - } - - [AsyncMethodBuilder(typeof(PoolingAsyncValueTaskMethodBuilder<>))] private async ValueTask ReadAsyncInternal(Memory buffer, CancellationToken cancellationToken) where TIOAdapter : IReadWriteAdapter diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 2dafbf2e2baa02..8dd348c7b7d1e0 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -970,49 +970,98 @@ internal void ProcessHandshakeSuccess() #endif } - internal ProtocolToken Encrypt(ReadOnlyMemory buffer) + private ProtocolToken EncryptData(ReadOnlyMemory buffer) { - if (NetEventSource.Log.IsEnabled()) NetEventSource.DumpBuffer(this, buffer.Span); + ThrowIfExceptionalOrNotAuthenticated(); - ProtocolToken token = SslStreamPal.EncryptMessage( - _securityContext!, - buffer, - _headerSize, - _trailerSize); - - if (token.Status.ErrorCode != SecurityStatusPalErrorCode.OK) + lock (_handshakeLock) { - if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(this, $"ERROR {token.Status}"); - } + if (_handshakeWaiter != null) + { + ProtocolToken token = default; + // avoid waiting under lock. + token.Status = new SecurityStatusPal(SecurityStatusPalErrorCode.TryAgain); + return token; + } - return token; + if (NetEventSource.Log.IsEnabled()) NetEventSource.DumpBuffer(this, buffer.Span); + + ProtocolToken token = SslStreamPal.EncryptMessage( + _securityContext!, + buffer, + _headerSize, + _trailerSize); + + if (token.Status.ErrorCode != SecurityStatusPalErrorCode.OK) + { + if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(this, $"ERROR {token.Status}"); + } + + return token; + } } - internal SecurityStatusPal Decrypt( - Span encrypted, - Span destination, - out int bytesWritten, - out int leftoverOffset, - out int leftoverLength) + // On some paltforms, the platform APIs decrypt in-place via single + // call (Schannel), while others have separate write-ciphertext + + // read-plaintext primitives. To allow the most efficient thing (copying + // plaintext straight to the `destination` buffer provided by the + // SslStream caller) on platforms that support it, the contract of this + // method is as follows: + // - After the call, first `bytesWritten` bytes of `destination` contain decrypted plaintext + // - Rest of the decrypted plaintext, if any, is stored in `_buffer.DecryptedSpan`. + private SecurityStatusPal DecryptData(int frameSize, Span destination, out int bytesWritten) { - SecurityStatusPal status = SslStreamPal.DecryptMessage( - _securityContext!, - encrypted, - destination, - out bytesWritten, - out leftoverOffset, - out leftoverLength); - - if (NetEventSource.Log.IsEnabled() && status.ErrorCode == SecurityStatusPalErrorCode.OK) + SecurityStatusPal status; + + lock (_handshakeLock) { - if (bytesWritten > 0) + ThrowIfExceptionalOrNotAuthenticated(); + + status = SslStreamPal.DecryptMessage( + _securityContext!, + _buffer.EncryptedSpanSliced(frameSize), + destination, + out bytesWritten, + out int leftoverOffset, + out int leftoverLength); + + _buffer.OnDecrypted(leftoverOffset, leftoverLength, frameSize); + + if (NetEventSource.Log.IsEnabled() && status.ErrorCode == SecurityStatusPalErrorCode.OK) { - NetEventSource.DumpBuffer(this, destination.Slice(0, bytesWritten)); + if (bytesWritten > 0) + { + NetEventSource.DumpBuffer(this, destination.Slice(0, bytesWritten)); + } + + if (_buffer.DecryptedSpan.Length > 0) + { + NetEventSource.DumpBuffer(this, _buffer.DecryptedSpan); + } } - if (leftoverLength > 0) + if (status.ErrorCode == SecurityStatusPalErrorCode.Renegotiate) { - NetEventSource.DumpBuffer(this, encrypted.Slice(leftoverOffset, leftoverLength)); + // The status indicates that peer wants to renegotiate. (Windows only) + // In practice, there can be some other reasons too - like TLS1.3 session creation + // of alert handling. We need to pass the data to lsass and it is not safe to do parallel + // write any more as that can change TLS state and the EncryptData() can fail in strange ways. + + // To handle this we call DecryptData() under lock and we create TCS waiter. + // EncryptData() checks that under same lock and if it exist it will not call low-level crypto. + // Instead it will wait synchronously or asynchronously and it will try again after the wait. + // The result will be set when ReplyOnReAuthenticationAsync() is finished e.g. lsass business is over. + // If that happen before EncryptData() runs, _handshakeWaiter will be set to null + // and EncryptData() will work normally e.g. no waiting, just exclusion with DecryptData() + + if (_sslAuthenticationOptions.AllowRenegotiation || SslProtocol == SslProtocols.Tls13 || _nestedAuth != NestedState.StreamNotInUse) + { + // create TCS only if we plan to proceed. If not, we will throw later outside of the lock. + // Tls1.3 does not have renegotiation. However on Windows this error code is used + // for session management e.g. anything lsass needs to see. + // We also allow it when explicitly requested using RenegotiateAsync(). + _handshakeWaiter = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + } } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index 8c87256f8d71c5..cf4c5492dec78b 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -79,16 +79,6 @@ public static ProtocolToken EncryptMessage(SafeDeleteSslContext securityContext, return token; } - - // Decrypt entry point. - // - // encrypted - the ciphertext frame; may also serve as the in-place plaintext target. - // destination - optional user-provided plaintext buffer. When non-empty and large enough - // to hold a full TLS record plaintext, decrypt writes there directly and - // reports the count via `bytesWritten`. - // bytesWritten - bytes written into `destination`. - // leftoverOffset / leftoverLength - location of the in-place decrypted region inside - // `encrypted`. public static SecurityStatusPal DecryptMessage( SafeDeleteSslContext securityContext, Span encrypted, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index 6189b6fb15b9ff..277337846fc11c 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -601,7 +601,6 @@ public static unsafe ProtocolToken EncryptMessage(SafeDeleteSslContext securityC return token; } - // Direct-decrypt to user buffer is supported only on the OpenSSL-backed Unix PAL. public static unsafe SecurityStatusPal DecryptMessage( SafeDeleteSslContext? securityContext, Span encrypted, diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c index 79685a6eb6da93..f74e18626f37f9 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c @@ -4,6 +4,9 @@ #include "pal_bio.h" #include +#include +#include +#include BIO* CryptoNative_CreateMemoryBio(void) { @@ -59,9 +62,66 @@ int32_t CryptoNative_BioCtrlPending(BIO* bio) return (int32_t)result; } -#include -#include -#include +/* + * Managed-span BIO + * ---------------- + * + * OpenSSL drives TLS by reading ciphertext from an "input BIO" and writing + * ciphertext to an "output BIO" that the application owns. The stock + * BIO_s_mem() implementation works, but it always copies the caller's data + * into an internal heap buffer and then OpenSSL copies again on the way out, + * resulting in two memcpys per TLS record in each direction. + * + * The "managed-span" BIO_METHOD defined below avoids one of those copies in + * each direction by letting the SSL operation read from / write to a + * caller-supplied buffer "window" directly. It is paired with the atomic + * SSL_{Handshake,Encrypt,Decrypt} entry points in pal_ssl.c which install + * the windows around an SSL operation and tear them down again afterwards. + * + * Lifetimes + * ~~~~~~~~~ + * The windows are only valid for the duration of a single SSL_* call and + * are installed via the (non-exported) helpers + * CryptoNative_BioSetReadWindow / CryptoNative_BioClearReadWindow + * CryptoNative_BioSetWriteWindow / CryptoNative_BioGetWriteResult + * (see pal_ssl.c). The caller pins/fixes the underlying managed buffers for + * exactly that scope, then unpins them on return. The BIO context itself + * outlives the SSL handle and persists across calls; only the {read,write}Ptr + * fields refer to caller memory while a call is in progress. + * + * Read side + * ~~~~~~~~~ + * BioSetReadWindow records (ptr, len) of the caller's ciphertext span. + * BIO_read inside SSL copies from there directly (one memcpy, into OpenSSL's + * record decode buffer). After the SSL call returns, BioClearReadWindow + * reports how many bytes are still unread (= len - readPos) so the caller + * knows what to keep in its own buffer for the next round; the window pointer + * is then cleared. If SSL did not advance the BIO at all (e.g. a renegotiate + * state-machine quirk that returns SSL_ERROR_NONE without consuming bytes) + * the unread tail simply stays in the caller's buffer and is re-supplied on + * the next call - the BIO holds no carry buffer of its own. + * + * Write side + * ~~~~~~~~~~ + * BioSetWriteWindow records (ptr, capacity) of the caller's outgoing-token + * buffer. BIO_write fills the window first (one memcpy, from OpenSSL's record + * encode buffer into the caller's span). If OpenSSL produces more output than + * fits in the window (because our upper-bound estimate was too small, or + * because alerts / KeyUpdate frames are emitted out-of-band during an + * SSL_read) the overflow goes into the per-BIO heap "spill" buffer. After the + * SSL call returns, BioGetWriteResult reports both counts; if any spill is + * present the caller drains it with BioDrainSpill (one extra memcpy, but only + * on the rare overflow path). + * + * Spill buffer reuse + * ~~~~~~~~~~~~~~~~~~ + * The spill buffer is owned by the BIO context (allocated lazily, grown by + * doubling, freed in BioDestroy). It is also the catch-all for output that + * OpenSSL writes outside an explicit window - notably TLS 1.3 KeyUpdate / + * post-handshake auth messages emitted while SSL_read is in progress. The + * managed wrapper drains the spill at the start of the next outgoing SSL + * operation so those bytes are not lost. + */ typedef struct { @@ -78,6 +138,11 @@ typedef struct int32_t spillLen; } ManagedSpanBioCtx; +static ManagedSpanBioCtx* GetManagedSpanBioCtx(BIO* bio) +{ + return (ManagedSpanBioCtx*)BIO_get_data(bio); +} + #define MANAGED_SPAN_SPILL_INITIAL 4096 static BIO_METHOD* g_managedSpanBioMethod = NULL; @@ -92,7 +157,7 @@ static int ManagedSpanBioRead(BIO* bio, char* buf, int len) return 0; } - ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); if (ctx == NULL) { return -1; @@ -154,7 +219,7 @@ static int ManagedSpanBioWrite(BIO* bio, const char* buf, int len) return 0; } - ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); if (ctx == NULL) { return -1; @@ -195,7 +260,7 @@ static long ManagedSpanBioCtrl(BIO* bio, int cmd, long num, void* ptr) (void)num; (void)ptr; - ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); switch (cmd) { @@ -240,7 +305,7 @@ static long ManagedSpanBioCtrl(BIO* bio, int cmd, long num, void* ptr) static int ManagedSpanBioCreate(BIO* bio) { - ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)calloc(1, sizeof(ManagedSpanBioCtx)); + ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); if (ctx == NULL) { return 0; @@ -258,7 +323,7 @@ static int ManagedSpanBioDestroy(BIO* bio) return 0; } - ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); if (ctx != NULL) { free(ctx->spillBuf); @@ -322,7 +387,7 @@ void CryptoNative_BioSetReadWindow(BIO* bio, const void* ptr, int32_t len) return; } - ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); if (ctx == NULL) { return; @@ -340,7 +405,7 @@ void CryptoNative_BioClearReadWindow(BIO* bio, int32_t* leftoverLength) return; } - ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); if (ctx == NULL) { return; @@ -363,7 +428,7 @@ void CryptoNative_BioSetWriteWindow(BIO* bio, void* ptr, int32_t capacity) return; } - ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); if (ctx == NULL) { return; @@ -390,7 +455,7 @@ void CryptoNative_BioGetWriteResult(BIO* bio, int32_t* writtenToWindow, int32_t* return; } - ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); if (ctx == NULL) { return; @@ -413,7 +478,7 @@ int32_t CryptoNative_BioDrainSpill(BIO* bio, void* dst, int32_t dstLen) return 0; } - ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)BIO_get_data(bio); + ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); if (ctx == NULL || ctx->spillLen == 0) { return 0; From b660616ebd7ea2155c209e9f9b1d51974641dd45 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Mon, 18 May 2026 22:20:31 +0200 Subject: [PATCH 10/20] Fix build: CS0136 (token name collision) and missing System.Threading.Tasks using in SslStream.Protocol.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/System/Net/Security/SslStream.Protocol.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 8dd348c7b7d1e0..5cf8c580433f0d 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -11,6 +11,7 @@ using System.Security.Authentication.ExtendedProtection; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; +using System.Threading.Tasks; namespace System.Net.Security { @@ -978,10 +979,10 @@ private ProtocolToken EncryptData(ReadOnlyMemory buffer) { if (_handshakeWaiter != null) { - ProtocolToken token = default; + ProtocolToken waitToken = default; // avoid waiting under lock. - token.Status = new SecurityStatusPal(SecurityStatusPalErrorCode.TryAgain); - return token; + waitToken.Status = new SecurityStatusPal(SecurityStatusPalErrorCode.TryAgain); + return waitToken; } if (NetEventSource.Log.IsEnabled()) NetEventSource.DumpBuffer(this, buffer.Span); From a84fdc75337a320ed141e7d883cee39935d41cdd Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 18 May 2026 22:59:38 +0200 Subject: [PATCH 11/20] Fix BIO allocation --- src/native/libs/System.Security.Cryptography.Native/pal_bio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c index f74e18626f37f9..c3de31987cb27c 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c @@ -305,7 +305,7 @@ static long ManagedSpanBioCtrl(BIO* bio, int cmd, long num, void* ptr) static int ManagedSpanBioCreate(BIO* bio) { - ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); + ManagedSpanBioCtx* ctx = (ManagedSpanBioCtx*)calloc(1, sizeof(ManagedSpanBioCtx)); if (ctx == NULL) { return 0; From c9b9a00a2e8e01a5ab86e3888dff2ba39cbd850c Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 18 May 2026 23:07:52 +0200 Subject: [PATCH 12/20] Address some code review feedback --- .../Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs | 2 +- .../src/System/Net/Security/SslStream.Protocol.cs | 2 +- src/native/libs/System.Security.Cryptography.Native/pal_ssl.c | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 2ce9aeff73ea59..7e6cad2945ce83 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -864,7 +864,7 @@ private static unsafe void DrainOutputBioSpill(SafeSslHandle context, ref Protoc internal static unsafe int Decrypt( SafeSslHandle context, - ReadOnlySpan input, + Span input, Span output, out int leftoverOffset, out int leftoverLength, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 5cf8c580433f0d..662708b3189944 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -1002,7 +1002,7 @@ private ProtocolToken EncryptData(ReadOnlyMemory buffer) } } - // On some paltforms, the platform APIs decrypt in-place via single + // On some platforms, the platform APIs decrypt in-place via single // call (Schannel), while others have separate write-ciphertext + // read-plaintext primitives. To allow the most efficient thing (copying // plaintext straight to the `destination` buffer provided by the diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 7da99b8c6994f8..8f221c1f040042 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -512,6 +512,7 @@ int32_t CryptoNative_SslHandshake( { if (outputWritten != NULL) *outputWritten = 0; if (outputPending != NULL) *outputPending = 0; + if (consumed != NULL) *consumed = 0; ERR_clear_error(); @@ -598,6 +599,7 @@ int32_t CryptoNative_SslDecrypt( { if (leftoverOffset != NULL) *leftoverOffset = 0; if (leftoverLength != NULL) *leftoverLength = 0; + if (consumed != NULL) *consumed = 0; ERR_clear_error(); From 63db871a47fe5159937a8763c18bbf348574fba3 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Mon, 18 May 2026 23:17:43 +0200 Subject: [PATCH 13/20] Address review feedback: update pal_ssl.h SslDecrypt and pal_bio.h BioGetWriteResult 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> --- .../pal_bio.c | 10 ++++- .../pal_bio.h | 17 +++++++-- .../pal_ssl.h | 37 ++++++++++++++----- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c index c3de31987cb27c..8319990f7d9834 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c @@ -74,7 +74,7 @@ int32_t CryptoNative_BioCtrlPending(BIO* bio) * * The "managed-span" BIO_METHOD defined below avoids one of those copies in * each direction by letting the SSL operation read from / write to a - * caller-supplied buffer "window" directly. It is paired with the atomic + * caller-supplied buffer "window" directly. It is paired with the single-shot * SSL_{Handshake,Encrypt,Decrypt} entry points in pal_ssl.c which install * the windows around an SSL operation and tear them down again afterwards. * @@ -243,6 +243,14 @@ static int ManagedSpanBioWrite(BIO* bio, const char* buf, int len) if (remaining > 0) { + // Guard against int32 overflow before computing the new spill size. + // remaining and spillLen are both non-negative int32; bail out if the + // sum would not fit so ManagedSpanBioGrowSpill cannot be tricked into + // sizing the buffer based on a wrapped value. + if (remaining > INT32_MAX - ctx->spillLen) + { + return -1; + } int32_t needed = ctx->spillLen + remaining; if (!ManagedSpanBioGrowSpill(ctx, needed)) { diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.h b/src/native/libs/System.Security.Cryptography.Native/pal_bio.h index af6d9378a4d4b9..f0520caf991252 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.h @@ -63,15 +63,26 @@ PALEXPORT BIO* CryptoNative_BioNewManagedSpan(void); /* Internal helpers used by pal_ssl.c to drive the managed-span BIO during -atomic SSL handshake/encrypt/decrypt operations. Not exported. +single-shot SSL handshake/encrypt/decrypt operations. Not exported. */ void CryptoNative_BioSetReadWindow(BIO* bio, const void* ptr, int32_t len); void CryptoNative_BioClearReadWindow(BIO* bio, int32_t* leftoverLength); void CryptoNative_BioSetWriteWindow(BIO* bio, void* ptr, int32_t capacity); /* -Returns the number of bytes written into the window and into the spill -buffer respectively since the last reset/window-set. +Reports the current state of the managed-span output BIO after an SSL +operation. + +writtenToWindow is the number of bytes BIO_write deposited directly into +the current caller-supplied window since the last CryptoNative_BioSetWriteWindow +call. CryptoNative_BioSetWriteWindow resets this counter to zero each time +a new window is installed. + +spillLen is the total number of bytes currently held in the per-BIO heap +spill buffer. The spill is *not* reset by CryptoNative_BioSetWriteWindow; +it accumulates across SSL operations (so out-of-band output such as alerts +or TLS 1.3 KeyUpdate frames emitted during SSL_read is preserved for the +caller) and is only drained by CryptoNative_BioDrainSpill. */ PALEXPORT void CryptoNative_BioGetWriteResult(BIO* bio, int32_t* writtenToWindow, int32_t* spillLen); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index 2de9d7b17cc052..97994bd4755e83 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -326,7 +326,7 @@ and by the specifications of the TLS/SSL protocol; PALEXPORT int32_t CryptoNative_SslDoHandshake(SSL* ssl, int32_t* error); /* -Atomically performs SSL_do_handshake with the input/output BIO windows set up +Performs SSL_do_handshake with the input/output BIO windows set up and torn down in a single P/Invoke. The input BIO window points at inputPtr (ciphertext from peer, may be NULL/0). The output BIO window receives outgoing handshake bytes into outputPtr/outputCap; outputWritten reports the count @@ -348,7 +348,7 @@ PALEXPORT int32_t CryptoNative_SslHandshake( int32_t* errorCode); /* -Atomically performs SSL_write with the output BIO window set up and torn down +Performs SSL_write with the output BIO window set up and torn down in a single P/Invoke. plaintextPtr/plaintextLen is the plaintext. outputPtr/ outputCap is the ciphertext destination window; outputWritten reports bytes written, outputPending reports any overflow now in the BIO spill (drain via @@ -367,14 +367,33 @@ PALEXPORT int32_t CryptoNative_SslEncrypt( int32_t* errorCode); /* -Atomically performs SSL_read with the input BIO window set up and torn down -in a single P/Invoke. inputPtr/inputLen is incoming ciphertext (may be 0 to -drain plaintext already buffered inside SSL from a prior partial read). -outputPtr/outputCap is the plaintext destination. plaintextPending receives -SSL_pending after the read (bytes still buffered inside the SSL object). +Performs SSL_read with the input BIO window set up and torn down +in a single P/Invoke. -Returns the SSL_read return value (>0 = bytes written to outputPtr); errorCode -receives SSL_get_error. +inputPtr/inputLen describes the incoming ciphertext window. The buffer is +also reused as in-place scratch space for plaintext that does not fit into +the caller-supplied destination (see leftoverOffset/leftoverLength below), so +inputPtr must point at writable memory. + +outputPtr/outputCap is the primary plaintext destination. The function reads +up to outputCap bytes of plaintext directly into outputPtr and returns that +count via the function return value. + +If outputCap is 0, or if OpenSSL has more plaintext available for the current +record after the first SSL_read (SSL_pending > 0, e.g. the destination was +smaller than the record), a second SSL_read drains the remaining plaintext +back into the input buffer in place starting at inputPtr. The leftover region +is reported through leftoverOffset / leftoverLength (offset is always 0 on +Unix; the Windows PAL uses non-zero offsets and the managed contract is +shared across PALs). The caller is expected to forward those bytes to its +internal decrypted-data buffer. + +consumed receives the number of ciphertext bytes the BIO consumed from the +input window (inputLen minus any unread tail). errorCode receives SSL_get_error +mapped on the SSL_read result (or on the second SSL_read if the first did +not produce data). + +Returns the first SSL_read return value (>0 = bytes written to outputPtr). */ PALEXPORT int32_t CryptoNative_SslDecrypt( SSL* ssl, From 649e1ad569ac067dd62710a5af1b6eaed480dbec Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 19 May 2026 09:27:08 +0200 Subject: [PATCH 14/20] Add force-spill test hook and tests for managed-span BIO 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> --- .../Interop.OpenSsl.cs | 21 ++ .../SslStreamForceSpillTests.cs | 208 ++++++++++++++++++ .../System.Net.Security.Tests.csproj | 1 + 3 files changed, 230 insertions(+) create mode 100644 src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamForceSpillTests.cs diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 7e6cad2945ce83..6e71c67c732682 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -34,6 +34,15 @@ internal static partial class OpenSsl private const SslProtocols FakeAlpnSslProtocol = (SslProtocols)1; // used to distinguish server sessions with ALPN private static readonly Lazy s_defaultSigAlgs = new(GetDefaultSignatureAlgorithms); +#if DEBUG + // Test-only knob: when DOTNET_OPENSSL_FORCE_BIO_SPILL=1 is set, the managed-span + // BIO is given a zero-length write window, which forces every byte SSL emits + // to take the spill (heap) path inside the BIO. Reading the environment variable + // once is safe because the value never changes during the lifetime of the process. + private static readonly bool s_forceBioSpill = + Environment.GetEnvironmentVariable("DOTNET_OPENSSL_FORCE_BIO_SPILL") == "1"; +#endif + private sealed class SafeSslContextCache : SafeHandleCache { } private static readonly SafeSslContextCache s_sslContexts = new(); @@ -687,6 +696,12 @@ internal static unsafe SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle c Ssl.SslErrorCode errorCode; Span outputSpan = token.AvailableSpan; +#if DEBUG + if (s_forceBioSpill) + { + outputSpan = default; + } +#endif fixed (byte* inputPtr = input) fixed (byte* outputPtr = outputSpan) { @@ -785,6 +800,12 @@ internal static unsafe Ssl.SslErrorCode Encrypt(SafeSslHandle context, ReadOnlyS Ssl.SslErrorCode errorCode; Span windowSpan = outToken.AvailableSpan; +#if DEBUG + if (s_forceBioSpill) + { + windowSpan = default; + } +#endif fixed (byte* plaintextPtr = input) fixed (byte* windowPtr = windowSpan) { diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamForceSpillTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamForceSpillTests.cs new file mode 100644 index 00000000000000..50093e58d9ea2c --- /dev/null +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamForceSpillTests.cs @@ -0,0 +1,208 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.IO; +using System.Net.Security; +using System.Security.Cryptography.X509Certificates; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.DotNet.RemoteExecutor; +using Xunit; + +namespace System.Net.Security.Tests +{ + using Configuration = System.Net.Test.Common.Configuration; + + // Exercises the spill path of the managed-span BIO used on Linux TLS. + // + // When DOTNET_OPENSSL_FORCE_BIO_SPILL=1 is set, the managed Interop.OpenSsl + // helpers pass a zero-length write window to SslHandshake/SslEncrypt so + // every byte written by SSL takes the spill (heap) path inside the BIO. + // These tests run the same SslStream scenarios with that override turned + // on so the spill path gets the same level of functional coverage as the + // fast (window) path. + // + // The knob is compiled out of release builds of System.Net.Security, so + // these tests only meaningfully exercise the spill path against Debug + // builds and are skipped otherwise. + [PlatformSpecific(TestPlatforms.Linux)] + public class SslStreamForceSpillTests + { + public static bool IsSupported => + RemoteExecutor.IsSupported && PlatformDetection.IsDebugLibrary(typeof(SslStream).Assembly); + + private static ProcessStartInfo CreateForceSpillStartInfo() + { + var psi = new ProcessStartInfo(); + psi.Environment["DOTNET_OPENSSL_FORCE_BIO_SPILL"] = "1"; + return psi; + } + + [ConditionalFact(nameof(IsSupported))] + public async Task ForceSpill_PingPong_Succeeds() + { + await RemoteExecutor.Invoke(static async () => + { + (Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams(); + using (clientStream) + using (serverStream) + using (var client = new SslStream(clientStream)) + using (var server = new SslStream(serverStream)) + using (X509Certificate2 certificate = Configuration.Certificates.GetServerCertificate()) + { + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions + { + RemoteCertificateValidationCallback = delegate { return true; }, + TargetHost = "localhost", + }; + + SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions + { + ServerCertificate = certificate, + }; + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout( + client.AuthenticateAsClientAsync(clientOptions), + server.AuthenticateAsServerAsync(serverOptions)); + + await TestHelper.PingPong(client, server); + } + }, new RemoteInvokeOptions { StartInfo = CreateForceSpillStartInfo() }).DisposeAsync(); + } + + [ConditionalTheory(nameof(IsSupported))] + [InlineData(1)] + [InlineData(64 * 1024)] + [InlineData(256 * 1024)] + [InlineData(1024 * 1024)] + public async Task ForceSpill_LargeTransfer_Succeeds(int payloadSize) + { + await RemoteExecutor.Invoke(static async (payloadSizeString) => + { + int payloadSize = int.Parse(payloadSizeString); + + (Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams(); + using (clientStream) + using (serverStream) + using (var client = new SslStream(clientStream)) + using (var server = new SslStream(serverStream)) + using (X509Certificate2 certificate = Configuration.Certificates.GetServerCertificate()) + { + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions + { + RemoteCertificateValidationCallback = delegate { return true; }, + TargetHost = "localhost", + }; + + SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions + { + ServerCertificate = certificate, + }; + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout( + client.AuthenticateAsClientAsync(clientOptions), + server.AuthenticateAsServerAsync(serverOptions)); + + byte[] sendBuffer = new byte[payloadSize]; + for (int i = 0; i < sendBuffer.Length; i++) + { + sendBuffer[i] = (byte)(i & 0xFF); + } + + byte[] receiveBuffer = new byte[payloadSize]; + + using var cts = new CancellationTokenSource(TestConfiguration.PassingTestTimeout); + + Task writeTask = client.WriteAsync(sendBuffer, cts.Token).AsTask(); + Task readTask = ReadExactlyAsync(server, receiveBuffer, cts.Token); + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout(writeTask, readTask); + + Assert.Equal(sendBuffer, receiveBuffer); + } + }, payloadSize.ToString(), new RemoteInvokeOptions { StartInfo = CreateForceSpillStartInfo() }).DisposeAsync(); + } + + [ConditionalFact(nameof(IsSupported))] + public async Task ForceSpill_BidirectionalStress_Succeeds() + { + await RemoteExecutor.Invoke(static async () => + { + const int Iterations = 64; + const int PayloadSize = 17 * 1024; // not a record-size multiple, crosses record boundaries + + (Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams(); + using (clientStream) + using (serverStream) + using (var client = new SslStream(clientStream)) + using (var server = new SslStream(serverStream)) + using (X509Certificate2 certificate = Configuration.Certificates.GetServerCertificate()) + { + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions + { + RemoteCertificateValidationCallback = delegate { return true; }, + TargetHost = "localhost", + }; + + SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions + { + ServerCertificate = certificate, + }; + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout( + client.AuthenticateAsClientAsync(clientOptions), + server.AuthenticateAsServerAsync(serverOptions)); + + byte[] payload = new byte[PayloadSize]; + for (int i = 0; i < payload.Length; i++) + { + payload[i] = (byte)((i * 31) & 0xFF); + } + + using var cts = new CancellationTokenSource(TestConfiguration.PassingTestTimeout); + + Task clientLoop = RunPingPongAsync(client, payload, Iterations, cts.Token); + Task serverLoop = RunEchoAsync(server, payload.Length, Iterations, cts.Token); + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout(clientLoop, serverLoop); + } + + static async Task RunPingPongAsync(SslStream stream, byte[] payload, int iterations, CancellationToken ct) + { + byte[] receive = new byte[payload.Length]; + for (int i = 0; i < iterations; i++) + { + await stream.WriteAsync(payload, ct); + await ReadExactlyAsync(stream, receive, ct); + Assert.Equal(payload, receive); + } + } + + static async Task RunEchoAsync(SslStream stream, int size, int iterations, CancellationToken ct) + { + byte[] buffer = new byte[size]; + for (int i = 0; i < iterations; i++) + { + await ReadExactlyAsync(stream, buffer, ct); + await stream.WriteAsync(buffer, ct); + } + } + }, new RemoteInvokeOptions { StartInfo = CreateForceSpillStartInfo() }).DisposeAsync(); + } + + private static async Task ReadExactlyAsync(Stream stream, Memory buffer, CancellationToken cancellationToken) + { + int total = 0; + while (total < buffer.Length) + { + int read = await stream.ReadAsync(buffer.Slice(total), cancellationToken); + if (read == 0) + { + throw new EndOfStreamException(); + } + total += read; + } + } + } +} diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj b/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj index 79610468fe7995..1e28799028ecef 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj @@ -124,6 +124,7 @@ + From fe4b93160a02006e471fa58fa1ef0adae4af0522 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 19 May 2026 10:05:49 +0200 Subject: [PATCH 15/20] Address review feedback: errorCode logic, BIO ctrl handler, retry flags, 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> --- .../Interop.OpenSsl.cs | 5 ++ .../Interop.Ssl.cs | 3 - .../src/System/Net/Security/SslStream.IO.cs | 2 - .../pal_bio.c | 63 ++++++------------- .../pal_ssl.c | 26 +++++--- 5 files changed, 43 insertions(+), 56 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 6e71c67c732682..1599614168f212 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -909,6 +909,11 @@ internal static unsafe int Decrypt( } if (retVal + leftoverLength > 0) { + // The managed callers always pass exactly one full TLS frame (sized via + // EnsureFullTlsFrameAsync). OpenSSL's SSL_read consumes whole records on + // success, so on any successful decrypt the entire frame is consumed - the + // input span has no residual ciphertext to forward and `consumed` is + // not plumbed through the managed surface. Debug.Assert(consumed == input.Length, "Expected all input to be consumed."); return retVal; } diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 187feb53df5136..2e502fa4aa3e66 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -169,9 +169,6 @@ internal static unsafe partial int SslDecrypt( [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioNewManagedSpan")] internal static partial SafeBioHandle BioNewManagedSpan(); - - - [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_BioGetWriteResult")] internal static partial void BioGetWriteResult(SafeBioHandle bio, out int writtenToWindow, out int spillLen); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index b94e58f6972a3c..98016da05b0859 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -867,8 +867,6 @@ private async ValueTask ReadAsyncInternal(Memory buffer, buffer = buffer.Slice(processedLength); } - - if (_receivedEOF && nextTlsFrameLength == UnknownTlsFrameLength) { // there should be no frames waiting for processing diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c index 8319990f7d9834..5c3e205c43f77a 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c @@ -150,13 +150,13 @@ static pthread_once_t g_managedSpanBioOnce = PTHREAD_ONCE_INIT; static int ManagedSpanBioRead(BIO* bio, char* buf, int len) { - BIO_clear_retry_flags(bio); - if (bio == NULL || buf == NULL || len <= 0) { return 0; } + BIO_clear_retry_flags(bio); + ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); if (ctx == NULL) { @@ -207,13 +207,13 @@ static int ManagedSpanBioGrowSpill(ManagedSpanBioCtx* ctx, int32_t needed) static int ManagedSpanBioWrite(BIO* bio, const char* buf, int len) { - BIO_clear_retry_flags(bio); - if (bio == NULL || buf == NULL || len < 0) { return 0; } + BIO_clear_retry_flags(bio); + if (len == 0) { return 0; @@ -265,50 +265,25 @@ static int ManagedSpanBioWrite(BIO* bio, const char* buf, int len) static long ManagedSpanBioCtrl(BIO* bio, int cmd, long num, void* ptr) { + (void)bio; (void)num; (void)ptr; - ManagedSpanBioCtx* ctx = GetManagedSpanBioCtx(bio); - - switch (cmd) - { - case BIO_CTRL_FLUSH: - return 1; - - case BIO_CTRL_PENDING: - if (ctx == NULL) - { - return 0; - } - return (long)(ctx->readLen - ctx->readPos); - - case BIO_CTRL_WPENDING: - if (ctx == NULL) - { - return 0; - } - return (long)(ctx->writePos + ctx->spillLen); - - case BIO_CTRL_RESET: - if (ctx != NULL) - { - ctx->readPtr = NULL; - ctx->readLen = 0; - ctx->readPos = 0; - ctx->writePtr = NULL; - ctx->writeCapacity = 0; - ctx->writePos = 0; - ctx->spillLen = 0; - BIO_clear_retry_flags(bio); - } - return 1; - - case BIO_CTRL_EOF: - return 0; - - default: - return 0; + // OpenSSL only invokes a small set of ctrl commands against this BIO when it is plugged + // into the SSL state machine via SSL_set_bio. Empirically (verified across the full + // System.Net.Security test suite) only BIO_CTRL_FLUSH needs a real response. Returning 0 + // from the default case correctly answers BIO_CTRL_PUSH/POP (no-op), the kTLS probes + // BIO_CTRL_GET_KTLS_SEND/RECV (not supported), and any other future query. We + // deliberately do not implement BIO_CTRL_RESET: the SSL flows we exercise never issue + // it, and a real reset would have to either preserve or drop the spill buffer of bytes + // that have not yet been drained - neither of which is a safe default. Returning 0 for + // an unhandled command surfaces that explicitly to OpenSSL rather than pretending + // success. + if (cmd == BIO_CTRL_FLUSH) + { + return 1; } + return 0; } static int ManagedSpanBioCreate(BIO* bio) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 8f221c1f040042..20cdf7e3c25bc7 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -610,27 +610,39 @@ int32_t CryptoNative_SslDecrypt( } int32_t result = 0; + int32_t leftover = 0; if (outputCap > 0) { result = SSL_read(ssl, outputPtr, outputCap); } - // if provided destination buffer was too small, leave the remaining plaintext in-place in the input buffer. - if (outputCap == 0 || SSL_pending(ssl) > 0) + // If the caller-provided destination is empty or full, look for additional plaintext that + // SSL would otherwise buffer internally and stash it in-place in the input buffer so the + // caller can pick it up on the next call. Skip the probe when the first SSL_read already + // failed - that error/state should be reported as-is. + if (result > 0 ? SSL_pending(ssl) > 0 : outputCap == 0) { - *leftoverLength = SSL_read(ssl, inputPtr, inputLen); + leftover = SSL_read(ssl, inputPtr, inputLen); } - *errorCode = (result + *leftoverLength > 0) ? SSL_ERROR_NONE : CryptoNative_SslGetError(ssl, result); + // The first SSL_read determines the outcome when it produced bytes; otherwise the second + // SSL_read (the one that was actually attempted) does. Only report leftoverLength when it + // is positive - a negative value from a failed second read is not user-visible plaintext. + int32_t outcome = result > 0 ? result : leftover; + *errorCode = (outcome > 0) ? SSL_ERROR_NONE : CryptoNative_SslGetError(ssl, outcome); + if (leftover > 0) + { + *leftoverLength = leftover; + } if (inputBio != NULL) { - int32_t leftover = 0; - CryptoNative_BioClearReadWindow(inputBio, &leftover); + int32_t unread = 0; + CryptoNative_BioClearReadWindow(inputBio, &unread); if (consumed != NULL) { - *consumed = inputLen - leftover; + *consumed = inputLen - unread; } } From 4e0f205ba2779537f32c8c4a5b1bf149744ca57e Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 19 May 2026 11:31:34 +0200 Subject: [PATCH 16/20] Document and assert that ComputeMaxTlsOutput inputLength fits in int32 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> --- .../Interop.OpenSsl.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 1599614168f212..cd0c8a847c50ad 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -861,6 +861,16 @@ private static int ComputeMaxTlsOutput(int inputLength) // tag, optional MAC, padding, and the inner content-type byte for TLS 1.3). // Always add slack for at least one record's overhead even when inputLength == 0, // since SSL_write of an empty buffer can still emit handshake/alert bytes. + // + // No overflow check is needed: SslStream chunks user writes to MaxDataSize before + // calling EncryptMessage (see WriteAsyncChunked in SslStream.IO.cs), and on Unix + // MaxDataSize is at most StreamSizes.Default.MaximumMessage = 32 * 1024. The + // resulting upper bound (~33 KiB) is several orders of magnitude below int.MaxValue. + // The assert below guards against accidentally breaking that invariant in the future. + const int MaxExpectedInput = 32 * 1024; + Debug.Assert( + (uint)inputLength <= MaxExpectedInput, + $"ComputeMaxTlsOutput: inputLength {inputLength} exceeds expected upper bound {MaxExpectedInput}; SslStream chunking invariant broken."); const int MaxRecordOverhead = 256; int records = (inputLength >> 14) + 2; return inputLength + (records * MaxRecordOverhead); From 3ff12f19a4530c9e15d1d038cf8a8ed95dd2b5d1 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 19 May 2026 11:40:45 +0200 Subject: [PATCH 17/20] Remove unused CryptoNative_SslDoHandshake export 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> --- .../Interop.Ssl.cs | 3 --- .../entrypoints.c | 1 - .../pal_ssl.c | 16 ---------------- .../pal_ssl.h | 11 ----------- 4 files changed, 31 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 2e502fa4aa3e66..c2bb91f7113794 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -117,9 +117,6 @@ internal static ushort[] GetDefaultSignatureAlgorithms() [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetBio")] internal static partial void SslSetBio(SafeSslHandle ssl, SafeBioHandle rbio, SafeBioHandle wbio); - [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslDoHandshake", SetLastError = true)] - internal static partial int SslDoHandshake(SafeSslHandle ssl, out SslErrorCode error); - [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslHandshake", SetLastError = true)] internal static unsafe partial int SslHandshake( SafeSslHandle ssl, diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index 18d66ae776a720..25d119a0c16563 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -381,7 +381,6 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslAddExtraChainCert) DllImportEntry(CryptoNative_SslAddClientCAs) DllImportEntry(CryptoNative_SslDestroy) - DllImportEntry(CryptoNative_SslDoHandshake) DllImportEntry(CryptoNative_SslHandshake) DllImportEntry(CryptoNative_SslEncrypt) DllImportEntry(CryptoNative_SslDecrypt) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 20cdf7e3c25bc7..62539c77ca288f 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -483,22 +483,6 @@ void CryptoNative_SslSetBio(SSL* ssl, BIO* rbio, BIO* wbio) SSL_set_bio(ssl, rbio, wbio); } -int32_t CryptoNative_SslDoHandshake(SSL* ssl, int32_t* error) -{ - ERR_clear_error(); - int32_t result = SSL_do_handshake(ssl); - if (result == 1) - { - *error = SSL_ERROR_NONE; - } - else - { - *error = CryptoNative_SslGetError(ssl, result); - } - - return result; -} - int32_t CryptoNative_SslHandshake( SSL* ssl, const uint8_t* inputPtr, diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index 97994bd4755e83..48e0a734c2ec65 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -314,17 +314,6 @@ Shims the SSL_set_bio method. */ PALEXPORT void CryptoNative_SslSetBio(SSL* ssl, BIO* rbio, BIO* wbio); -/* -Shims the SSL_do_handshake method. - -Returns: -1 if the handshake was successful; -0 if the handshake was not successful but was shut down controlled -and by the specifications of the TLS/SSL protocol; -<0 if the handshake was not successful because of a fatal error. -*/ -PALEXPORT int32_t CryptoNative_SslDoHandshake(SSL* ssl, int32_t* error); - /* Performs SSL_do_handshake with the input/output BIO windows set up and torn down in a single P/Invoke. The input BIO window points at inputPtr From 8b92fe50154794230dde9a338d4400ac1398fc9c Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 19 May 2026 11:52:32 +0200 Subject: [PATCH 18/20] Use hybrid Decrypt contract on OSX and Android PALs 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> --- .../Net/Security/SslStreamPal.Android.cs | 49 ++++++++++----- .../System/Net/Security/SslStreamPal.OSX.cs | 62 +++++++++++++------ 2 files changed, 76 insertions(+), 35 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs index 931c9f41c6c646..7b33f5a48dca9f 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs @@ -117,8 +117,6 @@ public static SecurityStatusPal DecryptMessage( out int leftoverOffset, out int leftoverLength) { - // The Android SSLStream PAL always decrypts in-place; `destination` is unused. - _ = destination; bytesWritten = 0; leftoverOffset = 0; leftoverLength = 0; @@ -129,27 +127,48 @@ public static SecurityStatusPal DecryptMessage( securityContext.Write(encrypted); - PAL_SSLStreamStatus ret = Interop.AndroidCrypto.SSLStreamRead(sslHandle, encrypted, out int read); - if (ret == PAL_SSLStreamStatus.Error) - return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError); + PAL_SSLStreamStatus ret; - leftoverLength = read; - - SecurityStatusPalErrorCode statusCode = ret switch + // Opportunistically decrypt directly into the caller-provided destination span + // when one was supplied (saves a memcpy through the SslStream-owned buffer). + // If the first read does not fill the destination, all currently available + // plaintext has been consumed and we report the resulting status as-is. + if (!destination.IsEmpty) { - PAL_SSLStreamStatus.OK => SecurityStatusPalErrorCode.OK, - PAL_SSLStreamStatus.NeedData => SecurityStatusPalErrorCode.OK, - PAL_SSLStreamStatus.Renegotiate => SecurityStatusPalErrorCode.Renegotiate, - PAL_SSLStreamStatus.Closed => SecurityStatusPalErrorCode.ContextExpired, - _ => SecurityStatusPalErrorCode.InternalError - }; + ret = Interop.AndroidCrypto.SSLStreamRead(sslHandle, destination, out int written); + if (ret == PAL_SSLStreamStatus.Error) + return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError); + + bytesWritten = written; + if (ret != PAL_SSLStreamStatus.OK || written < destination.Length) + { + return new SecurityStatusPal(MapSSLStreamStatus(ret)); + } + } - return new SecurityStatusPal(statusCode); + // Either destination was empty, or the first read filled it; capture any + // remaining plaintext in-place inside the ciphertext span so SslStream can + // pick it up via leftoverOffset/leftoverLength. + ret = Interop.AndroidCrypto.SSLStreamRead(sslHandle, encrypted, out int leftover); + if (ret == PAL_SSLStreamStatus.Error) + return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError); + + leftoverLength = leftover; + return new SecurityStatusPal(MapSSLStreamStatus(ret)); } catch (Exception e) { return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, e); } + + static SecurityStatusPalErrorCode MapSSLStreamStatus(PAL_SSLStreamStatus status) => status switch + { + PAL_SSLStreamStatus.OK => SecurityStatusPalErrorCode.OK, + PAL_SSLStreamStatus.NeedData => SecurityStatusPalErrorCode.OK, + PAL_SSLStreamStatus.Renegotiate => SecurityStatusPalErrorCode.Renegotiate, + PAL_SSLStreamStatus.Closed => SecurityStatusPalErrorCode.ContextExpired, + _ => SecurityStatusPalErrorCode.InternalError, + }; } public static ChannelBinding? QueryContextChannelBinding( diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index 2dd6d0fd261a3b..19cfd42baf353e 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -211,27 +211,35 @@ public static SecurityStatusPal DecryptMessage( out int leftoverOffset, out int leftoverLength) { - // SecureTransport always decrypts in-place; `destination` is unused. - _ = destination; bytesWritten = 0; + leftoverOffset = 0; + leftoverLength = 0; Debug.Assert(securityContext is SafeDeleteSslContext, "SafeDeleteSslContext expected"); SafeDeleteSslContext sslContext = (SafeDeleteSslContext)securityContext; - leftoverOffset = 0; - leftoverLength = 0; - try { SafeSslHandle sslHandle = sslContext.SslContext; sslContext.Write(encrypted); + PAL_TlsIo status; + unsafe { - fixed (byte* ptr = encrypted) + // Opportunistically decrypt directly into the caller-provided destination span + // when one was supplied (saves a memcpy through the SslStream-owned buffer). + // If the first read does not fill the destination, all currently available + // plaintext has been consumed and we report the resulting status as-is. + if (!destination.IsEmpty) { - PAL_TlsIo status = Interop.AppleCrypto.SslRead(sslHandle, ptr, encrypted.Length, out int written); + int written; + fixed (byte* destPtr = destination) + { + status = Interop.AppleCrypto.SslRead(sslHandle, destPtr, destination.Length, out written); + } + if (status < 0) { return new SecurityStatusPal( @@ -239,29 +247,43 @@ public static SecurityStatusPal DecryptMessage( Interop.AppleCrypto.CreateExceptionForOSStatus((int)status)); } - leftoverLength = written; - leftoverOffset = 0; + bytesWritten = written; + if (status != PAL_TlsIo.Success || written < destination.Length) + { + return MapTlsIoStatus(status); + } + } - switch (status) + // Either destination was empty, or the first read filled it; capture any + // remaining plaintext in-place inside the ciphertext span so SslStream can + // pick it up via leftoverOffset/leftoverLength. + fixed (byte* ptr = encrypted) + { + status = Interop.AppleCrypto.SslRead(sslHandle, ptr, encrypted.Length, out int leftover); + if (status < 0) { - case PAL_TlsIo.Success: - case PAL_TlsIo.WouldBlock: - return new SecurityStatusPal(SecurityStatusPalErrorCode.OK); - case PAL_TlsIo.ClosedGracefully: - return new SecurityStatusPal(SecurityStatusPalErrorCode.ContextExpired); - case PAL_TlsIo.Renegotiate: - return new SecurityStatusPal(SecurityStatusPalErrorCode.Renegotiate); - default: - Debug.Fail($"Unknown status value: {status}"); - return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError); + return new SecurityStatusPal( + SecurityStatusPalErrorCode.InternalError, + Interop.AppleCrypto.CreateExceptionForOSStatus((int)status)); } + leftoverLength = leftover; } } + + return MapTlsIoStatus(status); } catch (Exception e) { return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, e); } + + static SecurityStatusPal MapTlsIoStatus(PAL_TlsIo status) => status switch + { + PAL_TlsIo.Success or PAL_TlsIo.WouldBlock => new SecurityStatusPal(SecurityStatusPalErrorCode.OK), + PAL_TlsIo.ClosedGracefully => new SecurityStatusPal(SecurityStatusPalErrorCode.ContextExpired), + PAL_TlsIo.Renegotiate => new SecurityStatusPal(SecurityStatusPalErrorCode.Renegotiate), + _ => new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError), + }; } public static ChannelBinding? QueryContextChannelBinding( From 9a9f356db3799aaed93baead41d6cacd5481edc0 Mon Sep 17 00:00:00 2001 From: Radek Zikmund <32671551+rzikm@users.noreply.github.com> Date: Tue, 19 May 2026 13:27:58 +0200 Subject: [PATCH 19/20] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../src/System/Net/Security/SslStream.Protocol.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 662708b3189944..dff7c776cb5451 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -1043,10 +1043,11 @@ private SecurityStatusPal DecryptData(int frameSize, Span destination, out if (status.ErrorCode == SecurityStatusPalErrorCode.Renegotiate) { - // The status indicates that peer wants to renegotiate. (Windows only) - // In practice, there can be some other reasons too - like TLS1.3 session creation - // of alert handling. We need to pass the data to lsass and it is not safe to do parallel - // write any more as that can change TLS state and the EncryptData() can fail in strange ways. + // The status indicates that the peer or TLS implementation requires additional + // handshake/session processing. In practice, there can be other reasons too, + // like TLS1.3 session creation or alert handling. We need to pass the data to + // the underlying security provider and it is not safe to do parallel write any + // more as that can change TLS state and the EncryptData() can fail in strange ways. // To handle this we call DecryptData() under lock and we create TCS waiter. // EncryptData() checks that under same lock and if it exist it will not call low-level crypto. From ace805f630f714ee191beffe9d74454a928323df Mon Sep 17 00:00:00 2001 From: Radek Zikmund <32671551+rzikm@users.noreply.github.com> Date: Tue, 19 May 2026 13:43:50 +0200 Subject: [PATCH 20/20] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../src/System/Net/Security/SslStreamPal.Windows.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index 277337846fc11c..ac6d5175556c4d 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -651,8 +651,8 @@ public static unsafe SecurityStatusPal DecryptMessage( leftoverLength = unmanagedBuffers[i].cbBuffer; // destination is ignored on Windows. We always decrypt in place and we set leftoverOffset to indicate where the data start. - Debug.Assert(leftoverOffset >= 0 && leftoverLength >= 0, $"Expected offset and count greater than 0, got {leftoverOffset} and {leftoverLength}"); - Debug.Assert(checked(leftoverOffset + leftoverLength) <= encrypted.Length, $"Expected offset+count <= buffer.Length, got {leftoverOffset}+{leftoverLength}>={encrypted.Length}"); + Debug.Assert(leftoverOffset >= 0 && leftoverLength >= 0, $"Expected offset and length greater than or equal to 0, got {leftoverOffset} and {leftoverLength}"); + Debug.Assert(checked(leftoverOffset + leftoverLength) <= encrypted.Length, $"Expected offset+length <= encrypted.Length, got {leftoverOffset}+{leftoverLength}>{encrypted.Length}"); break; }