Fix malformed UNC pipe path for IPv6 literals in managed SNI

[cheenamalhotra]

Description

Ports the native SNI fix (ADO PR 8120) to the Managed SNI code base, with an IPv6 improvement over the native version.

A UNC path host component may never contain a colon, so an IPv6 literal server name composes a malformed pipe path such as \\::1\pipe\sql\query. Handing that path to the OS sends the SMB redirector into an SMB session setup whose SPNEGO/NegoEx target name embeds the IPv6 literal, hitting an access violation inside LSASS on Windows. LSASS is a critical process, so Windows forces a reboot.

Managed SNI does not walk the sm -> tcp -> np default protocol list the way native SNI does: with no protocol prefix it resolves to TCP, so Server=::1 is already safe there. The malformed path is still reachable when Named Pipes is selected explicitly, for example Server=np:::1, Server=np:[::1], or Server=\\::1\pipe\sql\query.

Rather than rejecting these outright (which would block IPv6 addresses that can work), the parser applies the UNC transcription Windows defines for exactly this case (MS-DTYP 2.2.57): replace each : with - and each % (zone index) with s, then append .ipv6-literal.net.

Data source	Resulting PipeHostName
np:::1	--1.ipv6-literal.net
np:[::1]	--1.ipv6-literal.net
np:2001:db8::1	2001-db8--1.ipv6-literal.net
np:fe80::1%3	fe80--1s3.ipv6-literal.net

Implementation, all in ManagedSni/SniProxy.netcore.cs:

• New DataSource.GetUncCompatibleHostName helper performs the transcription, accepting the bracketed [::1] spelling users often carry over from URL syntax.
• Applied at both Named Pipes host-assignment sites in InferNamedPipesInformation (the np:host form and the \\host\pipe\... UNC form).
• SniProxy.CreateNpHandle retains a colon check as a final safeguard, mirroring the Np::OpenPipe check in native SNI.

Edge behavior worth noting:

• Colon-free host names, including IPv4 literals and already-transcribed .ipv6-literal.net names, pass through untouched, so nothing that worked before changes. LocalDB, localhost and . are unaffected.
• ServerName deliberately keeps the original literal because it only feeds SPN creation; only PipeHostName, which is what reaches the OS, is transcribed.
• A colon-bearing host with no valid IPv6 interpretation (e.g. not:a:host) fails cleanly with the standard invalid-connection-string SNI error instead of composing a malformed path.

No public API changes.

Issues

Fixes #4523 in the Managed SNI code path. Native SNI counterpart: Microsoft.Data.SqlClient.sni PR 8120.

Testing

New unit tests in src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/DataSourceNamedPipesTests.cs:

• IPv6 literals over Named Pipes are transcribed to their .ipv6-literal.net form, covering the np:host and UNC forms, bracketed and unbracketed spellings, and a zone index.
• Colon-bearing hosts with no IPv6 interpretation are still rejected.
• Valid Named Pipes data sources still parse unchanged: np:127.0.0.1, np:localhost, np:., np:server\instance, \\127.0.0.1\pipe\sql\query, \\.\pipe\MSSQL$MYINSTANCE\sql\query, \\my-server\pipe\sql\query.
• ServerName keeps the original IPv6 literal for SPN purposes while PipeHostName is transcribed.
• IPv6 literals with no protocol prefix still resolve to TCP, so existing IPv6 TCP connections are untouched.
• Direct coverage of the GetUncCompatibleHostName helper, including the null and empty cases.

48 ManagedSni unit tests pass locally (dotnet test -f net9.0 --filter FullyQualifiedName~ManagedSni), and the driver builds clean across all TFMs. Pipe-name assertions deliberately avoid the UNC forms because that code path builds the name with Path.DirectorySeparatorChar, which is platform dependent.

Note that no automated test can cover the LSASS crash itself, since reproducing it reboots the machine. The native SNI PR was verified manually on Windows; this change removes the malformed path at the parser level, before it can reach the OS.

[Copilot]

Pull request overview

Ports a Managed SNI hardening fix to prevent composing/handing malformed UNC Named Pipes paths when the data source host component is an IPv6 literal (contains :), avoiding a Windows LSASS crash/reboot scenario when Named Pipes is explicitly selected.

Changes:

• Add pipe-host validation (IsValidPipeHostName) and enforce it during Named Pipes parsing (both np:host and \\host\pipe\... forms).
• Add a final defensive validation in SniProxy.CreateNpHandle before constructing the Named Pipes handle.
• Add new unit tests covering IPv6-literal rejection on the Named Pipes path and ensuring valid NP inputs still parse correctly.

