From 669b912dea8903df14ba2b3668aa5d8faa260462 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bodo=20M=C3=B6ller?= Date: Sun, 14 Sep 2008 13:51:49 +0000 Subject: [PATCH] Really get rid of unsafe double-checked locking. Also, "CHANGES" clean-ups. --- CHANGES | 26 ++++++++++---------------- crypto/rsa/rsa_eay.c | 39 ++++++++++++++++++++++----------------- 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)) -- 2.25.1