From be00793bb6e25872c6f202f93043ae5e5ef80dcd Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Fri, 2 Apr 2021 14:20:42 -0700 Subject: [PATCH 1/7] Add RSA tests for default-span inputs where zero length is valid --- .../RSA/SignVerify.netcoreapp.cs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs index 3b7c247068cbc2..6fdf81523462b1 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Xunit; + namespace System.Security.Cryptography.Rsa.Tests { public sealed class SignVerify_Span : SignVerify @@ -30,5 +32,54 @@ private static byte[] TryWithOutputArray(Func func) } } } + + [Fact] + public static void SignDefaultSpanHash() + { + using (RSA rsa = RSAFactory.Create()) + { + byte[] signature = new byte[2048 / 8]; + + Assert.ThrowsAny( + () => rsa.TrySignHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1, out _)); + + Assert.ThrowsAny( + () => rsa.TrySignHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pss, out _)); + } + } + + [Fact] + public static void VerifyDefaultSpanHash() + { + using (RSA rsa = RSAFactory.Create()) + { + byte[] signature = new byte[2048 / 8]; + + Assert.False( + rsa.VerifyHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1)); + + Assert.False( + rsa.VerifyHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pss)); + } + } + + [Fact] + public static void EncryptDefaultSpan() + { + using (RSA rsa = RSAFactory.Create()) + { + byte[] dest = new byte[2048 / 8]; + + Assert.True( + rsa.TryEncrypt(ReadOnlySpan.Empty, dest, RSAEncryptionPadding.Pkcs1, out int written)); + + Assert.Equal(dest.Length, written); + + Assert.True( + rsa.TryEncrypt(ReadOnlySpan.Empty, dest, RSAEncryptionPadding.OaepSHA256, out written)); + + Assert.Equal(dest.Length, written); + } + } } } From 4a92a18b47b735dea7a4f7c80404380276d53a26 Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Fri, 2 Apr 2021 14:21:39 -0700 Subject: [PATCH 2/7] Use EVP_PKEY for RSA public key operations Also change the storage model to SafeEvpPKeyHandle, only import and export use legacy SafeRsaHandle. --- .../Interop.EvpPkey.Rsa.cs | 75 +++++ .../Interop.Rsa.cs | 50 +--- .../Security/Cryptography/RSAOpenSsl.cs | 268 +++++------------- 3 files changed, 144 insertions(+), 249 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs index f89135a31d12ec..775e01fa433865 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs @@ -62,6 +62,41 @@ ref MemoryMarshal.GetReference(destination), return written; } + [DllImport(Libraries.CryptoNative)] + private static extern int CryptoNative_RsaEncrypt( + SafeEvpPKeyHandle pkey, + ref byte source, + int sourceLength, + RSAEncryptionPaddingMode paddingMode, + IntPtr digestAlgorithm, + ref byte destination, + int destinationLength); + + internal static int RsaEncrypt( + SafeEvpPKeyHandle pkey, + ReadOnlySpan source, + RSAEncryptionPaddingMode paddingMode, + IntPtr digestAlgorithm, + Span destination) + { + int written = CryptoNative_RsaEncrypt( + pkey, + ref MemoryMarshal.GetReference(source), + source.Length, + paddingMode, + digestAlgorithm, + ref MemoryMarshal.GetReference(destination), + destination.Length); + + if (written < 0) + { + Debug.Assert(written == -1); + throw CreateOpenSslCryptographicException(); + } + + return written; + } + [DllImport(Libraries.CryptoNative)] private static extern int CryptoNative_RsaSignHash( SafeEvpPKeyHandle pkey, @@ -97,6 +132,46 @@ ref MemoryMarshal.GetReference(destination), return written; } + [DllImport(Libraries.CryptoNative)] + private static extern int CryptoNative_RsaVerifyHash( + SafeEvpPKeyHandle pkey, + RSASignaturePaddingMode paddingMode, + IntPtr digestAlgorithm, + ref byte hash, + int hashLength, + ref byte signature, + int signatureLength); + + internal static bool RsaVerifyHash( + SafeEvpPKeyHandle pkey, + RSASignaturePaddingMode paddingMode, + IntPtr digestAlgorithm, + ReadOnlySpan hash, + ReadOnlySpan signature) + { + int ret = CryptoNative_RsaVerifyHash( + pkey, + paddingMode, + digestAlgorithm, + ref MemoryMarshal.GetReference(hash), + hash.Length, + ref MemoryMarshal.GetReference(signature), + signature.Length); + + if (ret == 1) + { + return true; + } + + if (ret == 0) + { + return false; + } + + Debug.Assert(ret == -1); + throw CreateOpenSslCryptographicException(); + } + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_EvpPkeyGetRsa")] internal static extern SafeRsaHandle EvpPkeyGetRsa(SafeEvpPKeyHandle pkey); diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Rsa.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Rsa.cs index 2d6023cce3666b..105258194845c7 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Rsa.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Rsa.cs @@ -27,61 +27,17 @@ internal static SafeRsaHandle DecodeRsaPublicKey(ReadOnlySpan buf) => [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_DecodeRsaPublicKey")] private static extern SafeRsaHandle DecodeRsaPublicKey(ref byte buf, int len); - internal static int RsaPublicEncrypt( - int flen, - ReadOnlySpan from, - Span to, - SafeRsaHandle rsa, - RsaPadding padding) => - RsaPublicEncrypt(flen, ref MemoryMarshal.GetReference(from), ref MemoryMarshal.GetReference(to), rsa, padding); - - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_RsaPublicEncrypt")] - private static extern int RsaPublicEncrypt( - int flen, - ref byte from, - ref byte to, - SafeRsaHandle rsa, - RsaPadding padding); - - internal static int RsaVerificationPrimitive( - ReadOnlySpan from, - Span to, - SafeRsaHandle rsa) => - RsaVerificationPrimitive(from.Length, ref MemoryMarshal.GetReference(from), ref MemoryMarshal.GetReference(to), rsa); - - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_RsaVerificationPrimitive")] - private static extern int RsaVerificationPrimitive( - int flen, - ref byte from, - ref byte to, - SafeRsaHandle rsa); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_RsaSize")] internal static extern int RsaSize(SafeRsaHandle rsa); - internal static bool RsaVerify(int type, ReadOnlySpan m, ReadOnlySpan sigbuf, SafeRsaHandle rsa) + internal static RSAParameters ExportRsaParameters(SafeEvpPKeyHandle key, bool includePrivateParameters) { - bool ret = RsaVerify( - type, - ref MemoryMarshal.GetReference(m), - m.Length, - ref MemoryMarshal.GetReference(sigbuf), - sigbuf.Length, - rsa); - - if (!ret) + using (SafeRsaHandle rsa = EvpPkeyGetRsa(key)) { - ErrClearError(); + return ExportRsaParameters(rsa, includePrivateParameters); } - - return ret; } - - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_RsaVerify")] - [return: MarshalAs(UnmanagedType.Bool)] - private static extern bool RsaVerify(int type, ref byte m, int m_len, ref byte sigbuf, int siglen, SafeRsaHandle rsa); - internal static RSAParameters ExportRsaParameters(SafeRsaHandle key, bool includePrivateParameters) { Debug.Assert( diff --git a/src/libraries/Common/src/System/Security/Cryptography/RSAOpenSsl.cs b/src/libraries/Common/src/System/Security/Cryptography/RSAOpenSsl.cs index 1bbcade3bbef6d..40381076b0321d 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/RSAOpenSsl.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/RSAOpenSsl.cs @@ -23,7 +23,7 @@ public sealed partial class RSAOpenSsl : RSA { private const int BitsPerByte = 8; - private Lazy _key; + private Lazy _key; public RSAOpenSsl() : this(2048) @@ -33,7 +33,7 @@ public RSAOpenSsl() public RSAOpenSsl(int keySize) { base.KeySize = keySize; - _key = new Lazy(GenerateKey); + _key = new Lazy(GenerateKey); } public override int KeySize @@ -50,7 +50,7 @@ public override int KeySize ThrowIfDisposed(); FreeKey(); - _key = new Lazy(GenerateKey); + _key = new Lazy(GenerateKey); } } @@ -84,7 +84,7 @@ public override byte[] Decrypt(byte[] data, RSAEncryptionPadding padding) throw new ArgumentNullException(nameof(padding)); ValidatePadding(padding); - SafeEvpPKeyHandle key = GetPKey(); + SafeEvpPKeyHandle key = GetKey(); int rsaSize = Interop.Crypto.EvpPKeySize(key); byte[]? buf = null; Span destination = default; @@ -101,8 +101,6 @@ public override byte[] Decrypt(byte[] data, RSAEncryptionPadding padding) { CryptographicOperations.ZeroMemory(destination); CryptoPool.Return(buf!, clearSize: 0); - // Until EVP_PKEY is what gets stored, free the temporary key handle. - key.Dispose(); } } @@ -116,7 +114,7 @@ public override bool TryDecrypt( throw new ArgumentNullException(nameof(padding)); ValidatePadding(padding); - SafeEvpPKeyHandle key = GetPKey(); + SafeEvpPKeyHandle key = GetKey(); int keySizeBytes = Interop.Crypto.EvpPKeySize(key); // OpenSSL requires that the decryption buffer be at least as large as EVP_PKEY_size. @@ -135,8 +133,6 @@ public override bool TryDecrypt( } int written = Decrypt(key, data, tmp, padding); - // Until EVP_PKEY is what gets stored, free the temporary key handle. - key.Dispose(); bool ret; if (destination.Length < written) @@ -165,8 +161,6 @@ public override bool TryDecrypt( } bytesWritten = Decrypt(key, data, destination, padding); - // Until EVP_PKEY is what gets stored, free the temporary key handle. - key.Dispose(); return true; } @@ -215,17 +209,16 @@ public override byte[] Encrypt(byte[] data, RSAEncryptionPadding padding) if (padding == null) throw new ArgumentNullException(nameof(padding)); - Interop.Crypto.RsaPadding rsaPadding = GetInteropPadding(padding, out RsaPaddingProcessor? oaepProcessor); - SafeRsaHandle key = GetKey(); + ValidatePadding(padding); + SafeEvpPKeyHandle key = GetKey(); - byte[] buf = new byte[Interop.Crypto.RsaSize(key)]; + byte[] buf = new byte[Interop.Crypto.EvpPKeySize(key)]; bool encrypted = TryEncrypt( key, data, buf, - rsaPadding, - oaepProcessor, + padding, out int bytesWritten); if (!encrypted || bytesWritten != buf.Length) @@ -244,21 +237,20 @@ public override bool TryEncrypt(ReadOnlySpan data, Span destination, throw new ArgumentNullException(nameof(padding)); } - Interop.Crypto.RsaPadding rsaPadding = GetInteropPadding(padding, out RsaPaddingProcessor? oaepProcessor); - SafeRsaHandle key = GetKey(); + ValidatePadding(padding); + SafeEvpPKeyHandle? key = GetKey(); - return TryEncrypt(key, data, destination, rsaPadding, oaepProcessor, out bytesWritten); + return TryEncrypt(key, data, destination, padding, out bytesWritten); } private static bool TryEncrypt( - SafeRsaHandle key, + SafeEvpPKeyHandle key, ReadOnlySpan data, Span destination, - Interop.Crypto.RsaPadding rsaPadding, - RsaPaddingProcessor? rsaPaddingProcessor, + RSAEncryptionPadding padding, out int bytesWritten) { - int rsaSize = Interop.Crypto.RsaSize(key); + int rsaSize = Interop.Crypto.EvpPKeySize(key); if (destination.Length < rsaSize) { @@ -266,69 +258,30 @@ private static bool TryEncrypt( return false; } - int returnValue; - - if (rsaPaddingProcessor != null) - { - Debug.Assert(rsaPadding == Interop.Crypto.RsaPadding.NoPadding); - byte[] rented = CryptoPool.Rent(rsaSize); - Span tmp = new Span(rented, 0, rsaSize); + IntPtr hashAlgorithm = IntPtr.Zero; - try - { - rsaPaddingProcessor.PadOaep(data, tmp); - returnValue = Interop.Crypto.RsaPublicEncrypt(tmp.Length, tmp, destination, key, rsaPadding); - } - finally - { - CryptographicOperations.ZeroMemory(tmp); - CryptoPool.Return(rented, clearSize: 0); - } - } - else + if (padding.Mode == RSAEncryptionPaddingMode.Oaep) { - Debug.Assert(rsaPadding != Interop.Crypto.RsaPadding.NoPadding); - - returnValue = Interop.Crypto.RsaPublicEncrypt(data.Length, data, destination, key, rsaPadding); + Debug.Assert(padding.OaepHashAlgorithm.Name != null); + hashAlgorithm = Interop.Crypto.HashAlgorithmToEvp(padding.OaepHashAlgorithm.Name); } - CheckReturn(returnValue); + int written = Interop.Crypto.RsaEncrypt( + key, + data, + padding.Mode, + hashAlgorithm, + destination); - bytesWritten = returnValue; - Debug.Assert(returnValue == rsaSize); + Debug.Assert(written == rsaSize); + bytesWritten = written; return true; - - } - - private static Interop.Crypto.RsaPadding GetInteropPadding( - RSAEncryptionPadding padding, - out RsaPaddingProcessor? rsaPaddingProcessor) - { - if (padding == RSAEncryptionPadding.Pkcs1) - { - rsaPaddingProcessor = null; - return Interop.Crypto.RsaPadding.Pkcs1; - } - - if (padding == RSAEncryptionPadding.OaepSHA1) - { - rsaPaddingProcessor = null; - return Interop.Crypto.RsaPadding.OaepSHA1; - } - - if (padding.Mode == RSAEncryptionPaddingMode.Oaep) - { - rsaPaddingProcessor = RsaPaddingProcessor.OpenProcessor(padding.OaepHashAlgorithm); - return Interop.Crypto.RsaPadding.NoPadding; - } - - throw PaddingModeNotSupported(); } public override RSAParameters ExportParameters(bool includePrivateParameters) { // It's entirely possible that this line will cause the key to be generated in the first place. - SafeRsaHandle key = GetKey(); + SafeEvpPKeyHandle key = GetKey(); RSAParameters rsaParameters = Interop.Crypto.ExportRsaParameters(key, includePrivateParameters); bool hasPrivateKey = rsaParameters.D != null; @@ -347,6 +300,7 @@ public override void ImportParameters(RSAParameters parameters) ThrowIfDisposed(); SafeRsaHandle key = Interop.Crypto.RsaCreate(); + SafeEvpPKeyHandle pkey = Interop.Crypto.EvpPkeyCreate(); bool imported = false; Interop.Crypto.CheckValidOpenSslHandle(key); @@ -385,12 +339,20 @@ public override void ImportParameters(RSAParameters parameters) } } + if (!Interop.Crypto.EvpPkeySetRsa(pkey, key)) + { + pkey.Dispose(); + key.Dispose(); + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + + key.Dispose(); FreeKey(); - _key = new Lazy(key); + _key = new Lazy(pkey); // Use ForceSet instead of the property setter to ensure that LegalKeySizes doesn't interfere // with the already loaded key. - ForceSetKeySize(BitsPerByte * Interop.Crypto.RsaSize(key)); + ForceSetKeySize(BitsPerByte * Interop.Crypto.EvpPKeySize(pkey)); } public override void ImportRSAPublicKey(ReadOnlySpan source, out int bytesRead) @@ -413,16 +375,26 @@ public override void ImportRSAPublicKey(ReadOnlySpan source, out int bytes throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding, e); } + SafeEvpPKeyHandle pkey = Interop.Crypto.EvpPkeyCreate(); SafeRsaHandle key = Interop.Crypto.DecodeRsaPublicKey(source.Slice(0, read)); Interop.Crypto.CheckValidOpenSslHandle(key); + if (!Interop.Crypto.EvpPkeySetRsa(pkey, key)) + { + key.Dispose(); + pkey.Dispose(); + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } + + key.Dispose(); + FreeKey(); - _key = new Lazy(key); + _key = new Lazy(pkey); // Use ForceSet instead of the property setter to ensure that LegalKeySizes doesn't interfere // with the already loaded key. - ForceSetKeySize(BitsPerByte * Interop.Crypto.RsaSize(key)); + ForceSetKeySize(BitsPerByte * Interop.Crypto.EvpPKeySize(pkey)); bytesRead = read; } @@ -460,7 +432,7 @@ private void FreeKey() { if (_key != null && _key.IsValueCreated) { - SafeRsaHandle handle = _key.Value; + SafeEvpPKeyHandle handle = _key.Value; handle?.Dispose(); } } @@ -516,35 +488,11 @@ private void ThrowIfDisposed() } } - private SafeEvpPKeyHandle GetPKey() - { - SafeRsaHandle currentKey = GetKey(); - SafeEvpPKeyHandle pkeyHandle = Interop.Crypto.EvpPkeyCreate(); - - try - { - // Wrapping our key in an EVP_PKEY will up_ref our key. - // When the EVP_PKEY is Disposed it will down_ref the key. - // So everything should be copacetic. - if (!Interop.Crypto.EvpPkeySetRsa(pkeyHandle, currentKey)) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - } - catch - { - pkeyHandle.Dispose(); - throw; - } - - return pkeyHandle; - } - - private SafeRsaHandle GetKey() + private SafeEvpPKeyHandle GetKey() { ThrowIfDisposed(); - SafeRsaHandle key = _key.Value; + SafeEvpPKeyHandle key = _key.Value; if (key == null || key.IsInvalid) { @@ -554,36 +502,9 @@ private SafeRsaHandle GetKey() return key; } - private static void CheckReturn(int returnValue) - { - if (returnValue == -1) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - } - - private static void CheckBoolReturn(int returnValue) - { - if (returnValue != 1) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - } - - private SafeRsaHandle GenerateKey() + private SafeEvpPKeyHandle GenerateKey() { - using (SafeEvpPKeyHandle pkey = Interop.Crypto.RsaGenerateKey(KeySize)) - { - SafeRsaHandle rsa = Interop.Crypto.EvpPkeyGetRsa(pkey); - - if (rsa.IsInvalid) - { - rsa.Dispose(); - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - - return rsa; - } + return Interop.Crypto.RsaGenerateKey(KeySize); } protected override byte[] HashData(byte[] data, int offset, int count, HashAlgorithmName hashAlgorithm) => @@ -665,7 +586,7 @@ private bool TrySignHash( signature = null; IntPtr digestAlgorithm = Interop.Crypto.HashAlgorithmToEvp(hashAlgorithm.Name); - SafeEvpPKeyHandle key = GetPKey(); + SafeEvpPKeyHandle key = GetKey(); int bytesRequired = Interop.Crypto.EvpPKeySize(key); if (allocateSignature) @@ -684,8 +605,6 @@ private bool TrySignHash( Debug.Assert(written == bytesRequired); bytesWritten = written; - // Until EVP_PKEY is what gets stored, free the temporary key handle. - key.Dispose(); return true; } @@ -713,73 +632,18 @@ public override bool VerifyHash(ReadOnlySpan hash, ReadOnlySpan sign { throw HashAlgorithmNameNullOrEmpty(); } - if (padding == null) - { - throw new ArgumentNullException(nameof(padding)); - } - if (padding == RSASignaturePadding.Pkcs1) - { - int algorithmNid = GetAlgorithmNid(hashAlgorithm); - SafeRsaHandle rsa = GetKey(); - return Interop.Crypto.RsaVerify(algorithmNid, hash, signature, rsa); - } - else if (padding == RSASignaturePadding.Pss) - { - RsaPaddingProcessor processor = RsaPaddingProcessor.OpenProcessor(hashAlgorithm); - SafeRsaHandle rsa = GetKey(); - - int requiredBytes = Interop.Crypto.RsaSize(rsa); - - if (signature.Length != requiredBytes) - { - return false; - } - - if (hash.Length != processor.HashLength) - { - return false; - } - - byte[] rented = CryptoPool.Rent(requiredBytes); - Span unwrapped = new Span(rented, 0, requiredBytes); - - try - { - int ret = Interop.Crypto.RsaVerificationPrimitive(signature, unwrapped, rsa); - - CheckReturn(ret); - - Debug.Assert( - ret == requiredBytes, - $"RSA_private_encrypt returned {ret} when {requiredBytes} was expected"); - - return processor.VerifyPss(hash, unwrapped, KeySize); - } - finally - { - CryptoPool.Return(rented, requiredBytes); - } - } - - throw PaddingModeNotSupported(); - } - - private static int GetAlgorithmNid(HashAlgorithmName hashAlgorithmName) - { - // All of the current HashAlgorithmName values correspond to the SN values in OpenSSL 0.9.8. - // If there's ever a new one that doesn't, translate it here. - string sn = hashAlgorithmName.Name!; - - int nid = Interop.Crypto.ObjSn2Nid(sn); + ValidatePadding(padding); - if (nid == Interop.Crypto.NID_undef) - { - Interop.Crypto.ErrClearError(); - throw new CryptographicException(SR.Cryptography_UnknownHashAlgorithm, hashAlgorithmName.Name); - } + IntPtr digestAlgorithm = Interop.Crypto.HashAlgorithmToEvp(hashAlgorithm.Name); + SafeEvpPKeyHandle key = GetKey(); - return nid; + return Interop.Crypto.RsaVerifyHash( + key, + padding.Mode, + digestAlgorithm, + hash, + signature); } private static void ValidatePadding(RSAEncryptionPadding padding) From cfaec60b052583201f71ebc6f5c435c9f61a6a50 Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Fri, 2 Apr 2021 14:36:47 -0700 Subject: [PATCH 3/7] Fix compilation errors in managed OpenSsl project --- .../Interop.EvpPkey.Rsa.cs | 4 ++ .../Security/Cryptography/RSAOpenSsl.cs | 48 +++++++++---------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs index 775e01fa433865..c8849d343d9cee 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs @@ -178,5 +178,9 @@ ref MemoryMarshal.GetReference(signature), [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_EvpPkeySetRsa")] [return: MarshalAs(UnmanagedType.Bool)] internal static extern bool EvpPkeySetRsa(SafeEvpPKeyHandle pkey, SafeRsaHandle rsa); + + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_EvpPkeySetRsa")] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool EvpPkeySetRsa(SafeEvpPKeyHandle pkey, IntPtr rsa); } } diff --git a/src/libraries/System.Security.Cryptography.OpenSsl/src/System/Security/Cryptography/RSAOpenSsl.cs b/src/libraries/System.Security.Cryptography.OpenSsl/src/System/Security/Cryptography/RSAOpenSsl.cs index a666cfc65472ae..eb7d7545928e8b 100644 --- a/src/libraries/System.Security.Cryptography.OpenSsl/src/System/Security/Cryptography/RSAOpenSsl.cs +++ b/src/libraries/System.Security.Cryptography.OpenSsl/src/System/Security/Cryptography/RSAOpenSsl.cs @@ -10,7 +10,7 @@ public sealed partial class RSAOpenSsl : RSA public RSAOpenSsl(RSAParameters parameters) { // Make _key be non-null before calling ImportParameters - _key = new Lazy(); + _key = new Lazy(); ImportParameters(parameters); } @@ -29,12 +29,18 @@ public RSAOpenSsl(IntPtr handle) if (handle == IntPtr.Zero) throw new ArgumentException(SR.Cryptography_OpenInvalidHandle, nameof(handle)); - SafeRsaHandle rsaHandle = SafeRsaHandle.DuplicateHandle(handle); + SafeEvpPKeyHandle pkey = Interop.Crypto.EvpPkeyCreate(); + + if (!Interop.Crypto.EvpPkeySetRsa(pkey, handle)) + { + pkey.Dispose(); + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } // Use ForceSet instead of the property setter to ensure that LegalKeySizes doesn't interfere // with the already loaded key. - ForceSetKeySize(BitsPerByte * Interop.Crypto.RsaSize(rsaHandle)); - _key = new Lazy(() => rsaHandle, isThreadSafe: true); + ForceSetKeySize(BitsPerByte * Interop.Crypto.EvpPKeySize(pkey)); + _key = new Lazy(pkey); } /// @@ -54,19 +60,21 @@ public RSAOpenSsl(SafeEvpPKeyHandle pkeyHandle) if (pkeyHandle.IsInvalid) throw new ArgumentException(SR.Cryptography_OpenInvalidHandle, nameof(pkeyHandle)); - // If rsa is valid it has already been up-ref'd, so we can just use this handle as-is. - SafeRsaHandle rsa = Interop.Crypto.EvpPkeyGetRsa(pkeyHandle); + SafeEvpPKeyHandle newKey = Interop.Crypto.EvpPkeyCreate(); - if (rsa.IsInvalid) + using (SafeRsaHandle rsa = Interop.Crypto.EvpPkeyGetRsa(pkeyHandle)) { - rsa.Dispose(); - throw Interop.Crypto.CreateOpenSslCryptographicException(); + if (rsa.IsInvalid || !Interop.Crypto.EvpPkeySetRsa(newKey, rsa)) + { + newKey.Dispose(); + throw Interop.Crypto.CreateOpenSslCryptographicException(); + } } // Use ForceSet instead of the property setter to ensure that LegalKeySizes doesn't interfere // with the already loaded key. - ForceSetKeySize(BitsPerByte * Interop.Crypto.RsaSize(rsa)); - _key = new Lazy(() => rsa, isThreadSafe: true); + ForceSetKeySize(BitsPerByte * Interop.Crypto.EvpPKeySize(newKey)); + _key = new Lazy(newKey); } /// @@ -76,26 +84,18 @@ public RSAOpenSsl(SafeEvpPKeyHandle pkeyHandle) /// A SafeHandle for the RSA key in OpenSSL public SafeEvpPKeyHandle DuplicateKeyHandle() { - SafeRsaHandle currentKey = _key.Value; SafeEvpPKeyHandle pkeyHandle = Interop.Crypto.EvpPkeyCreate(); - try + using (SafeRsaHandle rsa = Interop.Crypto.EvpPkeyGetRsa(GetKey())) { - // Wrapping our key in an EVP_PKEY will up_ref our key. - // When the EVP_PKEY is Disposed it will down_ref the key. - // So everything should be copacetic. - if (!Interop.Crypto.EvpPkeySetRsa(pkeyHandle, currentKey)) + if (rsa.IsInvalid || !Interop.Crypto.EvpPkeySetRsa(pkeyHandle, rsa)) { + pkeyHandle.Dispose(); throw Interop.Crypto.CreateOpenSslCryptographicException(); } - - return pkeyHandle; - } - catch - { - pkeyHandle.Dispose(); - throw; } + + return pkeyHandle; } } } From 434c2a287352054763fdd5fc3d3c3dacf0035f9c Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Fri, 2 Apr 2021 14:44:26 -0700 Subject: [PATCH 4/7] Fix compilation errors in X509Certificates --- .../src/System.Security.Cryptography.X509Certificates.csproj | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/System.Security.Cryptography.X509Certificates.csproj b/src/libraries/System.Security.Cryptography.X509Certificates/src/System.Security.Cryptography.X509Certificates.csproj index 00b5c63c0fd49f..29f958dcbc4db5 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/System.Security.Cryptography.X509Certificates.csproj +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/System.Security.Cryptography.X509Certificates.csproj @@ -346,8 +346,6 @@ Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.OCSP.cs" /> - - Date: Fri, 2 Apr 2021 16:52:59 -0700 Subject: [PATCH 5/7] Use EVP_PKEY for RSA public key operations (native code) --- .../entrypoints.c | 5 +- .../opensslshim.h | 10 +- .../pal_evp_pkey_rsa.c | 173 ++++++++++++++---- .../pal_evp_pkey_rsa.h | 27 +++ .../pal_rsa.c | 35 ---- .../pal_rsa.h | 51 ------ 6 files changed, 178 insertions(+), 123 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c index f92f0eabd5342e..1866691fa4df01 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c @@ -224,13 +224,12 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_RsaCreate) DllImportEntry(CryptoNative_RsaDecrypt) DllImportEntry(CryptoNative_RsaDestroy) + DllImportEntry(CryptoNative_RsaEncrypt) DllImportEntry(CryptoNative_RsaGenerateKey) - DllImportEntry(CryptoNative_RsaPublicEncrypt) DllImportEntry(CryptoNative_RsaSignHash) DllImportEntry(CryptoNative_RsaSize) DllImportEntry(CryptoNative_RsaUpRef) - DllImportEntry(CryptoNative_RsaVerificationPrimitive) - DllImportEntry(CryptoNative_RsaVerify) + DllImportEntry(CryptoNative_RsaVerifyHash) DllImportEntry(CryptoNative_SetRsaParameters) DllImportEntry(CryptoNative_UpRefEvpPkey) DllImportEntry(CryptoNative_X509ChainBuildOcspRequest) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index f405c5f6186c7e..b0174f998dd41e 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -397,6 +397,8 @@ void SSL_get0_alpn_selected(const SSL* ssl, const unsigned char** protocol, unsi REQUIRED_FUNCTION(EVP_PKEY_derive_set_peer) \ REQUIRED_FUNCTION(EVP_PKEY_derive_init) \ REQUIRED_FUNCTION(EVP_PKEY_derive) \ + REQUIRED_FUNCTION(EVP_PKEY_encrypt) \ + REQUIRED_FUNCTION(EVP_PKEY_encrypt_init) \ REQUIRED_FUNCTION(EVP_PKEY_free) \ FALLBACK_FUNCTION(EVP_PKEY_get0_RSA) \ REQUIRED_FUNCTION(EVP_PKEY_get1_DSA) \ @@ -412,6 +414,8 @@ void SSL_get0_alpn_selected(const SSL* ssl, const unsigned char** protocol, unsi REQUIRED_FUNCTION(EVP_PKEY_sign_init) \ REQUIRED_FUNCTION(EVP_PKEY_size) \ FALLBACK_FUNCTION(EVP_PKEY_up_ref) \ + REQUIRED_FUNCTION(EVP_PKEY_verify) \ + REQUIRED_FUNCTION(EVP_PKEY_verify_init) \ REQUIRED_FUNCTION(EVP_rc2_cbc) \ REQUIRED_FUNCTION(EVP_rc2_ecb) \ REQUIRED_FUNCTION(EVP_sha1) \ @@ -491,8 +495,6 @@ void SSL_get0_alpn_selected(const SSL* ssl, const unsigned char** protocol, unsi REQUIRED_FUNCTION(RSA_new) \ FALLBACK_FUNCTION(RSA_pkey_ctx_ctrl) \ RENAMED_FUNCTION(RSA_PKCS1_OpenSSL, RSA_PKCS1_SSLeay) \ - REQUIRED_FUNCTION(RSA_public_decrypt) \ - REQUIRED_FUNCTION(RSA_public_encrypt) \ FALLBACK_FUNCTION(RSA_set0_crt_params) \ FALLBACK_FUNCTION(RSA_set0_factors) \ FALLBACK_FUNCTION(RSA_set0_key) \ @@ -818,6 +820,8 @@ FOR_ALL_OPENSSL_FUNCTIONS #define EVP_PKEY_derive_set_peer EVP_PKEY_derive_set_peer_ptr #define EVP_PKEY_derive_init EVP_PKEY_derive_init_ptr #define EVP_PKEY_derive EVP_PKEY_derive_ptr +#define EVP_PKEY_encrypt_init EVP_PKEY_encrypt_init_ptr +#define EVP_PKEY_encrypt EVP_PKEY_encrypt_ptr #define EVP_PKEY_free EVP_PKEY_free_ptr #define EVP_PKEY_get0_RSA EVP_PKEY_get0_RSA_ptr #define EVP_PKEY_get1_DSA EVP_PKEY_get1_DSA_ptr @@ -833,6 +837,8 @@ FOR_ALL_OPENSSL_FUNCTIONS #define EVP_PKEY_sign EVP_PKEY_sign_ptr #define EVP_PKEY_size EVP_PKEY_size_ptr #define EVP_PKEY_up_ref EVP_PKEY_up_ref_ptr +#define EVP_PKEY_verify_init EVP_PKEY_verify_init_ptr +#define EVP_PKEY_verify EVP_PKEY_verify_ptr #define EVP_rc2_cbc EVP_rc2_cbc_ptr #define EVP_rc2_ecb EVP_rc2_ecb_ptr #define EVP_sha1 EVP_sha1_ptr diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c index 57c2b247b64f1f..65414073033f58 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c @@ -35,6 +35,36 @@ EVP_PKEY* CryptoNative_RsaGenerateKey(int keySize) return ret; } +static bool ConfigureEncryption(EVP_PKEY_CTX* ctx, RsaPaddingMode padding, const EVP_MD* digest) +{ + if (padding == RsaPaddingPkcs1) + { + if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_PADDING) <= 0) + { + return false; + } + } + else + { + assert(padding == RsaPaddingOaepOrPss); + + if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_OAEP_PADDING) <= 0) + { + return false; + } + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wcast-qual" + if (EVP_PKEY_CTX_set_rsa_oaep_md(ctx, digest) <= 0) +#pragma clang diagnostic pop + { + return false; + } + } + + return true; +} + int32_t CryptoNative_RsaDecrypt(EVP_PKEY* pkey, const uint8_t* source, int32_t sourceLen, @@ -58,29 +88,9 @@ int32_t CryptoNative_RsaDecrypt(EVP_PKEY* pkey, goto done; } - if (padding == RsaPaddingPkcs1) - { - if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_PADDING) <= 0) - { - goto done; - } - } - else + if (!ConfigureEncryption(ctx, padding, digest)) { - assert(padding == RsaPaddingOaepOrPss); - - if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_OAEP_PADDING) <= 0) - { - goto done; - } - -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wcast-qual" - if (EVP_PKEY_CTX_set_rsa_oaep_md(ctx, digest) <= 0) -#pragma clang diagnostic pop - { - goto done; - } + goto done; } // This check may no longer be needed on OpenSSL 3.0 @@ -110,13 +120,13 @@ int32_t CryptoNative_RsaDecrypt(EVP_PKEY* pkey, return ret; } -int32_t CryptoNative_RsaSignHash(EVP_PKEY* pkey, - RsaPaddingMode padding, - const EVP_MD* digest, - const uint8_t* hash, - int32_t hashLen, - uint8_t* destination, - int32_t destinationLen) +int32_t CryptoNative_RsaEncrypt(EVP_PKEY* pkey, + const uint8_t* source, + int32_t sourceLen, + RsaPaddingMode padding, + const EVP_MD* digest, + uint8_t* destination, + int32_t destinationLen) { assert(pkey != NULL); assert(destination != NULL); @@ -127,16 +137,39 @@ int32_t CryptoNative_RsaSignHash(EVP_PKEY* pkey, int ret = -1; - if (ctx == NULL || EVP_PKEY_sign_init(ctx) <= 0) + if (ctx == NULL || EVP_PKEY_encrypt_init(ctx) <= 0) { goto done; } + if (!ConfigureEncryption(ctx, padding, digest)) + { + goto done; + } + + size_t written = Int32ToSizeT(destinationLen); + + if (EVP_PKEY_encrypt(ctx, destination, &written, source, Int32ToSizeT(sourceLen)) > 0) + { + ret = SizeTToInt32(written); + } + +done: + if (ctx != NULL) + { + EVP_PKEY_CTX_free(ctx); + } + + return ret; +} + +static bool ConfigureSignature(EVP_PKEY_CTX* ctx, RsaPaddingMode padding, const EVP_MD* digest) +{ if (padding == RsaPaddingPkcs1) { if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_PADDING) <= 0) { - goto done; + return false; } } else @@ -146,7 +179,7 @@ int32_t CryptoNative_RsaSignHash(EVP_PKEY* pkey, if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_PSS_PADDING) <= 0 || EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx, RSA_PSS_SALTLEN_DIGEST) <= 0) { - goto done; + return false; } } @@ -154,6 +187,36 @@ int32_t CryptoNative_RsaSignHash(EVP_PKEY* pkey, #pragma clang diagnostic ignored "-Wcast-qual" if (EVP_PKEY_CTX_set_signature_md(ctx, digest) <= 0) #pragma clang diagnostic pop + { + return false; + } + + return true; +} + +int32_t CryptoNative_RsaSignHash(EVP_PKEY* pkey, + RsaPaddingMode padding, + const EVP_MD* digest, + const uint8_t* hash, + int32_t hashLen, + uint8_t* destination, + int32_t destinationLen) +{ + assert(pkey != NULL); + assert(destination != NULL); + assert(padding >= RsaPaddingPkcs1 && padding <= RsaPaddingOaepOrPss); + assert(digest != NULL || padding == RsaPaddingPkcs1); + + EVP_PKEY_CTX* ctx = EVP_PKEY_CTX_new(pkey, NULL); + + int ret = -1; + + if (ctx == NULL || EVP_PKEY_sign_init(ctx) <= 0) + { + goto done; + } + + if (!ConfigureSignature(ctx, padding, digest)) { goto done; } @@ -185,6 +248,52 @@ int32_t CryptoNative_RsaSignHash(EVP_PKEY* pkey, return ret; } +int32_t CryptoNative_RsaVerifyHash(EVP_PKEY* pkey, + RsaPaddingMode padding, + const EVP_MD* digest, + const uint8_t* hash, + int32_t hashLen, + const uint8_t* signature, + int32_t signatureLen) +{ + assert(pkey != NULL); + assert(signature != NULL); + assert(padding >= RsaPaddingPkcs1 && padding <= RsaPaddingOaepOrPss); + assert(digest != NULL || padding == RsaPaddingPkcs1); + + EVP_PKEY_CTX* ctx = EVP_PKEY_CTX_new(pkey, NULL); + + int ret = -1; + + if (ctx == NULL || EVP_PKEY_verify_init(ctx) <= 0) + { + goto done; + } + + if (!ConfigureSignature(ctx, padding, digest)) + { + goto done; + } + + // EVP_PKEY_verify is not consistent on whether a mis-sized hash is an error or just a mismatch. + // Normalize to mismatch. + if (hashLen != EVP_MD_size(digest)) + { + ret = 0; + goto done; + } + + ret = EVP_PKEY_verify(ctx, signature, Int32ToSizeT(signatureLen), hash, Int32ToSizeT(hashLen)); + +done: + if (ctx != NULL) + { + EVP_PKEY_CTX_free(ctx); + } + + return ret; +} + RSA* CryptoNative_EvpPkeyGetRsa(EVP_PKEY* pkey) { return EVP_PKEY_get1_RSA(pkey); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.h index 332696b6873acd..d913bc3554b93f 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.h @@ -33,6 +33,19 @@ PALEXPORT int32_t CryptoNative_RsaDecrypt(EVP_PKEY* pkey, uint8_t* destination, int32_t destinationLen); +/* +Encrypt source into destination using the specified RSA key (wrapped in an EVP_PKEY) and padding/digest options. + +Returns the number of bytes written to destination, -1 on error. +*/ +PALEXPORT int32_t CryptoNative_RsaEncrypt(EVP_PKEY* pkey, + const uint8_t* source, + int32_t sourceLen, + RsaPaddingMode padding, + const EVP_MD* digest, + uint8_t* destination, + int32_t destinationLen); + /* Complete the RSA signature generation for the specified hash using the provided RSA key (wrapped in an EVP_PKEY) and padding/digest options. @@ -47,6 +60,20 @@ PALEXPORT int32_t CryptoNative_RsaSignHash(EVP_PKEY* pkey, uint8_t* destination, int32_t destinationLen); +/* +Verify an RSA signature for the specified hash using the provided RSA key (wrapped in an EVP_PKEY) +and padding/digest options. + +Returns 1 on a verified signature, 0 on a mismatched signature, -1 on error. +*/ +PALEXPORT int32_t CryptoNative_RsaVerifyHash(EVP_PKEY* pkey, + RsaPaddingMode padding, + const EVP_MD* digest, + const uint8_t* hash, + int32_t hashLen, + const uint8_t* signature, + int32_t signatureLen); + /* Shims the EVP_PKEY_get1_RSA method. diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c index 1f115a5f2533fe..197f8bfd9bb443 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c @@ -32,46 +32,11 @@ RSA* CryptoNative_DecodeRsaPublicKey(const uint8_t* buf, int32_t len) return d2i_RSAPublicKey(NULL, &buf, len); } -static int GetOpenSslPadding(RsaPadding padding) -{ - assert(padding == Pkcs1 || padding == OaepSHA1 || padding == NoPadding); - - switch (padding) - { - case Pkcs1: - return RSA_PKCS1_PADDING; - case OaepSHA1: - return RSA_PKCS1_OAEP_PADDING; - case NoPadding: - return RSA_NO_PADDING; - } - - return RSA_NO_PADDING; -} - -int32_t -CryptoNative_RsaPublicEncrypt(int32_t flen, const uint8_t* from, uint8_t* to, RSA* rsa, RsaPadding padding) -{ - int openSslPadding = GetOpenSslPadding(padding); - return RSA_public_encrypt(flen, from, to, rsa, openSslPadding); -} - -int32_t CryptoNative_RsaVerificationPrimitive(int32_t flen, const uint8_t* from, uint8_t* to, RSA* rsa) -{ - return RSA_public_decrypt(flen, from, to, rsa, RSA_NO_PADDING); -} - int32_t CryptoNative_RsaSize(RSA* rsa) { return RSA_size(rsa); } -int32_t -CryptoNative_RsaVerify(int32_t type, const uint8_t* m, int32_t mlen, uint8_t* sigbuf, int32_t siglen, RSA* rsa) -{ - return RSA_verify(type, m, Int32ToUint32(mlen), sigbuf, Int32ToUint32(siglen), rsa); -} - int32_t CryptoNative_GetRsaParameters(const RSA* rsa, const BIGNUM** n, const BIGNUM** e, diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.h index c58a059833ea69..bebcaa34439436 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_rsa.h @@ -5,17 +5,6 @@ #include "pal_compiler.h" #include "opensslshim.h" -/* -Padding options for RsaPublicEncrypt and RsaPrivateDecrypt. -These values should be kept in sync with Interop.Crypto.RsaPadding. -*/ -typedef enum -{ - Pkcs1 = 0, - OaepSHA1 = 1, - NoPadding = 2, -} RsaPadding; - /* Shims the RSA_new method. @@ -46,30 +35,6 @@ Shims the d2i_RSAPublicKey method and makes it easier to invoke from managed cod */ PALEXPORT RSA* CryptoNative_DecodeRsaPublicKey(const uint8_t* buf, int32_t len); -/* -Shims the RSA_public_encrypt method. - -Returns the size of the signature, or -1 on error. -*/ -PALEXPORT int32_t -CryptoNative_RsaPublicEncrypt(int32_t flen, const uint8_t* from, uint8_t* to, RSA* rsa, RsaPadding padding); - -/* -Shims RSA_private_encrypt with a fixed value of RSA_NO_PADDING. - -Requires that the input be the size of the key. -Returns the number of bytes written (which should be flen), or -1 on error. -*/ -PALEXPORT int32_t CryptoNative_RsaSignPrimitive(int32_t flen, const uint8_t* from, uint8_t* to, RSA* rsa); - -/* -Shims RSA_public_decrypt with a fixed value of RSA_NO_PADDING. - -Requires that the input be the size of the key. -Returns the number of bytes written (which should be flen), or -1 on error. -*/ -PALEXPORT int32_t CryptoNative_RsaVerificationPrimitive(int32_t flen, const uint8_t* from, uint8_t* to, RSA* rsa); - /* Shims the RSA_size method. @@ -77,22 +42,6 @@ Returns the RSA modulus size in bytes. */ PALEXPORT int32_t CryptoNative_RsaSize(RSA* rsa); -/* -Shims the RSA_sign method. - -Returns 1 upon success, otherwise 0. -*/ -PALEXPORT int32_t -CryptoNative_RsaSign(int32_t type, const uint8_t* m, int32_t mlen, uint8_t* sigret, int32_t* siglen, RSA* rsa); - -/* -Shims the RSA_verify method. - -Returns 1 upon success, otherwise 0. -*/ -PALEXPORT int32_t -CryptoNative_RsaVerify(int32_t type, const uint8_t* m, int32_t mlen, uint8_t* sigbuf, int32_t siglen, RSA* rsa); - /* Gets all the parameters from the RSA instance. From 97eea86a77826d39c6438df346f777c3a07910c2 Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Fri, 9 Apr 2021 08:30:11 -0700 Subject: [PATCH 6/7] Fix new tests when run in the CSP library --- .../RSA/SignVerify.netcoreapp.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs index 6fdf81523462b1..785ac71b088103 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs @@ -58,8 +58,11 @@ public static void VerifyDefaultSpanHash() Assert.False( rsa.VerifyHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1)); - Assert.False( - rsa.VerifyHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pss)); + if (RSAFactory.SupportsPss) + { + Assert.False( + rsa.VerifyHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pss)); + } } } @@ -68,17 +71,20 @@ public static void EncryptDefaultSpan() { using (RSA rsa = RSAFactory.Create()) { - byte[] dest = new byte[2048 / 8]; + byte[] dest = new byte[rsa.KeySize / 8]; Assert.True( rsa.TryEncrypt(ReadOnlySpan.Empty, dest, RSAEncryptionPadding.Pkcs1, out int written)); Assert.Equal(dest.Length, written); - Assert.True( - rsa.TryEncrypt(ReadOnlySpan.Empty, dest, RSAEncryptionPadding.OaepSHA256, out written)); + if (RSAFactory.SupportsPss) + { + Assert.True( + rsa.TryEncrypt(ReadOnlySpan.Empty, dest, RSAEncryptionPadding.OaepSHA256, out written)); - Assert.Equal(dest.Length, written); + Assert.Equal(dest.Length, written); + } } } } From 457d7b0ad3daefff70ea02ae06f668f8f1cfb74a Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Fri, 9 Apr 2021 11:21:30 -0700 Subject: [PATCH 7/7] Make the new tests pass on macOS and Windows CSP --- .../RSA/EncryptDecrypt.netcoreapp.cs | 19 +++++++++++++ .../RSA/SignVerify.netcoreapp.cs | 28 ++----------------- .../pal_signverify.c | 9 ++++-- .../Cryptography/CapiHelper.Windows.cs | 22 +++++++++++++-- 4 files changed, 49 insertions(+), 29 deletions(-) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/EncryptDecrypt.netcoreapp.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/EncryptDecrypt.netcoreapp.cs index cf5a46e9d8dec0..435b0305ec6eb8 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/EncryptDecrypt.netcoreapp.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/EncryptDecrypt.netcoreapp.cs @@ -106,6 +106,25 @@ public void Decrypt_WrongKey_OAEP_SHA256() Decrypt_WrongKey(RSAEncryptionPadding.OaepSHA256); } + [Fact] + public static void EncryptDefaultSpan() + { + using (RSA rsa = RSAFactory.Create()) + { + byte[] dest = new byte[rsa.KeySize / 8]; + + Assert.True( + rsa.TryEncrypt(ReadOnlySpan.Empty, dest, RSAEncryptionPadding.Pkcs1, out int written)); + + Assert.Equal(dest.Length, written); + + Assert.True( + rsa.TryEncrypt(ReadOnlySpan.Empty, dest, RSAEncryptionPadding.OaepSHA1, out written)); + + Assert.Equal(dest.Length, written); + } + } + private static void Decrypt_WrongKey(RSAEncryptionPadding padding) { using (RSA rsa1 = RSAFactory.Create()) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs index 785ac71b088103..e79beb71246b95 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs @@ -41,10 +41,10 @@ public static void SignDefaultSpanHash() byte[] signature = new byte[2048 / 8]; Assert.ThrowsAny( - () => rsa.TrySignHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1, out _)); + () => rsa.TrySignHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA1, RSASignaturePadding.Pkcs1, out _)); Assert.ThrowsAny( - () => rsa.TrySignHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pss, out _)); + () => rsa.TrySignHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA1, RSASignaturePadding.Pss, out _)); } } @@ -56,7 +56,7 @@ public static void VerifyDefaultSpanHash() byte[] signature = new byte[2048 / 8]; Assert.False( - rsa.VerifyHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1)); + rsa.VerifyHash(ReadOnlySpan.Empty, signature, HashAlgorithmName.SHA1, RSASignaturePadding.Pkcs1)); if (RSAFactory.SupportsPss) { @@ -65,27 +65,5 @@ public static void VerifyDefaultSpanHash() } } } - - [Fact] - public static void EncryptDefaultSpan() - { - using (RSA rsa = RSAFactory.Create()) - { - byte[] dest = new byte[rsa.KeySize / 8]; - - Assert.True( - rsa.TryEncrypt(ReadOnlySpan.Empty, dest, RSAEncryptionPadding.Pkcs1, out int written)); - - Assert.Equal(dest.Length, written); - - if (RSAFactory.SupportsPss) - { - Assert.True( - rsa.TryEncrypt(ReadOnlySpan.Empty, dest, RSAEncryptionPadding.OaepSHA256, out written)); - - Assert.Equal(dest.Length, written); - } - } - } } } diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_signverify.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_signverify.c index f429771ad96763..f52345d904c0b2 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_signverify.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_signverify.c @@ -84,10 +84,15 @@ static int32_t VerifySignature(SecKeyRef publicKey, if (pErrorOut != NULL) *pErrorOut = NULL; - if (publicKey == NULL || pbDataHash == NULL || cbDataHash < 0 || pbSignature == NULL || cbSignature < 0 || - pErrorOut == NULL) + if (publicKey == NULL || cbDataHash < 0 || pbSignature == NULL || cbSignature < 0 || pErrorOut == NULL) return kErrorBadInput; + // A null hash is automatically the wrong length, so the signature will fail. + if (pbDataHash == NULL) + { + return 0; + } + CFDataRef dataHash = CFDataCreateWithBytesNoCopy(NULL, pbDataHash, cbDataHash, kCFAllocatorNull); if (dataHash == NULL) diff --git a/src/libraries/System.Security.Cryptography.Csp/src/System/Security/Cryptography/CapiHelper.Windows.cs b/src/libraries/System.Security.Cryptography.Csp/src/System/Security/Cryptography/CapiHelper.Windows.cs index 47d67ae2ae6b0f..913d8397e9f3ae 100644 --- a/src/libraries/System.Security.Cryptography.Csp/src/System/Security/Cryptography/CapiHelper.Windows.cs +++ b/src/libraries/System.Security.Cryptography.Csp/src/System/Security/Cryptography/CapiHelper.Windows.cs @@ -1204,8 +1204,13 @@ public static bool VerifySign(SafeProvHandle hProv, SafeKeyHandle hKey, int calg throw new InvalidOperationException(); } - using (SafeHashHandle hHash = hProv.CreateHashHandle(hash, calgHash)) + using (SafeHashHandle? hHash = hProv.CreateHashHandle(hash, calgHash, throwOnSizeError: false)) { + if (hHash == null) + { + return false; + } + bool verified = Interop.Advapi32.CryptVerifySignature(hHash, signature, signature.Length, hKey, null, Interop.Advapi32.CryptSignAndVerifyHashFlags.None); return verified; } @@ -1357,6 +1362,11 @@ private static void UnloadKey(SafeProvHandle hProv, SafeKeyHandle hKey, [NotNull /// Create a CAPI-1 hash handle that contains the specified bits as its hash value. /// private static SafeHashHandle CreateHashHandle(this SafeProvHandle hProv, byte[] hash, int calgHash) + { + return CreateHashHandle(hProv, hash, calgHash, throwOnSizeError: true)!; + } + + private static SafeHashHandle? CreateHashHandle(this SafeProvHandle hProv, byte[] hash, int calgHash, bool throwOnSizeError) { SafeHashHandle? hHash; if (!CryptCreateHash(hProv, calgHash, SafeKeyHandle.InvalidHandle, Interop.Advapi32.CryptCreateHashFlags.None, out hHash)) @@ -1377,8 +1387,16 @@ private static SafeHashHandle CreateHashHandle(this SafeProvHandle hProv, byte[] int hr = Marshal.GetHRForLastWin32Error(); throw hr.ToCryptographicException(); } + if (dwHashSize != hash.Length) - throw unchecked((int)CryptKeyError.NTE_BAD_HASH).ToCryptographicException(); + { + if (throwOnSizeError) + { + throw unchecked((int)CryptKeyError.NTE_BAD_HASH).ToCryptographicException(); + } + + return null; + } if (!Interop.Advapi32.CryptSetHashParam(hHash, Interop.Advapi32.CryptHashProperty.HP_HASHVAL, hash, 0)) {