Reviewed changes

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

File	Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs	Adds validation to reject pipe hostnames containing : during NP parsing and before NP handle creation.
src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/DataSourceNamedPipesTests.cs	Adds regression tests to ensure IPv6 literals are rejected for Named Pipes while valid NP data sources continue to parse.

Suppressed comments (4)

src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/DataSourceNamedPipesTests.cs:47

• This test method is missing an XML <summary> comment. Please add a brief summary describing what valid inputs are being verified and why (to match the documentation style used by other ManagedSni unit tests in this folder).

        [Theory]
        [InlineData(@"np:127.0.0.1", "127.0.0.1")]
        [InlineData(@"np:localhost", "localhost")]
        [InlineData(@"np:.", ".")]
        [InlineData(@"np:server\instance", "server")]
        [InlineData(@"\\127.0.0.1\pipe\sql\query", "127.0.0.1")]
        [InlineData(@"\\.\pipe\MSSQL$MYINSTANCE\sql\query", ".")]
        [InlineData(@"\\my-server\pipe\sql\query", "my-server")]
        public void ParseServerName_NamedPipesWithValidHost_IsAccepted(
            string dataSource, string expectedPipeHostName)
        {

src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/DataSourceNamedPipesTests.cs:62

• This test method is missing an XML <summary> comment. Please add a brief summary describing the default pipe-name composition being asserted (to match the documentation style used by other ManagedSni unit tests in this folder).

        [Theory]
        [InlineData(@"np:127.0.0.1", @"sql\query")]
        [InlineData(@"np:localhost", @"sql\query")]
        [InlineData(@"np:server\instance", @"MSSQL$instance\sql\query")]
        public void ParseServerName_NamedPipesWithoutUncPath_ComposesDefaultPipeName(
            string dataSource, string expectedPipeName)
        {

src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/DataSourceNamedPipesTests.cs:93

• This test method is missing an XML <summary> comment. Please add a brief summary describing the inputs/outputs for IsValidPipeHostName (to match the documentation style used by other ManagedSni unit tests in this folder).

        [Theory]
        [InlineData("::1", false)]
        [InlineData("[::1]", false)]
        [InlineData("", false)]
        [InlineData(".", true)]
        [InlineData("localhost", true)]
        [InlineData("127.0.0.1", true)]
        [InlineData("my-server.contoso.com", true)]
        public void IsValidPipeHostName_ReturnsExpected(string hostName, bool expected)

src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/DataSourceNamedPipesTests.cs:102

• This test method is missing an XML <summary> comment. Please add a brief summary describing the null-handling behavior being asserted (to match the documentation style used by other ManagedSni unit tests in this folder).

        [Fact]
        public void IsValidPipeHostName_Null_ReturnsFalse()
        {
            Assert.False(DataSource.IsValidPipeHostName(null));
        }

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[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.

Suppressed comments (3)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs:739

• In the UNC Named Pipes parsing branch, bracketed IPv6 hosts (e.g. "\[2001:db8::1]\pipe\...") are accepted/transcribed for PipeHostName but ServerName is still set to the original bracketed token. Since ServerName feeds DNS/SPN generation, it should be normalized to an unbracketed IPv6 literal when the host token is a bracketed IPv6 address.

                    ServerName = IsLocalHost(host) ? Environment.MachineName : host;
                    // Pipe hostname is the hostname after leading \\ which should be passed down as is to open Named Pipe.
                    // For Named Pipes the ServerName makes sense for SPN creation only.
                    PipeHostName = uncHost;

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs:770

• The PR metadata/title describe "rejecting" IPv6 literals on the Named Pipes path (and mention an IsValidPipeHostName helper), but the current implementation instead transcribes parseable IPv6 literals into the Windows ".ipv6-literal.net" form via GetUncCompatibleHostName and only rejects colon-bearing hosts that are not valid IPv6 literals. Please update the PR title/description to match the implemented behavior to avoid confusion during release notes/backports.

        /// <summary>
        /// Converts a host name into a form that can legally appear as the host component of a UNC
        /// pipe path (<c>\\host\pipe\sql\query</c>), returning <see langword="null"/> if no such
        /// form exists.
        /// </summary>

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs:671

• In the np:host parsing path, bracketed IPv6 literals (e.g. "np:[::1]") get transcribed for PipeHostName but ServerName remains bracketed. ServerName is later used for DNS/SPN generation (SniProxy.GetSqlServerSPNs), and the bracketed form is not a valid host/address there, which can lead to an invalid SPN being constructed. Consider normalizing ServerName to the unbracketed IPv6 literal when the original input is a bracketed IPv6 address.

This issue also appears on line 736 of the same file.

                    // An IPv6 literal must be transcribed before it can appear in a UNC pipe path.
                    // ServerName keeps the original form because it is only used for SPN creation.
                    // See GetUncCompatibleHostName for details.
                    PipeHostName = GetUncCompatibleHostName(PipeHostName);
                    if (PipeHostName is null)

[cheenamalhotra]

Follow-up on the latest Copilot review pass. It generated no new inline threads, but three suppressed comments were worth acting on, and two of them flagged a real bug I had introduced.

Bracketed IPv6 left ServerName bracketed (fixed in 615bd11). For np:[2001:db8::1] or \\[2001:db8::1]\pipe\sql\query, PipeHostName was transcribed correctly but ServerName kept its brackets. ServerName feeds Dns.GetHostEntry and SPN construction in SniProxy.GetSqlServerSPNs, neither of which accepts the bracketed spelling, so the lookup would fail and a malformed SPN such as MSSQLSvc/[2001:db8::1] could be produced.

The literal parsing is now factored into TryParseIPv6Literal, shared by two callers with distinct jobs:

• NormalizeHostName unwraps brackets to the canonical unbracketed form, used for ServerName.
• GetUncCompatibleHostName transcribes to .ipv6-literal.net, used for PipeHostName.

Both Named Pipes host-assignment sites now apply both. Added a theory covering all four bracketed/unbracketed x np:host/UNC combinations, plus direct NormalizeHostName coverage. 59 ManagedSni unit tests pass and the driver builds clean across all TFMs.

Title/description drift (third suppressed comment). Both said "reject", which stopped being accurate once I switched to transcription. The description was updated earlier; I have now retitled the PR to Fix malformed UNC pipe path for IPv6 literals in managed SNI so release notes and backports do not inherit the stale wording.

[Copilot]

Pull request overview

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

[codecov]

Codecov Report

❌ Patch coverage is 88.88889% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.61%. Comparing base (ee6fabd) to head (615bd11).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...soft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs	88.88%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4558      +/-   ##
==========================================
- Coverage   65.23%   63.61%   -1.62%     
==========================================
  Files         288      283       -5     
  Lines       44587    67691   +23104     
==========================================
+ Hits        29087    43064   +13977     
- Misses      15500    24627    +9127     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	63.61% <88.88%> (?)

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.

[priyankatiwari08]

Read through the parser paths around this change - the fix looks right and is at the correct layer (parser, before the malformed path can ever reach the OS). Four notes, mostly minor.

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/DataSourceNamedPipesTests.cs:162

• This test asserts that for a bracketed IPv6 literal (e.g. "[::1]") without a protocol prefix, DataSource.ServerName remains bracketed. However, the TCP path passes DataSource.ServerName directly into DNS resolution (SniProxy.netcore.cs:224 -> SniTcpHandle.netcore.cs:330 uses Dns.GetHostAddresses(serverName)), and "[::1]" is not a valid host string for Dns.GetHostAddresses. This means the behavior the test is locking in is likely non-functional for TCP connections. Consider normalizing ServerName for non-NP parsing as well (e.g., apply DataSource.NormalizeHostName in InferConnectionDetails / ParseServerName for TCP) and update this assertion to expect the unbracketed form instead.

            Assert.NotNull(details);
            Assert.NotEqual(DataSource.Protocol.NP, details.ResolvedProtocol);
            Assert.Equal(dataSource, details.ServerName);
        }

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs:803

• The remarks on NormalizeHostName currently state that ServerName feeds DNS resolution and SPN construction (which is true), and that bracketed IPv6 must be dropped before it’s used there. However, the non-NP (TCP) parsing path in this class does not call NormalizeHostName, so this remark reads broader than what the implementation currently guarantees. Either apply NormalizeHostName to the TCP parsing path as well, or narrow this remark to reflect that the normalization is only performed by Named Pipes parsing today.

        /// <remarks>
        /// <see cref="ServerName"/> feeds DNS resolution and SPN construction, neither of which
        /// accepts the bracketed spelling, so the brackets must be dropped before it is used there.
        /// </remarks>
        internal static string NormalizeHostName(string hostName) =>