From: Geoff Thorpe Date: Fri, 14 Feb 2003 23:21:19 +0000 (+0000) Subject: David Brumley noted and corrected a case in the X-Git-Tag: BEN_FIPS_TEST_1~38^2~307 X-Git-Url: https://git.openssl.org/?p=openssl.git;a=commitdiff_plain;h=79221bc26587c2f58c7198cc73d89eda6bdd6025 David Brumley noted and corrected a case in the verification step of CRT private key operations in the RSA code - previously no montgomery form was checked or used for 'n', and so it would be generated on the fly each time. As a result, private key operations are now a percent or two faster. Rather than adding this as another repetition of the nearly-identical montgomery "check for first-use" initialisation code blocks, I've taken this chance to create a helper function and macro-wrapper to replace them. PR: 475 --- diff --git a/crypto/rsa/rsa_eay.c b/crypto/rsa/rsa_eay.c index d4e30647d1..04cefd38b7 100644 --- a/crypto/rsa/rsa_eay.c +++ b/crypto/rsa/rsa_eay.c @@ -100,6 +100,43 @@ const RSA_METHOD *RSA_PKCS1_SSLeay(void) return(&rsa_pkcs1_eay_meth); } +/* Static helper to reduce oodles of code duplication. As a slight + * optimisation, the "MONT_HELPER() macro must be used as front-end to this + * function, to prevent unnecessary function calls - there is an initial test + * that is performed by the macro-generated code. */ +static int rsa_eay_mont_helper(BN_MONT_CTX **ptr, const BIGNUM *modulus, BN_CTX *ctx) + { + BN_MONT_CTX *bn_mont_ctx; + if((bn_mont_ctx = BN_MONT_CTX_new()) == NULL) + return 0; + if(!BN_MONT_CTX_set(bn_mont_ctx, modulus, ctx)) + { + BN_MONT_CTX_free(bn_mont_ctx); + return 0; + } + if (*ptr == NULL) /* other thread may have finished first */ + { + CRYPTO_w_lock(CRYPTO_LOCK_RSA); + if (*ptr == NULL) /* check again in the lock to stop races */ + { + *ptr = bn_mont_ctx; + bn_mont_ctx = NULL; + } + CRYPTO_w_unlock(CRYPTO_LOCK_RSA); + } + if (bn_mont_ctx) + BN_MONT_CTX_free(bn_mont_ctx); + return 1; + } +/* Usage example; + * MONT_HELPER(rsa, bn_ctx, p, rsa->flags & RSA_FLAG_CACHE_PRIVATE, goto err); + */ +#define MONT_HELPER(rsa, ctx, m, pre_cond, err_instr) \ + if((pre_cond) && ((rsa)->_method_mod_##m == NULL) && \ + !rsa_eay_mont_helper(&((rsa)->_method_mod_##m), \ + (rsa)->m, (ctx))) \ + err_instr + static int RSA_eay_public_encrypt(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding) { @@ -149,30 +186,8 @@ static int RSA_eay_public_encrypt(int flen, const unsigned char *from, goto err; } - if ((rsa->_method_mod_n == NULL) && (rsa->flags & RSA_FLAG_CACHE_PUBLIC)) - { - BN_MONT_CTX* bn_mont_ctx; - if ((bn_mont_ctx=BN_MONT_CTX_new()) == NULL) - goto err; - if (!BN_MONT_CTX_set(bn_mont_ctx,rsa->n,ctx)) - { - BN_MONT_CTX_free(bn_mont_ctx); - goto err; - } - if (rsa->_method_mod_n == NULL) /* other thread may have finished first */ - { - CRYPTO_w_lock(CRYPTO_LOCK_RSA); - if (rsa->_method_mod_n == NULL) - { - rsa->_method_mod_n = bn_mont_ctx; - bn_mont_ctx = NULL; - } - CRYPTO_w_unlock(CRYPTO_LOCK_RSA); - } - if (bn_mont_ctx) - BN_MONT_CTX_free(bn_mont_ctx); - } - + MONT_HELPER(rsa, ctx, n, rsa->flags & RSA_FLAG_CACHE_PUBLIC, goto err); + if (!rsa->meth->bn_mod_exp(&ret,&f,rsa->e,rsa->n,ctx, rsa->_method_mod_n)) goto err; @@ -418,31 +433,8 @@ static int RSA_eay_public_decrypt(int flen, const unsigned char *from, goto err; } - /* do the decrypt */ - if ((rsa->_method_mod_n == NULL) && (rsa->flags & RSA_FLAG_CACHE_PUBLIC)) - { - BN_MONT_CTX* bn_mont_ctx; - if ((bn_mont_ctx=BN_MONT_CTX_new()) == NULL) - goto err; - if (!BN_MONT_CTX_set(bn_mont_ctx,rsa->n,ctx)) - { - BN_MONT_CTX_free(bn_mont_ctx); - goto err; - } - if (rsa->_method_mod_n == NULL) /* other thread may have finished first */ - { - CRYPTO_w_lock(CRYPTO_LOCK_RSA); - if (rsa->_method_mod_n == NULL) - { - rsa->_method_mod_n = bn_mont_ctx; - bn_mont_ctx = NULL; - } - CRYPTO_w_unlock(CRYPTO_LOCK_RSA); - } - if (bn_mont_ctx) - BN_MONT_CTX_free(bn_mont_ctx); - } - + MONT_HELPER(rsa, ctx, n, rsa->flags & RSA_FLAG_CACHE_PUBLIC, goto err); + if (!rsa->meth->bn_mod_exp(&ret,&f,rsa->e,rsa->n,ctx, rsa->_method_mod_n)) goto err; @@ -487,57 +479,10 @@ static int RSA_eay_mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa) BN_init(&vrfy); if ((ctx=BN_CTX_new()) == NULL) goto err; - if (rsa->flags & RSA_FLAG_CACHE_PRIVATE) - { - if (rsa->_method_mod_p == NULL) - { - BN_MONT_CTX* bn_mont_ctx; - if ((bn_mont_ctx=BN_MONT_CTX_new()) == NULL) - goto err; - if (!BN_MONT_CTX_set(bn_mont_ctx,rsa->p,ctx)) - { - BN_MONT_CTX_free(bn_mont_ctx); - goto err; - } - if (rsa->_method_mod_p == NULL) /* other thread may have finished first */ - { - CRYPTO_w_lock(CRYPTO_LOCK_RSA); - if (rsa->_method_mod_p == NULL) - { - rsa->_method_mod_p = bn_mont_ctx; - bn_mont_ctx = NULL; - } - CRYPTO_w_unlock(CRYPTO_LOCK_RSA); - } - if (bn_mont_ctx) - BN_MONT_CTX_free(bn_mont_ctx); - } + MONT_HELPER(rsa, ctx, p, rsa->flags & RSA_FLAG_CACHE_PRIVATE, goto err); + MONT_HELPER(rsa, ctx, q, rsa->flags & RSA_FLAG_CACHE_PRIVATE, goto err); + MONT_HELPER(rsa, ctx, n, rsa->flags & RSA_FLAG_CACHE_PRIVATE, goto err); - if (rsa->_method_mod_q == NULL) - { - BN_MONT_CTX* bn_mont_ctx; - if ((bn_mont_ctx=BN_MONT_CTX_new()) == NULL) - goto err; - if (!BN_MONT_CTX_set(bn_mont_ctx,rsa->q,ctx)) - { - BN_MONT_CTX_free(bn_mont_ctx); - goto err; - } - if (rsa->_method_mod_q == NULL) /* other thread may have finished first */ - { - CRYPTO_w_lock(CRYPTO_LOCK_RSA); - if (rsa->_method_mod_q == NULL) - { - rsa->_method_mod_q = bn_mont_ctx; - bn_mont_ctx = NULL; - } - CRYPTO_w_unlock(CRYPTO_LOCK_RSA); - } - if (bn_mont_ctx) - BN_MONT_CTX_free(bn_mont_ctx); - } - } - if (!BN_mod(&r1,I,rsa->q,ctx)) goto err; if (!rsa->meth->bn_mod_exp(&m1,&r1,rsa->dmq1,rsa->q,ctx, rsa->_method_mod_q)) goto err; @@ -568,7 +513,7 @@ static int RSA_eay_mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa) if (rsa->e && rsa->n) { - if (!rsa->meth->bn_mod_exp(&vrfy,r0,rsa->e,rsa->n,ctx,NULL)) goto err; + if (!rsa->meth->bn_mod_exp(&vrfy,r0,rsa->e,rsa->n,ctx,rsa->_method_mod_n)) goto err; /* If 'I' was greater than (or equal to) rsa->n, the operation * will be equivalent to using 'I mod n'. However, the result of * the verify will *always* be less than 'n' so we don't check