[release/9.0] Make SafeEvpPKeyHandle.OpenKeyFromProvider throw PNSE on OSSL less than 3.0 (#106753)

github-actions[bot] · krwq · web-flow · commit 91ae78861b5c · 2024-08-23T14:55:39.000-07:00
* Make SafeEvpPKeyHandle.OpenKeyFromProvider throw PNSE on OSSL less than 3.0 * address feedback * Address recent feedback * Relax PNSE expectation for OSSL providers on Apple platforms (#106804) --------- Co-authored-by: Krzysztof Wicher <mordotymoja@gmail.com> Co-authored-by: Krzysztof Wicher <kwicher@microsoft.com>
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
@@ -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_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
@@ -834,6 +834,9 @@
   <data name="PlatformNotSupported_CryptographyOpenSSLNotFound" xml:space="preserve">
     <value>OpenSSL is required for algorithm '{0}' but could not be found or loaded.</value>
   </data>
+  <data name="PlatformNotSupported_CryptographyOpenSSLProvidersNotSupported" xml:space="preserve">
+    <value>OpenSSL providers are not supported on this platform.</value>
+  </data>
   <data name="Security_AccessDenied" xml:space="preserve">
     <value>Access is denied.</value>
   </data>
diff --git 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.IsOpenSsl3;
+        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,11 +91,30 @@ 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<PlatformNotSupportedException>(() => SafeEvpPKeyHandle.OpenPublicKeyFromEngine(TestEngineName, TestEngineKeyId));
             Assert.Throws<PlatformNotSupportedException>(() => SafeEvpPKeyHandle.OpenPrivateKeyFromEngine(TestEngineName, TestEngineKeyId));
-            Assert.Throws<PlatformNotSupportedException>(() => SafeEvpPKeyHandle.OpenKeyFromProvider(Tpm2ProviderName, AnyProviderKeyUri));
+        }
+
+        [ConditionalFact(nameof(ProvidersNotSupported))]
+        public static void ProvidersNotSupported_ThrowsPlatformNotSupported()
+        {
+            try
+            {
+                using SafeEvpPKeyHandle key = SafeEvpPKeyHandle.OpenKeyFromProvider("default", NonExistingEngineOrProviderKeyName);
+                Assert.Fail("We expected an exception to be thrown");
+            }
+            catch (PlatformNotSupportedException)
+            {
+                // Expected
+            }
+            catch (CryptographicException) when (PlatformDetection.IsApplePlatform)
+            {
+                // Our tests detect providers using PlatformDetection.IsOpenSsl3 which is always false for Apple platforms.
+                // Product on the other hand does feature detection and that might end up working
+                // in which case we should still throw any CryptographicException because the keyUri does not exist.
+            }
         }
 
         [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.OpenSslPresentOnSystem))]
@@ -111,10 +135,14 @@ public static void EmptyNameThroughNullCharacter()
         {
             Assert.ThrowsAny<CryptographicException>(() => SafeEvpPKeyHandle.OpenPrivateKeyFromEngine("\0", "foo"));
             Assert.ThrowsAny<CryptographicException>(() => SafeEvpPKeyHandle.OpenPublicKeyFromEngine("\0", "foo"));
-            Assert.ThrowsAny<CryptographicException>(() => SafeEvpPKeyHandle.OpenKeyFromProvider("\0", "foo"));
+
+            if (ProvidersSupported)
+            {
+                Assert.ThrowsAny<CryptographicException>(() => SafeEvpPKeyHandle.OpenKeyFromProvider("\0", "foo"));
+            }
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.OpenSslPresentOnSystem))]
+        [ConditionalFact(nameof(ProvidersSupported))]
         public static void EmptyUriThroughNullCharacter()
         {
             Assert.ThrowsAny<CryptographicException>(() => SafeEvpPKeyHandle.OpenKeyFromProvider("default", "\0"));
@@ -127,7 +155,7 @@ public static void Engine_NonExisting()
             Assert.ThrowsAny<CryptographicException>(() => SafeEvpPKeyHandle.OpenPublicKeyFromEngine(NonExistingEngineOrProviderKeyName, TestEngineKeyId));
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.OpenSslPresentOnSystem))]
+        [ConditionalFact(nameof(ProvidersSupported))]
         public static void Provider_NonExisting()
         {
             Assert.ThrowsAny<CryptographicException>(() => SafeEvpPKeyHandle.OpenKeyFromProvider(NonExistingEngineOrProviderKeyName, AnyProviderKeyUri));
@@ -146,6 +174,63 @@ public static void Provider_NonExistingKey()
             Assert.ThrowsAny<CryptographicException>(() => 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()
         {
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
@@ -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
@@ -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);
 
@@ -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.
+
+*haveProvider 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

Original file line number	Diff line number	Diff line change
`@@ -611,19 +611,20 @@ EVP_PKEY* CryptoNative_LoadPublicKeyFromEngine(const char* engineName, const cha`
`611`	`611`	`return NULL;`
`612`	`612`	`}`
`613`	`613`
`614`		`-EVP_PKEY* CryptoNative_LoadKeyFromProvider(const char* providerName, const char* keyUri, void** extraHandle)`
	`614`	`+EVP_PKEY* CryptoNative_LoadKeyFromProvider(const char* providerName, const char* keyUri, void** extraHandle, int32_t* haveProvider)`
`615`	`615`	`{`
`616`	`616`	`ERR_clear_error();`
`617`	`617`
`618`	`618`	`#ifdef FEATURE_DISTRO_AGNOSTIC_SSL`
`619`	`619`	`if (!API_EXISTS(OSSL_PROVIDER_load))`
`620`	`620`	`{`
`621`		`- ERR_put_error(ERR_LIB_NONE, 0, ERR_R_DISABLED, __FILE__, __LINE__);`
	`621`	`+ *haveProvider = 0;`
`622`	`622`	`return NULL;`
`623`	`623`	`}`
`624`	`624`	`#endif`
`625`	`625`
`626`	`626`	`#ifdef NEED_OPENSSL_3_0`
	`627`	`+ *haveProvider = 1;`
`627`	`628`	`EVP_PKEY* ret = NULL;`
`628`	`629`	`OSSL_LIB_CTX* libCtx = OSSL_LIB_CTX_new();`
`629`	`630`	`OSSL_PROVIDER* prov = NULL;`
`@@ -730,6 +731,7 @@ EVP_PKEY* CryptoNative_LoadKeyFromProvider(const char* providerName, const char*`
`730`	`731`	`(void)keyUri;`
`731`	`732`	`ERR_put_error(ERR_LIB_NONE, 0, ERR_R_DISABLED, __FILE__, __LINE__);`
`732`	`733`	`*extraHandle = NULL;`
	`734`	`+ *haveProvider = 0;`
`733`	`735`	`return NULL;`
`734`	`736`	`#endif`
`735`	`737`	`}`