Really get rid of unsafe double-checked locking.

Also, "CHANGES" clean-ups.

2 files changed, 32 insertions, 33 deletions

diff --git a/CHANGES b/CHANGES
index 443c3d84c0..7597c6d150 100644
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,16 @@
 
  Changes between 0.9.8h and 0.9.8i  [xx XXX xxxx]
 
+  *) The fix in 0.9.8c that supposedly got rid of unsafe
+     double-checked locking was incomplete for RSA blinding,
+     addressing just one layer of what turns out to have been
+     doubly unsafe triple-checked locking.
+
+     So now fix this for real by retiring the MONT_HELPER macro
+     in crypto/rsa/rsa_eay.c.
+
+     [Bodo Moeller; problem pointed out by Marius Schilder]
+
   *) Various precautionary measures:
 
      - Avoid size_t integer overflow in HASH_UPDATE (md32_common.h).
@@ -44,22 +54,6 @@
 
  Changes between 0.9.8g and 0.9.8h  [28 May 2008]
 
-  *) Various precautionary measures:
-
-     - Avoid size_t integer overflow in HASH_UPDATE (md32_common.h).
-
-     - Avoid a buffer overflow in d2i_SSL_SESSION() (ssl_asn1.c).
-       (NB: This would require knowledge of the secret session ticket key
-       to exploit, in which case you'd be SOL either way.)
-
-     - Change bn_nist.c so that it will properly handle input BIGNUMs
-       outside the expected range.
-
-     - Enforce the 'num' check in BN_div() (bn_div.c) for non-BN_DEBUG
-       builds.
-
-     [Neel Mehta, Bodo Moeller]
-
   *) Fix flaw if 'Server Key exchange message' is omitted from a TLS
      handshake which could lead to a cilent crash as found using the
      Codenomicon TLS test suite (CVE-2008-1672) 
diff --git a/crypto/rsa/rsa_eay.c b/crypto/rsa/rsa_eay.c
index ffadaab9a4..283ddd8f1f 100644
--- a/crypto/rsa/rsa_eay.c
+++ b/crypto/rsa/rsa_eay.c
@@ -150,16 +150,6 @@ const RSA_METHOD *RSA_PKCS1_SSLeay(void)
 	return(&rsa_pkcs1_eay_meth);
 	}
 
-/* Usage example;
- *    MONT_HELPER(rsa->_method_mod_p, bn_ctx, rsa->p, rsa->flags & RSA_FLAG_CACHE_PRIVATE, goto err);
- */
-#define MONT_HELPER(method_mod, ctx, m, pre_cond, err_instr) \
-	if ((pre_cond) && ((method_mod) == NULL) && \
-			!BN_MONT_CTX_set_locked(&(method_mod), \
-				CRYPTO_LOCK_RSA, \
-				(m), (ctx))) \
-		err_instr
-
 static int RSA_eay_public_encrypt(int flen, const unsigned char *from,
 	     unsigned char *to, RSA *rsa, int padding)
 	{
@@ -233,7 +223,9 @@ static int RSA_eay_public_encrypt(int flen, const unsigned char *from,
 		goto err;
 		}
 
-	MONT_HELPER(rsa->_method_mod_n, ctx, rsa->n, rsa->flags & RSA_FLAG_CACHE_PUBLIC, goto err);
+	if (rsa->flags & RSA_FLAG_CACHE_PUBLIC)
+		if (!BN_MONT_CTX_set_locked(&rsa->_method_mod_n, CRYPTO_LOCK_RSA, rsa->n, ctx))
+			goto err;
 
 	if (!rsa->meth->bn_mod_exp(ret,f,rsa->e,rsa->n,ctx,
 		rsa->_method_mod_n)) goto err;
@@ -438,7 +430,9 @@ static int RSA_eay_private_encrypt(int flen, const unsigned char *from,
 		else
 			d= rsa->d;
 
-		MONT_HELPER(rsa->_method_mod_n, ctx, rsa->n, rsa->flags & RSA_FLAG_CACHE_PUBLIC, goto err);
+		if (rsa->flags & RSA_FLAG_CACHE_PUBLIC)
+			if(!BN_MONT_CTX_set_locked(&rsa->_method_mod_n, CRYPTO_LOCK_RSA, rsa->n, ctx))
+				goto err;
 
 		if (!rsa->meth->bn_mod_exp(ret,f,d,rsa->n,ctx,
 				rsa->_method_mod_n)) goto err;
@@ -559,7 +553,9 @@ static int RSA_eay_private_decrypt(int flen, const unsigned char *from,
 		else
 			d = rsa->d;
 
-		MONT_HELPER(rsa->_method_mod_n, ctx, rsa->n, rsa->flags & RSA_FLAG_CACHE_PUBLIC, goto err);
+		if (rsa->flags & RSA_FLAG_CACHE_PUBLIC)
+			if (!BN_MONT_CTX_set_locked(&rsa->_method_mod_n, CRYPTO_LOCK_RSA, rsa->n, ctx))
+				goto err;
 		if (!rsa->meth->bn_mod_exp(ret,f,d,rsa->n,ctx,
 				rsa->_method_mod_n))
 		  goto err;
@@ -669,7 +665,9 @@ static int RSA_eay_public_decrypt(int flen, const unsigned char *from,
 		goto err;
 		}
 
-	MONT_HELPER(rsa->_method_mod_n, ctx, rsa->n, rsa->flags & RSA_FLAG_CACHE_PUBLIC, goto err);
+	if (rsa->flags & RSA_FLAG_CACHE_PUBLIC)
+		if (!BN_MONT_CTX_set_locked(&rsa->_method_mod_n, CRYPTO_LOCK_RSA, rsa->n, ctx))
+			goto err;
 
 	if (!rsa->meth->bn_mod_exp(ret,f,rsa->e,rsa->n,ctx,
 		rsa->_method_mod_n)) goto err;
@@ -747,11 +745,18 @@ static int RSA_eay_mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa, BN_CTX *ctx)
 			q = rsa->q;
 			}
 
-		MONT_HELPER(rsa->_method_mod_p, ctx, p, rsa->flags & RSA_FLAG_CACHE_PRIVATE, goto err);
-		MONT_HELPER(rsa->_method_mod_q, ctx, q, rsa->flags & RSA_FLAG_CACHE_PRIVATE, goto err);
+		if (rsa->flags & RSA_FLAG_CACHE_PRIVATE)
+			{
+			if (!BN_MONT_CTX_set_locked(&rsa->_method_mod_p, CRYPTO_LOCK_RSA, p, ctx))
+				goto err;
+			if (!BN_MONT_CTX_set_locked(&rsa->_method_mod_q, CRYPTO_LOCK_RSA, q, ctx))
+				goto err;
+			}
 	}
 
-	MONT_HELPER(rsa->_method_mod_n, ctx, rsa->n, rsa->flags & RSA_FLAG_CACHE_PUBLIC, goto err);
+	if (rsa->flags & RSA_FLAG_CACHE_PUBLIC)
+		if (!BN_MONT_CTX_set_locked(&rsa->_method_mod_n, CRYPTO_LOCK_RSA, rsa->n, ctx))
+			goto err;
 
 	/* compute I mod q */
 	if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME))