Generate TLS alerts on certificate validation failure on macOS#128316
Draft
liveans wants to merge 4 commits into
Draft
Generate TLS alerts on certificate validation failure on macOS#128316liveans wants to merge 4 commits into
liveans wants to merge 4 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Apple cert-validation path introduced by the previous commits stored the cert exception in alertToken.Status with InternalError, which caused the IO loop to wrap it as AuthenticationException(net_auth_SSPI, inner), diverging from the Windows/Linux/Android behavior where SendAuthResetSignal throws the cert exception directly. Fix by introducing SecurityStatusPalErrorCode.CertValidationFailed and having ForceAuthenticationAsync rethrow the inner exception via ExceptionDispatchInfo when this code is observed, mirroring the SendAuthResetSignal pattern. Also: - Drop redundant _handshakeCompleted = false (field is always false at this point since CompleteHandshake has not run yet). - Assert alertType == Fatal in SslStreamPal.OSX.ApplyAlertToken to document that SecureTransport only emits fatal alerts via SslSetError. - Strengthen the two new OSX cert-alert tests with Assert.Null on InnerException so any future regression of the parity gets caught. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the macOS (SecureTransport) SslStream handshake flow to explicitly emit a fatal TLS alert record when certificate validation fails, aligning behavior more closely with other platforms. It introduces new handshake/status plumbing for “cert validation needed” and adds Apple-native interop to set SecureTransport’s internal error state.
Changes:
- Add Apple PAL support to surface a “certificate validation needed” handshake state and to set a specific TLS alert via a new
AppleCryptoNative_SslSetErrorexport. - Update
SslStreamhandshake/token generation to run cert validation at the new pause point on Apple and (on failure) send an alert frame + preserve exception parity via a dedicatedCertValidationFailedstatus. - Add/extend unit + functional tests (including on-the-wire alert bytes capture) and adjust
TlsFrameHelper.CreateAlertFrameto handle TLS 1.0 framing.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.h | Adds PAL TLS alert message enum and declares AppleCryptoNative_SslSetError. |
| src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.c | Implements alert→OSStatus mapping and AppleCryptoNative_SslSetError. |
| src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c | Exposes AppleCryptoNative_SslSetError for managed P/Invoke resolution. |
| src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs | Adds managed SslSetError P/Invoke + wrapper. |
| src/libraries/System.Net.Security/src/System/Net/SecurityStatusPal.cs | Introduces CertValidationNeeded and CertValidationFailed internal status codes. |
| src/libraries/System.Net.Security/src/System/Net/Security/TlsFrameHelper.cs | Fixes TLS 1.0 inclusion in CreateAlertFrame version handling. |
| src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs | Adds CanGenerateCustomAlertsForContext shim (constant-backed). |
| src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs | Adds CanGenerateCustomAlertsForContext shim (constant-backed). |
| src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs | Adds CanGenerateCustomAlertsForContext shim (constant-backed). |
| src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs | Enables custom alerts for SecureTransport contexts and returns CertValidationNeeded at auth-complete pause; applies alerts via SslSetError. |
| src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs | Runs cert validation when Apple PAL signals CertValidationNeeded; generates on-wire alert payload for SecureTransport contexts; switches alert gating to per-context. |
| src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs | Rethrows cert validation exception directly for CertValidationFailed to match other platforms’ exception shape. |
| src/libraries/System.Net.Security/tests/UnitTests/TlsAlertsMatchWindowsInterop.cs | Adds coverage to pin CreateAlertFrame record version bytes for TLS 1.0/1.1/1.2. |
| src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlertsTest.cs | Adds macOS-only functional tests that assert the actual alert bytes appear on the wire using a recording stream wrapper. |
| // to the caller (matching Windows/Linux/Android behavior via SendAuthResetSignal), | ||
| // not wrapped in a generic AuthenticationException(SR.net_auth_SSPI, ...). | ||
| Assert.Null(clientException.InnerException); | ||
| Assert.Contains("certificate", clientException.Message, StringComparison.OrdinalIgnoreCase); |
rzikm
approved these changes
May 19, 2026
- Enable OSX in SslStream_NoCallback_UntrustedCert_SendsAlert and SslStream_NoCallback_UntrustedClientCert_ServerSendsAlert instead of adding separate _OSX-suffixed duplicates (per @rzikm). - Drop the culture-dependent assertions on the localized exception message; rely on InnerException-shape parity instead (per Copilot). - Skip the Ssl3 protocol case on macOS where SecureTransport no longer negotiates it (would otherwise time out). - Accept either UnknownCA or BadCertificate as the fatal cert-rejection alert on macOS; SecureTransport varies by protocol but both are valid 'untrusted cert' signals. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This pull request was prepared with AI assistance (GitHub Copilot CLI). The code, build, and test validation were performed by the assistant under my supervision.
Fixes #127053.
Summary
On macOS SecureTransport, when
SslStreamrejects the peer's certificate (chain build failure, name mismatch, missing cert, etc.) the handshake was aborted without putting a TLS alert on the wire — the peer just saw aTCP RST / EOF. Windows / Linux / Android all send a fatal alert (
unknown_ca,bad_certificate, …) per RFC 5246 §7.2 / RFC 8446 §6.2 before aborting. This PR brings macOS to parity.Previous behavior (macOS, SecureTransport)
errSSLServerAuthCompleted/errSSLClientAuthCompletedfromSSLHandshakeVerifyRemoteCertificatefails inCompleteHandshakeNew behavior
SecurityStatusPalErrorCode.CertValidationNeededwhen SecureTransport pauses at the auth-completed state, beforeCompleteHandshakeruns.SslStreaminvokes its existing cert validation pipeline at that point and, on failure, builds a fatal alert record viaTlsFrameHelper.CreateAlertFrameand writes it to the wire before tearing down the handshake.GetAlertMessageFromChainfor chain errors,BadCertificatefor name mismatch,CertificateUnknownfor missing cert).SafeDeleteSslContext) only —SafeDeleteNwContext(Network.framework, opt-in viaSystem.Net.Security.UseNetworkFramework) keeps its async-context behavior untouched.Design notes
Why an explicit plaintext alert frame?
AppleCryptoNative_SslSetError(new native shim aroundSSLSetError) sets SecureTransport's internal error state so subsequentSSLHandshake/SSLClosecalls return the desired OSStatus — but empirical A/B testingshowed it does not itself write an alert record to the BIO. The PR therefore builds and writes the alert frame directly using
TlsFrameHelper.CreateAlertFrame, which is the same code path Windows uses forprotocol_versionalert injection.SSLSetErroris still called so SecureTransport's internal state lines up with what we sent on the wire.This is a SecureTransport-only behavior; Network.framework manages its own alert handling and is excluded via
SslStreamPal.IsAsyncSecurityContext.CanGenerateCustomAlertsForContext
CanGenerateCustomAlertswas a per-PALconst bool. It becomes a per-context methodCanGenerateCustomAlertsForContext(SafeDeleteContext?)so the OSX PAL can answertrueonly forSafeDeleteSslContext. Windows /Unix / Android keep the constant value as the implementation.
Cross-platform exception parity
VerifyRemoteCertificateAndGenerateNextTokeninitially surfaced the cert exception viaSecurityStatusPalErrorCode.InternalError, which causedForceAuthenticationAsyncto wrap it insideAuthenticationException(SR.net_auth_SSPI, …). On Windows / Linux / Android the cert exception is thrown directly bySendAuthResetSignalviaExceptionDispatchInfo.Throw. A newSecurityStatusPalErrorCode.CertValidationFailedcode is introduced; when the IO loop sees it, the inner cert exception is rethrown directly so the user-visible message matches the other platforms exactly. The new testcases pin this behavior via
Assert.Null(ex.InnerException)and message-content assertions.Tests
SslStreamAlertsTest:SslStream_NoCallback_UntrustedCert_SendsUnknownCAAlert_OSX— client side, server cert untrusted, assertsunknown_ca(48)reaches the server's recorded read stream.SslStream_NoCallback_UntrustedClientCert_ServerSendsUnknownCAAlert_OSX— server side, mutual-auth, assertsunknown_ca(48)reaches the client's recorded read stream.RecordingReadStreamthat wraps the inner stream and matches alert framing), not just exception types.CreateAlertFrame_NonProtocolAlert_UsesRequestedVersioninTlsAlertsMatchWindowsInteroppins the TLS 1.0 / 1.1 / 1.2 record framing ofCreateAlertFrame, including the(int)version >= Tlsfix thatpreviously excluded TLS 1.0.