Skip to content

Commit

Permalink
[release/9.0] Make SafeEvpPKeyHandle.OpenKeyFromProvider throw PNSE o…
Browse files Browse the repository at this point in the history 
…n OSSL less than 3.0 (#106753)

* 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>
  • Loading branch information
@github-actions @krwq
3 people authored Aug 23, 2024
1 parent 2937bf3 commit 91ae788
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 15 deletions.
11 changes: 9 additions & 2 deletions src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
{
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Security.Cryptography/src/Resources/Strings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
103 changes: 94 additions & 9 deletions src/libraries/System.Security.Cryptography/tests/OpenSslNamedKeysTests.manual.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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))]
Expand All @@ -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"));
Expand All @@ -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));
Expand All @@ -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()
{
Expand Down
6 changes: 4 additions & 2 deletions src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
Expand Down
6 changes: 4 additions & 2 deletions src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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
Expand Down

0 comments on commit 91ae788

Please sign in to comment.