Skip to content

Commit e21f8cf

Browse files
bernd-edlingermattcaswell
authored andcommitted
Fix a padding oracle in PKCS7_dataDecode and CMS_decrypt_set1_pkey
An attack is simple, if the first CMS_recipientInfo is valid but the second CMS_recipientInfo is chosen ciphertext. If the second recipientInfo decodes to PKCS #1 v1.5 form plaintext, the correct encryption key will be replaced by garbage, and the message cannot be decoded, but if the RSA decryption fails, the correct encryption key is used and the recipient will not notice the attack. As a work around for this potential attack the length of the decrypted key must be equal to the cipher default key length, in case the certifiate is not given and all recipientInfo are tried out. The old behaviour can be re-enabled in the CMS code by setting the CMS_DEBUG_DECRYPT flag. Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from #9777) (cherry picked from commit 5840ed0)
1 parent 8bf7d77 commit e21f8cf

File tree

5 files changed

+45
-5
lines changed

5 files changed

+45
-5
lines changed

Diff for: CHANGES

+14
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,20 @@
3939
(CVE-2019-1547)
4040
[Billy Bob Brumley]
4141

42+
*) Fixed a padding oracle in PKCS7_dataDecode and CMS_decrypt_set1_pkey.
43+
An attack is simple, if the first CMS_recipientInfo is valid but the
44+
second CMS_recipientInfo is chosen ciphertext. If the second
45+
recipientInfo decodes to PKCS #1 v1.5 form plaintext, the correct
46+
encryption key will be replaced by garbage, and the message cannot be
47+
decoded, but if the RSA decryption fails, the correct encryption key is
48+
used and the recipient will not notice the attack.
49+
As a work around for this potential attack the length of the decrypted
50+
key must be equal to the cipher default key length, in case the
51+
certifiate is not given and all recipientInfo are tried out.
52+
The old behaviour can be re-enabled in the CMS code by setting the
53+
CMS_DEBUG_DECRYPT flag.
54+
[Bernd Edlinger]
55+
4256
*) Document issue with installation paths in diverse Windows builds
4357

4458
'/usr/local/ssl' is an unsafe prefix for location to install OpenSSL

Diff for: crypto/cms/cms_env.c

+17-1
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,7 @@ static int cms_RecipientInfo_ktri_decrypt(CMS_ContentInfo *cms,
422422
unsigned char *ek = NULL;
423423
size_t eklen;
424424
int ret = 0;
425+
size_t fixlen = 0;
425426
CMS_EncryptedContentInfo *ec;
426427
ec = cms->d.envelopedData->encryptedContentInfo;
427428

@@ -430,6 +431,19 @@ static int cms_RecipientInfo_ktri_decrypt(CMS_ContentInfo *cms,
430431
return 0;
431432
}
432433

434+
if (cms->d.envelopedData->encryptedContentInfo->havenocert
435+
&& !cms->d.envelopedData->encryptedContentInfo->debug) {
436+
X509_ALGOR *calg = ec->contentEncryptionAlgorithm;
437+
const EVP_CIPHER *ciph = EVP_get_cipherbyobj(calg->algorithm);
438+
439+
if (ciph == NULL) {
440+
CMSerr(CMS_F_CMS_RECIPIENTINFO_KTRI_DECRYPT, CMS_R_UNKNOWN_CIPHER);
441+
return 0;
442+
}
443+
444+
fixlen = EVP_CIPHER_key_length(ciph);
445+
}
446+
433447
ktri->pctx = EVP_PKEY_CTX_new(pkey, NULL);
434448
if (!ktri->pctx)
435449
return 0;
@@ -460,7 +474,9 @@ static int cms_RecipientInfo_ktri_decrypt(CMS_ContentInfo *cms,
460474

461475
if (EVP_PKEY_decrypt(ktri->pctx, ek, &eklen,
462476
ktri->encryptedKey->data,
463-
ktri->encryptedKey->length) <= 0) {
477+
ktri->encryptedKey->length) <= 0
478+
|| eklen == 0
479+
|| (fixlen != 0 && eklen != fixlen)) {
464480
CMSerr(CMS_F_CMS_RECIPIENTINFO_KTRI_DECRYPT, CMS_R_CMS_LIB);
465481
goto err;
466482
}

Diff for: crypto/cms/cms_lcl.h

+2
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@ struct CMS_EncryptedContentInfo_st {
172172
size_t keylen;
173173
/* Set to 1 if we are debugging decrypt and don't fake keys for MMA */
174174
int debug;
175+
/* Set to 1 if we have no cert and need extra safety measures for MMA */
176+
int havenocert;
175177
};
176178

177179
struct CMS_RecipientInfo_st {

Diff for: crypto/cms/cms_smime.c

+4
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,10 @@ int CMS_decrypt(CMS_ContentInfo *cms, EVP_PKEY *pk, X509 *cert,
737737
cms->d.envelopedData->encryptedContentInfo->debug = 1;
738738
else
739739
cms->d.envelopedData->encryptedContentInfo->debug = 0;
740+
if (!cert)
741+
cms->d.envelopedData->encryptedContentInfo->havenocert = 1;
742+
else
743+
cms->d.envelopedData->encryptedContentInfo->havenocert = 0;
740744
if (!pk && !cert && !dcont && !out)
741745
return 1;
742746
if (pk && !CMS_decrypt_set1_pkey(cms, pk, cert))

Diff for: crypto/pkcs7/pk7_doit.c

+8-4
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,8 @@ static int pkcs7_encode_rinfo(PKCS7_RECIP_INFO *ri,
191191
}
192192

193193
static int pkcs7_decrypt_rinfo(unsigned char **pek, int *peklen,
194-
PKCS7_RECIP_INFO *ri, EVP_PKEY *pkey)
194+
PKCS7_RECIP_INFO *ri, EVP_PKEY *pkey,
195+
size_t fixlen)
195196
{
196197
EVP_PKEY_CTX *pctx = NULL;
197198
unsigned char *ek = NULL;
@@ -224,7 +225,9 @@ static int pkcs7_decrypt_rinfo(unsigned char **pek, int *peklen,
224225
}
225226

226227
if (EVP_PKEY_decrypt(pctx, ek, &eklen,
227-
ri->enc_key->data, ri->enc_key->length) <= 0) {
228+
ri->enc_key->data, ri->enc_key->length) <= 0
229+
|| eklen == 0
230+
|| (fixlen != 0 && eklen != fixlen)) {
228231
ret = 0;
229232
PKCS7err(PKCS7_F_PKCS7_DECRYPT_RINFO, ERR_R_EVP_LIB);
230233
goto err;
@@ -571,13 +574,14 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert)
571574
for (i = 0; i < sk_PKCS7_RECIP_INFO_num(rsk); i++) {
572575
ri = sk_PKCS7_RECIP_INFO_value(rsk, i);
573576

574-
if (pkcs7_decrypt_rinfo(&ek, &eklen, ri, pkey) < 0)
577+
if (pkcs7_decrypt_rinfo(&ek, &eklen, ri, pkey,
578+
EVP_CIPHER_key_length(evp_cipher)) < 0)
575579
goto err;
576580
ERR_clear_error();
577581
}
578582
} else {
579583
/* Only exit on fatal errors, not decrypt failure */
580-
if (pkcs7_decrypt_rinfo(&ek, &eklen, ri, pkey) < 0)
584+
if (pkcs7_decrypt_rinfo(&ek, &eklen, ri, pkey, 0) < 0)
581585
goto err;
582586
ERR_clear_error();
583587
}

0 commit comments

Comments
 (0)