add better handling of SECBUFFER_EXTRA during TLS handshake on Windows

src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs

@@ -421,7 +421,7 @@ internal static unsafe int InitializeSecurityContext(
  }
 
  // Optional output buffer that may need to be freed.
- SafeFreeContextBuffer? outFreeContextBuffer = null;
+ IntPtr outoutBuffer = IntPtr.Zero;
  try
  {
  Span<Interop.SspiCli.SecBuffer> inUnmanagedBuffer = stackalloc Interop.SspiCli.SecBuffer[3];
@@ -494,11 +494,6 @@ internal static unsafe int InitializeSecurityContext(
  IntPtr.Zero :
  (IntPtr)(pinnedOutBytes + outSecBuffer.offset);
 
- if (isSspiAllocated)
- {
- outFreeContextBuffer = SafeFreeContextBuffer.CreateEmptyHandle();
- }
-
  if (refContext == null || refContext.IsInvalid)
  {
  // Previous versions unconditionally built a new "refContext" here, but would pass
@@ -526,23 +521,94 @@ internal static unsafe int InitializeSecurityContext(
  refContext!,
  ref outSecurityBufferDescriptor,
  ref outFlags,
- outFreeContextBuffer);
+ null);
+
+ if (isSspiAllocated)
+ {
+ outoutBuffer = outUnmanagedBuffer.pvBuffer;
+ }
+
+ // Get unmanaged buffer with index 0 as the only one passed into PInvoke.
+ outSecBuffer.size = outUnmanagedBuffer.cbBuffer;
+ outSecBuffer.type = outUnmanagedBuffer.BufferType;
+ outSecBuffer.token = outSecBuffer.size > 0 ?
+ new Span<byte>((byte*)outUnmanagedBuffer.pvBuffer, outUnmanagedBuffer.cbBuffer).ToArray() :
+ null;
+
+ if (inSecBuffers.Count > 1 && inUnmanagedBuffer[1].BufferType == SecurityBufferType.SECBUFFER_EXTRA && inSecBuffers._item1.Type == SecurityBufferType.SECBUFFER_EMPTY)
+ {
+ // OS function did not use all provided data and turned EMPTY to EXTRA
+ // https://docs.microsoft.com/en-us/windows/win32/secauthn/extra-buffers-returned-by-schannel
+
+ int leftover = inUnmanagedBuffer[1].cbBuffer;
+ int processed = inSecBuffers._item0.Token.Length - inUnmanagedBuffer[1].cbBuffer;
+
+ /* skip over processed data and try it again. */
+ inUnmanagedBuffer[0].cbBuffer = leftover;
+ inUnmanagedBuffer[0].pvBuffer = inUnmanagedBuffer[0].pvBuffer + processed;
+ inUnmanagedBuffer[1].BufferType = SecurityBufferType.SECBUFFER_EMPTY;
+ inUnmanagedBuffer[1].cbBuffer = 0;
+
+ outUnmanagedBuffer.cbBuffer = 0;
+
+ if (outoutBuffer != IntPtr.Zero)
+ {
+ Interop.SspiCli.FreeContextBuffer(outoutBuffer);
+ outoutBuffer = IntPtr.Zero;
+ }
+
+ errorCode = MustRunInitializeSecurityContext(
+ ref inCredentials,
+ isContextAbsent,
+ (byte*)(((object)targetName == (object)dummyStr) ? null : namePtr),
+ inFlags,
+ endianness,
+ &inSecurityBufferDescriptor,
+ refContext!,
+ ref outSecurityBufferDescriptor,
+ ref outFlags,
+ null);
+
+ if (isSspiAllocated)
+ {
+ outoutBuffer = outUnmanagedBuffer.pvBuffer;
+ }
+
+ if (outUnmanagedBuffer.cbBuffer > 0)
+ {
+ if (outSecBuffer.size == 0)
+ {
+ // We did not get anything in the first round.
+ outSecBuffer.size = outUnmanagedBuffer.cbBuffer;
+ outSecBuffer.type = outUnmanagedBuffer.BufferType;
+ outSecBuffer.token = new Span<byte>((byte*)outUnmanagedBuffer.pvBuffer, outUnmanagedBuffer.cbBuffer).ToArray();
+ }
+ else
+ {
+ byte[] buffer = new byte[outSecBuffer.size + outUnmanagedBuffer.cbBuffer];
+ Buffer.BlockCopy(outSecBuffer.token!, 0, buffer, 0, outSecBuffer.size);
+ new Span<byte>((byte*)outUnmanagedBuffer.pvBuffer, outUnmanagedBuffer.cbBuffer).CopyTo(new Span<byte>(buffer, outSecBuffer.size, outUnmanagedBuffer.cbBuffer));
+ outSecBuffer.size = buffer.Length;
+ outSecBuffer.token = buffer;
+ }
+ }
+
+ if (inUnmanagedBuffer[1].BufferType == SecurityBufferType.SECBUFFER_EXTRA)
+ {
+ // we are left with unprocessed data again. fail with SEC_E_INCOMPLETE_MESSAGE hResult.
+ errorCode = unchecked((int)0x80090318);
+ }
+ }
  }
-
- if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(null, "Marshalling OUT buffer");
-
- // Get unmanaged buffer with index 0 as the only one passed into PInvoke.
- outSecBuffer.size = outUnmanagedBuffer.cbBuffer;
- outSecBuffer.type = outUnmanagedBuffer.BufferType;
- outSecBuffer.token = outSecBuffer.size > 0 ?
- new Span<byte>((byte*)outUnmanagedBuffer.pvBuffer, outUnmanagedBuffer.cbBuffer).ToArray() :
- null;
  }
  }
  }
  finally
  {
- outFreeContextBuffer?.Dispose();
+ if (outoutBuffer != IntPtr.Zero)
+ {
+ Interop.SspiCli.FreeContextBuffer(outoutBuffer);
+ }
  }
 
  return errorCode;
@@ -671,8 +737,6 @@ internal static unsafe int AcceptSecurityContext(
  isContextAbsent = refContext._handle.IsZero;
  }
 
- // Optional output buffer that may need to be freed.
- SafeFreeContextBuffer? outFreeContextBuffer = null;
  Span<Interop.SspiCli.SecBuffer> outUnmanagedBuffer = stackalloc Interop.SspiCli.SecBuffer[2];
  outUnmanagedBuffer[1].pvBuffer = IntPtr.Zero;
  try
@@ -752,11 +816,6 @@ internal static unsafe int AcceptSecurityContext(
  outUnmanagedBuffer[1].cbBuffer = 0;
  outUnmanagedBuffer[1].BufferType = SecurityBufferType.SECBUFFER_ALERT;
 
- if (isSspiAllocated)
- {
- outFreeContextBuffer = SafeFreeContextBuffer.CreateEmptyHandle();
- }
-
  if (refContext == null || refContext.IsInvalid)
  {
  // Previous versions unconditionally built a new "refContext" here, but would pass
@@ -775,31 +834,85 @@ internal static unsafe int AcceptSecurityContext(
  refContext!,
  ref outSecurityBufferDescriptor,
  ref outFlags,
- outFreeContextBuffer);
-
- if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(null, "Marshaling OUT buffer");
+ null);
 
  // No data written out but there is Alert
- if (outUnmanagedBuffer[0].cbBuffer == 0 && outUnmanagedBuffer[1].cbBuffer > 0)
- {
- outSecBuffer.size = outUnmanagedBuffer[1].cbBuffer;
- outSecBuffer.type = outUnmanagedBuffer[1].BufferType;
- outSecBuffer.token = new Span<byte>((byte*)outUnmanagedBuffer[1].pvBuffer, outUnmanagedBuffer[1].cbBuffer).ToArray();
- }
- else
+ int index = outUnmanagedBuffer[0].cbBuffer == 0 && outUnmanagedBuffer[1].cbBuffer > 0 ? 1 : 0;
+
+ outSecBuffer.size = outUnmanagedBuffer[index].cbBuffer;
+ outSecBuffer.type = outUnmanagedBuffer[index].BufferType;
+ outSecBuffer.token = outSecBuffer.size > 0 ?
+ new Span<byte>((byte*)outUnmanagedBuffer[index].pvBuffer, outUnmanagedBuffer[0].cbBuffer).ToArray() :
+ null;
+
+ if (inSecBuffers.Count > 1 && inUnmanagedBuffer[1].BufferType == SecurityBufferType.SECBUFFER_EXTRA && inSecBuffers._item1.Type == SecurityBufferType.SECBUFFER_EMPTY)
  {
- outSecBuffer.size = outUnmanagedBuffer[0].cbBuffer;
- outSecBuffer.type = outUnmanagedBuffer[0].BufferType;
- outSecBuffer.token = outUnmanagedBuffer[0].cbBuffer > 0 ?
- new Span<byte>((byte*)outUnmanagedBuffer[0].pvBuffer, outUnmanagedBuffer[0].cbBuffer).ToArray() :
- null;
+ // OS function did not use all provided data and turned EMPTY to EXTRA
+ // https://docs.microsoft.com/en-us/windows/win32/secauthn/extra-buffers-returned-by-schannel
+
+ int leftover = inUnmanagedBuffer[1].cbBuffer;
+ int processed = inSecBuffers._item0.Token.Length - inUnmanagedBuffer[1].cbBuffer;
+
+ /* skip over processed data and try it again. */
+ inUnmanagedBuffer[0].cbBuffer = leftover;
+ inUnmanagedBuffer[0].pvBuffer = inUnmanagedBuffer[0].pvBuffer + processed;
+ inUnmanagedBuffer[1].BufferType = SecurityBufferType.SECBUFFER_EMPTY;
+ inUnmanagedBuffer[1].cbBuffer = 0;
+
+ outUnmanagedBuffer[0].cbBuffer = 0;
+ if (isSspiAllocated && outUnmanagedBuffer[0].pvBuffer != IntPtr.Zero)
+ {
+ Interop.SspiCli.FreeContextBuffer(outUnmanagedBuffer[0].pvBuffer);
+ outUnmanagedBuffer[0].pvBuffer = IntPtr.Zero;
+ }
+
+ errorCode = MustRunAcceptSecurityContext_SECURITY(
+ ref inCredentials,
+ isContextAbsent,
+ &inSecurityBufferDescriptor,
+ inFlags,
+ endianness,
+ refContext!,
+ ref outSecurityBufferDescriptor,
+ ref outFlags,
+ null);
+
+ index = outUnmanagedBuffer[0].cbBuffer == 0 && outUnmanagedBuffer[1].cbBuffer > 0 ? 1 : 0;
+ if (outUnmanagedBuffer[index].cbBuffer > 0)
+ {
+ if (outSecBuffer.size == 0)
+ {
+ // We did not get anything in the first round.
+ outSecBuffer.size = outUnmanagedBuffer[index].cbBuffer;
+ outSecBuffer.type = outUnmanagedBuffer[index].BufferType;
+ outSecBuffer.token = new Span<byte>((byte*)outUnmanagedBuffer[index].pvBuffer, outUnmanagedBuffer[index].cbBuffer).ToArray();
+ }
+ else
+ {
+ byte[] buffer = new byte[outSecBuffer.size + outUnmanagedBuffer[index].cbBuffer];
+ Buffer.BlockCopy(outSecBuffer.token!, 0, buffer, 0, outSecBuffer.size);
+ new Span<byte>((byte*)outUnmanagedBuffer[index].pvBuffer, outUnmanagedBuffer[index].cbBuffer).CopyTo(new Span<byte>(buffer, outSecBuffer.size, outUnmanagedBuffer[index].cbBuffer));
+ outSecBuffer.size = buffer.Length;
+ outSecBuffer.token = buffer;
+ }
+ }
+
+ if (inUnmanagedBuffer[1].BufferType == SecurityBufferType.SECBUFFER_EXTRA)
+ {
+ // we are left with unprocessed data again. fail with SEC_E_INCOMPLETE_MESSAGE hResult.
+ errorCode = unchecked((int)0x80090318);
+ }
  }
  }
  }
  }
  finally
  {
- outFreeContextBuffer?.Dispose();
+ if (isSspiAllocated && outUnmanagedBuffer[0].pvBuffer != IntPtr.Zero)
+ {
+ Interop.SspiCli.FreeContextBuffer(outUnmanagedBuffer[0].pvBuffer);
+ }
+
  if (outUnmanagedBuffer[1].pvBuffer != IntPtr.Zero)
  {
  Interop.SspiCli.FreeContextBuffer(outUnmanagedBuffer[1].pvBuffer);

src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationRemoteServer.cs

@@ -65,7 +65,28 @@ public async Task CertificateValidationRemoteServer_EndToEnd_Ok(bool useAsync)
  [InlineData("www.apple.com")]
  [InlineData("www.icloud.com")]
  [PlatformSpecific(TestPlatforms.OSX)]
- public async Task CertificateValidationApple_EndToEnd_Ok(string host)
+ public Task CertificateValidationApple_EndToEnd_Ok(string host)
+ {
+ return EndToEndHelper(host);
+ }
+
+ [ConditionalTheory]
+ [OuterLoop("Uses external servers")]
+ [InlineData("api.nuget.org")]
+ [InlineData("")]
+ public async Task DefaultConnect_EndToEnd_Ok(string host)
+ {
+ if (string.IsNullOrEmpty(host))
+ {
+ host = Configuration.Security.TlsServer.IdnHost;
+ }
+
+ await EndToEndHelper(host);
+ // Second try may change the handshake because of TLS resume.
+ await EndToEndHelper(host);
+ }
+
+ private async Task EndToEndHelper(string host)
  {
  using (var client = new TcpClient())
  {

src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamStreamToStreamTest.cs

@@ -95,7 +95,14 @@ public async Task SslStream_StreamToStream_Authentication_IncorrectServerName_Fa
  Task t2 = server.AuthenticateAsServerAsync(certificate);
 
  await Assert.ThrowsAsync<AuthenticationException>(() => t1);
- await t2;
+ try
+ {
+ await t2;
+ }
+ catch
+ {
+ // Ignore outcome of t2. It can succeed or fail depending on timing.
+ }
  }
  }