Commit 79221bc2 authored by Geoff Thorpe's avatar Geoff Thorpe
Browse files

David Brumley <dbrumley@stanford.edu> 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
parent ffa49dc3

crypto/rsa/rsa_eay.c

+45 −100
Original line number Diff line number Diff line
@@ -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,29 +186,7 @@ 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,30 +433,7 @@ 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,56 +479,9 @@ 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);

			}



		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);

			}

		}

	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 (!BN_mod(&r1,I,rsa->q,ctx)) goto err;

	if (!rsa->meth->bn_mod_exp(&m1,&r1,rsa->dmq1,rsa->q,ctx,
@@ -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