From 7111a92546253d6fc857f7cad8b0bff425df0798 Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Fri, 2 Apr 2021 09:10:08 -0700 Subject: [PATCH 03/11] Use EVP_PKEY for RSA signing operations With this change all RSA private key operations (excluding import/export) use the EVP_PKEY APIs. * RSAPaddingProcessor is no longer used in conjunction with the private keys, on Linux. * The pal_rsa.c copy of HasPrivateKey has been removed. --- .../Interop.EvpPkey.Rsa.cs | 35 ++++++ .../Interop.Rsa.cs | 20 ---- .../Security/Cryptography/RSAOpenSsl.cs | 87 ++++----------- .../opensslshim.h | 19 +++- .../pal_evp_pkey_rsa.c | 76 ++++++++++++- .../pal_evp_pkey_rsa.h | 14 +++ .../pal_rsa.c | 100 ------------------ 7 files changed, 156 insertions(+), 195 deletions(-) diff --git a/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs b/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs index f023ced7f9..6aab764cff 100644 --- a/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs +++ b/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs @@ -63,6 +63,41 @@ internal static SafeEvpPKeyHandle RsaGenerateKey(int keySize) return written; } + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_RsaSignHash")] + private static extern int CryptoNative_RsaSignHash( + SafeEvpPKeyHandle pkey, + RSASignaturePaddingMode paddingMode, + IntPtr digestAlgorithm, + ref byte hash, + int hashLength, + ref byte destination, + int destinationLength); + + internal static int RsaSignHash( + SafeEvpPKeyHandle pkey, + RSASignaturePaddingMode paddingMode, + IntPtr digestAlgorithm, + ReadOnlySpan hash, + Span destination) + { + int written = CryptoNative_RsaSignHash( + pkey, + paddingMode, + digestAlgorithm, + ref MemoryMarshal.GetReference(hash), + hash.Length, + ref MemoryMarshal.GetReference(destination), + destination.Length); + + if (written < 0) + { + Debug.Assert(written == -1); + throw CreateOpenSslCryptographicException(); + } + + return written; + } + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_EvpPkeyGetRsa")] internal static extern SafeRsaHandle EvpPkeyGetRsa(SafeEvpPKeyHandle pkey); diff --git a/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Rsa.cs b/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Rsa.cs index 5ad534a8f2..b2f250ffe9 100644 --- a/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Rsa.cs +++ b/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Rsa.cs @@ -44,19 +44,6 @@ internal static partial class Crypto SafeRsaHandle rsa, RsaPadding padding); - internal static int RsaSignPrimitive( - ReadOnlySpan from, - Span to, - SafeRsaHandle rsa) => - RsaSignPrimitive(from.Length, ref MemoryMarshal.GetReference(from), ref MemoryMarshal.GetReference(to), rsa); - - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_RsaSignPrimitive")] - private static extern int RsaSignPrimitive( - int flen, - ref byte from, - ref byte to, - SafeRsaHandle rsa); - internal static int RsaVerificationPrimitive( ReadOnlySpan from, Span to, @@ -73,13 +60,6 @@ internal static partial class Crypto [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_RsaSize")] internal static extern int RsaSize(SafeRsaHandle rsa); - internal static bool RsaSign(int type, ReadOnlySpan m, int m_len, Span sigret, out int siglen, SafeRsaHandle rsa) => - RsaSign(type, ref MemoryMarshal.GetReference(m), m_len, ref MemoryMarshal.GetReference(sigret), out siglen, rsa); - - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_RsaSign")] - [return: MarshalAs(UnmanagedType.Bool)] - private static extern bool RsaSign(int type, ref byte m, int m_len, ref byte sigret, out int siglen, SafeRsaHandle rsa); - internal static bool RsaVerify(int type, ReadOnlySpan m, ReadOnlySpan sigbuf, SafeRsaHandle rsa) { bool ret = RsaVerify( diff --git a/src/Common/src/System/Security/Cryptography/RSAOpenSsl.cs b/src/Common/src/System/Security/Cryptography/RSAOpenSsl.cs index 87e31b9dde..225968fc50 100644 --- a/src/Common/src/System/Security/Cryptography/RSAOpenSsl.cs +++ b/src/Common/src/System/Security/Cryptography/RSAOpenSsl.cs @@ -655,84 +655,33 @@ public override byte[] SignHash(byte[] hash, HashAlgorithmName hashAlgorithm, RS { Debug.Assert(!string.IsNullOrEmpty(hashAlgorithm.Name)); Debug.Assert(padding != null); + ValidatePadding(padding); signature = null; - // Do not factor out getting _key.Value, since the key creation should not happen on - // invalid padding modes. + IntPtr digestAlgorithm = Interop.Crypto.HashAlgorithmToEvp(hashAlgorithm.Name); + SafeEvpPKeyHandle key = GetPKey(); + int bytesRequired = Interop.Crypto.EvpPKeySize(key); - if (padding.Mode == RSASignaturePaddingMode.Pkcs1) + if (allocateSignature) { - int algorithmNid = GetAlgorithmNid(hashAlgorithm); - SafeRsaHandle rsa = GetKey(); - - int bytesRequired = Interop.Crypto.RsaSize(rsa); - - if (allocateSignature) - { - Debug.Assert(destination.Length == 0); - signature = new byte[bytesRequired]; - destination = signature; - } - - if (destination.Length < bytesRequired) - { - bytesWritten = 0; - return false; - } - - if (!Interop.Crypto.RsaSign(algorithmNid, hash, hash.Length, destination, out int signatureSize, rsa)) - { - throw Interop.Crypto.CreateOpenSslCryptographicException(); - } - - Debug.Assert( - signatureSize == bytesRequired, - $"RSA_sign reported signatureSize was {signatureSize}, when {bytesRequired} was expected"); - - bytesWritten = signatureSize; - return true; + Debug.Assert(destination.Length == 0); + signature = new byte[bytesRequired]; + destination = signature; } - else if (padding.Mode == RSASignaturePaddingMode.Pss) + else if (destination.Length < bytesRequired) { - RsaPaddingProcessor processor = RsaPaddingProcessor.OpenProcessor(hashAlgorithm); - SafeRsaHandle rsa = GetKey(); - - int bytesRequired = Interop.Crypto.RsaSize(rsa); - - if (allocateSignature) - { - Debug.Assert(destination.Length == 0); - signature = new byte[bytesRequired]; - destination = signature; - } - - if (destination.Length < bytesRequired) - { - bytesWritten = 0; - return false; - } - - byte[] pssRented = CryptoPool.Rent(bytesRequired); - Span pssBytes = new Span(pssRented, 0, bytesRequired); - - processor.EncodePss(hash, pssBytes, KeySize); - - int ret = Interop.Crypto.RsaSignPrimitive(pssBytes, destination, rsa); - - CryptoPool.Return(pssRented, bytesRequired); - - CheckReturn(ret); - - Debug.Assert( - ret == bytesRequired, - $"RSA_private_encrypt returned {ret} when {bytesRequired} was expected"); - - bytesWritten = ret; - return true; + bytesWritten = 0; + return false; } - throw PaddingModeNotSupported(); + int written = Interop.Crypto.RsaSignHash(key, padding.Mode, digestAlgorithm, hash, destination); + Debug.Assert(written == bytesRequired); + bytesWritten = written; + + // Until EVP_PKEY is what gets stored, free the temporary key handle. + key.Dispose(); + return true; } public override bool VerifyHash( diff --git a/src/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index 47cb142d25..4c15914d25 100644 --- a/src/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -179,11 +179,15 @@ int32_t X509_up_ref(X509* x509); #define EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, bits) \ RSA_pkey_ctx_ctrl(ctx, EVP_PKEY_OP_KEYGEN, EVP_PKEY_CTRL_RSA_KEYGEN_BITS, bits, NULL) +// EVP_PKEY_CTX_set_rsa_oaep_md doesn't call RSA_pkey_ctx_ctrl in 1.1, so don't redefine it here. + #undef EVP_PKEY_CTX_set_rsa_padding #define EVP_PKEY_CTX_set_rsa_padding(ctx, pad) \ RSA_pkey_ctx_ctrl(ctx, -1, EVP_PKEY_CTRL_RSA_PADDING, pad, NULL) -// EVP_PKEY_CTX_set_rsa_oaep_md doesn't call RSA_pkey_ctx_ctrl in 1.1, so don't redefine it here. +#undef EVP_PKEY_CTX_set_rsa_pss_saltlen +#define EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx, len) \ + RSA_pkey_ctx_ctrl(ctx, (EVP_PKEY_OP_SIGN|EVP_PKEY_OP_VERIFY), EVP_PKEY_CTRL_RSA_PSS_SALTLEN, len, NULL) #endif @@ -209,6 +213,11 @@ void SSL_CTX_set_alpn_select_cb(SSL_CTX* ctx, void SSL_get0_alpn_selected(const SSL* ssl, const unsigned char** protocol, unsigned int* len); #endif +// The value -1 has the correct meaning on 1.0.x, but the constant wasn't named. +#ifndef RSA_PSS_SALTLEN_DIGEST +#define RSA_PSS_SALTLEN_DIGEST -1 +#endif + #define API_EXISTS(fn) (fn != NULL) // List of all functions from the libssl that are used in the System.Security.Cryptography.Native. @@ -378,6 +387,8 @@ void SSL_get0_alpn_selected(const SSL* ssl, const unsigned char** protocol, unsi REQUIRED_FUNCTION(EVP_PKEY_set1_DSA) \ REQUIRED_FUNCTION(EVP_PKEY_set1_EC_KEY) \ REQUIRED_FUNCTION(EVP_PKEY_set1_RSA) \ + REQUIRED_FUNCTION(EVP_PKEY_sign) \ + REQUIRED_FUNCTION(EVP_PKEY_sign_init) \ REQUIRED_FUNCTION(EVP_PKEY_size) \ FALLBACK_FUNCTION(EVP_PKEY_up_ref) \ REQUIRED_FUNCTION(EVP_rc2_cbc) \ @@ -459,14 +470,12 @@ 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_private_encrypt) \ 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) \ REQUIRED_FUNCTION(RSA_set_method) \ - REQUIRED_FUNCTION(RSA_sign) \ REQUIRED_FUNCTION(RSA_size) \ REQUIRED_FUNCTION(RSA_up_ref) \ REQUIRED_FUNCTION(RSA_verify) \ @@ -774,6 +783,8 @@ FOR_ALL_OPENSSL_FUNCTIONS #define EVP_PKEY_set1_DSA EVP_PKEY_set1_DSA_ptr #define EVP_PKEY_set1_EC_KEY EVP_PKEY_set1_EC_KEY_ptr #define EVP_PKEY_set1_RSA EVP_PKEY_set1_RSA_ptr +#define EVP_PKEY_sign_init EVP_PKEY_sign_init_ptr +#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_rc2_cbc EVP_rc2_cbc_ptr @@ -855,14 +866,12 @@ FOR_ALL_OPENSSL_FUNCTIONS #define RSA_new RSA_new_ptr #define RSA_pkey_ctx_ctrl RSA_pkey_ctx_ctrl_ptr #define RSA_PKCS1_OpenSSL RSA_PKCS1_OpenSSL_ptr -#define RSA_private_encrypt RSA_private_encrypt_ptr #define RSA_public_decrypt RSA_public_decrypt_ptr #define RSA_public_encrypt RSA_public_encrypt_ptr #define RSA_set0_crt_params RSA_set0_crt_params_ptr #define RSA_set0_factors RSA_set0_factors_ptr #define RSA_set0_key RSA_set0_key_ptr #define RSA_set_method RSA_set_method_ptr -#define RSA_sign RSA_sign_ptr #define RSA_size RSA_size_ptr #define RSA_up_ref RSA_up_ref_ptr #define RSA_verify RSA_verify_ptr diff --git a/src/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c b/src/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c index 6235c905db..68b6a34a5d 100644 --- a/src/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c +++ b/src/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c @@ -91,7 +91,6 @@ int32_t CryptoNative_RsaDecrypt(EVP_PKEY* pkey, if (rsa == NULL || HasNoPrivateKey(rsa)) { ERR_PUT_error(ERR_LIB_RSA, RSA_F_RSA_NULL_PRIVATE_DECRYPT, RSA_R_VALUE_MISSING, __FILE__, __LINE__); - ret = -1; goto done; } } @@ -112,6 +111,81 @@ done: 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) +{ + 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 (padding == RsaPaddingPkcs1) + { + if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_PADDING) <= 0) + { + goto done; + } + } + else + { + assert(padding == RsaPaddingOaepOrPss); + + 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; + } + } + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wcast-qual" + if (EVP_PKEY_CTX_set_signature_md(ctx, digest) <= 0) +#pragma clang diagnostic pop + { + goto done; + } + + // This check may no longer be needed on OpenSSL 3.0 + { + RSA* rsa = EVP_PKEY_get0_RSA(pkey); + + if (rsa == NULL || HasNoPrivateKey(rsa)) + { + ERR_PUT_error(ERR_LIB_RSA, RSA_F_RSA_NULL_PRIVATE_DECRYPT, RSA_R_VALUE_MISSING, __FILE__, __LINE__); + goto done; + } + } + + size_t written = Int32ToSizeT(destinationLen); + + if (EVP_PKEY_sign(ctx, destination, &written, hash, Int32ToSizeT(hashLen)) > 0) + { + ret = SizeTToInt32(written); + } + +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/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.h b/src/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.h index d220065adf..f811523f78 100644 --- a/src/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.h +++ b/src/Native/Unix/System.Security.Cryptography.Native/pal_evp_pkey_rsa.h @@ -34,6 +34,20 @@ DLLEXPORT int32_t CryptoNative_RsaDecrypt(EVP_PKEY* pkey, 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. + +Returns the number of bytes written to destination, -1 on error. +*/ +DLLEXPORT 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); + /* Shims the EVP_PKEY_get1_RSA method. diff --git a/src/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c b/src/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c index 0c635dfca7..43268e88e1 100644 --- a/src/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c +++ b/src/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c @@ -48,60 +48,6 @@ static int GetOpenSslPadding(RsaPadding padding) } } -static int HasNoPrivateKey(RSA* rsa) -{ - if (rsa == NULL) - return 1; - - // Shared pointer, don't free. - const RSA_METHOD* meth = RSA_get_method(rsa); - - // The method has descibed itself as having the private key external to the structure. - // That doesn't mean it's actually present, but we can't tell. -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wcast-qual" - if (RSA_meth_get_flags((RSA_METHOD*)meth) & RSA_FLAG_EXT_PKEY) -#pragma clang diagnostic pop - { - return 0; - } - - // In the event that there's a middle-ground where we report failure when success is expected, - // one could do something like check if the RSA_METHOD intercepts all private key operations: - // - // * meth->rsa_priv_enc - // * meth->rsa_priv_dec - // * meth->rsa_sign (in 1.0.x this is only respected if the RSA_FLAG_SIGN_VER flag is asserted) - // - // But, for now, leave it at the EXT_PKEY flag test. - - // The module is documented as accepting either d or the full set of CRT parameters (p, q, dp, dq, qInv) - // So if we see d, we're good. Otherwise, if any of the rest are missing, we're public-only. - const BIGNUM* d; - RSA_get0_key(rsa, NULL, NULL, &d); - - if (d != NULL) - { - return 0; - } - - const BIGNUM* p; - const BIGNUM* q; - const BIGNUM* dmp1; - const BIGNUM* dmq1; - const BIGNUM* iqmp; - - RSA_get0_factors(rsa, &p, &q); - RSA_get0_crt_params(rsa, &dmp1, &dmq1, &iqmp); - - if (p == NULL || q == NULL || dmp1 == NULL || dmq1 == NULL || iqmp == NULL) - { - return 1; - } - - return 0; -} - int32_t CryptoNative_RsaPublicEncrypt(int32_t flen, const uint8_t* from, uint8_t* to, RSA* rsa, RsaPadding padding) { @@ -109,17 +55,6 @@ CryptoNative_RsaPublicEncrypt(int32_t flen, const uint8_t* from, uint8_t* to, RS return RSA_public_encrypt(flen, from, to, rsa, openSslPadding); } -int32_t CryptoNative_RsaSignPrimitive(int32_t flen, const uint8_t* from, uint8_t* to, RSA* rsa) -{ - if (HasNoPrivateKey(rsa)) - { - ERR_PUT_error(ERR_LIB_RSA, RSA_F_RSA_NULL_PRIVATE_ENCRYPT, RSA_R_VALUE_MISSING, __FILE__, __LINE__); - return -1; - } - - return RSA_private_encrypt(flen, from, to, rsa, RSA_NO_PADDING); -} - 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); @@ -130,41 +65,6 @@ int32_t CryptoNative_RsaSize(RSA* rsa) return RSA_size(rsa); } -int32_t -CryptoNative_RsaSign(int32_t type, const uint8_t* m, int32_t mlen, uint8_t* sigret, int32_t* siglen, RSA* rsa) -{ - if (siglen == NULL) - { - assert(false); - return 0; - } - - *siglen = 0; - - if (HasNoPrivateKey(rsa)) - { - ERR_PUT_error(ERR_LIB_RSA, RSA_F_RSA_SIGN, RSA_R_VALUE_MISSING, __FILE__, __LINE__); - return 0; - } - - // Shared pointer to the metadata about the message digest algorithm - const EVP_MD* digest = EVP_get_digestbynid(type); - - // If the digest itself isn't known then RSA_R_UNKNOWN_ALGORITHM_TYPE will get reported, but - // we have to check that the digest size matches what we expect. - if (digest != NULL && mlen != EVP_MD_size(digest)) - { - ERR_PUT_error(ERR_LIB_RSA, RSA_F_RSA_SIGN, RSA_R_INVALID_MESSAGE_LENGTH, __FILE__, __LINE__); - return 0; - } - - unsigned int unsignedSigLen = 0; - int32_t ret = RSA_sign(type, m, Int32ToUint32(mlen), sigret, &unsignedSigLen, rsa); - assert(unsignedSigLen <= INT32_MAX); - *siglen = (int32_t)unsignedSigLen; - return ret; -} - int32_t CryptoNative_RsaVerify(int32_t type, const uint8_t* m, int32_t mlen, uint8_t* sigbuf, int32_t siglen, RSA* rsa) { -- 2.31.1