From 8df9d35abe6f9387b9c161c4b7d4b9ce346b47b8 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Wed, 14 Aug 2024 16:30:31 +0200 Subject: [PATCH 1/3] Make SafeEvpPKeyHandle.OpenKeyFromProvider throw PNSE on OSSL less than 3.0 --- .../src/Resources/Strings.resx | 3 + .../Cryptography/SafeEvpPKeyHandle.OpenSsl.cs | 7 ++ .../tests/OpenSslNamedKeysTests.manual.cs | 87 +++++++++++++++++-- 3 files changed, 89 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx index 630f2f89bc6ff..bd54813d70361 100644 --- a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx @@ -834,6 +834,9 @@ OpenSSL is required for algorithm '{0}' but could not be found or loaded. + + OpenSSL 3.0 or higher is required but could not be found or loaded. + Access is denied. diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs index 073e6b0fec5dd..a8d166e1155a4 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs @@ -243,6 +243,13 @@ public static SafeEvpPKeyHandle OpenKeyFromProvider(string providerName, string throw new PlatformNotSupportedException(SR.PlatformNotSupported_CryptographyOpenSSL); } + // 3.0+ are M_NN_00_PP_p (Major, Minor, 0, Patch, Preview) + // 1.x.y are 1_XX_YY_PP_p + if (SafeEvpPKeyHandle.OpenSslVersion < 0x3_00_00_00_0) + { + throw new PlatformNotSupportedException(SR.PlatformNotSupported_CryptographyOpenSSL3NotFound); + } + return Interop.Crypto.LoadKeyFromProvider(providerName, keyUri); } } diff --git a/src/libraries/System.Security.Cryptography/tests/OpenSslNamedKeysTests.manual.cs b/src/libraries/System.Security.Cryptography/tests/OpenSslNamedKeysTests.manual.cs index b5aab83a1aa3b..9050e8aac427a 100644 --- a/src/libraries/System.Security.Cryptography/tests/OpenSslNamedKeysTests.manual.cs +++ b/src/libraries/System.Security.Cryptography/tests/OpenSslNamedKeysTests.manual.cs @@ -2,8 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Linq; +using System.Text; using Test.Cryptography; using Xunit; +using TempFileHolder = System.Security.Cryptography.X509Certificates.Tests.TempFileHolder; namespace System.Security.Cryptography.Tests { @@ -43,10 +45,13 @@ public class OpenSslNamedKeysTests private static string TpmRsaDecryptKeyHandleUri { get; } = GetHandleKeyUri(TpmRsaDecryptKeyHandle); public static bool ShouldRunEngineTests { get; } = PlatformDetection.OpenSslPresentOnSystem && StringToBool(Environment.GetEnvironmentVariable(TestEngineEnabledEnvVarName)); - public static bool ShouldRunProviderEcDsaTests { get; } = PlatformDetection.OpenSslPresentOnSystem && !string.IsNullOrEmpty(TpmEcDsaKeyHandleUri); - public static bool ShouldRunProviderEcDhTests { get; } = PlatformDetection.OpenSslPresentOnSystem && !string.IsNullOrEmpty(TpmEcDhKeyHandleUri); - public static bool ShouldRunProviderRsaSignTests { get; } = PlatformDetection.OpenSslPresentOnSystem && !string.IsNullOrEmpty(TpmRsaSignKeyHandleUri); - public static bool ShouldRunProviderRsaDecryptTests { get; } = PlatformDetection.OpenSslPresentOnSystem && !string.IsNullOrEmpty(TpmRsaDecryptKeyHandleUri); + + public static bool ProvidersSupported { get; } = PlatformDetection.OpenSslPresentOnSystem && PlatformDetection.OpenSslVersion >= new Version(3, 0, 0); + public static bool ProvidersNotSupported => !ProvidersSupported; + public static bool ShouldRunProviderEcDsaTests { get; } = ProvidersSupported && !string.IsNullOrEmpty(TpmEcDsaKeyHandleUri); + public static bool ShouldRunProviderEcDhTests { get; } = ProvidersSupported && !string.IsNullOrEmpty(TpmEcDhKeyHandleUri); + public static bool ShouldRunProviderRsaSignTests { get; } = ProvidersSupported && !string.IsNullOrEmpty(TpmRsaSignKeyHandleUri); + public static bool ShouldRunProviderRsaDecryptTests { get; } = ProvidersSupported && !string.IsNullOrEmpty(TpmRsaDecryptKeyHandleUri); public static bool ShouldRunAnyProviderTests => ShouldRunProviderEcDsaTests || ShouldRunProviderEcDhTests || ShouldRunProviderRsaSignTests || ShouldRunProviderRsaDecryptTests; public static bool ShouldRunTpmTssTests => ShouldRunEngineTests && !string.IsNullOrEmpty(TpmEcDsaKeyHandle); @@ -86,10 +91,15 @@ private static string GetHandleKeyUri(string handle) "B27434FA544BDAC679E1E16581D0E90203010001").HexToByteArray(); [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.OpenSslNotPresentOnSystem))] - public static void NotSupported() + public static void EngineNotSupported_ThrowsPlatformNotSupported() { Assert.Throws(() => SafeEvpPKeyHandle.OpenPublicKeyFromEngine(TestEngineName, TestEngineKeyId)); Assert.Throws(() => SafeEvpPKeyHandle.OpenPrivateKeyFromEngine(TestEngineName, TestEngineKeyId)); + } + + [ConditionalFact(nameof(ProvidersNotSupported))] + public static void ProvidersNotSupported_ThrowsPlatformNotSupported() + { Assert.Throws(() => SafeEvpPKeyHandle.OpenKeyFromProvider(Tpm2ProviderName, AnyProviderKeyUri)); } @@ -111,10 +121,14 @@ public static void EmptyNameThroughNullCharacter() { Assert.ThrowsAny(() => SafeEvpPKeyHandle.OpenPrivateKeyFromEngine("\0", "foo")); Assert.ThrowsAny(() => SafeEvpPKeyHandle.OpenPublicKeyFromEngine("\0", "foo")); - Assert.ThrowsAny(() => SafeEvpPKeyHandle.OpenKeyFromProvider("\0", "foo")); + + if (ProvidersSupported) + { + Assert.ThrowsAny(() => SafeEvpPKeyHandle.OpenKeyFromProvider("\0", "foo")); + } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.OpenSslPresentOnSystem))] + [ConditionalFact(nameof(ProvidersSupported))] public static void EmptyUriThroughNullCharacter() { Assert.ThrowsAny(() => SafeEvpPKeyHandle.OpenKeyFromProvider("default", "\0")); @@ -127,7 +141,7 @@ public static void Engine_NonExisting() Assert.ThrowsAny(() => SafeEvpPKeyHandle.OpenPublicKeyFromEngine(NonExistingEngineOrProviderKeyName, TestEngineKeyId)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.OpenSslPresentOnSystem))] + [ConditionalFact(nameof(ProvidersSupported))] public static void Provider_NonExisting() { Assert.ThrowsAny(() => SafeEvpPKeyHandle.OpenKeyFromProvider(NonExistingEngineOrProviderKeyName, AnyProviderKeyUri)); @@ -146,6 +160,63 @@ public static void Provider_NonExistingKey() Assert.ThrowsAny(() => SafeEvpPKeyHandle.OpenKeyFromProvider(Tpm2ProviderName, NonExistingEngineOrProviderKeyName)); } + [ConditionalFact(nameof(ProvidersSupported))] + public static void Provider_Default_RSASignAndDecrypt() + { + using RSA originalKey = RSA.Create(); + string pem = originalKey.ExportRSAPrivateKeyPem(); + + using TempFileHolder pemFile = new TempFileHolder(Encoding.UTF8.GetBytes(pem)); + Uri fileUri = new Uri(pemFile.FilePath); + string keyUri = fileUri.AbsoluteUri; + using SafeEvpPKeyHandle priKeyHandle = SafeEvpPKeyHandle.OpenKeyFromProvider("default", keyUri); + using RSA rsaPri = new RSAOpenSsl(priKeyHandle); + byte[] data = new byte[] { 1, 2, 3, 1, 1, 2, 3 }; + byte[] signature = rsaPri.SignData(data, HashAlgorithmName.SHA256, RSASignaturePadding.Pss); + Assert.True(originalKey.VerifyData(data, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pss), "signature does not verify with the right key"); + + byte[] encrypted = originalKey.Encrypt(data, RSAEncryptionPadding.OaepSHA256); + byte[] decrypted = rsaPri.Decrypt(encrypted, RSAEncryptionPadding.OaepSHA256); + Assert.Equal(data, decrypted); + } + + [ConditionalFact(nameof(ProvidersSupported))] + public static void Provider_Default_ECDsaSignAndVerify() + { + using ECDsa originalKey = ECDsa.Create(); + string pem = originalKey.ExportECPrivateKeyPem(); + + using TempFileHolder pemFile = new TempFileHolder(Encoding.UTF8.GetBytes(pem)); + Uri fileUri = new Uri(pemFile.FilePath); + string keyUri = fileUri.AbsoluteUri; + using SafeEvpPKeyHandle priKeyHandle = SafeEvpPKeyHandle.OpenKeyFromProvider("default", keyUri); + using ECDsa ecdsaPri = new ECDsaOpenSsl(priKeyHandle); + byte[] data = new byte[] { 1, 2, 3, 1, 1, 2, 3 }; + byte[] signature = ecdsaPri.SignData(data, HashAlgorithmName.SHA256); + Assert.True(originalKey.VerifyData(data, signature, HashAlgorithmName.SHA256), "signature does not verify with the right key"); + } + + [ConditionalFact(nameof(ProvidersSupported))] + public static void Provider_Default_ECDHKeyExchange() + { + using ECDiffieHellman originalAliceKey = ECDiffieHellman.Create(); + string pem = originalAliceKey.ExportECPrivateKeyPem(); + + using TempFileHolder pemFile = new TempFileHolder(Encoding.UTF8.GetBytes(pem)); + Uri fileUri = new Uri(pemFile.FilePath); + string keyUri = fileUri.AbsoluteUri; + using SafeEvpPKeyHandle priKeyHandle = SafeEvpPKeyHandle.OpenKeyFromProvider("default", keyUri); + using ECDiffieHellman alicePri = new ECDiffieHellmanOpenSsl(priKeyHandle); + using ECDiffieHellman bobPri = ECDiffieHellman.Create(alicePri.ExportParameters(false).Curve); + + byte[] sharedSecret1 = originalAliceKey.DeriveRawSecretAgreement(bobPri.PublicKey); + byte[] sharedSecret2 = alicePri.DeriveRawSecretAgreement(bobPri.PublicKey); + byte[] sharedSecret3 = bobPri.DeriveRawSecretAgreement(alicePri.PublicKey); + + Assert.Equal(sharedSecret1, sharedSecret2); + Assert.Equal(sharedSecret1, sharedSecret3); + } + [ConditionalFact(nameof(ShouldRunEngineTests))] public static void Engine_OpenExistingPrivateKey() { From a59d9b2bc42f74743b83894ef6280d069d447ef0 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Wed, 14 Aug 2024 19:40:19 +0200 Subject: [PATCH 2/3] address feedback --- .../Interop.EvpPkey.cs | 11 +++++++++-- .../Cryptography/SafeEvpPKeyHandle.OpenSsl.cs | 7 ------- .../tests/OpenSslNamedKeysTests.manual.cs | 2 +- .../pal_evp_pkey.c | 6 ++++-- .../pal_evp_pkey.h | 4 +++- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.cs index 68f4f8cb6433f..42f6d847379e5 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.cs @@ -281,7 +281,8 @@ internal static SafeEvpPKeyHandle LoadPublicKeyFromEngine( private static partial IntPtr CryptoNative_LoadKeyFromProvider( string providerName, string keyUri, - ref IntPtr extraHandle); + ref IntPtr extraHandle, + [MarshalAs(UnmanagedType.Bool)] out bool haveProvider); internal static SafeEvpPKeyHandle LoadKeyFromProvider( string providerName, @@ -292,7 +293,13 @@ internal static SafeEvpPKeyHandle LoadKeyFromProvider( try { - evpPKeyHandle = CryptoNative_LoadKeyFromProvider(providerName, keyUri, ref extraHandle); + evpPKeyHandle = CryptoNative_LoadKeyFromProvider(providerName, keyUri, ref extraHandle, out bool haveProvider); + + if (!haveProvider) + { + Debug.Assert(evpPKeyHandle == IntPtr.Zero && extraHandle == IntPtr.Zero, "both handles should be null if provider is not supported"); + throw new PlatformNotSupportedException(SR.PlatformNotSupported_CryptographyOpenSSL3NotFound); + } if (evpPKeyHandle == IntPtr.Zero || extraHandle == IntPtr.Zero) { diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs index a8d166e1155a4..073e6b0fec5dd 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs @@ -243,13 +243,6 @@ public static SafeEvpPKeyHandle OpenKeyFromProvider(string providerName, string throw new PlatformNotSupportedException(SR.PlatformNotSupported_CryptographyOpenSSL); } - // 3.0+ are M_NN_00_PP_p (Major, Minor, 0, Patch, Preview) - // 1.x.y are 1_XX_YY_PP_p - if (SafeEvpPKeyHandle.OpenSslVersion < 0x3_00_00_00_0) - { - throw new PlatformNotSupportedException(SR.PlatformNotSupported_CryptographyOpenSSL3NotFound); - } - return Interop.Crypto.LoadKeyFromProvider(providerName, keyUri); } } diff --git a/src/libraries/System.Security.Cryptography/tests/OpenSslNamedKeysTests.manual.cs b/src/libraries/System.Security.Cryptography/tests/OpenSslNamedKeysTests.manual.cs index 9050e8aac427a..a72f9af97b60c 100644 --- a/src/libraries/System.Security.Cryptography/tests/OpenSslNamedKeysTests.manual.cs +++ b/src/libraries/System.Security.Cryptography/tests/OpenSslNamedKeysTests.manual.cs @@ -46,7 +46,7 @@ public class OpenSslNamedKeysTests public static bool ShouldRunEngineTests { get; } = PlatformDetection.OpenSslPresentOnSystem && StringToBool(Environment.GetEnvironmentVariable(TestEngineEnabledEnvVarName)); - public static bool ProvidersSupported { get; } = PlatformDetection.OpenSslPresentOnSystem && PlatformDetection.OpenSslVersion >= new Version(3, 0, 0); + public static bool ProvidersSupported { get; } = PlatformDetection.IsOpenSsl3; public static bool ProvidersNotSupported => !ProvidersSupported; public static bool ShouldRunProviderEcDsaTests { get; } = ProvidersSupported && !string.IsNullOrEmpty(TpmEcDsaKeyHandleUri); public static bool ShouldRunProviderEcDhTests { get; } = ProvidersSupported && !string.IsNullOrEmpty(TpmEcDhKeyHandleUri); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c index 2e0e8efee114e..f80ef99c02515 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c @@ -611,19 +611,20 @@ EVP_PKEY* CryptoNative_LoadPublicKeyFromEngine(const char* engineName, const cha return NULL; } -EVP_PKEY* CryptoNative_LoadKeyFromProvider(const char* providerName, const char* keyUri, void** extraHandle) +EVP_PKEY* CryptoNative_LoadKeyFromProvider(const char* providerName, const char* keyUri, void** extraHandle, int32_t* haveProvider) { ERR_clear_error(); #ifdef FEATURE_DISTRO_AGNOSTIC_SSL if (!API_EXISTS(OSSL_PROVIDER_load)) { - ERR_put_error(ERR_LIB_NONE, 0, ERR_R_DISABLED, __FILE__, __LINE__); + *haveProvider = 0; return NULL; } #endif #ifdef NEED_OPENSSL_3_0 + *haveProvider = 1; EVP_PKEY* ret = NULL; OSSL_LIB_CTX* libCtx = OSSL_LIB_CTX_new(); OSSL_PROVIDER* prov = NULL; @@ -730,6 +731,7 @@ EVP_PKEY* CryptoNative_LoadKeyFromProvider(const char* providerName, const char* (void)keyUri; ERR_put_error(ERR_LIB_NONE, 0, ERR_R_DISABLED, __FILE__, __LINE__); *extraHandle = NULL; + *haveProvider = 0; return NULL; #endif } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.h b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.h index 084ae28d77cd4..766fd1251c1c2 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.h @@ -114,8 +114,10 @@ Load a key by URI from a specified OSSL_PROVIDER. Returns a valid EVP_PKEY* on success, NULL on failure. On success extraHandle may be non-null value which we need to keep alive until the EVP_PKEY is destroyed. + +providersSupported is 1 if OpenSSL providers are supported, otherwise 0. */ -PALEXPORT EVP_PKEY* CryptoNative_LoadKeyFromProvider(const char* providerName, const char* keyUri, void** extraHandle); +PALEXPORT EVP_PKEY* CryptoNative_LoadKeyFromProvider(const char* providerName, const char* keyUri, void** extraHandle, int32_t* haveProvider); /* It's a wrapper for EVP_PKEY_CTX_new_from_pkey and EVP_PKEY_CTX_new From 9af1d0e500bfa8590a3832e55f06b86ad13d3ef9 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Tue, 20 Aug 2024 14:46:37 +0200 Subject: [PATCH 3/3] Address recent feedback --- .../System.Security.Cryptography.Native/Interop.EvpPkey.cs | 2 +- .../System.Security.Cryptography/src/Resources/Strings.resx | 4 ++-- .../libs/System.Security.Cryptography.Native/pal_evp_pkey.h | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.cs index 42f6d847379e5..f5129fe32d643 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.cs @@ -298,7 +298,7 @@ internal static SafeEvpPKeyHandle LoadKeyFromProvider( if (!haveProvider) { Debug.Assert(evpPKeyHandle == IntPtr.Zero && extraHandle == IntPtr.Zero, "both handles should be null if provider is not supported"); - throw new PlatformNotSupportedException(SR.PlatformNotSupported_CryptographyOpenSSL3NotFound); + throw new PlatformNotSupportedException(SR.PlatformNotSupported_CryptographyOpenSSLProvidersNotSupported); } if (evpPKeyHandle == IntPtr.Zero || extraHandle == IntPtr.Zero) diff --git a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx index bd54813d70361..412cfbf03a8ac 100644 --- a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx @@ -834,8 +834,8 @@ OpenSSL is required for algorithm '{0}' but could not be found or loaded. - - OpenSSL 3.0 or higher is required but could not be found or loaded. + + OpenSSL providers are not supported on this platform. Access is denied. diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.h b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.h index 766fd1251c1c2..f71b64cb07c09 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.h @@ -104,7 +104,7 @@ PALEXPORT EVP_PKEY* CryptoNative_LoadPrivateKeyFromEngine(const char* engineName Load a named key, via ENGINE_load_public_key, from the named engine. Returns a valid EVP_PKEY* on success, NULL on failure. -haveEngine is 1 if OpenSSL ENGINE's are supported, otherwise 0. +*haveEngine is 1 if OpenSSL ENGINE's are supported, otherwise 0. */ PALEXPORT EVP_PKEY* CryptoNative_LoadPublicKeyFromEngine(const char* engineName, const char* keyName, int32_t* haveEngine); @@ -115,7 +115,7 @@ Returns a valid EVP_PKEY* on success, NULL on failure. On success extraHandle may be non-null value which we need to keep alive until the EVP_PKEY is destroyed. -providersSupported is 1 if OpenSSL providers are supported, otherwise 0. +*haveProvider is 1 if OpenSSL providers are supported, otherwise 0. */ PALEXPORT EVP_PKEY* CryptoNative_LoadKeyFromProvider(const char* providerName, const char* keyUri, void** extraHandle, int32_t* haveProvider);