From a0d76d8c97137fbaa0c688dd022c1afdb1e74ec2 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Fri, 11 Feb 2022 17:24:24 +0100 Subject: [PATCH 01/15] Send CA names on Linux --- .../Interop.OpenSsl.cs | 74 +++++++---- .../Interop.Ssl.cs | 3 + .../Net/Security/SslCertificateTrust.cs | 4 +- .../Net/Security/SslStream.Implementation.cs | 5 + .../SslStreamCertificateTrustTests.cs | 122 ++++++++++++++++++ .../System.Net.Security.Tests.csproj | 1 + .../entrypoints.c | 1 + .../opensslshim.h | 2 + .../pal_ssl.c | 10 ++ .../pal_ssl.h | 9 ++ 10 files changed, 201 insertions(+), 30 deletions(-) create mode 100644 src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index ed807385c9b574..9bfce9593af4d7 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -12,6 +12,7 @@ using System.Security.Authentication; using System.Security.Authentication.ExtendedProtection; using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; using Microsoft.Win32.SafeHandles; internal static partial class Interop @@ -46,32 +47,32 @@ internal static partial class OpenSsl return bindingHandle; } - private static volatile int s_disableTlsResume = -1; - - private static bool DisableTlsResume - { - get - { - int disableTlsResume = s_disableTlsResume; - if (disableTlsResume != -1) - { - return disableTlsResume != 0; - } - - // First check for the AppContext switch, giving it priority over the environment variable. - if (AppContext.TryGetSwitch(DisableTlsResumeCtxSwitch, out bool value)) - { - s_disableTlsResume = value ? 1 : 0; - } - else - { - // AppContext switch wasn't used. Check the environment variable. + private static volatile int s_disableTlsResume = -1; + + private static bool DisableTlsResume + { + get + { + int disableTlsResume = s_disableTlsResume; + if (disableTlsResume != -1) + { + return disableTlsResume != 0; + } + + // First check for the AppContext switch, giving it priority over the environment variable. + if (AppContext.TryGetSwitch(DisableTlsResumeCtxSwitch, out bool value)) + { + s_disableTlsResume = value ? 1 : 0; + } + else + { + // AppContext switch wasn't used. Check the environment variable. s_disableTlsResume = Environment.GetEnvironmentVariable(DisableTlsResumeEnvironmentVariable) is string envVar && (envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase)) ? 1 : 0; - } + } - return s_disableTlsResume != 0; + return s_disableTlsResume != 0; } } @@ -147,7 +148,7 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c // Sets policy and security level if (!Ssl.SetEncryptionPolicy(sslCtx, sslAuthenticationOptions.EncryptionPolicy)) { - throw new SslException( SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); + throw new SslException(SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); } } @@ -274,7 +275,7 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia if (cacheSslContext) { - sslAuthenticationOptions.CertificateContext!.SslContexts!.TryGetValue(protocols | (SslProtocols)(hasAlpn ? 1 : 0), out sslCtxHandle); + sslAuthenticationOptions.CertificateContext!.SslContexts!.TryGetValue(protocols | (SslProtocols)(hasAlpn ? 1 : 0), out sslCtxHandle); } if (sslCtxHandle == null) @@ -339,10 +340,27 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia // if server actually requests a certificate. Ssl.SslSetClientCertCallback(sslHandle, 1); } - - if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.RemoteCertRequired) + else // sslAuthenticationOptions.IsServer { - Ssl.SslSetVerifyPeer(sslHandle); + if (sslAuthenticationOptions.RemoteCertRequired) + { + Ssl.SslSetVerifyPeer(sslHandle); + } + + if (sslAuthenticationOptions.CertificateContext?.Trust?._sendTrustInHandshake == true) + { + SslCertificateTrust trust = sslAuthenticationOptions.CertificateContext!.Trust!; + X509Certificate2Collection certList = (trust._trustList ?? trust._store!.Certificates); + + Debug.Assert(certList != null, "certList != null"); + foreach (X509Certificate2 cert in certList) + { + if (!Ssl.SslAddClientCA(sslHandle, cert.Handle)) + { + Debug.Fail("Failed to add issuer to trusted CA list."); + } + } + } } } catch @@ -576,7 +594,7 @@ private static unsafe int AlpnServerSelectCallback(IntPtr ssl, byte** outp, byte { *outp = null; *outlen = 0; - IntPtr sslData = Ssl.SslGetData(ssl); + IntPtr sslData = Ssl.SslGetData(ssl); // reset application data to avoid dangling pointer. Ssl.SslSetData(ssl, IntPtr.Zero); diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index d599fe7ecf7372..10d49f15a204fd 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -221,6 +221,9 @@ internal static unsafe int SslSetAlpnProtos(SafeSslHandle ssl, Span serial [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslAddExtraChainCert")] internal static partial bool SslAddExtraChainCert(SafeSslHandle ssl, SafeX509Handle x509); + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslAddClientCA")] + internal static partial bool SslAddClientCA(SafeSslHandle ssl, IntPtr x509); + internal static bool AddExtraChainCertificates(SafeSslHandle ssl, X509Certificate2[] chain) { // send pre-computed list of intermediates. diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslCertificateTrust.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslCertificateTrust.cs index 982d206c859353..0bf6019af02f32 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslCertificateTrust.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslCertificateTrust.cs @@ -21,7 +21,7 @@ public static SslCertificateTrust CreateForX509Store(X509Store store, bool sendT throw new PlatformNotSupportedException(SR.net_ssl_trust_store); } #else - if (sendTrustInHandshake) + if (sendTrustInHandshake && !System.OperatingSystem.IsLinux()) { // to be removed when implemented. throw new PlatformNotSupportedException("Not supported yet."); @@ -41,7 +41,7 @@ public static SslCertificateTrust CreateForX509Store(X509Store store, bool sendT [UnsupportedOSPlatform("windows")] public static SslCertificateTrust CreateForX509Collection(X509Certificate2Collection trustList, bool sendTrustInHandshake = false) { - if (sendTrustInHandshake) + if (sendTrustInHandshake && !System.OperatingSystem.IsLinux()) { // to be removed when implemented. throw new PlatformNotSupportedException("Not supported yet."); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index 9dc0538cfca292..67925bd9c63739 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -747,6 +747,11 @@ private async ValueTask EnsureFullTlsFrameAsync(CancellationTok return frameSize; } + if (frameSize < int.MaxValue) + { + _buffer.EnsureAvailableSpace(frameSize - _buffer.EncryptedLength); + } + while (_buffer.EncryptedLength < frameSize) { // there should be space left to read into diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs new file mode 100644 index 00000000000000..6b82867738fad5 --- /dev/null +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs @@ -0,0 +1,122 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using System.IO.Tests; +using System.Security.Authentication; +using System.Security.Cryptography.X509Certificates; +using System.Threading.Tasks; +using Xunit; + +namespace System.Net.Security.Tests +{ + using Configuration = System.Net.Test.Common.Configuration; + + public class SslStreamCertificateTrustTest + { + [Fact] + // not supported on Windows, not implemented elsewhere + [PlatformSpecific(TestPlatforms.Linux)] + public async Task SslStream_SendCertificateTrust_CertificateCollection() + { + (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); + using (client) + using (server) + using (X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate()) + using (X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate()) + { + SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions + { + ServerCertificate = serverCertificate, + ClientCertificateRequired = true, + RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, + ServerCertificateContext = SslStreamCertificateContext.Create( + serverCertificate, + null, + trust: SslCertificateTrust.CreateForX509Collection( + new X509Certificate2Collection { serverCertificate, clientCertificate }, + sendTrustInHandshake: true)) + }; + + string[] acceptableIssuers = Array.Empty(); + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions + { + TargetHost = "localhost", + EnabledSslProtocols = SslProtocols.Tls12, + RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, + LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, issuers) => + { + if (remoteCertificate == null) + { + // ignore the first call, we should receive acceptable issuers in the next one + return null; + } + + acceptableIssuers = issuers; + return clientCertificate; + }, + + }; + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout( + client.AuthenticateAsClientAsync(clientOptions), + server.AuthenticateAsServerAsync(serverOptions)); + + Assert.Equal(2, acceptableIssuers.Length); + Assert.Contains(serverCertificate.Subject, acceptableIssuers); + Assert.Contains(clientCertificate.Subject, acceptableIssuers); + } + } + + [Fact] + public async Task SslStream_SendCertificateTrust_CertificateStore() + { + (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); + using (client) + using (server) + using (X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate()) + using (X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate()) + using (X509Store store = new X509Store("Root", StoreLocation.LocalMachine)) + { + SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions + { + ServerCertificate = serverCertificate, + ClientCertificateRequired = true, + RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, + ServerCertificateContext = SslStreamCertificateContext.Create( + serverCertificate, + null, + trust: SslCertificateTrust.CreateForX509Store(store, sendTrustInHandshake: true)) + }; + + string[] acceptableIssuers = Array.Empty(); + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions + { + TargetHost = "localhost", + EnabledSslProtocols = SslProtocols.Tls12, + RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, + LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, issuers) => + { + if (remoteCertificate == null) + { + // ignore the first call, we should receive acceptable issuers in the next one + return null; + } + + acceptableIssuers = issuers; + return clientCertificate; + }, + + }; + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout( + client.AuthenticateAsClientAsync(clientOptions), + server.AuthenticateAsServerAsync(serverOptions)); + + // don't assert individual ellements, just that some issuers were sent + // we use Root cert store which should always contain at least some certs + Assert.NotEmpty(acceptableIssuers); + } + } + } +} \ No newline at end of file diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj b/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj index 8f2d6f40e17efa..6fe027ca2d2804 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj @@ -100,6 +100,7 @@ + diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index 0518fa7987b74b..7a428606855199 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -299,6 +299,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslCtxUseCertificate) DllImportEntry(CryptoNative_SslCtxUsePrivateKey) DllImportEntry(CryptoNative_SslAddExtraChainCert) + DllImportEntry(CryptoNative_SslAddClientCA) DllImportEntry(CryptoNative_SslDestroy) DllImportEntry(CryptoNative_SslDoHandshake) DllImportEntry(CryptoNative_SslGetClientCAList) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index f1b39f2f5020d4..6f60a3c78900eb 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -460,6 +460,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); LIGHTUP_FUNCTION(SSL_CIPHER_get_name) \ LIGHTUP_FUNCTION(SSL_CIPHER_get_version) \ REQUIRED_FUNCTION(SSL_ctrl) \ + REQUIRED_FUNCTION(SSL_add_client_CA) \ REQUIRED_FUNCTION(SSL_set_alpn_protos) \ REQUIRED_FUNCTION(SSL_set_quiet_shutdown) \ REQUIRED_FUNCTION(SSL_CTX_check_private_key) \ @@ -923,6 +924,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_CIPHER_get_name SSL_CIPHER_get_name_ptr #define SSL_CIPHER_get_version SSL_CIPHER_get_version_ptr #define SSL_ctrl SSL_ctrl_ptr +#define SSL_add_client_CA SSL_add_client_CA_ptr #define SSL_set_alpn_protos SSL_set_alpn_protos_ptr #define SSL_set_quiet_shutdown SSL_set_quiet_shutdown_ptr #define SSL_CTX_check_private_key SSL_CTX_check_private_key_ptr diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 5ef0e681182d35..7d1b736246fe51 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -772,6 +772,16 @@ int32_t CryptoNative_SslAddExtraChainCert(SSL* ssl, X509* x509) return 0; } +int32_t CryptoNative_SslAddClientCA(SSL* ssl, X509* x509) +{ + if (!x509 || !ssl) + { + return 0; + } + + return SSL_add_client_CA(ssl, x509); +} + void CryptoNative_SslCtxSetAlpnSelectCb(SSL_CTX* ctx, SslCtxSetAlpnCallback cb, void* arg) { // void shim functions don't lead to exceptions, so skip the unconditional error clearing. diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index 5bb758eeff16ee..409cf7d20583e6 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -405,6 +405,15 @@ Returns 1 if success and 0 in case of failure */ PALEXPORT int32_t CryptoNative_SslAddExtraChainCert(SSL* ssl, X509* x509); +/* +Adds the name of the given certificate to the list of acceptable issuers sent to +client when requesting a client certificate. Shims the SSL_add_client_CA function. + +No transfer of ownership or refcount changes. +Returns 1 if success and 0 in case of failure +*/ +PALEXPORT int32_t CryptoNative_SslAddClientCA(SSL* ssl, X509* x509); + /* Shims the ssl_ctx_set_alpn_select_cb method. */ From 23cfd08191589b5f3b1e6d6acf0e55c8365dd31f Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 14 Feb 2022 10:40:25 +0100 Subject: [PATCH 02/15] Optimize setting certname list in OpenSSL --- .../Interop.OpenSsl.cs | 19 ++++++++++++++----- .../Interop.Ssl.cs | 12 ++++++++++-- .../SslStreamCertificateTrustTests.cs | 2 -- .../entrypoints.c | 2 +- .../pal_ssl.c | 15 ++++++++++++--- .../pal_ssl.h | 4 ++-- 6 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 9bfce9593af4d7..19a3596377f43b 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -353,12 +353,21 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia X509Certificate2Collection certList = (trust._trustList ?? trust._store!.Certificates); Debug.Assert(certList != null, "certList != null"); - foreach (X509Certificate2 cert in certList) + Span handles = certList.Count <= 256 + ? stackalloc IntPtr[256] + : new IntPtr[certList.Count]; + + for (int i = 0; i < certList.Count; i++) + { + handles[i] = certList[i].Handle; + } + + if (!Ssl.SslAddClientCAs(sslHandle, handles.Slice(0, certList.Count))) { - if (!Ssl.SslAddClientCA(sslHandle, cert.Handle)) - { - Debug.Fail("Failed to add issuer to trusted CA list."); - } + // The method can fail only when the number of cert names exceeds the maximum capacity + // supported by STACK_OF(X509_NAME) structure, which should not happen under normal + // operation. + Debug.Fail("Failed to add issuer to trusted CA list."); } } } diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 10d49f15a204fd..9e2c100b2a8d5b 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -221,8 +221,16 @@ internal static unsafe int SslSetAlpnProtos(SafeSslHandle ssl, Span serial [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslAddExtraChainCert")] internal static partial bool SslAddExtraChainCert(SafeSslHandle ssl, SafeX509Handle x509); - [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslAddClientCA")] - internal static partial bool SslAddClientCA(SafeSslHandle ssl, IntPtr x509); + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslAddClientCAs")] + private static unsafe partial bool SslAddClientCAs(SafeSslHandle ssl, IntPtr* x509s, int count); + + internal static unsafe bool SslAddClientCAs(SafeSslHandle ssl, Span x509handles) + { + fixed (IntPtr* pHandles = &MemoryMarshal.GetReference(x509handles)) + { + return SslAddClientCAs(ssl, pHandles, x509handles.Length); + } + } internal static bool AddExtraChainCertificates(SafeSslHandle ssl, X509Certificate2[] chain) { diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs index 6b82867738fad5..d0325999fe6221 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs @@ -42,7 +42,6 @@ public async Task SslStream_SendCertificateTrust_CertificateCollection() SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions { TargetHost = "localhost", - EnabledSslProtocols = SslProtocols.Tls12, RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, issuers) => { @@ -93,7 +92,6 @@ public async Task SslStream_SendCertificateTrust_CertificateStore() SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions { TargetHost = "localhost", - EnabledSslProtocols = SslProtocols.Tls12, RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, issuers) => { diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index 7a428606855199..1951011467a0b9 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -299,7 +299,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslCtxUseCertificate) DllImportEntry(CryptoNative_SslCtxUsePrivateKey) DllImportEntry(CryptoNative_SslAddExtraChainCert) - DllImportEntry(CryptoNative_SslAddClientCA) + DllImportEntry(CryptoNative_SslAddClientCAs) DllImportEntry(CryptoNative_SslDestroy) DllImportEntry(CryptoNative_SslDoHandshake) DllImportEntry(CryptoNative_SslGetClientCAList) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 7d1b736246fe51..ead2acea2bb6a9 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -772,14 +772,23 @@ int32_t CryptoNative_SslAddExtraChainCert(SSL* ssl, X509* x509) return 0; } -int32_t CryptoNative_SslAddClientCA(SSL* ssl, X509* x509) +int32_t CryptoNative_SslAddClientCAs(SSL* ssl, X509** x509s, uint32_t count) { - if (!x509 || !ssl) + if (!x509s || !ssl) { return 0; } - return SSL_add_client_CA(ssl, x509); + for (uint32_t i = 0; i < count; i++) + { + int res = SSL_add_client_CA(ssl, x509s[i]); + if (res != 1) + { + return res; + } + } + + return 1; } void CryptoNative_SslCtxSetAlpnSelectCb(SSL_CTX* ctx, SslCtxSetAlpnCallback cb, void* arg) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index 409cf7d20583e6..522c170e7cf073 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -406,13 +406,13 @@ Returns 1 if success and 0 in case of failure PALEXPORT int32_t CryptoNative_SslAddExtraChainCert(SSL* ssl, X509* x509); /* -Adds the name of the given certificate to the list of acceptable issuers sent to +Adds the names of the given certificates to the list of acceptable issuers sent to client when requesting a client certificate. Shims the SSL_add_client_CA function. No transfer of ownership or refcount changes. Returns 1 if success and 0 in case of failure */ -PALEXPORT int32_t CryptoNative_SslAddClientCA(SSL* ssl, X509* x509); +PALEXPORT int32_t CryptoNative_SslAddClientCAs(SSL* ssl, X509** x509s, uint32_t count); /* Shims the ssl_ctx_set_alpn_select_cb method. From 717e8664e4b0a1ee3015961732395df0ed16448e Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 14 Feb 2022 11:35:54 +0100 Subject: [PATCH 03/15] First shot at OSX implementation --- .../Interop.Ssl.cs | 16 +++++++++ .../Security/Pal.OSX/SafeDeleteSslContext.cs | 36 +++++++++++++++++++ .../Net/Security/SslCertificateTrust.cs | 4 +-- .../System/Net/Security/SslStreamPal.OSX.cs | 17 --------- .../SslStreamCertificateTrustTests.cs | 3 +- .../pal_ssl.c | 11 +++++- .../pal_ssl.h | 7 ++++ 7 files changed, 73 insertions(+), 21 deletions(-) diff --git a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs index af6596efd51da6..fd562f83347327 100644 --- a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs @@ -161,6 +161,22 @@ private static partial int AppleCryptoNative_SslIsHostnameMatch( [GeneratedDllImport(Interop.Libraries.AppleCryptoNative, EntryPoint = "AppleCryptoNative_SslSetEnabledCipherSuites")] internal static unsafe partial int SslSetEnabledCipherSuites(SafeSslHandle sslHandle, uint* cipherSuites, int numCipherSuites); + [GeneratedDllImport(Interop.Libraries.AppleCryptoNative, EntryPoint = "AppleCryptoNative_SslSetCertificateAuthorities")] + internal static partial int SslSetCertificateAuthorities(SafeSslHandle sslHandle, SafeCreateHandle certificateOrArray, int replaceExisting); + + internal static unsafe void SslSetCertificateAuthorities(SafeSslHandle sslHandle, IntPtr[] certificates, bool replaceExisting) + { + using (SafeCreateHandle cfCertRefs = CoreFoundation.CFArrayCreate(certificates, (UIntPtr)certificates.Length)) + { + int osStatus = SslSetCertificateAuthorities(sslHandle, cfCertRefs, replaceExisting ? 1 : 0); + + if (osStatus != 0) + { + throw CreateExceptionForOSStatus(osStatus); + } + } + } + internal static void SslSetAcceptClientCert(SafeSslHandle sslHandle) { int osStatus = AppleCryptoNative_SslSetAcceptClientCert(sslHandle); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs index 9e1a445d9dc750..7f665fd0e9f2b7 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs @@ -92,6 +92,42 @@ public SafeDeleteSslContext(SafeFreeSslCredentials credential, SslAuthentication Dispose(); throw; } + + if (!string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) && !sslAuthenticationOptions.IsServer) + { + Interop.AppleCrypto.SslSetTargetName(_sslContext, sslAuthenticationOptions.TargetHost); + } + + if (sslAuthenticationOptions.CertificateContext == null && sslAuthenticationOptions.CertSelectionDelegate != null) + { + // certificate was not provided but there is user callback. We can break handshake if server asks for certificate + // and we can try to get it based on remote certificate and trusted issuers. + Interop.AppleCrypto.SslBreakOnCertRequested(_sslContext, true); + } + + if (sslAuthenticationOptions.IsServer) + { + if (sslAuthenticationOptions.RemoteCertRequired) + { + Interop.AppleCrypto.SslSetAcceptClientCert(_sslContext); + } + + if (sslAuthenticationOptions.CertificateContext?.Trust?._sendTrustInHandshake == true) + { + SslCertificateTrust trust = sslAuthenticationOptions.CertificateContext!.Trust!; + X509Certificate2Collection certList = (trust._trustList ?? trust._store!.Certificates); + + Debug.Assert(certList != null, "certList != null"); + IntPtr[] handles = new IntPtr[certList.Count]; + + for (int i = 0; i < certList.Count; i++) + { + handles[i] = certList[i].Handle; + } + + Interop.AppleCrypto.SslSetCertificateAuthorities(_sslContext, handles, true); + } + } } private static SafeSslHandle CreateSslContext(SafeFreeSslCredentials credential, bool isServer) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslCertificateTrust.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslCertificateTrust.cs index 0bf6019af02f32..479d83ec430ab7 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslCertificateTrust.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslCertificateTrust.cs @@ -21,7 +21,7 @@ public static SslCertificateTrust CreateForX509Store(X509Store store, bool sendT throw new PlatformNotSupportedException(SR.net_ssl_trust_store); } #else - if (sendTrustInHandshake && !System.OperatingSystem.IsLinux()) + if (sendTrustInHandshake && !System.OperatingSystem.IsLinux() && !System.OperatingSystem.IsMacOS()) { // to be removed when implemented. throw new PlatformNotSupportedException("Not supported yet."); @@ -41,7 +41,7 @@ public static SslCertificateTrust CreateForX509Store(X509Store store, bool sendT [UnsupportedOSPlatform("windows")] public static SslCertificateTrust CreateForX509Collection(X509Certificate2Collection trustList, bool sendTrustInHandshake = false) { - if (sendTrustInHandshake && !System.OperatingSystem.IsLinux()) + if (sendTrustInHandshake && !System.OperatingSystem.IsLinux() && !System.OperatingSystem.IsMacOS()) { // to be removed when implemented. throw new PlatformNotSupportedException("Not supported yet."); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index 6b1b13ca9f2a70..7fbea273f95ef2 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -248,23 +248,6 @@ private static SecurityStatusPal HandshakeInternal( { sslContext = new SafeDeleteSslContext((credential as SafeFreeSslCredentials)!, sslAuthenticationOptions); context = sslContext; - - if (!string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) && !sslAuthenticationOptions.IsServer) - { - Interop.AppleCrypto.SslSetTargetName(sslContext.SslContext, sslAuthenticationOptions.TargetHost); - } - - if (sslAuthenticationOptions.CertificateContext == null && sslAuthenticationOptions.CertSelectionDelegate != null) - { - // certificate was not provided but there is user callback. We can break handshake if server asks for certificate - // and we can try to get it based on remote certificate and trusted issuers. - Interop.AppleCrypto.SslBreakOnCertRequested(sslContext.SslContext, true); - } - - if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.RemoteCertRequired) - { - Interop.AppleCrypto.SslSetAcceptClientCert(sslContext.SslContext); - } } if (inputBuffer.Length > 0) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs index d0325999fe6221..67103878a0ebf9 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs @@ -16,7 +16,7 @@ public class SslStreamCertificateTrustTest { [Fact] // not supported on Windows, not implemented elsewhere - [PlatformSpecific(TestPlatforms.Linux)] + [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.OSX)] public async Task SslStream_SendCertificateTrust_CertificateCollection() { (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); @@ -68,6 +68,7 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( } [Fact] + [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux | TestPlatforms.OSX)] public async Task SslStream_SendCertificateTrust_CertificateStore() { (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); diff --git a/src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.c index 7a8a0eebe1317d..745f577adb1b7d 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.c @@ -641,8 +641,17 @@ int32_t AppleCryptoNative_SslSetEnabledCipherSuites(SSLContextRef sslContext, co } } +PALEXPORT int32_t AppleCryptoNative_SslSetCertificateAuthorities(SSLContextRef sslContext, CFArrayRef certificates, int32_t replaceExisting) +{ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + // The underlying call handles NULL inputs, so just pass it through + return SSLSetCertificateAuthorities(sslContext, certificates, replaceExisting); +#pragma clang diagnostic pop +} + __attribute__((constructor)) static void InitializeAppleCryptoSslShim() { SSLSetALPNProtocolsPtr = (OSStatus(*)(SSLContextRef, CFArrayRef))dlsym(RTLD_DEFAULT, "SSLSetALPNProtocols"); SSLCopyALPNProtocolsPtr = (OSStatus(*)(SSLContextRef, CFArrayRef*))dlsym(RTLD_DEFAULT, "SSLCopyALPNProtocols"); -} +} \ No newline at end of file diff --git a/src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.h index 2c5961a72ac095..312dacb9e7c210 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.h @@ -249,3 +249,10 @@ Sets enabled cipher suites for the current session. Returns the output of SSLSetEnabledCiphers. */ PALEXPORT int32_t AppleCryptoNative_SslSetEnabledCipherSuites(SSLContextRef sslContext, const uint32_t* cipherSuites, int32_t numCipherSuites); + +/* +Adds one or more certificates to a server's list of certification authorities (CAs) acceptable for client authentication. + +Returns the output of SSLSetCertificateAuthorities. +*/ +PALEXPORT int32_t AppleCryptoNative_SslSetCertificateAuthorities(SSLContextRef sslContext, CFArrayRef certificates, int32_t replaceExisting); From 7247db031b267036c0c51f51d27947a363108907 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 14 Feb 2022 14:30:11 +0100 Subject: [PATCH 04/15] Fix tests on OSX --- .../SslStreamCertificateTrustTests.cs | 76 ++++++------------- .../entrypoints.c | 5 +- 2 files changed, 28 insertions(+), 53 deletions(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs index 67103878a0ebf9..eb5947be183b6e 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs @@ -6,6 +6,7 @@ using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using System.Threading.Tasks; +using System.Linq; using Xunit; namespace System.Net.Security.Tests @@ -19,74 +20,49 @@ public class SslStreamCertificateTrustTest [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.OSX)] public async Task SslStream_SendCertificateTrust_CertificateCollection() { - (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); - using (client) - using (server) - using (X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate()) - using (X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate()) - { - SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions - { - ServerCertificate = serverCertificate, - ClientCertificateRequired = true, - RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, - ServerCertificateContext = SslStreamCertificateContext.Create( - serverCertificate, - null, - trust: SslCertificateTrust.CreateForX509Collection( - new X509Certificate2Collection { serverCertificate, clientCertificate }, - sendTrustInHandshake: true)) - }; + using X509Store store = new X509Store("Root", StoreLocation.LocalMachine); + store.Open(OpenFlags.ReadOnly | OpenFlags.OpenExistingOnly); + X509Certificate2[] certList = store.Certificates.DistinctBy(c => c.Subject).Take(2).ToArray(); - string[] acceptableIssuers = Array.Empty(); - SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions - { - TargetHost = "localhost", - RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, - LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, issuers) => - { - if (remoteCertificate == null) - { - // ignore the first call, we should receive acceptable issuers in the next one - return null; - } - - acceptableIssuers = issuers; - return clientCertificate; - }, + SslCertificateTrust trust = SslCertificateTrust.CreateForX509Collection( + new X509Certificate2Collection(certList), + sendTrustInHandshake: true); - }; + string[] acceptableIssuers = await ConnectAndGatherAcceptableIssuers(trust); - await TestConfiguration.WhenAllOrAnyFailedWithTimeout( - client.AuthenticateAsClientAsync(clientOptions), - server.AuthenticateAsServerAsync(serverOptions)); - - Assert.Equal(2, acceptableIssuers.Length); - Assert.Contains(serverCertificate.Subject, acceptableIssuers); - Assert.Contains(clientCertificate.Subject, acceptableIssuers); - } + Assert.Equal(2, acceptableIssuers.Length); + Assert.Contains(certList[0].Subject, acceptableIssuers); + Assert.Contains(certList[1].Subject, acceptableIssuers); } [Fact] [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux | TestPlatforms.OSX)] public async Task SslStream_SendCertificateTrust_CertificateStore() + { + using X509Store store = new X509Store("Root", StoreLocation.LocalMachine); + + SslCertificateTrust trust = SslCertificateTrust.CreateForX509Store(store, sendTrustInHandshake: true); + string[] acceptableIssuers = await ConnectAndGatherAcceptableIssuers(trust); + + // don't assert individual ellements, just that some issuers were sent + // we use Root cert store which should always contain at least some certs + Assert.NotEmpty(acceptableIssuers); + } + + private async Task ConnectAndGatherAcceptableIssuers(SslCertificateTrust trust) { (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); using (client) using (server) using (X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate()) using (X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate()) - using (X509Store store = new X509Store("Root", StoreLocation.LocalMachine)) { SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions { ServerCertificate = serverCertificate, ClientCertificateRequired = true, RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, - ServerCertificateContext = SslStreamCertificateContext.Create( - serverCertificate, - null, - trust: SslCertificateTrust.CreateForX509Store(store, sendTrustInHandshake: true)) + ServerCertificateContext = SslStreamCertificateContext.Create(serverCertificate, null, false, trust) }; string[] acceptableIssuers = Array.Empty(); @@ -112,9 +88,7 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( client.AuthenticateAsClientAsync(clientOptions), server.AuthenticateAsServerAsync(serverOptions)); - // don't assert individual ellements, just that some issuers were sent - // we use Root cert store which should always contain at least some certs - Assert.NotEmpty(acceptableIssuers); + return acceptableIssuers; } } } diff --git a/src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c index 875ce803165628..5bd8d16efc7fcb 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native.Apple/entrypoints.c @@ -7,7 +7,9 @@ #include "pal_digest.h" #include "pal_ecc.h" #include "pal_hmac.h" +#include "pal_keyagree.h" #include "pal_keychain_macos.h" +#include "pal_keyderivation_macos.h" #include "pal_random.h" #include "pal_rsa.h" #include "pal_sec.h" @@ -20,8 +22,6 @@ #include "pal_x509.h" #include "pal_x509_macos.h" #include "pal_x509chain.h" -#include "pal_keyderivation_macos.h" -#include "pal_keyagree.h" static const Entry s_cryptoAppleNative[] = { @@ -84,6 +84,7 @@ static const Entry s_cryptoAppleNative[] = DllImportEntry(AppleCryptoNative_SslSetMinProtocolVersion) DllImportEntry(AppleCryptoNative_SslSetMaxProtocolVersion) DllImportEntry(AppleCryptoNative_SslSetCertificate) + DllImportEntry(AppleCryptoNative_SslSetCertificateAuthorities) DllImportEntry(AppleCryptoNative_SslSetTargetName) DllImportEntry(AppleCryptoNative_SSLSetALPNProtocols) DllImportEntry(AppleCryptoNative_SslGetAlpnSelected) From 09e01eb4eae143d3014ebced86dc1e981eef4304 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 15 Feb 2022 14:48:04 +0100 Subject: [PATCH 05/15] Optimize setting certificates on OSX --- .../src/Interop/OSX/Interop.CoreFoundation.cs | 23 +++++++++++++++---- .../Interop.Ssl.cs | 4 ++-- .../Security/Pal.OSX/SafeDeleteSslContext.cs | 6 +++-- .../pal_ssl.c | 13 ++++++++++- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/libraries/Common/src/Interop/OSX/Interop.CoreFoundation.cs b/src/libraries/Common/src/Interop/OSX/Interop.CoreFoundation.cs index b5d64a92606edc..52d8de2836bd59 100644 --- a/src/libraries/Common/src/Interop/OSX/Interop.CoreFoundation.cs +++ b/src/libraries/Common/src/Interop/OSX/Interop.CoreFoundation.cs @@ -131,10 +131,9 @@ internal static unsafe SafeCreateHandle CFStringCreateFromSpan(ReadOnlySpanShould be IntPtr.Zero /// Returns a pointer to a CFArray on success; otherwise, returns IntPtr.Zero [GeneratedDllImport(Interop.Libraries.CoreFoundationLibrary)] - private static partial SafeCreateHandle CFArrayCreate( + private static unsafe partial SafeCreateHandle CFArrayCreate( IntPtr allocator, - [MarshalAs(UnmanagedType.LPArray)] - IntPtr[] values, + IntPtr* values, UIntPtr numValues, IntPtr callbacks); @@ -146,7 +145,23 @@ private static partial SafeCreateHandle CFArrayCreate( /// Returns a valid SafeCreateHandle to a CFArray on success; otherwise, returns an invalid SafeCreateHandle internal static SafeCreateHandle CFArrayCreate(IntPtr[] values, UIntPtr numValues) { - return CFArrayCreate(IntPtr.Zero, values, numValues, IntPtr.Zero); + fixed (IntPtr* pValues = values) + { + return CFArrayCreate(IntPtr.Zero, pValues, (UIntPtr)values.Length, IntPtr.Zero); + } + } + + /// + /// Creates a pointer to an unmanaged CFArray containing the input values. Follows the "Create Rule" where if you create it, you delete it. + /// + /// The values to put in the array + /// Returns a valid SafeCreateHandle to a CFArray on success; otherwise, returns an invalid SafeCreateHandle + internal static unsafe SafeCreateHandle CFArrayCreate(Span values) + { + fixed (IntPtr* pValues = MemoryMarshal) + { + return CFArrayCreate(IntPtr.Zero, pValues, (UIntPtr)values.Length, IntPtr.Zero); + } } /// diff --git a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs index fd562f83347327..b6bbe902b9f7b3 100644 --- a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs @@ -164,9 +164,9 @@ private static partial int AppleCryptoNative_SslIsHostnameMatch( [GeneratedDllImport(Interop.Libraries.AppleCryptoNative, EntryPoint = "AppleCryptoNative_SslSetCertificateAuthorities")] internal static partial int SslSetCertificateAuthorities(SafeSslHandle sslHandle, SafeCreateHandle certificateOrArray, int replaceExisting); - internal static unsafe void SslSetCertificateAuthorities(SafeSslHandle sslHandle, IntPtr[] certificates, bool replaceExisting) + internal static unsafe void SslSetCertificateAuthorities(SafeSslHandle sslHandle, Span certificates, bool replaceExisting) { - using (SafeCreateHandle cfCertRefs = CoreFoundation.CFArrayCreate(certificates, (UIntPtr)certificates.Length)) + using (SafeCreateHandle cfCertRefs = CoreFoundation.CFArrayCreate(certificates)) { int osStatus = SslSetCertificateAuthorities(sslHandle, cfCertRefs, replaceExisting ? 1 : 0); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs index 7f665fd0e9f2b7..660a55087d2438 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs @@ -118,14 +118,16 @@ public SafeDeleteSslContext(SafeFreeSslCredentials credential, SslAuthentication X509Certificate2Collection certList = (trust._trustList ?? trust._store!.Certificates); Debug.Assert(certList != null, "certList != null"); - IntPtr[] handles = new IntPtr[certList.Count]; + Span handles = certList.Count <= 256 + ? stackalloc IntPtr[256] + : new IntPtr[certList.Count]; for (int i = 0; i < certList.Count; i++) { handles[i] = certList[i].Handle; } - Interop.AppleCrypto.SslSetCertificateAuthorities(_sslContext, handles, true); + Interop.AppleCrypto.SslSetCertificateAuthorities(_sslContext, handles.Slice(0, certList.Count), true); } } } diff --git a/src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.c index 745f577adb1b7d..f0adc55eecab2c 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native.Apple/pal_ssl.c @@ -641,17 +641,28 @@ int32_t AppleCryptoNative_SslSetEnabledCipherSuites(SSLContextRef sslContext, co } } +// This API is present on macOS 10.5 and newer only +static OSStatus (*SSLSetCertificateAuthoritiesPtr)(SSLContextRef context, CFArrayRef certificates, int32_t replaceExisting) = NULL; + PALEXPORT int32_t AppleCryptoNative_SslSetCertificateAuthorities(SSLContextRef sslContext, CFArrayRef certificates, int32_t replaceExisting) { #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" // The underlying call handles NULL inputs, so just pass it through - return SSLSetCertificateAuthorities(sslContext, certificates, replaceExisting); + + if (!SSLSetCertificateAuthoritiesPtr) + { + // not available. + return 0; + } + + return SSLSetCertificateAuthoritiesPtr(sslContext, certificates, replaceExisting); #pragma clang diagnostic pop } __attribute__((constructor)) static void InitializeAppleCryptoSslShim() { + SSLSetCertificateAuthoritiesPtr = (OSStatus(*)(SSLContextRef, CFArrayRef, int32_t))dlsym(RTLD_DEFAULT, "SSLSetCertificateAuthorities"); SSLSetALPNProtocolsPtr = (OSStatus(*)(SSLContextRef, CFArrayRef))dlsym(RTLD_DEFAULT, "SSLSetALPNProtocols"); SSLCopyALPNProtocolsPtr = (OSStatus(*)(SSLContextRef, CFArrayRef*))dlsym(RTLD_DEFAULT, "SSLCopyALPNProtocols"); } \ No newline at end of file From 9ee85202a9142da4933dc3d9288cd1ae41612b1a Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 15 Feb 2022 15:26:16 +0100 Subject: [PATCH 06/15] Improve message in PNSE --- src/libraries/System.Net.Security/src/Resources/Strings.resx | 3 +++ .../src/System/Net/Security/SslCertificateTrust.cs | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Security/src/Resources/Strings.resx b/src/libraries/System.Net.Security/src/Resources/Strings.resx index dd3b86e0b682bd..0aef32953c3b09 100644 --- a/src/libraries/System.Net.Security/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Security/src/Resources/Strings.resx @@ -386,4 +386,7 @@ Sending trust from collection is not supported on Windows. + + Sending trust in handshake is not supported on this platform. + diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslCertificateTrust.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslCertificateTrust.cs index 479d83ec430ab7..98fd52fad3c5b6 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslCertificateTrust.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslCertificateTrust.cs @@ -24,7 +24,7 @@ public static SslCertificateTrust CreateForX509Store(X509Store store, bool sendT if (sendTrustInHandshake && !System.OperatingSystem.IsLinux() && !System.OperatingSystem.IsMacOS()) { // to be removed when implemented. - throw new PlatformNotSupportedException("Not supported yet."); + throw new PlatformNotSupportedException(SR.net_ssl_trust_handshake); } #endif if (!store.IsOpen) @@ -44,7 +44,7 @@ public static SslCertificateTrust CreateForX509Collection(X509Certificate2Collec if (sendTrustInHandshake && !System.OperatingSystem.IsLinux() && !System.OperatingSystem.IsMacOS()) { // to be removed when implemented. - throw new PlatformNotSupportedException("Not supported yet."); + throw new PlatformNotSupportedException(SR.net_ssl_trust_handshake); } #if TARGET_WINDOWS From 17b573cfe0f20cbeb8fcdfde693ece64d0fc5808 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 17 Feb 2022 14:02:13 +0100 Subject: [PATCH 07/15] Fix compilation --- .../src/Interop/OSX/Interop.CoreFoundation.cs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/libraries/Common/src/Interop/OSX/Interop.CoreFoundation.cs b/src/libraries/Common/src/Interop/OSX/Interop.CoreFoundation.cs index 52d8de2836bd59..6295ae373937cc 100644 --- a/src/libraries/Common/src/Interop/OSX/Interop.CoreFoundation.cs +++ b/src/libraries/Common/src/Interop/OSX/Interop.CoreFoundation.cs @@ -20,21 +20,21 @@ internal static partial class CoreFoundation /// private enum CFStringBuiltInEncodings : uint { - kCFStringEncodingMacRoman = 0, - kCFStringEncodingWindowsLatin1 = 0x0500, - kCFStringEncodingISOLatin1 = 0x0201, - kCFStringEncodingNextStepLatin = 0x0B01, - kCFStringEncodingASCII = 0x0600, - kCFStringEncodingUnicode = 0x0100, - kCFStringEncodingUTF8 = 0x08000100, - kCFStringEncodingNonLossyASCII = 0x0BFF, - - kCFStringEncodingUTF16 = 0x0100, - kCFStringEncodingUTF16BE = 0x10000100, - kCFStringEncodingUTF16LE = 0x14000100, - kCFStringEncodingUTF32 = 0x0c000100, - kCFStringEncodingUTF32BE = 0x18000100, - kCFStringEncodingUTF32LE = 0x1c000100 + kCFStringEncodingMacRoman = 0, + kCFStringEncodingWindowsLatin1 = 0x0500, + kCFStringEncodingISOLatin1 = 0x0201, + kCFStringEncodingNextStepLatin = 0x0B01, + kCFStringEncodingASCII = 0x0600, + kCFStringEncodingUnicode = 0x0100, + kCFStringEncodingUTF8 = 0x08000100, + kCFStringEncodingNonLossyASCII = 0x0BFF, + + kCFStringEncodingUTF16 = 0x0100, + kCFStringEncodingUTF16BE = 0x10000100, + kCFStringEncodingUTF16LE = 0x14000100, + kCFStringEncodingUTF32 = 0x0c000100, + kCFStringEncodingUTF32BE = 0x18000100, + kCFStringEncodingUTF32LE = 0x1c000100 } /// @@ -143,7 +143,7 @@ private static unsafe partial SafeCreateHandle CFArrayCreate( /// The values to put in the array /// The number of values in the array /// Returns a valid SafeCreateHandle to a CFArray on success; otherwise, returns an invalid SafeCreateHandle - internal static SafeCreateHandle CFArrayCreate(IntPtr[] values, UIntPtr numValues) + internal static unsafe SafeCreateHandle CFArrayCreate(IntPtr[] values, UIntPtr numValues) { fixed (IntPtr* pValues = values) { @@ -158,7 +158,7 @@ internal static SafeCreateHandle CFArrayCreate(IntPtr[] values, UIntPtr numValue /// Returns a valid SafeCreateHandle to a CFArray on success; otherwise, returns an invalid SafeCreateHandle internal static unsafe SafeCreateHandle CFArrayCreate(Span values) { - fixed (IntPtr* pValues = MemoryMarshal) + fixed (IntPtr* pValues = &MemoryMarshal.GetReference(values)) { return CFArrayCreate(IntPtr.Zero, pValues, (UIntPtr)values.Length, IntPtr.Zero); } From 5cad62d00ba897bd5de528284bb65af6aa1b47fc Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 17 Feb 2022 14:02:33 +0100 Subject: [PATCH 08/15] Use only CA certificates in test --- .../SslStreamCertificateTrustTests.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs index eb5947be183b6e..a3b49fc6591cf5 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs @@ -20,19 +20,13 @@ public class SslStreamCertificateTrustTest [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.OSX)] public async Task SslStream_SendCertificateTrust_CertificateCollection() { - using X509Store store = new X509Store("Root", StoreLocation.LocalMachine); - store.Open(OpenFlags.ReadOnly | OpenFlags.OpenExistingOnly); - X509Certificate2[] certList = store.Certificates.DistinctBy(c => c.Subject).Take(2).ToArray(); - - SslCertificateTrust trust = SslCertificateTrust.CreateForX509Collection( - new X509Certificate2Collection(certList), - sendTrustInHandshake: true); + (X509Certificate2 certificate, X509Certificate2Collection caCerts) = TestHelper.GenerateCertificates("foo"); + SslCertificateTrust trust = SslCertificateTrust.CreateForX509Collection(caCerts, sendTrustInHandshake: true); string[] acceptableIssuers = await ConnectAndGatherAcceptableIssuers(trust); - Assert.Equal(2, acceptableIssuers.Length); - Assert.Contains(certList[0].Subject, acceptableIssuers); - Assert.Contains(certList[1].Subject, acceptableIssuers); + Assert.Equal(caCerts.Count, acceptableIssuers.Length); + Assert.Equal(caCerts.Select(c => c.Subject), acceptableIssuers); } [Fact] From 33fda736b4ea3f8d6cd19f210601f8295268cac5 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 17 Feb 2022 14:04:41 +0100 Subject: [PATCH 09/15] Remove unwanted formatting changes --- .../src/Interop/OSX/Interop.CoreFoundation.cs | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/libraries/Common/src/Interop/OSX/Interop.CoreFoundation.cs b/src/libraries/Common/src/Interop/OSX/Interop.CoreFoundation.cs index 6295ae373937cc..2409b4903a05b2 100644 --- a/src/libraries/Common/src/Interop/OSX/Interop.CoreFoundation.cs +++ b/src/libraries/Common/src/Interop/OSX/Interop.CoreFoundation.cs @@ -20,21 +20,21 @@ internal static partial class CoreFoundation /// private enum CFStringBuiltInEncodings : uint { - kCFStringEncodingMacRoman = 0, - kCFStringEncodingWindowsLatin1 = 0x0500, - kCFStringEncodingISOLatin1 = 0x0201, - kCFStringEncodingNextStepLatin = 0x0B01, - kCFStringEncodingASCII = 0x0600, - kCFStringEncodingUnicode = 0x0100, - kCFStringEncodingUTF8 = 0x08000100, - kCFStringEncodingNonLossyASCII = 0x0BFF, - - kCFStringEncodingUTF16 = 0x0100, - kCFStringEncodingUTF16BE = 0x10000100, - kCFStringEncodingUTF16LE = 0x14000100, - kCFStringEncodingUTF32 = 0x0c000100, - kCFStringEncodingUTF32BE = 0x18000100, - kCFStringEncodingUTF32LE = 0x1c000100 + kCFStringEncodingMacRoman = 0, + kCFStringEncodingWindowsLatin1 = 0x0500, + kCFStringEncodingISOLatin1 = 0x0201, + kCFStringEncodingNextStepLatin = 0x0B01, + kCFStringEncodingASCII = 0x0600, + kCFStringEncodingUnicode = 0x0100, + kCFStringEncodingUTF8 = 0x08000100, + kCFStringEncodingNonLossyASCII = 0x0BFF, + + kCFStringEncodingUTF16 = 0x0100, + kCFStringEncodingUTF16BE = 0x10000100, + kCFStringEncodingUTF16LE = 0x14000100, + kCFStringEncodingUTF32 = 0x0c000100, + kCFStringEncodingUTF32BE = 0x18000100, + kCFStringEncodingUTF32LE = 0x1c000100 } /// From 6432d877941af58932e494aae4df64cce12446dc Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 17 Feb 2022 14:47:03 +0100 Subject: [PATCH 10/15] Skip non-CA certificates --- .../Interop.OpenSsl.cs | 53 ++++++++++++------- .../Security/Pal.OSX/SafeDeleteSslContext.cs | 34 ++++++++---- 2 files changed, 59 insertions(+), 28 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 19a3596377f43b..e4ccd1b4c61cf0 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -350,25 +350,7 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia if (sslAuthenticationOptions.CertificateContext?.Trust?._sendTrustInHandshake == true) { SslCertificateTrust trust = sslAuthenticationOptions.CertificateContext!.Trust!; - X509Certificate2Collection certList = (trust._trustList ?? trust._store!.Certificates); - - Debug.Assert(certList != null, "certList != null"); - Span handles = certList.Count <= 256 - ? stackalloc IntPtr[256] - : new IntPtr[certList.Count]; - - for (int i = 0; i < certList.Count; i++) - { - handles[i] = certList[i].Handle; - } - - if (!Ssl.SslAddClientCAs(sslHandle, handles.Slice(0, certList.Count))) - { - // The method can fail only when the number of cert names exceeds the maximum capacity - // supported by STACK_OF(X509_NAME) structure, which should not happen under normal - // operation. - Debug.Fail("Failed to add issuer to trusted CA list."); - } + SetCertificateAuthorities(sslHandle, trust._trustList ?? trust._store!.Certificates); } } } @@ -389,6 +371,39 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia return sslHandle; } + internal static void SetCertificateAuthorities(SafeSslHandle sslHandle, X509Certificate2Collection certList) + { + Debug.Assert(certList != null, "certList != null"); + Span handles = certList.Count <= 256 + ? stackalloc IntPtr[256] + : new IntPtr[certList.Count]; + + int certCount = 0; + for (int i = 0; i < certList.Count; i++) + { + foreach (X509Extension ext in certList[i].Extensions) + { + // filter out non-CA certificates + if (ext is X509BasicConstraintsExtension ex) + { + if (ex.CertificateAuthority) + { + handles[certCount++] = certList[i].Handle; + } + break; + } + } + } + + if (!Ssl.SslAddClientCAs(sslHandle, handles.Slice(0, certCount))) + { + // The method can fail only when the number of cert names exceeds the maximum capacity + // supported by STACK_OF(X509_NAME) structure, which should not happen under normal + // operation. + Debug.Fail("Failed to add issuer to trusted CA list."); + } + } + internal static SecurityStatusPal SslRenegotiate(SafeSslHandle sslContext, out byte[]? outputBuffer) { int ret = Interop.Ssl.SslRenegotiate(sslContext, out Ssl.SslErrorCode errorCode); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs index 660a55087d2438..c2160115126123 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs @@ -115,23 +115,39 @@ public SafeDeleteSslContext(SafeFreeSslCredentials credential, SslAuthentication if (sslAuthenticationOptions.CertificateContext?.Trust?._sendTrustInHandshake == true) { SslCertificateTrust trust = sslAuthenticationOptions.CertificateContext!.Trust!; - X509Certificate2Collection certList = (trust._trustList ?? trust._store!.Certificates); + SetCertificateAuthorities(trust._trustList ?? trust._store!.Certificates); + } + } + } - Debug.Assert(certList != null, "certList != null"); - Span handles = certList.Count <= 256 - ? stackalloc IntPtr[256] - : new IntPtr[certList.Count]; + internal void SetCertificateAuthorities(X509Certificate2Collection certList) + { + Debug.Assert(certList != null, "certList != null"); + Span handles = certList.Count <= 256 + ? stackalloc IntPtr[256] + : new IntPtr[certList.Count]; - for (int i = 0; i < certList.Count; i++) + int certCount = 0; + for (int i = 0; i < certList.Count; i++) + { + foreach (X509Extension ext in certList[i].Extensions) + { + // filter out non-CA certificates + if (ext is X509BasicConstraintsExtension ex) { - handles[i] = certList[i].Handle; + if (ex.CertificateAuthority) + { + handles[certCount++] = certList[i].Handle; + } + break; } - - Interop.AppleCrypto.SslSetCertificateAuthorities(_sslContext, handles.Slice(0, certList.Count), true); } } + + Interop.AppleCrypto.SslSetCertificateAuthorities(_sslContext, handles.Slice(0, certList.Count), true); } + private static SafeSslHandle CreateSslContext(SafeFreeSslCredentials credential, bool isServer) { switch (credential.Policy) From 27fd404f9888186a9d3ef4f2f9e1fb8ada80d067 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 17 Feb 2022 15:01:56 +0100 Subject: [PATCH 11/15] Fix crash on OSX --- .../src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs index c2160115126123..0d5db350f473e8 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs @@ -144,7 +144,7 @@ internal void SetCertificateAuthorities(X509Certificate2Collection certList) } } - Interop.AppleCrypto.SslSetCertificateAuthorities(_sslContext, handles.Slice(0, certList.Count), true); + Interop.AppleCrypto.SslSetCertificateAuthorities(_sslContext, handles.Slice(0, certCount), true); } From e064a9d590d3bad9602a97815c0464cbfeae5fb8 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 17 Feb 2022 17:42:20 +0100 Subject: [PATCH 12/15] Disable new tests on Windows --- .../tests/FunctionalTests/SslStreamCertificateTrustTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs index a3b49fc6591cf5..f4e365111488c8 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs @@ -30,6 +30,7 @@ public async Task SslStream_SendCertificateTrust_CertificateCollection() } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/65515", TestPlatforms.Windows)] [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux | TestPlatforms.OSX)] public async Task SslStream_SendCertificateTrust_CertificateStore() { From 15cc494fa607798a188443030fc1ab913203a995 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Fri, 18 Feb 2022 13:56:24 +0100 Subject: [PATCH 13/15] Revert "Skip non-CA certificates" This reverts commit 7d864a185a7150cd384f16bba5efd616f27759ca. --- .../Interop.OpenSsl.cs | 53 +++++++------------ .../Security/Pal.OSX/SafeDeleteSslContext.cs | 34 ++++-------- 2 files changed, 28 insertions(+), 59 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index e4ccd1b4c61cf0..19a3596377f43b 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -350,7 +350,25 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia if (sslAuthenticationOptions.CertificateContext?.Trust?._sendTrustInHandshake == true) { SslCertificateTrust trust = sslAuthenticationOptions.CertificateContext!.Trust!; - SetCertificateAuthorities(sslHandle, trust._trustList ?? trust._store!.Certificates); + X509Certificate2Collection certList = (trust._trustList ?? trust._store!.Certificates); + + Debug.Assert(certList != null, "certList != null"); + Span handles = certList.Count <= 256 + ? stackalloc IntPtr[256] + : new IntPtr[certList.Count]; + + for (int i = 0; i < certList.Count; i++) + { + handles[i] = certList[i].Handle; + } + + if (!Ssl.SslAddClientCAs(sslHandle, handles.Slice(0, certList.Count))) + { + // The method can fail only when the number of cert names exceeds the maximum capacity + // supported by STACK_OF(X509_NAME) structure, which should not happen under normal + // operation. + Debug.Fail("Failed to add issuer to trusted CA list."); + } } } } @@ -371,39 +389,6 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia return sslHandle; } - internal static void SetCertificateAuthorities(SafeSslHandle sslHandle, X509Certificate2Collection certList) - { - Debug.Assert(certList != null, "certList != null"); - Span handles = certList.Count <= 256 - ? stackalloc IntPtr[256] - : new IntPtr[certList.Count]; - - int certCount = 0; - for (int i = 0; i < certList.Count; i++) - { - foreach (X509Extension ext in certList[i].Extensions) - { - // filter out non-CA certificates - if (ext is X509BasicConstraintsExtension ex) - { - if (ex.CertificateAuthority) - { - handles[certCount++] = certList[i].Handle; - } - break; - } - } - } - - if (!Ssl.SslAddClientCAs(sslHandle, handles.Slice(0, certCount))) - { - // The method can fail only when the number of cert names exceeds the maximum capacity - // supported by STACK_OF(X509_NAME) structure, which should not happen under normal - // operation. - Debug.Fail("Failed to add issuer to trusted CA list."); - } - } - internal static SecurityStatusPal SslRenegotiate(SafeSslHandle sslContext, out byte[]? outputBuffer) { int ret = Interop.Ssl.SslRenegotiate(sslContext, out Ssl.SslErrorCode errorCode); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs index 0d5db350f473e8..660a55087d2438 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs @@ -115,39 +115,23 @@ public SafeDeleteSslContext(SafeFreeSslCredentials credential, SslAuthentication if (sslAuthenticationOptions.CertificateContext?.Trust?._sendTrustInHandshake == true) { SslCertificateTrust trust = sslAuthenticationOptions.CertificateContext!.Trust!; - SetCertificateAuthorities(trust._trustList ?? trust._store!.Certificates); - } - } - } + X509Certificate2Collection certList = (trust._trustList ?? trust._store!.Certificates); - internal void SetCertificateAuthorities(X509Certificate2Collection certList) - { - Debug.Assert(certList != null, "certList != null"); - Span handles = certList.Count <= 256 - ? stackalloc IntPtr[256] - : new IntPtr[certList.Count]; + Debug.Assert(certList != null, "certList != null"); + Span handles = certList.Count <= 256 + ? stackalloc IntPtr[256] + : new IntPtr[certList.Count]; - int certCount = 0; - for (int i = 0; i < certList.Count; i++) - { - foreach (X509Extension ext in certList[i].Extensions) - { - // filter out non-CA certificates - if (ext is X509BasicConstraintsExtension ex) + for (int i = 0; i < certList.Count; i++) { - if (ex.CertificateAuthority) - { - handles[certCount++] = certList[i].Handle; - } - break; + handles[i] = certList[i].Handle; } + + Interop.AppleCrypto.SslSetCertificateAuthorities(_sslContext, handles.Slice(0, certList.Count), true); } } - - Interop.AppleCrypto.SslSetCertificateAuthorities(_sslContext, handles.Slice(0, certCount), true); } - private static SafeSslHandle CreateSslContext(SafeFreeSslCredentials credential, bool isServer) { switch (credential.Policy) From fbba39995ff0f1204c8bf2f1c4f6fbe187de7966 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 22 Feb 2022 20:11:57 +0100 Subject: [PATCH 14/15] Fix tests on Ubuntu 18.04 --- .../tests/FunctionalTests/SslStreamCertificateTrustTests.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs index f4e365111488c8..72a0f024e46237 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs @@ -64,12 +64,15 @@ private async Task ConnectAndGatherAcceptableIssuers(SslCertificateTru SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions { TargetHost = "localhost", + // Force Tls 1.2 to avoid issues with certain OpenSSL versions and Tls 1.3 + // https://github.com/openssl/openssl/issues/7384 + EnabledSslProtocols = SslProtocols.Tls12, RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, issuers) => { if (remoteCertificate == null) { - // ignore the first call, we should receive acceptable issuers in the next one + // ignore the first call that is called before handshake return null; } From e40e1b0eb7bf366e49baa1a80bf936b11ed7186a Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 24 Feb 2022 13:53:43 +0100 Subject: [PATCH 15/15] Change cert name --- .../tests/FunctionalTests/SslStreamCertificateTrustTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs index 72a0f024e46237..fe3a977d479b69 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateTrustTests.cs @@ -20,7 +20,7 @@ public class SslStreamCertificateTrustTest [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.OSX)] public async Task SslStream_SendCertificateTrust_CertificateCollection() { - (X509Certificate2 certificate, X509Certificate2Collection caCerts) = TestHelper.GenerateCertificates("foo"); + (X509Certificate2 certificate, X509Certificate2Collection caCerts) = TestHelper.GenerateCertificates(nameof(SslStream_SendCertificateTrust_CertificateCollection)); SslCertificateTrust trust = SslCertificateTrust.CreateForX509Collection(caCerts, sendTrustInHandshake: true); string[] acceptableIssuers = await ConnectAndGatherAcceptableIssuers(trust);