[DO NOT MERGE] [Draft] Connection encryption info – Slice C: Managed SNI certificate capture by priyankatiwari08 · Pull Request #4835 · dotnet/SqlClient

priyankatiwari08 · 2026-10-13T03:41:58Z

Summary

Managed SNI now captures server certificate details from the RemoteCertificateValidationCallback during the TLS handshake and flows them through TdsParserStateObject into the per-attempt EncryptionInfoCapture from Slice B. Both SqlConnection.EncryptionInfo (success) and SqlException.EncryptionInfo (failure) include them. Capture reuses the callback's inputs and the existing verdict. It doesn't run extra validation or network work, and it doesn't change trust decisions, exception messages, or tracing. Native SNI returns no certificate data yet (Slice D).

Part of #2353
Stacked on #4831

Certificate instead of hash

Following the Threat Model Review, the SHA-256 hash is removed. Managed SNI now captures the presented certificate itself: GetRawCertData() in the validation callback's finally block (so it is present on reject and on TrustServerCertificate bypass), skipped above 64 KiB, first certificate per attempt wins. ServerCertificateCapture.Hash256 became RawCertificate and EncryptionInfoCapture.CertificateHash256 became ServerCertificateRawData; the bytes go to the snapshot constructor (presentedServerCertificate).

Capture points

