Feature | Implement SSRP packet parsing by edwardneal · Pull Request #4481 · dotnet/SqlClient

edwardneal · 2026-07-28T17:29:48Z

Description

This builds on the earlier test cases laid in #3741, and implements parsing of SSRP responses (for both SVR_RESP & SVR_RESP (DAC) packets.)

These responses will be parsed from untrusted/malicious network packets, so I've split the parsers into their own PR to make sure they can get a dedicated review. Each of the parsers stand alone, and SVR_RESP (DAC) parsing is the simplest by some margin!

SVR_RESP parsing is the more complex part, since it has a top-level header followed by RESP_DATA parsing. I've broken this down as seems reasonable:

SqlDataSourceResponse parses the header
During parsing, we parse RESP_DATA into a chain of RespDataComponents (key + value pairs)
Once RespDataComponents have been extracted and verified as structurally valid, verify that they're valid for the keys we care about
Gather the RespDataComponents for TCP and Named Pipes in an ExposedProtocols and verify that the SQL Server instance has at least one usable endpoint
When we know that the above are all true, populate the properties on SqlDataSourceResponse

The end goal will be to gather packet buffers asynchronously from a UDP client or socket, then pass them to SqlDataSourceResponseReader and DacResponseReader and populate the DataTable or TCP port.

Issues

Follows up #3741
Contributes to #3700

Link to any relevant issues, bugs, or discussions (e.g., Closes #123, Fixes issue #456).

Testing

Automated tests have been added, and I've added several test cases as I wrote the parsers.

These data are issued as unicast responses, and we should recognise the first response, not the last.

These relate to invalid data and empty packet buffers

Add assertion on bytesRead in DacResponseReader

* Locate one reference to Encoding.UTF8 rather than to s_mbcsEncoding. * Indentation for conditional compilation. * Permit underscores in machine names. * Complete rename from DacResponseProcessorTest to DacResponseReaderTest. * XML documentation updates.

Also provide an early break if the BV_INFO protocol component is too short

azure-pipelines · 2026-07-28T17:29:57Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

edwardneal

Comments to aid review.

edwardneal · 2026-07-28T20:20:42Z

+        return GetByteCount(encoding, slicedString);
+    }
+
+    public static int GetByteCount(this Encoding encoding, ReadOnlySpan<char> chars)


This refactor provides the ability to restrict the number of bytes in a particular component of RESP_DATA after having been converted to a string in order to parse it.

edwardneal · 2026-07-28T20:23:25Z

+
+    private const byte ResponseHeaderValue = 0x05;
+
+    private static readonly Encoding s_mbcsEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true);


The question of which encoding to use is odd. The specification just refers to a multi-byte character set, but the existing (partial) implementation just uses ASCII. I've chosen UTF8 because it meets the specification requirements while also maintaining the property of normal characters being single bytes.

edwardneal · 2026-07-28T20:29:25Z

+            ReadOnlySpan<char> valueCandidate = respDataSpan.Slice(terminatorPos + 1);
+
+            // There could potentially be a number of terminators within the RESP_DATA token's value. The Banyan VINES parameters contain five.
+            // BV_PARAMETERS = "bv;<ITEM NAME>;<GROUP NAME>;<ITEM NAME>;<GROUP NAME>;<ORG NAME>;"


Note the repetition here. This comes from the below statements in the SVR_RESP page:

BV_INFO=SEMICOLON "bv" SEMICOLON ITEMNAME SEMICOLON GROUPNAME SEMICOLON BV_PARAMETERS
BV_PARAMETERS=ITEMNAME SEMICOLON GROUPNAME SEMICOLON ORGNAME

This might well be a document error, but I've not got a real-world implementation to validate against so I've taken the specification as written.

We care about Banyan VINES but will never use it, because this is the only protocol component where the key can contain semicolons; as such, trying to parse it as part of the RESP_DATA string when it appears first in the list of protocols could lead to the succeeding protocol entries being misinterpreted.

edwardneal · 2026-07-28T20:31:21Z

+
+            // If a maximum value length is specified, this is in bytes. Calculate the number of
+            // bytes in the string, and compare.
+            if (maxValueLength != -1)


This is used for ServerName, InstanceName and VersionString. MC-SQLR references the bytes underlying RESP_DATA as a string, then requires that the string's components are converted back to bytes in order to have their length checked.

edwardneal · 2026-07-28T20:36:12Z

+    /// A successfully parsed instance does not guarantee secure data. Callers must ensure that
+    /// <see cref="NamedPipe"/> points to the same server identified by the SSRP response's server
+    /// name before they connect to it.


This statement isn't currently important because we don't actually resolve SSRP responses to anything other than a TCP port number, and we don't expose the named pipe in the SqlDataSource return value. If either of these assumptions change, it'll be a problem.

Section 3.1.5.2 notes:

The SQL Server Resolution Protocol SHOULD NOT verify the length or content of the PIPENAME field, which is provided by the higher layer. It is the upper layer's responsibility to ensure that PIPENAME conforms to the specification of a valid pipe name.

This is the only field which the specification explicitly bars a compliant client from validating, and it's the reason why the constructor just reads RespDataComponent.Value directly rather than using a TryGetX method.

edwardneal · 2026-07-28T20:41:57Z

+
+        public ReadOnlySpan<char> NamedPipe { get; }
+
+        public bool Valid => TcpEnabled || NamedPipeEnabled;


This type rolls data up into a piece of coherent connectivity metadata and takes responsibility for determining where SqlClient needs to care about it. Its notion of Valid is thus more involved than RespDataComponent, which considers a component valid if that component has been successfully parsed from the RESP_DATA string.

edwardneal · 2026-07-28T20:47:32Z

+            // selected based upon the key - a protocol name. A failure to match a known key will
+            // allow the default value (a managed reference to a boolean true) value to stand, forcing
+            // this method to return false.
+            if (key.Equals(NamedPipesInfoKey.AsSpan(), StringComparison.Ordinal))


These are deliberately case sensitive - the specification is silent on this, so I've chosen to take a more conservative approach.

edwardneal · 2026-07-28T20:52:48Z

+        }
+
+        // Parse and validate the header components
+        if (!RespDataComponent.TryParse(decodedRespData, ref currRespDataOffset, RespDataComponent.MaxServerNameLength, out RespDataComponent serverNameToken)


This is a long if statement (same style as in the DacResponseReader) but it's well-commented, all of the clauses come together to form the fixed-position and mandatory components, and splitting it into one statement per token also looked slightly clumsy. I've got no objection to changing the style if anyone thinks there's a clearer approach.

edwardneal · 2026-07-28T21:09:28Z

+/// <summary>
+/// Utilities used to extract zero or more parsed SSRP DAC responses from a set of packet buffers.
+/// </summary>
+internal static class DacResponseReader


The name change here and in SqlDataSourceResponseReader is purely a layering point:

DacResponse handles the parsing of one SVR_RESPONSE (DAC) from the start of a ReadOnlySequence;

DacResponseReader builds on that to read the first/last DacResponse from the ReadOnlySequence;

In future, the layer above will handle the networking and interpretation of the results to output the DAC port. This will be the processing layer.

edwardneal · 2026-07-28T21:16:56Z

+    /// This method parses SVR_RESP messages sent as the result of issuing a CLNT_BCAST_EX or a CLNT_UCAST_EX
+    /// message.
+    /// </remarks>
+    public static bool TryReadLast(ReadOnlySequence<byte> sourceSequence, out SqlDataSourceResponse lastResponse)


This is in addition to the mechanism of DacResponseReader, and it supports the extra functionality in the subnet-wide SSRP discovery mechanism. The processing layer for those SSRP responses will broadcast a request into the network, then listen for 30s. At the end of this, we'll have one ReadOnlySequence per source IP address and we'll want to know the most recent announcement for each of them.

In theory, this also means that if a malicious client were to announce another server's name, this would be detected. We can't break the assumption that SERVERNAME\INSTANCENAME will be a unique key for the resultant DataTable, but we can at least log it.

edwardneal added 11 commits July 25, 2026 16:59

Update DAC response test data

0526e31

These data are issued as unicast responses, and we should recognise the first response, not the last.

Add IndexOf utility function

fa505b0

Add more test cases to DAC response test data

9f951f0

These relate to invalid data and empty packet buffers

Implement DacResponse and DacResponseReader

863d1f4

Implement DacResponseReader tests

7a7cec1

Documentation updates and debug assertion

d648939

Add assertion on bytesRead in DacResponseReader

Implement SqlDataSourceResponse and SqlDataSourceResponseReader

3695a00

Add new test cases

3806b1c

Implement and rename SqlDataSourceResponseReaderTest

68edbb3

Follow-ups

8df8cd8

* Locate one reference to Encoding.UTF8 rather than to s_mbcsEncoding. * Indentation for conditional compilation. * Permit underscores in machine names. * Complete rename from DacResponseProcessorTest to DacResponseReaderTest. * XML documentation updates.

Add additional test cases

11185a5

Also provide an early break if the BV_INFO protocol component is too short

edwardneal requested a review from a team as a code owner July 28, 2026 17:29

github-project-automation Bot added this to SqlClient Board Jul 28, 2026

github-project-automation Bot moved this to To triage in SqlClient Board Jul 28, 2026

edwardneal commented Jul 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature | Implement SSRP packet parsing - #4481

Feature | Implement SSRP packet parsing#4481
edwardneal wants to merge 11 commits into
dotnet:mainfrom
edwardneal:feat/ssrp/02-parsing

edwardneal commented Jul 28, 2026

Uh oh!

azure-pipelines Bot commented Jul 28, 2026

Uh oh!

edwardneal left a comment

Uh oh!

edwardneal Jul 28, 2026

Uh oh!

edwardneal Jul 28, 2026

Uh oh!

edwardneal Jul 28, 2026

Uh oh!

edwardneal Jul 28, 2026

Uh oh!

edwardneal Jul 28, 2026

Uh oh!

edwardneal Jul 28, 2026

Uh oh!

edwardneal Jul 28, 2026

Uh oh!

edwardneal Jul 28, 2026

Uh oh!

edwardneal Jul 28, 2026

Uh oh!

edwardneal Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant


		private const byte ResponseHeaderValue = 0x05;

		private static readonly Encoding s_mbcsEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true);


		public ReadOnlySpan<char> NamedPipe { get; }

		public bool Valid => TcpEnabled \|\| NamedPipeEnabled;

Uh oh!

Conversation

edwardneal commented Jul 28, 2026

Description

Issues

Testing

Uh oh!

azure-pipelines Bot commented Jul 28, 2026

Uh oh!

edwardneal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant