Memory leak in WebListener when using NTLM

[vanjar]

Memory leak occurs in lsass.exe process when using WebListener (.NET Core 1.1). Authentication scheme must be set to AuthenticationSchemes.NTLM.

Client is sending requests using System.Net.HttpClient (.NET 4.6) with credentials set to CredentialCache.DefaultCredentials.

Client and server must be on different machines.

Server:

 public class Program
    {
        public static void Main(string[] args)
        {
            var builder = new WebHostBuilder()
                  .UseContentRoot(Directory.GetCurrentDirectory())
                  .UseStartup<Startup>()
                  .UseUrls($"http://+:8080")
                  .UseWebListener(options =>
                  {
                      options.ListenerSettings.Authentication.Schemes = AuthenticationSchemes.NTLM;
                      options.ListenerSettings.Authentication.AllowAnonymous = false;
                  });
            var host = builder.Build();
            host.Run();
        }
    }

Client:

  public class Program
    {
        private static string _url;
        private static HttpClient _client;
        public static void Main(string[] args)
        {
            _client = new HttpClient(new HttpClientHandler
            {
                Credentials = CredentialCache.DefaultCredentials
            });
            _url = "http://192.166.1.122:8080/";
            try
            {
                for (int i = 0; i < 10000; i++)
                    using (HttpResponseMessage response = _client.GetAsync(_url).Result)
                        Console.WriteLine($"StatusCode: {response.StatusCode}");
            }
            catch (Exception e)
            {
                Console.WriteLine(e);
            }
        }
    }

Sample solution attached:
WebListenerNtlmLsassMemoryLeak.zip

[vanjar]

After further investigation (rel/1.1.0 and also dev branch), we found two problems that were causing leaks. Following fixes are for rel/1.1.0 branch.

1. Disposing of User.Identity in Microsoft.Net.Http.Server.Request was left to GC and that was causing temporary memory leak. We fixed that by adding (User?.Identity as IDisposable)?.Dispose(); in Dispose() function of Microsoft.Net.Http.Server.Request.

 internal void Dispose()
{
    // TODO: Verbose log
    _isDisposed = true;
    _nativeRequestContext.Dispose();
    if (_nativeStream != null)
    {
        _nativeStream.Dispose();
    }
    (User?.Identity as IDisposable)?.Dispose();
}

commit

2. The second one was a little more serious, because it was actually causing handle leak and memory leak in lsass.exe and we were forced to restart our server application periodically.

After successful NTLM challenge an access token is created but never disposed. According to Http Server API version 2.0 documentation, section AccessToken (link), handle should always be closed when it is no longer required.

We fixed that by adding ReleaseAccessToken() function in Microsoft.Net.Http.Server.NativeRequestContext and calling it in ReleasePins() function.

internal void ReleasePins()
{
    Debug.Assert(_nativeRequest != null || _backingBuffer == null, "RequestContextBase::ReleasePins()|ReleasePins() called twice.");
    ReleaseAccessTokens();
    _originalBufferAddress = (IntPtr)_nativeRequest;
    _nativeRequest = null;
    _nativeOverlapped?.Dispose();
    _nativeOverlapped = null;
}

private void ReleaseAccessTokens()
{
    var requestInfo = NativeRequestV2->pRequestInfo;
    var infoCount = NativeRequestV2->RequestInfoCount;
    for (int i = 0; i < infoCount; i++)
    {
        var info = &requestInfo[i];
        if (info->pInfo != null)
        {
            var accessToken = info->pInfo->AccessToken;
            if (accessToken != IntPtr.Zero)
                UnsafeNclNativeMethods.SafeNetHandles.CloseHandle(accessToken);
        }
    }
}

commit

[Tratcher]

Conceptually this makes sense, but I haven't been able to reproduce the leak on Win10 Insiders branch. What OS version are you using? I would expect Http.Sys to close the AccessToken handle at the end of the request. "This token is valid only for the lifetime of the request."

[vanjar]

Leaks are happening only when server and client are on different machines. We were not able to reproduce it when both were on localhost.
Testing environments for server were Microsoft Windows 10 Enterprise (10.0.14393 N/A Build 14393) and Windows Server 2012 R2 (6.3.9600 N/A Build 9600)

[Tratcher]

I was using different machines.

[vanjar]

Are you trying to reproduce bug on example provided? If not, please configure HttpClient, so that NTLM negotiation occurs on every request:

 new HttpClient(new WebRequestHandler
            {
                UnsafeAuthenticatedConnectionSharing = false
            });

[Tratcher]

That made a difference. Over lunch lsass.exe leaked about 165,000 handles and 75mb.

[Tratcher]

Preview builds of this patch fix should be available on the following feeds:

• https://dotnet.myget.org/gallery/aspnet-1-0-5-may2017-patch-public (for fixes in the 1.0.x release)
  • Feed URL to add to NuGet.config: https://dotnet.myget.org/F/aspnet-1-0-5-may2017-patch-public/api/v3/index.json
• https://dotnet.myget.org/gallery/aspnet-1-1-2-may2017-patch-public (for fixes in the 1.1.x release)
  • Feed URL to add to NuGet.config: https://dotnet.myget.org/F/aspnet-1-1-2-may2017-patch-public/api/v3/index.json

If you have a chance, please try out these preview builds and let us know if you have any feedback!