Skip to content

Commit 631f94d

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 d382345 commit 631f94d

File tree

5 files changed

+45
-5
lines changed

5 files changed

+45
-5
lines changed

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
*) Use Windows installation paths in the mingw builds
4357

4458
Mingw isn't a POSIX environment per se, which means that Windows

crypto/cms/cms_env.c

+17-1
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ static int cms_RecipientInfo_ktri_decrypt(CMS_ContentInfo *cms,
363363
unsigned char *ek = NULL;
364364
size_t eklen;
365365
int ret = 0;
366+
size_t fixlen = 0;
366367
CMS_EncryptedContentInfo *ec;
367368
ec = cms->d.envelopedData->encryptedContentInfo;
368369

@@ -371,6 +372,19 @@ static int cms_RecipientInfo_ktri_decrypt(CMS_ContentInfo *cms,
371372
return 0;
372373
}
373374

375+
if (cms->d.envelopedData->encryptedContentInfo->havenocert
376+
&& !cms->d.envelopedData->encryptedContentInfo->debug) {
377+
X509_ALGOR *calg = ec->contentEncryptionAlgorithm;
378+
const EVP_CIPHER *ciph = EVP_get_cipherbyobj(calg->algorithm);
379+
380+
if (ciph == NULL) {
381+
CMSerr(CMS_F_CMS_RECIPIENTINFO_KTRI_DECRYPT, CMS_R_UNKNOWN_CIPHER);
382+
return 0;
383+
}
384+
385+
fixlen = EVP_CIPHER_key_length(ciph);
386+
}
387+
374388
ktri->pctx = EVP_PKEY_CTX_new(pkey, NULL);
375389
if (ktri->pctx == NULL)
376390
return 0;
@@ -401,7 +415,9 @@ static int cms_RecipientInfo_ktri_decrypt(CMS_ContentInfo *cms,
401415

402416
if (EVP_PKEY_decrypt(ktri->pctx, ek, &eklen,
403417
ktri->encryptedKey->data,
404-
ktri->encryptedKey->length) <= 0) {
418+
ktri->encryptedKey->length) <= 0
419+
|| eklen == 0
420+
|| (fixlen != 0 && eklen != fixlen)) {
405421
CMSerr(CMS_F_CMS_RECIPIENTINFO_KTRI_DECRYPT, CMS_R_CMS_LIB);
406422
goto err;
407423
}

crypto/cms/cms_lcl.h

+2
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ struct CMS_EncryptedContentInfo_st {
129129
size_t keylen;
130130
/* Set to 1 if we are debugging decrypt and don't fake keys for MMA */
131131
int debug;
132+
/* Set to 1 if we have no cert and need extra safety measures for MMA */
133+
int havenocert;
132134
};
133135

134136
struct CMS_RecipientInfo_st {

crypto/cms/cms_smime.c

+4
Original file line numberDiff line numberDiff line change
@@ -743,6 +743,10 @@ int CMS_decrypt(CMS_ContentInfo *cms, EVP_PKEY *pk, X509 *cert,
743743
cms->d.envelopedData->encryptedContentInfo->debug = 1;
744744
else
745745
cms->d.envelopedData->encryptedContentInfo->debug = 0;
746+
if (!cert)
747+
cms->d.envelopedData->encryptedContentInfo->havenocert = 1;
748+
else
749+
cms->d.envelopedData->encryptedContentInfo->havenocert = 0;
746750
if (!pk && !cert && !dcont && !out)
747751
return 1;
748752
if (pk && !CMS_decrypt_set1_pkey(cms, pk, cert))

crypto/pkcs7/pk7_doit.c

+8-4
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,8 @@ static int pkcs7_encode_rinfo(PKCS7_RECIP_INFO *ri,
137137
}
138138

139139
static int pkcs7_decrypt_rinfo(unsigned char **pek, int *peklen,
140-
PKCS7_RECIP_INFO *ri, EVP_PKEY *pkey)
140+
PKCS7_RECIP_INFO *ri, EVP_PKEY *pkey,
141+
size_t fixlen)
141142
{
142143
EVP_PKEY_CTX *pctx = NULL;
143144
unsigned char *ek = NULL;
@@ -170,7 +171,9 @@ static int pkcs7_decrypt_rinfo(unsigned char **pek, int *peklen,
170171
}
171172

172173
if (EVP_PKEY_decrypt(pctx, ek, &eklen,
173-
ri->enc_key->data, ri->enc_key->length) <= 0) {
174+
ri->enc_key->data, ri->enc_key->length) <= 0
175+
|| eklen == 0
176+
|| (fixlen != 0 && eklen != fixlen)) {
174177
ret = 0;
175178
PKCS7err(PKCS7_F_PKCS7_DECRYPT_RINFO, ERR_R_EVP_LIB);
176179
goto err;
@@ -499,13 +502,14 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert)
499502
for (i = 0; i < sk_PKCS7_RECIP_INFO_num(rsk); i++) {
500503
ri = sk_PKCS7_RECIP_INFO_value(rsk, i);
501504

502-
if (pkcs7_decrypt_rinfo(&ek, &eklen, ri, pkey) < 0)
505+
if (pkcs7_decrypt_rinfo(&ek, &eklen, ri, pkey,
506+
EVP_CIPHER_key_length(evp_cipher)) < 0)
503507
goto err;
504508
ERR_clear_error();
505509
}
506510
} else {
507511
/* Only exit on fatal errors, not decrypt failure */
508-
if (pkcs7_decrypt_rinfo(&ek, &eklen, ri, pkey) < 0)
512+
if (pkcs7_decrypt_rinfo(&ek, &eklen, ri, pkey, 0) < 0)
509513
goto err;
510514
ERR_clear_error();
511515
}

0 commit comments

Comments
 (0)