Fix SslStream detection of exactly-5-byte TLS frames

[rzikm]

Summary

SslStream.EnsureFullTlsFrameAsync loops until a complete TLS record is buffered. When it starts with fewer than HeaderSize (5) bytes, frameSize is UnknownTlsFrameLength and the code recalculates the real size once more data arrives. However, the recalculation guard used a strict comparison:

if (frameSize == int.MaxValue && _buffer.EncryptedLength > TlsFrameHelper.HeaderSize)

A TLS record that is exactly the 5-byte header with a zero-length payload (e.g. 16 03 01 00 00) never satisfies EncryptedLength > 5, so frameSize stays at UnknownTlsFrameLength and the read loop keeps waiting for data that never comes.

Fix

Recalculate the frame size as soon as at least HeaderSize bytes are available (>= instead of >). Also replaced the int.MaxValue literal with the UnknownTlsFrameLength constant it aliases, for consistency with the surrounding code.

Testing

Added SslStreamFramingTests.Read_ExactlyFiveByteTlsRecord_DetectedAsCompleteFrame, which completes a real handshake and then injects a raw, exactly-5-byte zero-length TLS record on the next read, asserting SslStream detects a complete frame rather than hanging for a sixth byte. A real encrypted record is never exactly 5 bytes (AEAD overhead), so the record is injected artificially. Verified locally that the test hangs and fails at the 60s timeout against the unfixed code, and passes in <1s with the fix.

Note

This PR was authored by GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 4 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Fixes a TLS record buffering edge case in SslStream where a record that is exactly the 5-byte TLS header (including a valid zero-length payload) could fail to trigger frame-size recalculation and cause EnsureFullTlsFrameAsync to keep waiting for more data.

Changes:

• Recomputes TLS frame size as soon as _buffer.EncryptedLength reaches TlsFrameHelper.HeaderSize (uses >= instead of >).
• Replaces the int.MaxValue sentinel check with the existing UnknownTlsFrameLength constant for consistency.

[wfurt]

can we add some unit tests? e.g. artificially constructed case.

[rzikm]

Added a regression test in SslStreamFramingTest.cs (Read_ExactlyFiveByteTlsRecord_DetectedAsCompleteFrame).

It completes a real handshake, then injects a raw, exactly-5-byte TLS record with a zero-length payload (17 03 03 00 00) on the next read and asserts SslStream treats it as a complete frame instead of waiting forever for a sixth byte. A real encrypted record is never exactly 5 bytes on the wire (AEAD tag/overhead), so the record is injected artificially.

Verified both directions locally: against the unfixed code the read hangs and the test fails at the 60s timeout ("SslStream hung waiting for more data"); with the fix it passes in <1s.

Note

This comment was generated by GitHub Copilot.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

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

[rzikm]

The regression test hung on TLS 1.2-only CI legs (macOS, Mono, older Windows images). Root cause: the test injects a raw, exactly-5-byte zero-length application_data record. Under TLS 1.3 that record can't be decrypted (no room for the AEAD tag), so once framing recognizes the complete 5-byte frame the read fails fast — which is the behavior under test. Under TLS 1.2 a zero-length record is a legal empty fragment that decrypts to zero bytes and is silently skipped, so even the fixed code loops to read the next frame and hangs when the peer withholds data.

Pinned the test to TLS 1.3 ([ConditionalFact(..., SupportsTls13)] + EnabledSslProtocols = Tls13) so its observable signal is deterministic. The product fix itself is protocol-independent and unchanged.

Note

This comment was generated by GitHub Copilot.

[Copilot]

Pull request overview

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

[rzikm]

/ba-g Test failures are unrelated.