From: Dr. David von Oheimb Date: Thu, 15 Sep 2022 09:51:30 +0000 (+0200) Subject: CMS_decrypt*(): fix misconceptions and mem leak X-Git-Tag: openssl-3.2.0-alpha1~1670 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=25dd78048b69c2a780ab1a5378b62447c77a5e75;p=thirdparty%2Fopenssl.git CMS_decrypt*(): fix misconceptions and mem leak Reviewed-by: Tomas Mraz Reviewed-by: Paul Dale Reviewed-by: David von Oheimb (Merged from https://github.com/openssl/openssl/pull/19222) --- diff --git a/crypto/cms/cms_env.c b/crypto/cms/cms_env.c index 7887defe252..d25504a03f7 100644 --- a/crypto/cms/cms_env.c +++ b/crypto/cms/cms_env.c @@ -138,7 +138,7 @@ int ossl_cms_env_asn1_ctrl(CMS_RecipientInfo *ri, int cmd) return 1; } -CMS_EncryptedContentInfo* ossl_cms_get0_env_enc_content(const CMS_ContentInfo *cms) +CMS_EncryptedContentInfo *ossl_cms_get0_env_enc_content(const CMS_ContentInfo *cms) { switch (cms_get_enveloped_type(cms)) { case CMS_ENVELOPED_STANDARD: @@ -266,7 +266,8 @@ BIO *CMS_EnvelopedData_decrypt(CMS_EnvelopedData *env, BIO *detached_data, ASN1_STRING_get0_data(secret), ASN1_STRING_length(secret)) != 1) goto end; - res = CMS_decrypt(ci, pkey, cert, detached_data, bio, flags); + res = CMS_decrypt(ci, secret == NULL ? pkey : NULL, + secret == NULL ? cert : NULL, detached_data, bio, flags); end: if (ci != NULL) diff --git a/crypto/cms/cms_smime.c b/crypto/cms/cms_smime.c index c5b82cf9562..e49c611a289 100644 --- a/crypto/cms/cms_smime.c +++ b/crypto/cms/cms_smime.c @@ -707,10 +707,16 @@ int CMS_decrypt_set1_pkey_and_peer(CMS_ContentInfo *cms, EVP_PKEY *pk, CMS_RecipientInfo *ri; int i, r, cms_pkey_ri_type; int debug = 0, match_ri = 0; + CMS_EncryptedContentInfo *ec = ossl_cms_get0_env_enc_content(cms); + + /* Prevent mem leak on earlier CMS_decrypt_set1_{pkey_and_peer,password} */ + OPENSSL_clear_free(ec->key, ec->keylen); + ec->key = NULL; + ec->keylen = 0; ris = CMS_get0_RecipientInfos(cms); if (ris != NULL) - debug = ossl_cms_get0_env_enc_content(cms)->debug; + debug = ec->debug; cms_pkey_ri_type = ossl_cms_pkey_get_ri_type(pk); if (cms_pkey_ri_type == CMS_RECIPINFO_NONE) { @@ -845,7 +851,7 @@ int CMS_decrypt(CMS_ContentInfo *cms, EVP_PKEY *pk, X509 *cert, { int r; BIO *cont; - + CMS_EncryptedContentInfo *ec; int nid = OBJ_obj2nid(CMS_get0_type(cms)); if (nid != NID_pkcs7_enveloped @@ -855,14 +861,9 @@ int CMS_decrypt(CMS_ContentInfo *cms, EVP_PKEY *pk, X509 *cert, } if (dcont == NULL && !check_content(cms)) return 0; - if (flags & CMS_DEBUG_DECRYPT) - ossl_cms_get0_env_enc_content(cms)->debug = 1; - else - ossl_cms_get0_env_enc_content(cms)->debug = 0; - if (cert == NULL) - ossl_cms_get0_env_enc_content(cms)->havenocert = 1; - else - ossl_cms_get0_env_enc_content(cms)->havenocert = 0; + ec = ossl_cms_get0_env_enc_content(cms); + ec->debug = (flags & CMS_DEBUG_DECRYPT) != 0; + ec->havenocert = cert == NULL; if (pk == NULL && cert == NULL && dcont == NULL && out == NULL) return 1; if (pk != NULL && !CMS_decrypt_set1_pkey(cms, pk, cert)) diff --git a/doc/man3/CMS_EncryptedData_decrypt.pod b/doc/man3/CMS_EncryptedData_decrypt.pod index 07a2449c12e..fe77701fe01 100644 --- a/doc/man3/CMS_EncryptedData_decrypt.pod +++ b/doc/man3/CMS_EncryptedData_decrypt.pod @@ -26,15 +26,17 @@ to and I is an optional set of flags. I is used in the rare case where the encrypted content is detached. It will normally be set to NULL. -The following flags can be passed in the B parameter. +The following flags can be passed in the I parameter. -If the B flag is set MIME headers for type B are deleted -from the content. If the content is not of type B then an error is +If the B flag is set MIME headers for type C are deleted +from the content. If the content is not of type C then an error is returned. CMS_EnvelopedData_decrypt() decrypts, similarly to CMS_decrypt(3), -a CMS EnvelopedData object I using the symmetric key I or -the private key of the recipient B and its associated certificate B. +a CMS EnvelopedData object I using the symmetric key I if it +is not NULL, otherwise the private key of the recipient I. +If I is given, it is recommended to provide also the associated +certificate in I - see L and the NOTES on I there. The optional parameters I and I are used as described above. The optional parameters library context I and property query I are used when retrieving algorithms from providers. diff --git a/doc/man3/CMS_decrypt.pod b/doc/man3/CMS_decrypt.pod index 3d3a4e0693a..121b74a30a1 100644 --- a/doc/man3/CMS_decrypt.pod +++ b/doc/man3/CMS_decrypt.pod @@ -20,23 +20,35 @@ CMS_decrypt_set1_pkey, CMS_decrypt_set1_password =head1 DESCRIPTION -CMS_decrypt() extracts and decrypts the content from a CMS EnvelopedData -or AuthEnvelopedData structure. I is the private key of the recipient, -I is the recipient's certificate, I is a BIO to write the content to -and I is an optional set of flags. - +CMS_decrypt() extracts the decrypted content from a CMS EnvelopedData +or AuthEnvelopedData structure. +It uses CMS_decrypt_set1_pkey() to decrypt the content +with the recipient private key I if I is not NULL. +In this case, the associated certificate is recommended to provide in I - +see the NOTES below. +I is a BIO to write the content to and +I is an optional set of flags. +If I is NULL the function assumes that decryption was already done +(e.g., using CMS_decrypt_set1_pkey() or CMS_decrypt_set1_password()) and just +provides the content unless I, I, and I are NULL as well. The I parameter is used in the rare case where the encrypted content is detached. It will normally be set to NULL. -CMS_decrypt_set1_pkey_and_peer() associates the private key I, the -corresponding certificate I and the originator certificate I with -the CMS_ContentInfo structure I. +CMS_decrypt_set1_pkey_and_peer() decrypts the CMS_ContentInfo structure I +using the private key I, the corresponding certificate I, which is +recommended but may be NULL, and the (optional) originator certificate I. +On success, it also records in I the decryption key I, and then +should be followed by C. +This call deallocates any decryption key stored in I. -CMS_decrypt_set1_pkey() associates the private key I and the corresponding -certificate I with the CMS_ContentInfo structure I. +CMS_decrypt_set1_pkey() is the same as +CMS_decrypt_set1_pkey_and_peer() with I being NULL. -CMS_decrypt_set1_password() associates the secret I of length I -with the CMS_ContentInfo structure I. +CMS_decrypt_set1_password() decrypts the CMS_ContentInfo structure I +using the secret I of length I. +On success, it also records in I the decryption key used, and then +should be followed by C. +This call deallocates any decryption key stored in I. =head1 NOTES @@ -60,8 +72,9 @@ open to attack. It is possible to determine the correct recipient key by other means (for example looking them up in a database) and setting them in the CMS structure -in advance using the CMS utility functions such as CMS_set1_pkey(). In this -case both I and I should be set to NULL. +in advance using the CMS utility functions such as CMS_set1_pkey(), +or use CMS_decrypt_set1_password() if the recipient has a symmetric key. +In these cases both I and I should be set to NULL. To process KEKRecipientInfo types CMS_set1_key() or CMS_RecipientInfo_set0_key() and CMS_RecipientInfo_decrypt() should be called before CMS_decrypt() and @@ -82,6 +95,9 @@ The error can be obtained from ERR_get_error(3). =head1 BUGS +The B part of these function names is misleading +and should better read: B. + The lack of single pass processing and the need to hold all data in memory as mentioned in CMS_verify() also applies to CMS_decrypt().