From db99c52509bd79138924c84d127e9872359689c4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bodo=20M=C3=B6ller?= Date: Sun, 14 Sep 2008 13:51:44 +0000 Subject: [PATCH] Really get rid of unsafe double-checked locking. Also, "CHANGES" clean-ups. --- CHANGES | 57 +++++++++++++++++++++++++++++++++++++++++--- crypto/rsa/rsa_eay.c | 39 +++++++++++++++++------------- 2 files changed, 76 insertions(+), 20 deletions(-) diff --git a/CHANGES b/CHANGES index b9a6d83641..4fd140968d 100644 --- a/CHANGES +++ b/CHANGES @@ -705,6 +705,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). @@ -739,6 +749,10 @@ 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 @@ -771,7 +785,26 @@ the 'db' section contains nothing but zeroes (there is a one-byte invalid read after the end of 'db'). [Ivan Nestlerode ] - + + *) 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 @@ -790,10 +823,24 @@ 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. - [Paul Sheer , Geoff Thorpe] + [Paul Sheer ] *) Zlib compression BIO. This is a filter BIO which compressed and uncompresses any data passed through it. @@ -1007,6 +1054,10 @@ 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 @@ -2210,7 +2261,7 @@ 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. diff --git a/crypto/rsa/rsa_eay.c b/crypto/rsa/rsa_eay.c index 7321349237..c5eaeeae6b 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; @@ -440,7 +432,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; @@ -561,7 +555,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; @@ -671,7 +667,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; @@ -749,11 +747,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)) -- 2.34.1