Field	Source	`null` when
`PresentedServerCertificateRawData` (internal) / `GetServerCertificate()`	A copy of the presented leaf certificate's DER (`GetRawCertData()`). The first certificate presented in the attempt is kept	No certificate presented, or its DER exceeds 64 KiB. Also captured under `TrustServerCertificate` bypass and on validation failure
`ServerCertificateValidationErrors`	Effective SqlClient verdict: `ServerCertificateMissing` (null cert / `RemoteCertificateNotAvailable`), `ChainError`, `HostNameMismatch` (after `HostNameInCertificate`), `PinnedCertificateMismatch`; `None` when passed	Validation bypassed (`TrustServerCertificate`), or the `ServerCertificate` file couldn't be read
`ServerCertificateChainStatus`	OR of the original callback `X509Chain.ChainStatus` flags (no rebuild); `NoError` when clean	Bypass, `ServerCertificate` pin mode (chain not consulted, even on match), no certificate, null chain, or `RemoteCertificateChainErrors` with no flags
`NegotiatedEncryptionProtocol` (Managed contribution)	`SslStream.SslProtocol`, read inside the validation callback (try/catch-guarded; `None` maps to null). Used only if the attempt has no protocol yet. net10.0 only (Managed SNI isn't compiled for net462)	Sender isn't an `SslStream`, the read throws, or the value is `None`
`NegotiatedCipherSuiteId` / .NET `NegotiatedCipherSuite` (Managed contribution)	`(int)SslStream.NegotiatedCipherSuite`, read after the handshake completes (authoritative, recorded via `RecordCipherSuite`) and inside the validation callback (adopted only if none recorded). TCP and named pipe; MARS passes through. try/catch-guarded; `0` maps to null. net10.0 only	No TLS, sender isn't an `SslStream`, the read throws, or the value is `0`
`NegotiatedEncryptionMode`	Slice B. On a failed handshake, `RecordFailedTlsAttempt` records the mode only when a protocol was observed (G2/D15)	No protocol observed

On Managed SNI, a certificate-validation failure now reports mode Full, the observed protocol (for example Tls12) and the observed cipher suite together with the presented certificate, ServerCertificateValidationErrors and ServerCertificateChainStatus. Slice B's DiscardUnobservedNegotiation keeps them because a protocol was observed.

Named pipes over Managed SNI validate against the target .. X509Certificate2.MatchesHostname(".") throws ArgumentException, which was already the case on main. To make the capture survive that throw, ValidateSslServerCertificate records the errors it already knows (for example ChainError and the callback chain status) before the host name check. The exception is not caught or changed. If no error is known before the check, the fields stay null rather than reporting None.

Values reset at the start of every TLS handshake / login attempt. They're stored through a volatile field written in the callback's finally and read after AuthenticateAsClient returns synchronously inside EnableSsl.

Tests

SniCommonServerCertificateCaptureTest (net10.0): table-driven verdicts (null cert, each SslPolicyErrors combination, HNIC override/mismatch, pin match/mismatch/missing/unreadable), outcome and message unchanged vs the legacy overload, every X509ChainStatusFlags value and the full set preserved, captured bytes equal the DER of a CertificateRequest self-signed cert (and are a copy), over-64-KiB cert not captured, first certificate wins, snapshot round-trip via GetServerCertificate().RawData, null vs zero, unbuilt chain not rebuilt, no leak into Message/ToString().
ManagedSniCertificateCaptureTests (simulated TDS server, managed SNI forced on Windows, Open and OpenAsync): TSC bypass (certificate only), matching pin, untrusted self-signed (ChainError + chain flags + certificate), hostname mismatch, HNIC override, pin mismatch, attempt isolation.
ManagedSniCertificateCaptureTests failure cases (untrusted, hostname mismatch, HNIC override mismatch, pin mismatch, failure-then-success) assert mode Full and protocol Tls12 alongside the certificate fields. A new SniCommon test covers a sender that isn't an SslStream, which leaves the protocol unavailable.
Updated ConnectionEncryptionInfoTests: bypass on managed SNI reports the certificate, and an untrusted failure on managed SNI reports a non-null protocol.
ValidateSslServerCertificate_NamedPipeDotTarget_CapturesKnownErrorsAndKeepsException:
- With chain and name errors, ChainError and the callback chain flags are captured.
- With only a name mismatch, the fields stay null.
- In both cases, the exception type and message match the legacy overload.
- Without the fix, the test fails with Expected ChainError, Actual null.
Head e4e6869, ManagedSni, SniCommon and EncryptionInfo filters on net10.0: 230/230 passed, with 0 warnings.
Results on head 502f3f3 (rebased on [DO NOT MERGE] [Draft] Connection encryption info – Slice B: negotiated mode and TLS protocol #4831 @ 43b09dd), using the filters ManagedSni, SniCommon, SimulatedServerTests, SqlException, SqlConnection and EncryptionInfo:
- net10.0: 485/485 passed.
- net462 (clean PSModulePath): 358/358 passed.
- Both builds: 0 warnings, 0 errors.
Cipher suite (head e9fbf4b, rebased on [DO NOT MERGE] [Draft] Connection encryption info – Slice B: negotiated mode and TLS protocol #4831 @ e6c78ca):
- TSC success asserts that NegotiatedCipherSuite equals SslStream.NegotiatedCipherSuite read from the client handle.
- The untrusted-certificate failure asserts that the suite captured in the callback equals the suite a trusted handshake with the same server negotiates, which confirms it is readable and real mid-handshake.
- ConnectionEncryptionInfoTests asserts a non-null suite with Managed TLS and null without TLS.
- Unit tests cover a null or unauthenticated SslStream (null, no throw) and adopt-if-none.
Results on head e9fbf4b, with the filters ManagedSni, SniCommon, SimulatedServerTests, SqlException, SqlConnection and EncryptionInfo:
- net10.0: 516/516 passed.
- net462 (clean PSModulePath): 385/385 passed.
- Both builds: 0 warnings.
Results on head a0c686e (rebased on [DO NOT MERGE] [Draft] Connection encryption info – Slice B: negotiated mode and TLS protocol #4831 @ a1d9ba6, full certificate instead of hash), same filters:
- net10.0: 838/838 passed.
- net462 (clean PSModulePath): 671/671 passed.
- Driver, ref and UnitTests builds: 0 warnings, 0 errors.

Manual tests still needed

Real SQL Server with trusted, untrusted, and hostname-mismatch certificates on Managed SNI (Windows and Linux)
Named Pipes over TLS
TDS 8 / Encrypt=Strict (the simulated server only supports PRELOGIN TLS)
MARS over TLS

Copilot

🔵 Needs a closer look

TLS transport changes still lack effective same-attempt isolation coverage and require the listed real-server validation.

4 open findings

What changed in this PR

Adds Slice C of connection encryption diagnostics, capturing Managed SNI server certificate details for successful and failed connections.

Changes:

Captures certificate hashes, validation verdicts, and chain status.
Propagates captured data through TDS and MARS layers.
Adds unit coverage and design documentation.

File	Description
`ManagedSniCertificateCaptureTests.cs`	Adds simulated TLS integration tests.
`ConnectionEncryptionInfoTests.cs`	Updates managed-SNI hash expectations.
`SniCommonServerCertificateCaptureTest.cs`	Tests validation and capture logic.
`TdsParserStateObjectManaged.netcore.cs`	Exposes managed transport capture.
`TdsParserStateObject.cs`	Defines the transport capture hook.
`TdsParser.cs`	Records capture after TLS negotiation.
`SniTcpHandle.netcore.cs`	Captures TCP TLS certificate details.
`SniNpHandle.netcore.cs`	Captures Named Pipes TLS certificate details.
`SniMarsHandle.netcore.cs`	Forwards capture through MARS handles.
`SniMarsConnection.netcore.cs`	Exposes the physical MARS capture.
`SniHandle.netcore.cs`	Adds the capture property contract.
`SniCommon.netcore.cs`	Produces validation verdicts, hashes, and chain flags.
`ServerCertificateCapture.cs`	Defines the immutable transport capture.
`EncryptionInfoCapture.cs`	Publishes certificate fields in snapshots.
`2353-connection-encryption-info.md`	Documents Managed SNI capture semantics.

🧠 Review effort: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+            Exception? legacy = null;
+            try
+            {
+                SniCommon.ValidateSslServerCertificate(Guid.NewGuid(), targetServerName, hostNameInCertificate, cert, pinFile?.Path, policyErrors);


+        [Theory]
+        [InlineData(false)]
+        [InlineData(true)]
+        public async Task Open_FailureThenSuccess_DoesNotCarryPreviousAttemptCertificate(bool async)


+        [Theory]
+        [MemberData(nameof(VerdictCases))]
+        public void ValidateSslServerCertificate_RecordsEffectiveVerdict(


+        [Theory]
+        [InlineData(false)]
+        [InlineData(true)]
+        public async Task Open_TrustServerCertificate_PublishesHashWithoutVerdict(bool async)


codecov · 2026-10-13T04:51:08Z

Codecov Report

❌ Patch coverage is 84.65909% with 27 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (dev/automation/encryption-info-slice-b-mode-protocol@e1ab4e4). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
...t/Data/SqlClient/ManagedSni/SniNpHandle.netcore.cs	0.00%	13 Missing ⚠️
...oft/Data/SqlClient/ManagedSni/SniCommon.netcore.cs	93.02%	6 Missing ⚠️
...oft/Data/SqlClient/ManagedSni/SniHandle.netcore.cs	0.00%	2 Missing ⚠️
.../SqlClient/ManagedSni/SniMarsConnection.netcore.cs	0.00%	2 Missing ⚠️
...Data/SqlClient/ManagedSni/SniMarsHandle.netcore.cs	0.00%	2 Missing ⚠️
...soft/Data/SqlClient/Parser/TdsParserStateObject.cs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@                                   Coverage Diff                                   @@
##             dev/automation/encryption-info-slice-b-mode-protocol    #4835   +/-   ##
=======================================================================================
  Coverage                                                        ?   66.33%           
=======================================================================================
  Files                                                           ?      308           
  Lines                                                           ?    68599           
  Branches                                                        ?        0           
=======================================================================================
  Hits                                                            ?    45506           
  Misses                                                          ?    23093           
  Partials                                                        ?        0

Flag	Coverage Δ
PR-SqlClient-Project	`66.33% <84.65%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

🟡 Changes recommended

Four unresolved review findings leave regression and attempt-isolation coverage incomplete.

4 open findings

🧠 Review effort: Balanced

Copilot

🔵 Needs a closer look

Security-sensitive TLS changes still need the documented cross-platform real-server, Strict encryption, Named Pipes, and MARS validation.

4 open findings

🧠 Review effort: Balanced

Copilot

🔵 Needs a closer look

Retry isolation needs correction, and real-server, Strict, Named Pipes, and MARS TLS validation remains pending.

5 open issues

_{Review effort: Balanced}

+            // The certificate validation callback runs synchronously within EnableSsl, so whatever the
+            // transport observed is complete here; record it before any failure is reported so that
+            // the login exception carries it.
+            _connHandler?.EncryptionInfoCapture?.RecordServerCertificate(_physicalStateObj.GetServerCertificateCapture());


Copilot

🔵 Needs a closer look

Human validation remains necessary for real-server certificate scenarios on Windows/Linux, named-pipe TLS, Strict/TDS 8, and MARS.

5 open issues

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

The acknowledged real-server, named-pipe TLS, Strict-mode, and MARS validation remains outstanding and warrants final human review.

5 open issues

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

The public XML documentation still promises a null protocol when no TLS handshake completes, but the callback now exposes an observed protocol even when certificate validation aborts the handshake.

6 open issues

_{Review effort: Balanced}

+            if (Protocol is null && capture.Protocol is SslProtocols protocol && protocol != SslProtocols.None)
+            {
+                Protocol = protocol;


Copilot

🔵 Needs a closer look

Real-SQL-Server validation remains pending for Windows/Linux Managed SNI, Strict/TDS 8, named-pipe TLS, and MARS, leaving certificate-capture and negotiation correctness unestablished across those paths.

6 open issues

_{Review effort: Balanced}

…tails (Slice C) Capture SHA-256 of the presented leaf certificate, the effective SqlClient validation verdict, and the original callback chain status flags in Managed SNI, and flow them into the per-attempt encryption info capture so both SqlConnection.EncryptionInfo and SqlException.EncryptionInfo report them. No trust decisions, messages, or tracing are changed. Part of #2353 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Named pipes validate against the target '.', which X509Certificate2.MatchesHostname rejects with ArgumentException before the final verdict is recorded. Record the errors already established (e.g. ChainError and callback chain status) before the host name check so the capture survives. The exception is not caught or changed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Record (int)SslStream.NegotiatedCipherSuite after the handshake (authoritative) and inside the certificate validation callback (adopt-if-none), so failed certificate validation keeps the observed suite. TCP and named-pipe handles; MARS passes through. Reads are exception-safe; 0 is treated as unavailable. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

… failures Main's #4891 makes the TDS simulator rethrow connection faults on Stop. The untrusted, host-name-mismatch and pin-mismatch capture tests now set ExpectTlsHandshakeFailure so the server treats the client's rejection as expected. Test-only change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Assert the public CLS-compliant NegotiatedCipherSuiteId property in the Managed SNI simulator tests and describe the two-property shape in the Managed capture section. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

… SNI) Managed SNI now captures a copy of the presented leaf certificate's DER encoding (GetRawCertData) instead of its SHA-256 hash, and passes it to the snapshot so SqlConnectionEncryptionInfo.GetServerCertificate() can return it. Certificates over 64 KiB are not captured, and the first certificate presented in an attempt is kept. Capture still happens in the validation callback's finally block, so it is present on TrustServerCertificate bypass and on validation failure. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🔵 Needs a closer look

Platform-sensitive TLS diagnostics still require runtime validation on real SQL Server on Windows/Linux and for named-pipe TLS, Strict/TDS 8, and MARS, which the simulated TLS 1.2 results do not establish.

6 open issues

_{Review effort: Balanced}

Copilot AI balanced review requested due to automatic review settings October 13, 2026 03:41

github-project-automation Bot added this to SqlClient Board Oct 13, 2026

Copilot started reviewing on behalf of priyankatiwari08 October 13, 2026 03:42

View session

Copilot AI reviewed Oct 13, 2026

View reviewed changes

priyankatiwari08 mentioned this pull request Oct 13, 2026

[DO NOT MERGE] [Draft] Connection encryption info – Slice D: Native SNI interop #4836

Draft

4 of 13 tasks

priyankatiwari08 added this pull request to stack #4837 October 13, 2026 05:54

Copilot AI balanced review requested due to automatic review settings October 13, 2026 09:20

priyankatiwari08 force-pushed the dev/automation/encryption-info-slice-c-managed-sni branch from 7d60501 to 549fd10 Compare October 13, 2026 09:20

Copilot started reviewing on behalf of priyankatiwari08 October 13, 2026 09:21

View session

Copilot AI reviewed Oct 13, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 15, 2026 09:28

priyankatiwari08 force-pushed the dev/automation/encryption-info-slice-c-managed-sni branch from 549fd10 to 502f3f3 Compare October 15, 2026 09:28

Copilot started reviewing on behalf of priyankatiwari08 October 15, 2026 09:29

View session

Copilot AI reviewed Oct 15, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 19, 2026 09:33

Copilot started reviewing on behalf of priyankatiwari08 October 19, 2026 09:33

View session

Copilot AI reviewed Oct 19, 2026

View reviewed changes

priyankatiwari08 added this to the 8.0.0-preview1 milestone Oct 19, 2026

priyankatiwari08 changed the title ~~[Draft] Connection encryption info – Slice C: Managed SNI certificate capture~~ [DO NOT MERGE] [Draft] Connection encryption info – Slice C: Managed SNI certificate capture Oct 19, 2026

mdaigle linked an issue Oct 20, 2026 that may be closed by this pull request

Feature | Expose connection encryption information to clients #2353

Open

cheenamalhotra modified the milestones: 8.0.0-preview1, 8.1.0-preview1 Oct 21, 2026

Copilot AI balanced review requested due to automatic review settings October 22, 2026 08:31

priyankatiwari08 force-pushed the dev/automation/encryption-info-slice-c-managed-sni branch from e4e6869 to e9fbf4b Compare October 22, 2026 08:31

Copilot started reviewing on behalf of priyankatiwari08 October 22, 2026 08:31

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

priyankatiwari08 force-pushed the dev/automation/encryption-info-slice-c-managed-sni branch from e9fbf4b to 78f101e Compare October 26, 2026 17:45

Copilot AI balanced review requested due to automatic review settings October 26, 2026 17:45

Copilot started reviewing on behalf of priyankatiwari08 October 26, 2026 17:46

View session

Copilot AI reviewed Oct 26, 2026

View reviewed changes

priyankatiwari08 force-pushed the dev/automation/encryption-info-slice-c-managed-sni branch from 78f101e to 1a6e5c6 Compare October 28, 2026 09:39

Copilot AI balanced review requested due to automatic review settings October 28, 2026 09:40

Copilot started reviewing on behalf of priyankatiwari08 October 28, 2026 09:40

View session

Copilot AI reviewed Oct 28, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/EncryptionInfoCapture.cs

Comment on lines +126 to +128

if (Protocol is null && capture.Protocol is SslProtocols protocol && protocol != SslProtocols.None)

{

Protocol = protocol;

priyankatiwari08 force-pushed the dev/automation/encryption-info-slice-c-managed-sni branch from 1a6e5c6 to a0c686e Compare October 30, 2026 07:34

Copilot AI balanced review requested due to automatic review settings October 30, 2026 07:34

Copilot started reviewing on behalf of priyankatiwari08 October 30, 2026 07:35

View session

Copilot AI reviewed Oct 30, 2026

View reviewed changes

priyankatiwari08 and others added 6 commits November 2, 2026 11:35

priyankatiwari08 force-pushed the dev/automation/encryption-info-slice-c-managed-sni branch from a0c686e to 4d35501 Compare November 2, 2026 06:11

Copilot AI balanced review requested due to automatic review settings November 2, 2026 06:11

Copilot started reviewing on behalf of priyankatiwari08 November 2, 2026 06:11

View session

Copilot AI reviewed Nov 2, 2026

View reviewed changes

Navigation Menu

Repository navigation

[DO NOT MERGE] [Draft] Connection encryption info – Slice C: Managed SNI certificate capture - #4835

[DO NOT MERGE] [Draft] Connection encryption info – Slice C: Managed SNI certificate capture#4835
priyankatiwari08 wants to merge 6 commits into
dev/automation/encryption-info-slice-b-mode-protocolfrom
dev/automation/encryption-info-slice-c-managed-sni

priyankatiwari08 commented Oct 13, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

codecov Bot commented Oct 13, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

priyankatiwari08 commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Certificate instead of hash

Capture points

Tests

Manual tests still needed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

codecov Bot commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

priyankatiwari08 commented Oct 13, 2026 •

edited

Loading

codecov Bot commented Oct 13, 2026 •

edited

Loading