diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs index a4be717e6c59c..f9d3afe570af6 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs @@ -71,7 +71,7 @@ internal static SslPolicyErrors VerifyCertificateProperties( { long chainSize = Interop.AppleCrypto.X509ChainGetChainSize(chainHandle); - if (retrieveChainCertificates) + if (retrieveChainCertificates && chainSize > 1) { chain ??= new X509Chain(); if (chainPolicy != null) @@ -79,7 +79,9 @@ internal static SslPolicyErrors VerifyCertificateProperties( 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)); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.OSX.cs index 9cef66806ac06..314848b5b08a5 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.OSX.cs +++ b/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(), null); - } + internal static SslStreamCertificateContext Create(X509Certificate2 target) => Create(target, null, offline: false, trust: null, noOcspFetch: true); } } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index e27f6f635ac90..9e43cfe74ed88 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/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 streams = new List(); - 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(); } - using (var chain = new X509Chain()) + // 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) { - chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllFlags; - chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; - chain.ChainPolicy.DisableCertificateDownloads = false; - bool chainStatus = chain.Build(clientCertificate); - // Verify we can construct full chain - if (chain.ChainElements.Count < clientChain.Count) + using (var chain = new X509Chain()) { - throw new SkipTestException($"chain cannot be built {chain.ChainElements.Count}"); + chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllFlags; + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + chain.ChainPolicy.DisableCertificateDownloads = true; + bool chainStatus = chain.Build(clientCertificate); + if (chainStatus && chain.ChainElements.Count >= clientChain.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) { diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs index a8b4fba179d6a..aa2e8e46f23d9 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs @@ -105,12 +105,12 @@ internal static (NetworkStream ClientStream, NetworkStream ServerStream) GetConn } } - 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)