Client certs with HttpClient on Windows prevents X509Certificate2 cleanup

[bartonjs]

Due to Windows S/Channel requirements, TLS server and TLS client certificates have to have key files on disk for SslStream to make use of them. Because filling the key storage directories with discarded data is bad, a certificate imported from a PFX blob/file will delete the "temporary" key file when the SafeHandle that powers it is released.

This cleanup works on certificates used without networking, and works with direct usage of SslStream; but when used in conjunction with HttpClient the certificate's SafeHandle ends up in state 6 (Disposed, but has an outstanding reference count of 1). In my debugging I was able to track this back to a SafeFreeCredentials_SECURITY instance that was also in state 6; but I wasn't able to go back farther.

This function demonstrates that HttpClient is involved in the minimal repro. It must, of course, be run on Windows.

        private static void CertLifetimeTest()
        {
            CspParameters cspParameters = new CspParameters(24)
            {
                KeyContainerName = Guid.NewGuid().ToString("D"),
            };

            byte[] pfx;

            int before = CountAndPrintCertTempFiles("Preparing certificate");

            using (RSACryptoServiceProvider key = new RSACryptoServiceProvider(2048, cspParameters))
            {
                key.PersistKeyInCsp = false;

                CertificateRequest req = new CertificateRequest(
                    "CN=Self-Signed Client Cert",
                    key,
                    HashAlgorithmName.SHA256,
                    RSASignaturePadding.Pkcs1);

                req.CertificateExtensions.Add(
                    new X509EnhancedKeyUsageExtension(
                        new OidCollection()
                        {
                            new Oid("1.3.6.1.5.5.7.3.2", null)
                        },
                        critical: false));

                DateTimeOffset now = DateTimeOffset.UtcNow;

                using (X509Certificate2 cert = req.CreateSelfSigned(now.AddMinutes(-5), now.AddMinutes(10)))
                {
                    pfx = cert.Export(X509ContentType.Pkcs12);
                }
            }

            int after = CountAndPrintCertTempFiles("Created PFX with CAPI key");
            Evaluate(before, after);
            before = after;

            using (X509Certificate2 cert = new X509Certificate2(pfx))
            {
                int mid = CountAndPrintCertTempFiles("Opened CAPI PFX");
                Evaluate(before, mid);
            }

            after = CountAndPrintCertTempFiles("Disposed CAPI PFX");
            Evaluate(before, after);
            before = after;

            const string TargetHost = "client.badssl.com";

            using (X509Certificate2 cert = new X509Certificate2(pfx))
            using (TcpClient client = new TcpClient(TargetHost, 443))
            using (SslStream stream = new SslStream(client.GetStream()))
            {
                stream.AuthenticateAsClient(new SslClientAuthenticationOptions
                {
                    ClientCertificates = new X509CertificateCollection { cert },
                    TargetHost = TargetHost,
                });

                stream.Write("GET / HTTP/1.0\n\n"u8);

                byte[] data = new byte[100];
                int read = stream.ReadAtLeast(data, 10, throwOnEndOfStream: false);

                Console.WriteLine($"Read {read} byte(s): {Convert.ToHexString(data.AsSpan(0, read))}");
            }

            after = CountAndPrintCertTempFiles("Used certificate in SslStream");
            Evaluate(before, after);
            before = after;

            SafeHandle savedHandle;

            using (X509Certificate2 cert = new X509Certificate2(pfx))
            using (HttpClientHandler handler = new HttpClientHandler())
            {
                handler.ClientCertificates.Add(cert);
                savedHandle = NoseyGetSafeHandle(cert);

                using (HttpClient client = new HttpClient(handler))
                {
                    try
                    {
                        string resp = client.GetStringAsync("https://client.badssl.com/").Result;
                        Console.WriteLine($"HttpClient got {resp}");
                    }
                    catch (Exception e)
                    {
                        Console.WriteLine($"HttpClient got angry: {e}");
                    }
                }
            }

            after = CountAndPrintCertTempFiles("Used certificate in HttpClient");
            Evaluate(before, after);
            before = after;

            Console.WriteLine($"Saved SafeHandle state: {NoseyGetState(savedHandle)}");

            static void Evaluate(int before, int after)
            {
                int delta = after - before;

                Console.WriteLine(
                    delta switch
                    {
                        0 => "Hooray, no net new files!",
                        1 => "1 new file was gained.",
                        > 0 => $"{delta} files were gained.",
                        _ => $"{int.Abs(delta)} files were lost.  Bad experiment?",
                    });
            }

            static int CountAndPrintCertTempFiles(string msg)
            {
                WindowsIdentity id = WindowsIdentity.GetCurrent();
                string sid = id.User!.Value;
                string capiRsaPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), "Microsoft", "Crypto", "RSA", sid);
                DirectoryInfo dirInfo = new DirectoryInfo(capiRsaPath);
                IEnumerable<FileInfo> files = dirInfo.EnumerateFiles().Where(static file => file.CreationTimeUtc >= DateTime.UtcNow.Date);

                Console.WriteLine($"Total CAPI RSA key files created today after {msg}: {files.Count()}");

                return files.Count();
            }

            static SafeHandle NoseyGetSafeHandle(X509Certificate2 cert)
            {
                object pal = typeof(X509Certificate).GetProperty("Pal", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(cert);
                object handle = pal.GetType().GetField("_certContext", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(pal);

                return (SafeHandle)handle;
            }

            static int NoseyGetState(SafeHandle handle)
            {
                return (int)typeof(SafeHandle).GetField("_state", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(handle);
            }
        }

When run on my machine just now, it prints:

Total CAPI RSA key files created today after Preparing certificate: 230
Total CAPI RSA key files created today after Created PFX with CAPI key: 230
Hooray, no net new files!
Total CAPI RSA key files created today after Opened CAPI PFX: 231
1 new file was gained.
Total CAPI RSA key files created today after Disposed CAPI PFX: 230
Hooray, no net new files!
Read 100 byte(s): 485454502F312E3120343231204D6973646972656374656420526571756573740D0A5365727665723A206E67696E782F312E31302E3320285562756E7475290D0A446174653A204D6F6E2C2031352041707220323032342032323A34323A313420474D54
Total CAPI RSA key files created today after Used certificate in SslStream: 230
Hooray, no net new files!
HttpClient got angry: System.AggregateException: One or more errors occurred. (Response status code does not indicate success: 400 (Bad Request).)
 ---> System.Net.Http.HttpRequestException: Response status code does not indicate success: 400 (Bad Request).
   at System.Net.Http.HttpResponseMessage.EnsureSuccessStatusCode()
   at System.Net.Http.HttpClient.GetStringAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
   at System.Threading.Tasks.Task`1.get_Result()
   at WWDNCD.Program.CertLifetimeTest() in C:\Users\jbarton\source\repos\WWDNCD\WWDNCD\Program.cs:line 376
Total CAPI RSA key files created today after Used certificate in HttpClient: 231
1 new file was gained.
Saved SafeHandle state: 6

Ignoring the HTTP 400 failure, what the test shows is:

• CAPI key cleanup inherently works
• The PFX is causing a CAPI key to be written, and if all goes well it gets erased.
• Using the certificate directly with SslStream causes cleanup to work
• Using the certificate with HttpClient causes one net new file to be left behind at program exit.
• The SafeHandle for the last instance of the certificate has one more DangerousAddRef than it does DangerousRelease.

GC.Collect() and GC.WaitForPendingFinalizers(), run after the repro function has exited, does not result in successful cleanup. So either there's an unbalanced AddRef/Release, or the release is pending on a rooted object somewhere.

This state of affairs is causing trouble for hosted program models that execute with a very small user profile directory quota... it is filling up the key storage drive and preventing further program executions.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

[rzikm]

In this specific case, the certificate seems to be rooted in the SslSessions cache

0:000> !gcroot 256b84133e8
Caching GC roots, this may take a while.
Subsequent runs of this command will be faster.

HandleTable:
    00000256b5d91388 (strong handle)
          -> 0256b6004018     System.Object[] 
          -> 0256b8416650     System.Collections.Concurrent.ConcurrentDictionary<System.Net.Security.SslSessionsCache+SslCredKey, System.Net.Security.SafeCredentialReference> (static variable: System.Byte[].s_http11Utf8)
          -> 0256b8416a98     System.Collections.Concurrent.ConcurrentDictionary<System.Net.Security.SslSessionsCache+SslCredKey, System.Net.Security.SafeCredentialReference>+Tables 
          -> 0256b8416958     System.Collections.Concurrent.ConcurrentDictionary<System.Net.Security.SslSessionsCache+SslCredKey, System.Net.Security.SafeCredentialReference>+VolatileNode[] 
          -> 0256b841be10     System.Collections.Concurrent.ConcurrentDictionary<System.Net.Security.SslSessionsCache+SslCredKey, System.Net.Security.SafeCredentialReference>+Node 
          -> 0256b841bdf8     System.Net.Security.SafeCredentialReference 
          -> 0256b841b360     System.Net.Security.SafeFreeCredential_SECURITY 
          -> 0256b841b488     System.Security.Cryptography.X509Certificates.X509Certificate2 
          -> 0256b841b528     System.Security.Cryptography.X509Certificates.CertificatePal 
          -> 0256b841b540     Microsoft.Win32.SafeHandles.SafeCertContextHandle 
          -> 0256b84133e8     Microsoft.Win32.SafeHandles.SafeCertContextHandleWithKeyContainerDeletion 

Found 1 unique roots.

The same thing can be reproduced with plain SslStream. However, client.badssl.com sends trusted issuers list which we use to filter out certificates provided by the SslClientAuthenticationOptions.ClientCertificates collection, so in your code, no certificate is actually being sent. To repro with SslStream, just switch to LocalCertificateSelectionCallback.

    stream.AuthenticateAsClient(new SslClientAuthenticationOptions
    {
-       ClientCertificates = new X509CertificateCollection { cert },
+       LocalCertificateSelectionCallback = delegate { return cert; },
        TargetHost = TargetHost,
    });

That being said, the certificate instance is rooted because of

runtime/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs

Lines 229 to 238 in ae184d5

	// Windows can fail to get local credentials in case of TLS Resume.
	// We will store associated certificate in credentials and use it in case
	// of TLS resume. It will be disposed when the credentials are.
	if (newCredentialsRequested && sslAuthenticationOptions.CertificateContext != null)
	{
	SafeFreeCredential_SECURITY handle = (SafeFreeCredential_SECURITY)cred;
	// We need to create copy to avoid Disposal issue.
	handle.LocalCertificate = new X509Certificate2(sslAuthenticationOptions.CertificateContext.TargetCertificate);
	}

which was part of #79898

[rzikm]

The only place where handle.LocalCertificate seems to be used is in

runtime/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs

Lines 93 to 109 in d64c653

	// Check that local certificate was used by schannel.
	internal static bool IsLocalCertificateUsed(SafeFreeCredentials? _credentialsHandle, SafeDeleteContext securityContext)
	{
	SecPkgContext_SessionInfo info = default;
	// fails on Server 2008 and older. We will fall-back to probing LOCAL_CERT_CONTEXT in that case.
	if (SSPIWrapper.QueryBlittableContextAttributes(
	GlobalSSPI.SSPISecureChannel,
	securityContext,
	Interop.SspiCli.ContextAttribute.SECPKG_ATTR_SESSION_INFO,
	ref info) &&
	((SecPkgContext_SessionInfo.Flags)info.dwFlags).HasFlag(SecPkgContext_SessionInfo.Flags.SSL_SESSION_RECONNECT))
	{
	// This is TLS Resumed session. Windows can fail to query the local cert bellow.
	// Instead, we will determine the usage form used credentials.
	SafeFreeCredential_SECURITY creds = (SafeFreeCredential_SECURITY)_credentialsHandle!;
	return creds.LocalCertificate != null;
	}

since the actual value is not used anywhere, I think we might just switch to a bool instead of X509Certificate reference.

cc @wfurt.