TdsParser.ProcessSSPI in NetCore uses ArrayPool.Shared.Rent which might return an non zero-initialized array

[stuka120]

Describe the bug

TdsParser for NetCore uses ArrayPool.Shared.Rent(...) which might return an non-zero-initialized array.

SqlClient/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs

Lines 8654 to 8683 in faf9b95

	byte[] receivedBuff = ArrayPool<byte>.Shared.Rent(receivedLength);
	// read SSPI data received from server
	Debug.Assert(_physicalStateObj._syncOverAsync, "Should not attempt pends in a synchronous call");
	bool result = _physicalStateObj.TryReadByteArray(receivedBuff, receivedLength);
	if (!result)
	{
	throw SQL.SynchronousCallMayNotPend();
	}
	// allocate send buffer and initialize length
	byte[] rentedSendBuff = ArrayPool<byte>.Shared.Rent((int)s_maxSSPILength);
	byte[] sendBuff = rentedSendBuff;
	uint sendLength = s_maxSSPILength;
	// make call for SSPI data
	SSPIData(receivedBuff, (uint)receivedLength, ref sendBuff, ref sendLength);
	// DO NOT SEND LENGTH - TDS DOC INCORRECT! JUST SEND SSPI DATA!
	_physicalStateObj.WriteByteArray(sendBuff, (int)sendLength, 0);
	ArrayPool<byte>.Shared.Return(rentedSendBuff, clearArray: true);
	ArrayPool<byte>.Shared.Return(receivedBuff, clearArray: true);
	// set message type so server knows its a SSPI response
	_physicalStateObj._outputMessageType = TdsEnums.MT_SSPI;
	// send to server
	_physicalStateObj.WritePacket(TdsEnums.HARDFLUSH);
	_physicalStateObj.SniContext = outerContext;

The first bytes of that array are then replaced with the negTokenResp.

But as the array has some bytes set from the beginning, there might be some additional data after the actual negTokenResp, which causes the MIT-krb5 implementation to fail with a DEFECTIVE-TOKEN here.

Exception message:
Cannot authenticate using Kerberos. Ensure Kerberos has been initialized on the client with 'kinit' and a Service Principal Name has been registered for the SQL Server to allow Kerberos authentication.\nErrorCode=InternalError, Exception=Interop+NetSecurityNative+GssApiException: GSSAPI operation failed with error - Invalid token was supplied (Unknown error).\n  

Stack trace:
   at System.Net.Security.NegotiateStreamPal.GssInitSecurityContext(SafeGssContextHandle& context, SafeGssCredHandle credential, Boolean isNtlm, SafeGssNameHandle targetName, GssFlags inFlags, Byte[] buffer, Byte[]& outputBuffer, UInt32& outFlags, Int32& isNtlmUsed)\n  
   at System.Net.Security.NegotiateStreamPal.EstablishSecurityContext(SafeFreeNegoCredentials credential, SafeDeleteContext& context, String targetName, ContextFlagsPal inFlags, SecurityBuffer inputBuffer, SecurityBuffer outputBuffer, ContextFlagsPal& outFlags)\n  
   at Microsoft.Data.SqlClient.SNI.SNIProxy.GenSspiClientContext(SspiClientContextStatus sspiClientContextStatus, Byte[] receivedBuff, Byte[]& sendBuff, Byte[][] serverName)\n  
   at Microsoft.Data.SqlClient.SNI.TdsParserStateObjectManaged.GenerateSspiClientContext(Byte[] receivedBuff, UInt32 receivedLength, Byte[]& sendBuff, UInt32& sendLength, Byte[][] _sniSpnBuffer)\n  
   at Microsoft.Data.SqlClient.TdsParser.SSPIData(Byte[] receivedBuff, UInt32 receivedLength, Byte[]& sendBuff, UInt32& sendLength)

To reproduce

I'm using an application with multiple connection-strings which - for testing purposes - queries the database very often, afterwards it invokes SqlConnection.ClearAllPools();. And this is done in an endless loop -> This causes the client to establish new security-contexts heavily which causes the error to appear.

Expected behavior

Only the negTokenResp bytes are sent to the underlying GSSAPI implementation

Further technical details

Microsoft.Data.SqlClient version: (found on the nuget or Microsoft.Data.SqlClient.dll)
.NET target: (e.g. .NET6)
SQL Server version: Microsoft SQL Server 2019
Operating system: aspnet:7.0-bookworm-slim

Additional context

I event tried the latest debian distro, but there the latest krb5 version is 1.20 which still does that check. In 1.21 they kind of revamped that check, but it's not available for debian as it stands now.

[vonzshik]

The problem is in GenerateSspiClientContext, which while does receive the size of the array (receivedLength), but completely ignores it.

SqlClient/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs

Lines 392 to 414 in faf9b95

	internal override uint GenerateSspiClientContext(byte[] receivedBuff,
	uint receivedLength,
	ref byte[] sendBuff,
	ref uint sendLength,
	byte[][] _sniSpnBuffer)
	{
	#if NET7_0_OR_GREATER
	_negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = Encoding.Unicode.GetString(_sniSpnBuffer[0]) });
	sendBuff = _negotiateAuth.GetOutgoingBlob(receivedBuff, out NegotiateAuthenticationStatusCode statusCode)!;
	SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {0}, StatusCode={1}", _sessionHandle?.ConnectionId, statusCode);
	if (statusCode is not NegotiateAuthenticationStatusCode.Completed and not NegotiateAuthenticationStatusCode.ContinueNeeded)
	{
	throw new InvalidOperationException(SQLMessage.SSPIGenerateError() + Environment.NewLine + statusCode);
	}
	#else
	_sspiClientContextStatus ??= new SspiClientContextStatus();
	SNIProxy.GenSspiClientContext(_sspiClientContextStatus, receivedBuff, ref sendBuff, _sniSpnBuffer);
	SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObjectManaged.GenerateSspiClientContext | Info | Session Id {0}", _sessionHandle?.ConnectionId);
	#endif
	sendLength = (uint)(sendBuff != null ? sendBuff.Length : 0);
	return 0;
	}

[stuka120]

If we need to respect the actual size of the negTokenResp, which is available in receivedLength, does it then make sense to use the ArrayPool.Shared.Rent(...) mechanism at all?

Maybe a byte[] receivedBuff = new byte[receivedLength];, like it has been done in the old System.Data.SqlClient#TdsParser would be a better fit here?

[JRahnama]

PR #2447 is under review to solve this issue.