Really get rid of unsafe double-checked locking.
authorBodo Möller <bodo@openssl.org>
Sun, 14 Sep 2008 13:51:44 +0000 (13:51 +0000)
committerBodo Möller <bodo@openssl.org>
Sun, 14 Sep 2008 13:51:44 +0000 (13:51 +0000)
Also, "CHANGES" clean-ups.

CHANGES
crypto/rsa/rsa_eay.c

diff --git a/CHANGES b/CHANGES
index b9a6d8364117636c95958d77b0794ab84647bc1c..4fd140968d9f84ae08bc7b8ecec8460bd65f7b17 100644 (file)
--- a/CHANGES
+++ b/CHANGES
 
  Changes between 0.9.8h and 0.9.8i  [xx XXX xxxx]
 
 
  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).
   *) Various precautionary measures:
 
      - Avoid size_t integer overflow in HASH_UPDATE (md32_common.h).
      This work was sponsored by Logica.
      [Steve Henson]
 
      This work was sponsored by Logica.
      [Steve Henson]
 
+  *) Allow engines to be "soft loaded" - i.e. optionally don't die if
+     the load fails. Useful for distros.
+     [Ben Laurie and the FreeBSD team]
+
  Changes between 0.9.8g and 0.9.8h  [28 May 2008]
 
   *) Fix flaw if 'Server Key exchange message' is omitted from a TLS
  Changes between 0.9.8g and 0.9.8h  [28 May 2008]
 
   *) Fix flaw if 'Server Key exchange message' is omitted from a TLS
      the 'db' section contains nothing but zeroes (there is a one-byte
      invalid read after the end of 'db').
      [Ivan Nestlerode <inestlerode@us.ibm.com>]
      the 'db' section contains nothing but zeroes (there is a one-byte
      invalid read after the end of 'db').
      [Ivan Nestlerode <inestlerode@us.ibm.com>]
-  
+
+  *) Partial backport from 0.9.9-dev:
+
+     Introduce bn_mul_mont (dedicated Montgomery multiplication
+     procedure) as a candidate for BIGNUM assembler implementation.
+     While 0.9.9-dev uses assembler for various architectures, only
+     x86_64 is available by default here in the 0.9.8 branch, and
+     32-bit x86 is available through a compile-time setting.
+
+     To try the 32-bit x86 assembler implementation, use Configure
+     option "enable-montasm" (which exists only for this backport).
+
+     As "enable-montasm" for 32-bit x86 disclaims code stability
+     anyway, in this constellation we activate additional code
+     backported from 0.9.9-dev for further performance improvements,
+     namely BN_from_montgomery_word.  (To enable this otherwise,
+     e.g. x86_64, try "-DMONT_FROM_WORD___NON_DEFAULT_0_9_8_BUILD".)
+
+     [Andy Polyakov (backport partially by Bodo Moeller)]
+
   *) Add TLS session ticket callback. This allows an application to set
      TLS ticket cipher and HMAC keys rather than relying on hardcoded fixed
      values. This is useful for key rollover for example where several key
   *) Add TLS session ticket callback. This allows an application to set
      TLS ticket cipher and HMAC keys rather than relying on hardcoded fixed
      values. This is useful for key rollover for example where several key
      implementation.
      [Ian Lister (tweaked by Geoff Thorpe)]
 
      implementation.
      [Ian Lister (tweaked by Geoff Thorpe)]
 
+  *) Backport of CMS code to OpenSSL 0.9.8. This differs from the 0.9.9
+     implemention in the following ways:
+
+     Lack of EVP_PKEY_ASN1_METHOD means algorithm parameters have to be
+     hard coded.
+
+     Lack of BER streaming support means one pass streaming processing is
+     only supported if data is detached: setting the streaming flag is
+     ignored for embedded content.
+
+     CMS support is disabled by default and must be explicitly enabled
+     with the enable-cms configuration option.
+     [Steve Henson]
+
   *) Update the GMP engine glue to do direct copies between BIGNUM and
      mpz_t when openssl and GMP use the same limb size. Otherwise the
      existing "conversion via a text string export" trick is still used.
   *) Update the GMP engine glue to do direct copies between BIGNUM and
      mpz_t when openssl and GMP use the same limb size. Otherwise the
      existing "conversion via a text string export" trick is still used.
