From 4eadfbea0bb863050959559916d0503e647cc8f5 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Mon, 5 Feb 2024 11:01:54 -0500 Subject: [PATCH] Implement faster RSA key check --- .../apibridge.c | 23 ++ .../apibridge.h | 4 + .../openssl_1_0_structs.h | 11 +- .../opensslshim.h | 35 +++ .../osslcompat_111.h | 7 + .../pal_evp_pkey.c | 249 +++++++++++++++++- 6 files changed, 317 insertions(+), 12 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/apibridge.c b/src/native/libs/System.Security.Cryptography.Native/apibridge.c index bad65ee2b5021..5c8d05d17c74d 100644 --- a/src/native/libs/System.Security.Cryptography.Native/apibridge.c +++ b/src/native/libs/System.Security.Cryptography.Native/apibridge.c @@ -442,6 +442,14 @@ void local_RSA_get0_crt_params(const RSA* rsa, const BIGNUM** dmp1, const BIGNUM } } +int local_RSA_get_multi_prime_extra_count(const RSA* rsa) +{ + (void)rsa; + // OpenSSL before 1.1 does not support multi-prime RSA, so it implicitly + // has zero extra primes. + return 0; +} + int32_t local_RSA_set0_key(RSA* rsa, BIGNUM* n, BIGNUM* e, BIGNUM* d) { if (rsa == NULL) @@ -909,4 +917,19 @@ int local_BN_is_zero(const BIGNUM* a) return a->top == 0; } +int local_BN_is_one(const BIGNUM* a) +{ + return BN_abs_is_word(a, 1) && !a->neg; +} + +int local_BN_abs_is_word(const BIGNUM *a, const BN_ULONG w) +{ + return ((a->top == 1) && (a->d[0] == w)) || ((w == 0) && (a->top == 0)); +} + +int local_BN_is_odd(const BIGNUM* a) +{ + return (a->top > 0) && (a->d[0] & 1); +} + #endif diff --git a/src/native/libs/System.Security.Cryptography.Native/apibridge.h b/src/native/libs/System.Security.Cryptography.Native/apibridge.h index 93cff0d2fd11e..06bfa4c2fe8fb 100644 --- a/src/native/libs/System.Security.Cryptography.Native/apibridge.h +++ b/src/native/libs/System.Security.Cryptography.Native/apibridge.h @@ -7,7 +7,10 @@ #include "pal_types.h" int local_ASN1_TIME_to_tm(const ASN1_TIME* s, struct tm* tm); +int local_BN_abs_is_word(const BIGNUM *a, const BN_ULONG w); int local_BN_is_zero(const BIGNUM* a); +int local_BN_is_odd(const BIGNUM* a); +int local_BN_is_one(const BIGNUM* a); int local_BIO_up_ref(BIO *a); const BIGNUM* local_DSA_get0_key(const DSA* dsa, const BIGNUM** pubKey, const BIGNUM** privKey); void local_DSA_get0_pqg(const DSA* dsa, const BIGNUM** p, const BIGNUM** q, const BIGNUM** g); @@ -27,6 +30,7 @@ long local_OpenSSL_version_num(void); void local_RSA_get0_crt_params(const RSA* rsa, const BIGNUM** dmp1, const BIGNUM** dmq1, const BIGNUM** iqmp); void local_RSA_get0_factors(const RSA* rsa, const BIGNUM** p, const BIGNUM** q); void local_RSA_get0_key(const RSA* rsa, const BIGNUM** n, const BIGNUM** e, const BIGNUM** d); +int local_RSA_get_multi_prime_extra_count(const RSA* r); int32_t local_RSA_meth_get_flags(const RSA_METHOD* meth); int32_t local_RSA_set0_crt_params(RSA* rsa, BIGNUM* dmp1, BIGNUM* dmq1, BIGNUM* iqmp); int32_t local_RSA_set0_factors(RSA* rsa, BIGNUM* p, BIGNUM* q); diff --git a/src/native/libs/System.Security.Cryptography.Native/openssl_1_0_structs.h b/src/native/libs/System.Security.Cryptography.Native/openssl_1_0_structs.h index 486dbb902de3b..f13fa7ba79d00 100644 --- a/src/native/libs/System.Security.Cryptography.Native/openssl_1_0_structs.h +++ b/src/native/libs/System.Security.Cryptography.Native/openssl_1_0_structs.h @@ -185,10 +185,11 @@ struct bio_st int references; }; -struct bignum_st { - const void* _ignored1; +struct bignum_st +{ + BN_ULONG *d; int top; - int _ignored2; - int _ignored3; - int _ignored4; + int dmax; + int neg; + int flags; }; diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 8591940942620..b48d39de83cdd 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -57,6 +57,9 @@ #if OPENSSL_VERSION_NUMBER < OPENSSL_VERSION_1_1_0_RTM // Remove problematic #defines +#undef BN_abs_is_word +#undef BN_is_odd +#undef BN_is_one #undef BN_is_zero #undef SSL_get_state #undef SSL_is_init_finished @@ -210,15 +213,28 @@ int EVP_DigestFinalXOF(EVP_MD_CTX *ctx, unsigned char *md, size_t len); FALLBACK_FUNCTION(BIO_up_ref) \ REQUIRED_FUNCTION(BIO_s_mem) \ REQUIRED_FUNCTION(BIO_write) \ + FALLBACK_FUNCTION(BN_abs_is_word) \ REQUIRED_FUNCTION(BN_bin2bn) \ REQUIRED_FUNCTION(BN_bn2bin) \ REQUIRED_FUNCTION(BN_clear_free) \ + REQUIRED_FUNCTION(BN_cmp) \ + REQUIRED_FUNCTION(BN_div) \ REQUIRED_FUNCTION(BN_dup) \ REQUIRED_FUNCTION(BN_free) \ + REQUIRED_FUNCTION(BN_gcd) \ + FALLBACK_FUNCTION(BN_is_odd) \ + FALLBACK_FUNCTION(BN_is_one) \ FALLBACK_FUNCTION(BN_is_zero) \ + REQUIRED_FUNCTION(BN_mod_inverse) \ + REQUIRED_FUNCTION(BN_mod_mul) \ + REQUIRED_FUNCTION(BN_mul) \ REQUIRED_FUNCTION(BN_new) \ REQUIRED_FUNCTION(BN_num_bits) \ REQUIRED_FUNCTION(BN_set_word) \ + REQUIRED_FUNCTION(BN_sub) \ + REQUIRED_FUNCTION(BN_value_one) \ + REQUIRED_FUNCTION(BN_CTX_new) \ + REQUIRED_FUNCTION(BN_CTX_free) \ LEGACY_FUNCTION(CRYPTO_add_lock) \ REQUIRED_FUNCTION(CRYPTO_free) \ REQUIRED_FUNCTION(CRYPTO_get_ex_new_index) \ @@ -496,6 +512,7 @@ int EVP_DigestFinalXOF(EVP_MD_CTX *ctx, unsigned char *md, size_t len); REQUIRED_FUNCTION(RSA_free) \ REQUIRED_FUNCTION(RSA_generate_key_ex) \ REQUIRED_FUNCTION(RSA_get_method) \ + FALLBACK_FUNCTION(RSA_get_multi_prime_extra_count) \ FALLBACK_FUNCTION(RSA_get0_crt_params) \ FALLBACK_FUNCTION(RSA_get0_factors) \ FALLBACK_FUNCTION(RSA_get0_key) \ @@ -722,15 +739,28 @@ FOR_ALL_OPENSSL_FUNCTIONS #define BIO_up_ref BIO_up_ref_ptr #define BIO_s_mem BIO_s_mem_ptr #define BIO_write BIO_write_ptr +#define BN_abs_is_word BN_abs_is_word_ptr #define BN_bin2bn BN_bin2bn_ptr #define BN_bn2bin BN_bn2bin_ptr #define BN_clear_free BN_clear_free_ptr +#define BN_cmp BN_cmp_ptr +#define BN_div BN_div_ptr #define BN_dup BN_dup_ptr #define BN_free BN_free_ptr +#define BN_gcd BN_gcd_ptr +#define BN_is_odd BN_is_odd_ptr +#define BN_is_one BN_is_one_ptr #define BN_is_zero BN_is_zero_ptr +#define BN_mod_inverse BN_mod_inverse_ptr +#define BN_mod_mul BN_mod_mul_ptr +#define BN_mul BN_mul_ptr #define BN_new BN_new_ptr #define BN_num_bits BN_num_bits_ptr #define BN_set_word BN_set_word_ptr +#define BN_sub BN_sub_ptr +#define BN_value_one BN_value_one_ptr +#define BN_CTX_free BN_CTX_free_ptr +#define BN_CTX_new BN_CTX_new_ptr #define CRYPTO_add_lock CRYPTO_add_lock_ptr #define CRYPTO_free CRYPTO_free_ptr #define CRYPTO_get_ex_new_index CRYPTO_get_ex_new_index_ptr @@ -1011,6 +1041,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #define RSA_get0_factors RSA_get0_factors_ptr #define RSA_get0_key RSA_get0_key_ptr #define RSA_get_method RSA_get_method_ptr +#define RSA_get_multi_prime_extra_count RSA_get_multi_prime_extra_count_ptr #define RSA_meth_get_flags RSA_meth_get_flags_ptr #define RSA_new RSA_new_ptr #define RSA_pkey_ctx_ctrl RSA_pkey_ctx_ctrl_ptr @@ -1270,6 +1301,9 @@ FOR_ALL_OPENSSL_FUNCTIONS // Alias "future" API to the local_ version. #define ASN1_TIME_to_tm local_ASN1_TIME_to_tm +#define BN_abs_is_word local_BN_abs_is_word +#define BN_is_odd local_BN_is_odd +#define BN_is_one local_BN_is_one #define BN_is_zero local_BN_is_zero #define BIO_up_ref local_BIO_up_ref #define DSA_get0_key local_DSA_get0_key @@ -1287,6 +1321,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #define HMAC_CTX_free local_HMAC_CTX_free #define HMAC_CTX_new local_HMAC_CTX_new #define OpenSSL_version_num local_OpenSSL_version_num +#define RSA_get_multi_prime_extra_count local_RSA_get_multi_prime_extra_count #define RSA_get0_crt_params local_RSA_get0_crt_params #define RSA_get0_factors local_RSA_get0_factors #define RSA_get0_key local_RSA_get0_key diff --git a/src/native/libs/System.Security.Cryptography.Native/osslcompat_111.h b/src/native/libs/System.Security.Cryptography.Native/osslcompat_111.h index 2581e7f815068..a56becf4f8557 100644 --- a/src/native/libs/System.Security.Cryptography.Native/osslcompat_111.h +++ b/src/native/libs/System.Security.Cryptography.Native/osslcompat_111.h @@ -6,6 +6,9 @@ #pragma once #include "pal_types.h" +#undef BN_abs_is_word +#undef BN_is_odd +#undef BN_is_one #undef BN_is_zero #undef SSL_CTX_set_options #undef SSL_set_options @@ -21,6 +24,9 @@ typedef struct stack_st OPENSSL_STACK; #define OPENSSL_INIT_LOAD_SSL_STRINGS 0x00200000L int ASN1_TIME_to_tm(const ASN1_TIME* s, struct tm* tm); +int BN_abs_is_word(const BIGNUM *a, const BN_ULONG w); +int BN_is_odd(const BIGNUM* a); +int BN_is_one(const BIGNUM* a); int BN_is_zero(const BIGNUM* a); int BIO_up_ref(BIO* a); const BIGNUM* DSA_get0_key(const DSA* dsa, const BIGNUM** pubKey, const BIGNUM** privKey); @@ -52,6 +58,7 @@ const RSA_METHOD* RSA_PKCS1_OpenSSL(void); void RSA_get0_crt_params(const RSA* rsa, const BIGNUM** dmp1, const BIGNUM** dmq1, const BIGNUM** iqmp); void RSA_get0_factors(const RSA* rsa, const BIGNUM** p, const BIGNUM** q); void RSA_get0_key(const RSA* rsa, const BIGNUM** n, const BIGNUM** e, const BIGNUM** d); +int RSA_get_multi_prime_extra_count(const RSA* r); int32_t RSA_meth_get_flags(const RSA_METHOD* meth); int32_t RSA_pkey_ctx_ctrl(EVP_PKEY_CTX* ctx, int32_t optype, int32_t cmd, int32_t p1, void* p2); int32_t RSA_set0_crt_params(RSA* rsa, BIGNUM* dmp1, BIGNUM* dmq1, BIGNUM* iqmp); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c index 6cf59b97d057e..80183b97a77c9 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c @@ -89,7 +89,231 @@ int32_t CryptoNative_UpRefEvpPkey(EVP_PKEY* pkey) return EVP_PKEY_up_ref(pkey); } -static bool CheckKey(EVP_PKEY* key, int32_t algId, int32_t (*check_func)(EVP_PKEY_CTX*)) +static bool Lcm(const BIGNUM* num1, const BIGNUM* num2, BN_CTX* ctx, BIGNUM* result) +{ + assert(result); + + // lcm(num1, num2) = (num1 * num2) / gcd(num1, num2) + BIGNUM* mul = NULL; + BIGNUM* gcd = NULL; + bool ret = false; + + if ((mul = BN_new()) == NULL || + (gcd = BN_new()) == NULL || + !BN_mul(mul, num1, num2, ctx) || + !BN_gcd(gcd, num1, num2, ctx) || + !BN_div(result, NULL, mul, gcd, ctx)) + { + goto done; + } + + ret = true; +done: + BN_clear_free(mul); + BN_clear_free(gcd); + return ret; +} + +static int32_t QuickRsaCheck(const RSA* rsa, bool isPublic) +{ + // This method does some lightweight key consistency checks on an RSA key to make sure all supplied values are + // sensible. This is not intended to be a strict key check that verifies a key conforms to any particular set + // of criteria or standards. + + const BIGNUM* n = NULL; + const BIGNUM* e = NULL; + const BIGNUM* d = NULL; + const BIGNUM* p = NULL; + const BIGNUM* q = NULL; + const BIGNUM* dp = NULL; + const BIGNUM* dq = NULL; + const BIGNUM* inverseQ = NULL; + BN_CTX* ctx = NULL; + + // x and y are scratch integers that receive the result of some operations. + BIGNUM* x = NULL; + BIGNUM* y = NULL; + + // pM1 and qM1 are to hold p-1 and q-1, respectively. We need these values a couple of times, so don't waste time + // recomputing them in scratch integers. + BIGNUM* pM1 = NULL; + BIGNUM* qM1 = NULL; + int ret = 0; + + RSA_get0_key(rsa, &n, &e, &d); + + // Always need public parameters. + if (!n || !e) + { + ERR_PUT_error(ERR_LIB_RSA, 0, RSA_R_VALUE_MISSING, __FILE__, __LINE__); + goto done; + } + + // Compatibility: We put this error for OpenSSL 1.0.2 and 1.1.x when the modulus is zero because OpenSSL did not + // handle this correctly. Continue to use the same error if the modulus is zero. + if (BN_is_zero(n)) + { + ERR_put_error(ERR_LIB_EVP, 0, EVP_R_DECODE_ERROR, __FILE__, __LINE__); + goto done; + } + + // OpenSSL has kept this value at 16,384 for all versions. + if (BN_num_bits(n) > OPENSSL_RSA_MAX_MODULUS_BITS) + { + ERR_PUT_error(ERR_LIB_RSA, 0, RSA_R_MODULUS_TOO_LARGE, __FILE__, __LINE__); + goto done; + } + + // Exponent cannot be 1 and must be odd + if (BN_is_one(e) || !BN_is_odd(e)) + { + ERR_PUT_error(ERR_LIB_RSA, 0, RSA_R_BAD_E_VALUE, __FILE__, __LINE__); + goto done; + } + + // At this point everything that is public has been checked. Mark as successful and clean up. + if (isPublic) + { + ret = 1; + goto done; + } + + // We do not support validating multi-prime RSA. Multi-prime RSA is not common. For these, we fall back to the + // OpenSSL key check. + if (RSA_get_multi_prime_extra_count(rsa) != 0) + { + ret = -1; + goto done; + } + + // Get the private components now that we've moved on to checking the private parameters. + RSA_get0_factors(rsa, &p, &q); + + // Need all the private parameters now. If we cannot get them, fall back to the OpenSSL key check so the provider + // or engine routine can be used. + if (!d || !p || !q) + { + ret = -1; + goto done; + } + + // Check that d is less than n. + if (BN_cmp(d, n) >= 0) + { + ERR_put_error(ERR_LIB_EVP, 0, EVP_R_DECODE_ERROR, __FILE__, __LINE__); + goto done; + } + + // Set up the scratch integers + if ((ctx = BN_CTX_new()) == NULL || + (x = BN_new()) == NULL || + (y = BN_new()) == NULL || + (pM1 = BN_new()) == NULL || + (qM1 = BN_new()) == NULL) + { + goto done; + } + + // multiply p and q and put the result in x. + if (!BN_mul(x, p, q, ctx)) + { + goto done; + } + + // p * q == n + if (BN_cmp(x, n) != 0) + { + ERR_PUT_error(ERR_LIB_RSA, 0, RSA_R_N_DOES_NOT_EQUAL_P_Q, __FILE__, __LINE__); + goto done; + } + + // Checking congruence of private parameters. + // de = 1 % lambda(n) + // lambda(n) = lcm(p-1, q-1) + // lambda(n) is known to be lambda(pq) already. + // pM1 = p-1 + // qM1 = q-1 + // x = lcm(pM1, qM1) + // Note that we are checking that these values have a valid relationship, not that the parameters + // are canonically correct. The d parameter (and subsequent derived parameters) may have more than + // one working value, but we do not check the the value is the "best" possible value. + if (!BN_sub(pM1, p, BN_value_one()) || !BN_sub(qM1, q, BN_value_one()) || !Lcm(pM1, qM1, ctx, x)) + { + goto done; + } + + // de % lambda(n) + // put the result in y + if (!BN_mod_mul(y, d, e, x, ctx)) + { + goto done; + } + + // Given de = 1 % lambda(n), de % lambda(n) should be one. + if (!BN_is_one(y)) + { + ERR_PUT_error(ERR_LIB_RSA, 0, RSA_R_D_E_NOT_CONGRUENT_TO_1, __FILE__, __LINE__); + goto done; + } + + // Move on to checking the CRT parameters. In compatibility with what OpenSSL does, + // these are optional and only check them if all are present. + RSA_get0_crt_params(rsa, &dp, &dq, &inverseQ); + + if (dp && dq && inverseQ) + { + // Check dp = d % (p-1) + // compute d % (p-1) and put in x + if (!BN_div(NULL, x, d, pM1, ctx)) + { + goto done; + } + + if (BN_cmp(x, dp) != 0) + { + ERR_PUT_error(ERR_LIB_RSA, 0, RSA_R_DMP1_NOT_CONGRUENT_TO_D, __FILE__, __LINE__); + goto done; + } + + // Check dq = d % (q-1) + // compute d % (q-1) and put in x + if (!BN_div(NULL, x, d, qM1, ctx)) + { + goto done; + } + + if (BN_cmp(x, dq) != 0) + { + ERR_PUT_error(ERR_LIB_RSA, 0, RSA_R_DMQ1_NOT_CONGRUENT_TO_D, __FILE__, __LINE__); + goto done; + } + + // Check inverseQ = q^-1 % p + // Use mod_inverse and put the result in x. + if (!BN_mod_inverse(x, q, p, ctx)) + { + goto done; + } + + if (BN_cmp(x, inverseQ) != 0) + { + ERR_PUT_error(ERR_LIB_RSA, 0, RSA_R_IQMP_NOT_INVERSE_OF_Q, __FILE__, __LINE__); + goto done; + } + } + + // If we made it to the end, everything looks good. + ret = 1; +done: + if (x) BN_clear_free(x); + if (y) BN_clear_free(y); + if (pM1) BN_clear_free(pM1); + if (qM1) BN_clear_free(qM1); + if (ctx) BN_CTX_free(ctx); + return ret; +} + +static bool CheckKey(EVP_PKEY* key, int32_t algId, bool isPublic, int32_t (*check_func)(EVP_PKEY_CTX*)) { if (algId != NID_undef && EVP_PKEY_get_base_id(key) != algId) { @@ -104,16 +328,27 @@ static bool CheckKey(EVP_PKEY* key, int32_t algId, int32_t (*check_func)(EVP_PKE { const RSA* rsa = EVP_PKEY_get0_RSA(key); + // If we can get the RSA object, use that for a faster path to validating the key that skips primality tests. if (rsa != NULL) { - const BIGNUM* modulus = NULL; - RSA_get0_key(rsa, &modulus, NULL, NULL); + // 0 = key check failed + // 1 = key check passed + // -1 = could not assess private key. + int32_t result = QuickRsaCheck(rsa, isPublic); - if (modulus != NULL && BN_is_zero(modulus)) + if (result == 0) { - ERR_put_error(ERR_LIB_EVP, 0, EVP_R_DECODE_ERROR, __FILE__, __LINE__); return false; } + if (result == 1) + { + return true; + } + + // -1 falls though. + // If the fast check was indeterminate, fall though and use the OpenSSL routine. + // Clear out any errors we may have accumulated in our check. + ERR_clear_error(); } } @@ -149,7 +384,7 @@ EVP_PKEY* CryptoNative_DecodeSubjectPublicKeyInfo(const uint8_t* buf, int32_t le EVP_PKEY* key = d2i_PUBKEY(NULL, &buf, len); - if (key != NULL && !CheckKey(key, algId, EVP_PKEY_public_check)) + if (key != NULL && !CheckKey(key, algId, true, EVP_PKEY_public_check)) { EVP_PKEY_free(key); key = NULL; @@ -175,7 +410,7 @@ EVP_PKEY* CryptoNative_DecodePkcs8PrivateKey(const uint8_t* buf, int32_t len, in EVP_PKEY* key = EVP_PKCS82PKEY(p8info); PKCS8_PRIV_KEY_INFO_free(p8info); - if (key != NULL && !CheckKey(key, algId, EVP_PKEY_check)) + if (key != NULL && !CheckKey(key, algId, false, EVP_PKEY_check)) { EVP_PKEY_free(key); key = NULL;