Really get rid of unsafe double-checked locking.

Also, "CHANGES" clean-ups.

diff --git a/CHANGES b/CHANGES
index 443c3d84c0d33e26d9b70645d0701ca06314ad2d..7597c6d150e3887792addff292eb5e2ac7dd47d9 100644 (file)
--- 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 ffadaab9a4d705fb0d1022a5b9a1a62832c15305..283ddd8f1f0a7c446e6dae4d5e17a875079d4df9 100644 (file)
--- 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))