Skip to content

Fix handling of errSSLClosedGraceful from SSLWrite on macOS#123987

Merged
rzikm merged 8 commits intomainfrom
copilot/fix-sslwrite-error-handling
Feb 10, 2026
Merged

Fix handling of errSSLClosedGraceful from SSLWrite on macOS#123987
rzikm merged 8 commits intomainfrom
copilot/fix-sslwrite-error-handling

Conversation

Copy link
Contributor

Copilot AI commented Feb 4, 2026

Description

On macOS with Secure Transport, writing to an SslStream after the remote side sends close_notify triggers Debug.Fail (debug builds) or throws IOException with misleading "Bad address" message (release builds). Root cause: EncryptMessage doesn't handle PAL_TlsIo.ClosedGracefully status from SSLWrite.

Changes

Production code (SslStreamPal.OSX.cs):

  • Added PAL_TlsIo.ClosedGracefully case in EncryptMessage switch statement
  • Returns SecurityStatusPal(SecurityStatusPalErrorCode.ContextExpired)
  • Matches existing DecryptMessage pattern

Test (SslStreamAlertsTest.cs):

  • Added SslStream_WriteAfterRemoteCloseNotify_ThrowsIOException test
  • Validates write-after-close_notify behavior using async operations
  • Uses runtime platform detection (PlatformDetection.IsOSX) with try-catch pattern to handle different behaviors across platforms:
    • macOS with Secure Transport: Validates the fix by asserting that IOException must be thrown when writing after close_notify
    • Linux/Windows: Accepts either success or IOException to accommodate different SSL implementation behaviors (OpenSSL/Schannel allow the first write to succeed)
  • Uses Task.WhenAll for concurrent client/server handshake to prevent deadlock
  • Includes proper timeouts to prevent test hangs in CI
  • Test runs on all major platforms for comprehensive validation

Impact

  • Eliminates Debug.Fail crash in debug builds on macOS
  • Replaces misleading "Bad address" error with proper IOException derived from ContextExpired on macOS
  • Consistent error handling between encryption and decryption paths
  • 3-line production code change
  • Test validates correct behavior across all platforms without platform-specific restrictions
Original prompt

This section details on the original issue you should resolve

<issue_title>Improper handling of errSSLClosedGraceful returned from SSLWrite on macOS</issue_title>
<issue_description>### Description

This issue applies to the original Secure Transport-based SSL implementation on macOS. It does not apply to the new Network.framework-based implementation which seems slated to replace it, but for now Secure Transport is still the default implementation.

If you try to write to a System.Net.Security.SslStream after the other side has sent a TLS close_notify alert (which is a standard way to close a connection), you'll get an unexpected exception:

Unhandled exception. System.IO.IOException: The encryption operation failed, see inner exception.
 ---> System.ComponentModel.Win32Exception (14): Bad address

Cause

There are actually two bugs here.

1. No check for ClosedGracefully

In this case, the native function SSLWrite is returning errSSLClosedGraceful, which Interop.AppleCrypto.SslWrite translates to PAL_TlsIo.ClosedGracefully. But the caller, SslStreamPal.EncryptMessage (in SslStreamPal.OSX.cs), doesn't have a case for this error code. DecryptMessage does have a case for it:

                            case PAL_TlsIo.ClosedGracefully:
                                return new SecurityStatusPal(SecurityStatusPalErrorCode.ContextExpired);

But EncryptMessage doesn’t, so we fall into:

                            default:
                                Debug.Fail($"Unknown status value: {status}");
                                token.Status = new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError);
                                break;

Note that Debug.Fail crashes the program on debug builds of System.Net.Security (but does nothing on release builds). Clearly we don’t want to fall into this case in any legitimate circumstance.

2. Misinterpreted error code

If we’re on a release build, then we proceed past the Debug.Fail to token.Status = new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError); However, this is later translated to an exception with the wrong message: “Win32Exception (14): Bad address”. There is no actual bad address here. Instead, this function in SslStreamPal.OSX.cs is extracting the numeric value of SecurityStatusPalErrorCode.InternalError, 14, and passing it to new Win32Exception:

        public static Exception GetException(SecurityStatusPal status)
        {
            return status.Exception ?? new Win32Exception((int)status.ErrorCode);
        }

Which picks this constructor:

        public Win32Exception(int error) : this(error, Marshal.GetPInvokeErrorMessage(error))

But GetPInvokeErrorMessage interprets the integer as a Unix errno value and calls out to strerror to get a string description. "Bad address" is the description of EFAULT, which just happens to be the errno value with numeric value 14 on macOS.

This should be fixed in addition to the first bug, since it could still apply to other error cases.

Test case

To reproduce, compile and run Program.cs.

(On macOS this will use Secure Transport by default. To use Network Framework, change the SslProtocols argument to either SslProtocols.Tls13 | SslProtocols.Tls12 or SslProtocols.Tls13, and pass DOTNET_SYSTEM_NET_SECURITY_USENETWORKFRAMEWORK=1. If you only pass SslProtocols.Tls12 then it will fall back to Secure Transport regardless of the environment variable, so to test Network.framework with TLS 1.2 you need to pass SslProtocols.Tls13 | SslProtocols.Tls12 and connect to a server that doesn’t support TLS 1.3. For convenience, I’m running such a server on port 444 at the same host from the test case.)

Here are some pcaps taken from running the program on various systems. They have the TLS secrets injected, so they should be viewable in Wireshark without any additional files. However, to view these properly, you should go to Preferences -> Protocols -> TLS and uncheck “Reassemble TLS Application Data spanning multiple TLS records”. Otherwise, some of the close_notify alerts will not be decoded properly. This appears to be a Wireshark bug.

What do we expect to happen?

This section is merely informative.

For practical compatibility, it would be nice if the specific exception thrown was the same on all platforms, but this is probably not achievable.

In my testing, on both Linux and Windows, SslStream disregards the close_notify alert and attempts to send data anyway. Therefore, the first Write call succeeds and sends an Application Data packet to the server; the server responds with a TCP RST, and subsequent Write...


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits February 4, 2026 09:31
Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix improper handling of errSSLClosedGraceful in SSLWrite Fix handling of errSSLClosedGraceful from SSLWrite on macOS Feb 4, 2026
Copilot AI requested a review from rzikm February 4, 2026 09:39
Copy link
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

@copilot Please address my review comments

…outs, simplify test

Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 4, 2026

@copilot Please address my review comments

Addressed all review comments in 3197819

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

@rzikm
Copy link
Member

rzikm commented Feb 5, 2026

@copilot The new test seems to be failing on all platforms, can you take a look?

Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 5, 2026

@copilot The new test seems to be failing on all platforms, can you take a look?

Fixed in 306d7c3. The test was failing on Linux/Windows because those platforms don't throw an exception on the first write after close_notify (they disregard the alert and attempt to send data). The fix in SslStreamPal.OSX.cs only affects macOS with Secure Transport, so I've restored the PlatformSpecific(TestPlatforms.OSX) attribute to make the test run only where the behavior applies.

Copy link
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

@copilot Please review my comments

Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
Copilot AI requested a review from rzikm February 5, 2026 15:15
Copy link
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

@copilot review my comments

Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
Copilot AI requested a review from rzikm February 9, 2026 13:08
@rzikm rzikm marked this pull request as ready for review February 9, 2026 14:22
@rzikm rzikm requested review from a team and Copilot February 9, 2026 14:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes macOS Secure Transport behavior when SSLWrite returns errSSLClosedGraceful (mapped to PAL_TlsIo.ClosedGracefully) by treating it as a graceful TLS close instead of an unknown/internal error, and adds a regression test for write-after-close_notify.

Changes:

  • Handle PAL_TlsIo.ClosedGracefully in SslStreamPal.OSX.EncryptMessage by returning SecurityStatusPalErrorCode.ContextExpired.
  • Add a functional test covering client write after server close_notify.

Reviewed changes

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

File Description
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs Adds ClosedGracefully handling during encryption to avoid falling into the “unknown status” path.
src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlertsTest.cs Adds a regression test for writing after receiving close_notify, with platform-conditional expectations.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

🤖 Copilot Code Review — PR #123987

Holistic Assessment

Motivation: This PR addresses a valid bug where writing to an SslStream after the remote side sends a TLS close_notify alert causes a Debug.Fail crash in debug builds or throws a misleading IOException with "Bad address" message in release builds on macOS with Secure Transport. The problem is well-documented in issue #121272 with clear root cause analysis.

Approach: The fix correctly adds handling for PAL_TlsIo.ClosedGracefully in EncryptMessage, mapping it to SecurityStatusPalErrorCode.ContextExpired. This mirrors the existing pattern in DecryptMessage and is the correct semantic representation for a closed TLS session.

Net positive: ✅ This is a net positive change — it fixes a crash/bug on macOS and improves error reporting consistency with a minimal 3-line production code change.


Detailed Findings

✅ Production Code — Correct fix pattern

The addition of the case PAL_TlsIo.ClosedGracefully: block in SslStreamPal.OSX.cs correctly mirrors the existing pattern in DecryptMessage (lines 240-241 in the original file):

case PAL_TlsIo.ClosedGracefully:
    return new SecurityStatusPal(SecurityStatusPalErrorCode.ContextExpired);

Mapping this status to ContextExpired is semantically correct — it signals that the TLS context is no longer valid for operations, which the upper SslStream layer translates into an appropriate IOException. This eliminates the Debug.Fail crash path.

✅ Consistency — Matches sibling DecryptMessage implementation

The fix maintains symmetry between EncryptMessage and DecryptMessage — both now handle ClosedGracefully identically. This is the expected pattern for this codebase.

✅ Test Coverage — Adequate regression test

The new test SslStream_WriteAfterRemoteCloseNotify_MayThrowIOException correctly exercises the fix:

  1. Establishes a TLS 1.2 connection between client and server
  2. Server initiates shutdown (sending close_notify)
  3. Client reads the closure (receives 0 bytes)
  4. Client attempts to write after receiving close_notify
  5. Verifies that macOS throws IOException while other platforms may succeed

The test uses proper timeouts (TestConfiguration.PassingTestTimeout) to prevent hangs in CI, and the platform-conditional assertion accurately reflects the behavioral difference between Secure Transport and OpenSSL/Schannel.

💡 Test Design — Platform divergence codified

The test's structure with Assert.False(PlatformDetection.IsOSX, ...) inside the try block explicitly codifies that macOS behaves more strictly (throwing immediately on write after close) than Windows/Linux (which may allow the first write to succeed). This is acceptable as it reflects the reality of the underlying SSL implementations, but note this commits the test to maintaining this documented divergence.


Cross-Cutting Analysis

Sibling platform implementations: I verified that SslStreamPal.Windows.cs, SslStreamPal.Unix.cs, and SslStreamPal.Android.cs exist. The fix is appropriately scoped to the OSX implementation since this bug is specific to how macOS Secure Transport returns errSSLClosedGraceful from SSLWrite.

Network Framework path: The PR description notes this issue doesn't apply to the new Network.framework-based implementation (SafeDeleteNwContext). The fix correctly targets only the SafeDeleteSslContext (Secure Transport) code path.

Issue #121272 mentions a second bug: The issue notes that GetException produces misleading error messages when SecurityStatusPalErrorCode.InternalError (value 14) is passed to Win32Exception, which interprets it as Unix errno EFAULT. This PR doesn't address that secondary issue, but that's acceptable as the fix prevents reaching that code path for this particular scenario.


Summary

LGTM. The fix is correct, minimal, consistent with existing patterns, and verified by an appropriate regression test. The PR has already been approved by @wfurt.

Review performed with: Claude Sonnet 4, Gemini 3 Pro. (GPT-5.1-Codex timed out)

@stephentoub
Copy link
Member

/ba-g unrelated failure

@rzikm rzikm enabled auto-merge (squash) February 10, 2026 08:07
@rzikm
Copy link
Member

rzikm commented Feb 10, 2026

/ba-g Test failure is unrelated

@rzikm rzikm merged commit c214ea0 into main Feb 10, 2026
87 of 90 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improper handling of errSSLClosedGraceful returned from SSLWrite on macOS

4 participants