From 6a0a48433b3ad29027a3f2315a5a0e119b96ddc4 Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Mon, 12 Mar 2012 14:22:59 +0000 Subject: [PATCH] Fix for CMS/PKCS7 MMA. If RSA decryption fails use a random key and continue with symmetric decryption process to avoid leaking timing information to an attacker. Thanks to Ivan Nestlerode for discovering this issue. (CVE-2012-0884) --- CHANGES | 11 +++++++ crypto/cms/cms.h | 1 + crypto/cms/cms_enc.c | 60 ++++++++++++++++++++++++++++--------- crypto/cms/cms_env.c | 12 ++++++-- crypto/cms/cms_lcl.h | 2 ++ crypto/cms/cms_smime.c | 37 ++++++++++++++++++++--- crypto/pkcs7/pk7_doit.c | 65 +++++++++++++++++++++++++++++------------ 7 files changed, 150 insertions(+), 38 deletions(-) diff --git a/CHANGES b/CHANGES index 0ee54e3f2f..5a93a271da 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,17 @@ Changes between 1.0.0g and 1.0.0h [xx XXX xxxx] + *) Fix MMA (Bleichenbacher's attack on PKCS #1 v1.5 RSA padding) weakness + in CMS and PKCS7 code. When RSA decryption fails use a random key for + content decryption and always return the same error. Note: this attack + needs on average 2^20 messages so it only affects automated senders. The + old behaviour can be reenabled in the CMS code by setting the + CMS_DEBUG_DECRYPT flag: this is useful for debugging and testing where + an MMA defence is not necessary. + Thanks to Ivan Nestlerode for discovering + this issue. (CVE-2012-0884) + [Steve Henson] + *) Fix CVE-2011-4619: make sure we really are receiving a client hello before rejecting multiple SGC restarts. Thanks to Ivan Nestlerode for discovering this bug. diff --git a/crypto/cms/cms.h b/crypto/cms/cms.h index 09c45d0412..ba378e80f8 100644 --- a/crypto/cms/cms.h +++ b/crypto/cms/cms.h @@ -111,6 +111,7 @@ DECLARE_ASN1_PRINT_FUNCTION(CMS_ContentInfo) #define CMS_PARTIAL 0x4000 #define CMS_REUSE_DIGEST 0x8000 #define CMS_USE_KEYID 0x10000 +#define CMS_DEBUG_DECRYPT 0x20000 const ASN1_OBJECT *CMS_get0_type(CMS_ContentInfo *cms); diff --git a/crypto/cms/cms_enc.c b/crypto/cms/cms_enc.c index bab26235bd..580083b45f 100644 --- a/crypto/cms/cms_enc.c +++ b/crypto/cms/cms_enc.c @@ -73,6 +73,8 @@ BIO *cms_EncryptedContent_init_bio(CMS_EncryptedContentInfo *ec) const EVP_CIPHER *ciph; X509_ALGOR *calg = ec->contentEncryptionAlgorithm; unsigned char iv[EVP_MAX_IV_LENGTH], *piv = NULL; + unsigned char *tkey = NULL; + size_t tkeylen; int ok = 0; @@ -137,32 +139,57 @@ BIO *cms_EncryptedContent_init_bio(CMS_EncryptedContentInfo *ec) CMS_R_CIPHER_PARAMETER_INITIALISATION_ERROR); goto err; } - - - if (enc && !ec->key) + /* Generate random session key */ + if (!enc || !ec->key) { - /* Generate random key */ - if (!ec->keylen) - ec->keylen = EVP_CIPHER_CTX_key_length(ctx); - ec->key = OPENSSL_malloc(ec->keylen); - if (!ec->key) + tkeylen = EVP_CIPHER_CTX_key_length(ctx); + tkey = OPENSSL_malloc(tkeylen); + if (!tkey) { CMSerr(CMS_F_CMS_ENCRYPTEDCONTENT_INIT_BIO, ERR_R_MALLOC_FAILURE); goto err; } - if (EVP_CIPHER_CTX_rand_key(ctx, ec->key) <= 0) + if (EVP_CIPHER_CTX_rand_key(ctx, tkey) <= 0) goto err; - keep_key = 1; } - else if (ec->keylen != (unsigned int)EVP_CIPHER_CTX_key_length(ctx)) + + if (!ec->key) + { + ec->key = tkey; + ec->keylen = tkeylen; + tkey = NULL; + if (enc) + keep_key = 1; + else + ERR_clear_error(); + + } + + if (ec->keylen != tkeylen) { /* If necessary set key length */ if (EVP_CIPHER_CTX_set_key_length(ctx, ec->keylen) <= 0) { - CMSerr(CMS_F_CMS_ENCRYPTEDCONTENT_INIT_BIO, - CMS_R_INVALID_KEY_LENGTH); - goto err; + /* Only reveal failure if debugging so we don't + * leak information which may be useful in MMA. + */ + if (ec->debug) + { + CMSerr(CMS_F_CMS_ENCRYPTEDCONTENT_INIT_BIO, + CMS_R_INVALID_KEY_LENGTH); + goto err; + } + else + { + /* Use random key */ + OPENSSL_cleanse(ec->key, ec->keylen); + OPENSSL_free(ec->key); + ec->key = tkey; + ec->keylen = tkeylen; + tkey = NULL; + ERR_clear_error(); + } } } @@ -198,6 +225,11 @@ BIO *cms_EncryptedContent_init_bio(CMS_EncryptedContentInfo *ec) OPENSSL_free(ec->key); ec->key = NULL; } + if (tkey) + { + OPENSSL_cleanse(tkey, tkeylen); + OPENSSL_free(tkey); + } if (ok) return b; BIO_free(b); diff --git a/crypto/cms/cms_env.c b/crypto/cms/cms_env.c index b3237d4b94..b398f6990b 100644 --- a/crypto/cms/cms_env.c +++ b/crypto/cms/cms_env.c @@ -371,6 +371,8 @@ static int cms_RecipientInfo_ktri_decrypt(CMS_ContentInfo *cms, unsigned char *ek = NULL; size_t eklen; int ret = 0; + CMS_EncryptedContentInfo *ec; + ec = cms->d.envelopedData->encryptedContentInfo; if (ktri->pkey == NULL) { @@ -417,8 +419,14 @@ static int cms_RecipientInfo_ktri_decrypt(CMS_ContentInfo *cms, ret = 1; - cms->d.envelopedData->encryptedContentInfo->key = ek; - cms->d.envelopedData->encryptedContentInfo->keylen = eklen; + if (ec->key) + { + OPENSSL_cleanse(ec->key, ec->keylen); + OPENSSL_free(ec->key); + } + + ec->key = ek; + ec->keylen = eklen; err: if (pctx) diff --git a/crypto/cms/cms_lcl.h b/crypto/cms/cms_lcl.h index c8ecfa724a..2d8f54b1b6 100644 --- a/crypto/cms/cms_lcl.h +++ b/crypto/cms/cms_lcl.h @@ -175,6 +175,8 @@ struct CMS_EncryptedContentInfo_st const EVP_CIPHER *cipher; unsigned char *key; size_t keylen; + /* Set to 1 if we are debugging decrypt and don't fake keys for MMA */ + int debug; }; struct CMS_RecipientInfo_st diff --git a/crypto/cms/cms_smime.c b/crypto/cms/cms_smime.c index 4a799eb897..83edea87bd 100644 --- a/crypto/cms/cms_smime.c +++ b/crypto/cms/cms_smime.c @@ -611,7 +611,10 @@ int CMS_decrypt_set1_pkey(CMS_ContentInfo *cms, EVP_PKEY *pk, X509 *cert) STACK_OF(CMS_RecipientInfo) *ris; CMS_RecipientInfo *ri; int i, r; + int debug = 0; ris = CMS_get0_RecipientInfos(cms); + if (ris) + debug = cms->d.envelopedData->encryptedContentInfo->debug; for (i = 0; i < sk_CMS_RecipientInfo_num(ris); i++) { ri = sk_CMS_RecipientInfo_value(ris, i); @@ -625,17 +628,38 @@ int CMS_decrypt_set1_pkey(CMS_ContentInfo *cms, EVP_PKEY *pk, X509 *cert) CMS_RecipientInfo_set0_pkey(ri, pk); r = CMS_RecipientInfo_decrypt(cms, ri); CMS_RecipientInfo_set0_pkey(ri, NULL); - if (r > 0) - return 1; if (cert) { + /* If not debugging clear any error and + * return success to avoid leaking of + * information useful to MMA + */ + if (!debug) + { + ERR_clear_error(); + return 1; + } + if (r > 0) + return 1; CMSerr(CMS_F_CMS_DECRYPT_SET1_PKEY, CMS_R_DECRYPT_ERROR); return 0; } - ERR_clear_error(); + /* If no cert and not debugging don't leave loop + * after first successful decrypt. Always attempt + * to decrypt all recipients to avoid leaking timing + * of a successful decrypt. + */ + else if (r > 0 && debug) + return 1; } } + /* If no cert and not debugging always return success */ + if (!cert && !debug) + { + ERR_clear_error(); + return 1; + } CMSerr(CMS_F_CMS_DECRYPT_SET1_PKEY, CMS_R_NO_MATCHING_RECIPIENT); return 0; @@ -694,9 +718,14 @@ int CMS_decrypt(CMS_ContentInfo *cms, EVP_PKEY *pk, X509 *cert, } if (!dcont && !check_content(cms)) return 0; + if (flags & CMS_DEBUG_DECRYPT) + cms->d.envelopedData->encryptedContentInfo->debug = 1; + else + cms->d.envelopedData->encryptedContentInfo->debug = 0; + if (!pk && !cert && !dcont && !out) + return 1; if (pk && !CMS_decrypt_set1_pkey(cms, pk, cert)) return 0; - cont = CMS_dataInit(cms, dcont); if (!cont) return 0; diff --git a/crypto/pkcs7/pk7_doit.c b/crypto/pkcs7/pk7_doit.c index 3bf1a367bb..e4b095e195 100644 --- a/crypto/pkcs7/pk7_doit.c +++ b/crypto/pkcs7/pk7_doit.c @@ -204,11 +204,11 @@ static int pkcs7_decrypt_rinfo(unsigned char **pek, int *peklen, unsigned char *ek = NULL; size_t eklen; - int ret = 0; + int ret = -1; pctx = EVP_PKEY_CTX_new(pkey, NULL); if (!pctx) - return 0; + return -1; if (EVP_PKEY_decrypt_init(pctx) <= 0) goto err; @@ -235,12 +235,19 @@ static int pkcs7_decrypt_rinfo(unsigned char **pek, int *peklen, if (EVP_PKEY_decrypt(pctx, ek, &eklen, ri->enc_key->data, ri->enc_key->length) <= 0) { + ret = 0; PKCS7err(PKCS7_F_PKCS7_DECRYPT_RINFO, ERR_R_EVP_LIB); goto err; } ret = 1; + if (*pek) + { + OPENSSL_cleanse(*pek, *peklen); + OPENSSL_free(*pek); + } + *pek = ek; *peklen = eklen; @@ -500,8 +507,8 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert) int max; X509_OBJECT ret; #endif - unsigned char *ek = NULL; - int eklen; + unsigned char *ek = NULL, *tkey = NULL; + int eklen, tkeylen; if ((etmp=BIO_new(BIO_f_cipher())) == NULL) { @@ -534,29 +541,28 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert) } /* If we haven't got a certificate try each ri in turn */ - if (pcert == NULL) { + /* Always attempt to decrypt all rinfo even + * after sucess as a defence against MMA timing + * attacks. + */ for (i=0; i 0) - break; + ri, pkey) < 0) + goto err; ERR_clear_error(); - ri = NULL; - } - if (ri == NULL) - { - PKCS7err(PKCS7_F_PKCS7_DATADECODE, - PKCS7_R_NO_RECIPIENT_MATCHES_KEY); - goto err; } } else { - if (pkcs7_decrypt_rinfo(&ek, &eklen, ri, pkey) <= 0) + /* Only exit on fatal errors, not decrypt failure */ + if (pkcs7_decrypt_rinfo(&ek, &eklen, ri, pkey) < 0) goto err; + ERR_clear_error(); } evp_ctx=NULL; @@ -565,6 +571,19 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert) goto err; if (EVP_CIPHER_asn1_to_param(evp_ctx,enc_alg->parameter) < 0) goto err; + /* Generate random key as MMA defence */ + tkeylen = EVP_CIPHER_CTX_key_length(evp_ctx); + tkey = OPENSSL_malloc(tkeylen); + if (!tkey) + goto err; + if (EVP_CIPHER_CTX_rand_key(evp_ctx, tkey) <= 0) + goto err; + if (ek == NULL) + { + ek = tkey; + eklen = tkeylen; + tkey = NULL; + } if (eklen != EVP_CIPHER_CTX_key_length(evp_ctx)) { /* Some S/MIME clients don't use the same key @@ -573,11 +592,16 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert) */ if(!EVP_CIPHER_CTX_set_key_length(evp_ctx, eklen)) { - PKCS7err(PKCS7_F_PKCS7_DATADECODE, - PKCS7_R_DECRYPTED_KEY_IS_WRONG_LENGTH); - goto err; + /* Use random key as MMA defence */ + OPENSSL_cleanse(ek, eklen); + OPENSSL_free(ek); + ek = tkey; + eklen = tkeylen; + tkey = NULL; } } + /* Clear errors so we don't leak information useful in MMA */ + ERR_clear_error(); if (EVP_CipherInit_ex(evp_ctx,NULL,NULL,ek,NULL,0) <= 0) goto err; @@ -586,6 +610,11 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert) OPENSSL_cleanse(ek,eklen); OPENSSL_free(ek); } + if (tkey) + { + OPENSSL_cleanse(tkey,tkeylen); + OPENSSL_free(tkey); + } if (out == NULL) out=etmp; -- 2.25.1