fix handling of client cert on macOS (#73574)

* fix handling of client cert on macOS

* update test

* fix test check

* add debug for building chain

* increase retry

* more debug

* more debug

* more debug

* log ChainStatus

* update test

* add comment

src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs

@@ -71,7 +71,7 @@ internal static partial class CertificateValidationPal
             {
                 long chainSize = Interop.AppleCrypto.X509ChainGetChainSize(chainHandle);
 
-                if (retrieveChainCertificates)
+                if (retrieveChainCertificates && chainSize > 1)
                 {
                     chain ??= new X509Chain();
                     if (chainPolicy != null)
@@ -79,7 +79,9 @@ internal static partial class CertificateValidationPal
                         chain.ChainPolicy = chainPolicy;
                     }
 
-                    for (int i = 0; i < chainSize; i++)
+                    // First certificate is peer's certificate.
+                    // Any any additional intermediate CAs to ExtraStore.
+                    for (int i = 1; i < chainSize; i++)
                     {
                         IntPtr certHandle = Interop.AppleCrypto.X509ChainGetCertificateAtIndex(chainHandle, i);
                         chain.ChainPolicy.ExtraStore.Add(new X509Certificate2(certHandle));

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.OSX.cs

@@ -17,10 +17,6 @@ private SslStreamCertificateContext(X509Certificate2 target, X509Certificate2[]
             Trust = trust;
         }
 
-        internal static SslStreamCertificateContext Create(X509Certificate2 target)
-        {
-            // On OSX we do not need to build chain unless we are asked for it.
-            return new SslStreamCertificateContext(target, Array.Empty<X509Certificate2>(), null);
-        }
+        internal static SslStreamCertificateContext Create(X509Certificate2 target) => Create(target, null, offline: false, trust: null, noOcspFetch: true);
     }
 }

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

@@ -856,15 +856,19 @@ public async Task SslStream_UntrustedCaWithCustomCallback_Throws(bool customCall
             }
         }
 
-        [ConditionalFact]
+        [Fact]
         [SkipOnPlatform(TestPlatforms.Android, "Self-signed certificates are rejected by Android before the .NET validation is reached")]
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/73862")]
         public async Task SslStream_ClientCertificate_SendsChain()
         {
+            // macOS ignores CertificateAuthority
+            // https://github.com/dotnet/runtime/issues/48207
+            StoreName storeName = OperatingSystem.IsMacOS() ? StoreName.My : StoreName.CertificateAuthority;
             List<SslStream> streams = new List<SslStream>();
-            TestHelper.CleanupCertificates();
-            (X509Certificate2 clientCertificate, X509Certificate2Collection clientChain) = TestHelper.GenerateCertificates("SslStream_ClinetCertificate_SendsChain", serverCertificate: false);
+            TestHelper.CleanupCertificates(nameof(SslStream_ClientCertificate_SendsChain), storeName);
+            (X509Certificate2 clientCertificate, X509Certificate2Collection clientChain) = TestHelper.GenerateCertificates(nameof(SslStream_ClientCertificate_SendsChain), serverCertificate: false);
 
-            using (X509Store store = new X509Store(StoreName.CertificateAuthority, StoreLocation.CurrentUser))
+            using (X509Store store = new X509Store(storeName, StoreLocation.CurrentUser))
             {
                 // add chain certificate so we can construct chain since there is no way how to pass intermediates directly.
                 store.Open(OpenFlags.ReadWrite);
@@ -872,19 +876,30 @@ public async Task SslStream_ClientCertificate_SendsChain()
                 store.Close();
             }
 
+            // make sure we can build chain. There may be some race conditions after certs being added to the store.
+            int retries = 10;
+            int delay = 100;
+            while (retries > 0)
+            {
                 using (var chain = new X509Chain())
                 {
                     chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllFlags;
                     chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
-                chain.ChainPolicy.DisableCertificateDownloads = false;
+                    chain.ChainPolicy.DisableCertificateDownloads = true;
                     bool chainStatus = chain.Build(clientCertificate);
-                // Verify we can construct full chain
-                if (chain.ChainElements.Count < clientChain.Count)
+                    if (chainStatus && chain.ChainElements.Count >= clientChain.Count)
                     {
-                    throw new SkipTestException($"chain cannot be built {chain.ChainElements.Count}");
+                        break;
                     }
                 }
 
+                Thread.Sleep(delay);
+                delay *= 2;
+                retries--;
+            }
+
+            Assert.NotEqual(0, retries);
+
             var clientOptions = new SslClientAuthenticationOptions() { TargetHost = "localhost" };
             clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true;
             clientOptions.LocalCertificateSelectionCallback = (sender, target, certificates, remoteCertificate, issuers) => clientCertificate;
@@ -902,7 +917,9 @@ public async Task SslStream_ClientCertificate_SendsChain()
                     _output.WriteLine("received {0}", c.Subject);
                 }
 
-                Assert.Equal(clientChain.Count - 1, chain.ChainPolicy.ExtraStore.Count);
+                // We may get completed chain from OS even if client sent less.
+                // As minimum, it should send more than the leaf cert
+                Assert.True(chain.ChainPolicy.ExtraStore.Count >= clientChain.Count - 1);
                 Assert.Contains(clientChain[0], chain.ChainPolicy.ExtraStore);
                 return true;
             };
@@ -923,7 +940,7 @@ public async Task SslStream_ClientCertificate_SendsChain()
                 streams.Add(server);
             }
 
-            TestHelper.CleanupCertificates();
+            TestHelper.CleanupCertificates(nameof(SslStream_ClientCertificate_SendsChain), storeName);
             clientCertificate.Dispose();
             foreach (X509Certificate c in clientChain)
             {

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

@@ -105,12 +105,12 @@ internal static async Task<(NetworkStream ClientStream, NetworkStream ServerStre
             }
         }
 
-        internal static void CleanupCertificates([CallerMemberName] string? testName = null)
+        internal static void CleanupCertificates([CallerMemberName] string? testName = null, StoreName storeName = StoreName.CertificateAuthority)
         {
             string caName = $"O={testName}";
             try
             {
-                using (X509Store store = new X509Store(StoreName.CertificateAuthority, StoreLocation.LocalMachine))
+                using (X509Store store = new X509Store(storeName, StoreLocation.LocalMachine))
                 {
                     store.Open(OpenFlags.ReadWrite);
                     foreach (X509Certificate2 cert in store.Certificates)
@@ -127,7 +127,7 @@ internal static void CleanupCertificates([CallerMemberName] string? testName = n
 
             try
             {
-                using (X509Store store = new X509Store(StoreName.CertificateAuthority, StoreLocation.CurrentUser))
+                using (X509Store store = new X509Store(storeName, StoreLocation.CurrentUser))
                 {
                     store.Open(OpenFlags.ReadWrite);
                     foreach (X509Certificate2 cert in store.Certificates)