From 56dd4252b31b9479e63313cbc67a9eafb37f7b0a Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Mon, 10 Jun 2024 18:11:50 +0000 Subject: [PATCH 1/8] Fix GenerateSspiClientContext to retry negotiation with default port number appended to SPN. --- .../SSPI/NegotiateSSPIContextProvider.cs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs index 2c68839e3f..61d0aff940 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs @@ -14,13 +14,25 @@ internal sealed class NegotiateSSPIContextProvider : SSPIContextProvider internal override void GenerateSspiClientContext(ReadOnlyMemory received, ref byte[] sendBuff, ref uint sendLength, byte[][] _sniSpnBuffer) { - _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = Encoding.Unicode.GetString(_sniSpnBuffer[0]) }); - sendBuff = _negotiateAuth.GetOutgoingBlob(received.Span, out NegotiateAuthenticationStatusCode statusCode)!; - SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {0}, StatusCode={1}", _physicalStateObj.SessionId, statusCode); + NegotiateAuthenticationStatusCode statusCode = NegotiateAuthenticationStatusCode.UnknownCredentials; + + foreach (byte[] spn in _sniSpnBuffer) + { + _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = Encoding.Unicode.GetString(spn) }); + sendBuff = _negotiateAuth.GetOutgoingBlob(received.Span, out statusCode)!; + SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {0}, StatusCode={1}", _physicalStateObj.SessionId, statusCode); + + if (statusCode == NegotiateAuthenticationStatusCode.Completed || statusCode == NegotiateAuthenticationStatusCode.ContinueNeeded) + break; + else + _negotiateAuth = null; + } + if (statusCode is not NegotiateAuthenticationStatusCode.Completed and not NegotiateAuthenticationStatusCode.ContinueNeeded) { throw new InvalidOperationException(SQLMessage.SSPIGenerateError() + Environment.NewLine + statusCode); } + sendLength = (uint)(sendBuff != null ? sendBuff.Length : 0); } } From a9c52edd9cdb22bb4d93c037f7cf5ea1cfa8b5db Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Mon, 10 Jun 2024 18:32:29 +0000 Subject: [PATCH 2/8] Added comment why _negotiateAuth is reset to null when retrying. --- .../Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs index 61d0aff940..633eeeeddf 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs @@ -25,7 +25,7 @@ internal override void GenerateSspiClientContext(ReadOnlyMemory received, if (statusCode == NegotiateAuthenticationStatusCode.Completed || statusCode == NegotiateAuthenticationStatusCode.ContinueNeeded) break; else - _negotiateAuth = null; + _negotiateAuth = null; // Reset _negotiateAuth to be generated again for next SPN. } if (statusCode is not NegotiateAuthenticationStatusCode.Completed and not NegotiateAuthenticationStatusCode.ContinueNeeded) From ca7841a511fad651ea18829bf0be1a13529b995c Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Mon, 10 Jun 2024 23:21:18 +0000 Subject: [PATCH 3/8] Apply suggestions from PR review. --- .../SqlClient/SSPI/NegotiateSSPIContextProvider.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs index 633eeeeddf..8d70b4b8bd 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs @@ -16,14 +16,16 @@ internal override void GenerateSspiClientContext(ReadOnlyMemory received, { NegotiateAuthenticationStatusCode statusCode = NegotiateAuthenticationStatusCode.UnknownCredentials; - foreach (byte[] spn in _sniSpnBuffer) + for (int i = 0; i < _sniSpnBuffer.Length; i++) { - _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = Encoding.Unicode.GetString(spn) }); + string spnName = Encoding.Unicode.GetString(_sniSpnBuffer[i]); + _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = spnName }); sendBuff = _negotiateAuth.GetOutgoingBlob(received.Span, out statusCode)!; - SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {0}, StatusCode={1}", _physicalStateObj.SessionId, statusCode); + // Log session id, status code and the actual SPN used in the negotiation + SqlClientEventSource.Log.TryTraceEvent($"TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {_physicalStateObj.SessionId}, StatusCode={statusCode}, SPN={_negotiateAuth.TargetName}"); if (statusCode == NegotiateAuthenticationStatusCode.Completed || statusCode == NegotiateAuthenticationStatusCode.ContinueNeeded) - break; + break; // Successful case, exit the loop with current SPN. else _negotiateAuth = null; // Reset _negotiateAuth to be generated again for next SPN. } From 9cc0280f708c1e9da3123619f42c9fb6ecaba3a5 Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Tue, 11 Jun 2024 15:43:26 +0000 Subject: [PATCH 4/8] Apply suggestion from PM review. --- .../Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs index 8d70b4b8bd..98cd9c4f62 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs @@ -22,7 +22,7 @@ internal override void GenerateSspiClientContext(ReadOnlyMemory received, _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = spnName }); sendBuff = _negotiateAuth.GetOutgoingBlob(received.Span, out statusCode)!; // Log session id, status code and the actual SPN used in the negotiation - SqlClientEventSource.Log.TryTraceEvent($"TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {_physicalStateObj.SessionId}, StatusCode={statusCode}, SPN={_negotiateAuth.TargetName}"); + SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {0}, StatusCode={1}, SPN={2}", _physicalStateObj.SessionId, statusCode, _negotiateAuth.TargetName); if (statusCode == NegotiateAuthenticationStatusCode.Completed || statusCode == NegotiateAuthenticationStatusCode.ContinueNeeded) break; // Successful case, exit the loop with current SPN. From 9aa47f6923570780df22cb93cec340135cc38ecc Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Tue, 11 Jun 2024 15:43:26 +0000 Subject: [PATCH 5/8] Apply suggestion from PR review. --- .../Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs index 8d70b4b8bd..98cd9c4f62 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs @@ -22,7 +22,7 @@ internal override void GenerateSspiClientContext(ReadOnlyMemory received, _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = spnName }); sendBuff = _negotiateAuth.GetOutgoingBlob(received.Span, out statusCode)!; // Log session id, status code and the actual SPN used in the negotiation - SqlClientEventSource.Log.TryTraceEvent($"TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {_physicalStateObj.SessionId}, StatusCode={statusCode}, SPN={_negotiateAuth.TargetName}"); + SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {0}, StatusCode={1}, SPN={2}", _physicalStateObj.SessionId, statusCode, _negotiateAuth.TargetName); if (statusCode == NegotiateAuthenticationStatusCode.Completed || statusCode == NegotiateAuthenticationStatusCode.ContinueNeeded) break; // Successful case, exit the loop with current SPN. From d1399f488988db74da11e61f138c86a5c1231278 Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Tue, 11 Jun 2024 16:26:04 +0000 Subject: [PATCH 6/8] Revert "Apply suggestion from PM review." This reverts commit 9cc0280f708c1e9da3123619f42c9fb6ecaba3a5. --- .../Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs index 98cd9c4f62..8d70b4b8bd 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs @@ -22,7 +22,7 @@ internal override void GenerateSspiClientContext(ReadOnlyMemory received, _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = spnName }); sendBuff = _negotiateAuth.GetOutgoingBlob(received.Span, out statusCode)!; // Log session id, status code and the actual SPN used in the negotiation - SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {0}, StatusCode={1}, SPN={2}", _physicalStateObj.SessionId, statusCode, _negotiateAuth.TargetName); + SqlClientEventSource.Log.TryTraceEvent($"TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {_physicalStateObj.SessionId}, StatusCode={statusCode}, SPN={_negotiateAuth.TargetName}"); if (statusCode == NegotiateAuthenticationStatusCode.Completed || statusCode == NegotiateAuthenticationStatusCode.ContinueNeeded) break; // Successful case, exit the loop with current SPN. From d02a792d14a1a50af42f5259d42e5b5151a5ca14 Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Tue, 11 Jun 2024 16:36:06 +0000 Subject: [PATCH 7/8] Apply suggestion from PR review. --- .../Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs index 8d70b4b8bd..6209b5161c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs @@ -18,8 +18,7 @@ internal override void GenerateSspiClientContext(ReadOnlyMemory received, for (int i = 0; i < _sniSpnBuffer.Length; i++) { - string spnName = Encoding.Unicode.GetString(_sniSpnBuffer[i]); - _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = spnName }); + _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = Encoding.Unicode.GetString(_sniSpnBuffer[i]) }); sendBuff = _negotiateAuth.GetOutgoingBlob(received.Span, out statusCode)!; // Log session id, status code and the actual SPN used in the negotiation SqlClientEventSource.Log.TryTraceEvent($"TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {_physicalStateObj.SessionId}, StatusCode={statusCode}, SPN={_negotiateAuth.TargetName}"); From 3e413c59065a643b3548da99e6b38b7af16d5f5c Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Tue, 11 Jun 2024 17:58:38 +0000 Subject: [PATCH 8/8] Apply suggestion from PR review. --- .../Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs index 6209b5161c..21ab4a15b7 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs @@ -21,8 +21,7 @@ internal override void GenerateSspiClientContext(ReadOnlyMemory received, _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = Encoding.Unicode.GetString(_sniSpnBuffer[i]) }); sendBuff = _negotiateAuth.GetOutgoingBlob(received.Span, out statusCode)!; // Log session id, status code and the actual SPN used in the negotiation - SqlClientEventSource.Log.TryTraceEvent($"TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {_physicalStateObj.SessionId}, StatusCode={statusCode}, SPN={_negotiateAuth.TargetName}"); - + SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {0}, StatusCode={1}, SPN={2}", _physicalStateObj.SessionId, statusCode, _negotiateAuth.TargetName); if (statusCode == NegotiateAuthenticationStatusCode.Completed || statusCode == NegotiateAuthenticationStatusCode.ContinueNeeded) break; // Successful case, exit the loop with current SPN. else