-     [Paul Sheer <paulsheer@gmail.com>, Geoff Thorpe]
+     [Paul Sheer <paulsheer@gmail.com>]
 
   *) Zlib compression BIO. This is a filter BIO which compressed and
      uncompresses any data passed through it.
 
   *) Zlib compression BIO. This is a filter BIO which compressed and
      uncompresses any data passed through it.
      authentication-only ciphersuites.
      [Bodo Moeller]
 
      authentication-only ciphersuites.
      [Bodo Moeller]
 
+  *) Update the SSL_get_shared_ciphers() fix CVE-2006-3738 which was
+     not complete and could lead to a possible single byte overflow
+     (CVE-2007-5135) [Ben Laurie]
+
  Changes between 0.9.8d and 0.9.8e  [23 Feb 2007]
 
   *) Since AES128 and AES256 (and similarly Camellia128 and
  Changes between 0.9.8d and 0.9.8e  [23 Feb 2007]
 
   *) Since AES128 and AES256 (and similarly Camellia128 and
 
      BN_mod_exp_mont_consttime() is the new exponentiation implementation,
      and this is automatically used by BN_mod_exp_mont() if the new flag
 
      BN_mod_exp_mont_consttime() is the new exponentiation implementation,
      and this is automatically used by BN_mod_exp_mont() if the new flag
-     BN_FLG_EXP_CONSTTIME is set for the exponent.  RSA, DSA, and DH 
+     BN_FLG_EXP_CONSTTIME is set for the exponent.  RSA, DSA, and DH
      will use this BN flag for private exponents unless the flag
      RSA_FLAG_NO_EXP_CONSTTIME, DSA_FLAG_NO_EXP_CONSTTIME, or
      DH_FLAG_NO_EXP_CONSTTIME, respectively, is set.
      will use this BN flag for private exponents unless the flag
      RSA_FLAG_NO_EXP_CONSTTIME, DSA_FLAG_NO_EXP_CONSTTIME, or
      DH_FLAG_NO_EXP_CONSTTIME, respectively, is set.
index 7321349237cbc732c8a5e651dfe71689a315a7d6..c5eaeeae6bdd3825a8aadf6e0bd9c422e7150e5a 100644 (file)
@@ -150,16 +150,6 @@ const RSA_METHOD *RSA_PKCS1_SSLeay(void)
        return(&rsa_pkcs1_eay_meth);
        }
 
        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)
        {
 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;
                }
 
                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;
 
        if (!rsa->meth->bn_mod_exp(ret,f,rsa->e,rsa->n,ctx,
                rsa->_method_mod_n)) goto err;
@@ -440,7 +432,9 @@ static int RSA_eay_private_encrypt(int flen, const unsigned char *from,
                else
                        d= rsa->d;
 
                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;
 
                if (!rsa->meth->bn_mod_exp(ret,f,d,rsa->n,ctx,
                                rsa->_method_mod_n)) goto err;
@@ -561,7 +555,9 @@ static int RSA_eay_private_decrypt(int flen, const unsigned char *from,
                else
                        d = rsa->d;
 
                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;
                if (!rsa->meth->bn_mod_exp(ret,f,d,rsa->n,ctx,
                                rsa->_method_mod_n))
                  goto err;
@@ -671,7 +667,9 @@ static int RSA_eay_public_decrypt(int flen, const unsigned char *from,
                goto err;
                }
 
                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;
 
        if (!rsa->meth->bn_mod_exp(ret,f,rsa->e,rsa->n,ctx,
                rsa->_method_mod_n)) goto err;
@@ -749,11 +747,18 @@ static int RSA_eay_mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa, BN_CTX *ctx)
                        q = rsa->q;
                        }
 
                        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))
 
        /* compute I mod q */
        if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME))