free oaep label-octet-string on error
authorJames Muir <james@openssl.org>
Wed, 25 Oct 2023 00:08:54 +0000 (20:08 -0400)
committerTomas Mraz <tomas@openssl.org>
Wed, 1 Nov 2023 11:04:04 +0000 (12:04 +0100)
When X509_ALGOR_set0() fails, ownership of the the ASN1 object "los"
(label octet string) has not been passed on to the X509_ALGOR object
"oaep->pSourceFunc", so we need to free "los" in that case.

Check return value of X509_ALGOR_set0(), change the scope of "los" and
ensure it is freed on failure (on success, set it to NULL so it is not
freed inside the function).

Fixes #22336

Testing:
You can use the following script to test cms encryption with rsa-oaep:

  #!/bin/bash -x

  OSSLCMD="apps/openssl"

  # check we are calling the right openssl app
  LD_LIBRARY_PATH=. valgrind $OSSLCMD version

  echo "this is a confidential message." > msg.txt

  LD_LIBRARY_PATH=. valgrind $OSSLCMD cms -encrypt -in msg.txt \
   -stream -out msg.txt.cms \
   -recip test/smime-certs/smrsa1.pem \
          -keyopt rsa_padding_mode:oaep \
          -keyopt rsa_oaep_md:sha256 \
          -keyopt rsa_oaep_label:deadbeef

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22556)

(cherry picked from commit a9a1b3da876456e1eecffbba15fb6d1820e8f379)

crypto/cms/cms_rsa.c

index 61fd43fb54d051189aab5662e7fc425f5058458a..68545e5fb7e170df437ac207813fcec68b6d86ce 100644 (file)
@@ -114,6 +114,7 @@ static int rsa_cms_encrypt(CMS_RecipientInfo *ri)
     const EVP_MD *md, *mgf1md;
     RSA_OAEP_PARAMS *oaep = NULL;
     ASN1_STRING *os = NULL;
+    ASN1_OCTET_STRING *los = NULL;
     X509_ALGOR *alg;
     EVP_PKEY_CTX *pkctx = CMS_RecipientInfo_get0_pkey_ctx(ri);
     int pad_mode = RSA_PKCS1_PADDING, rv = 0, labellen;
@@ -125,10 +126,10 @@ static int rsa_cms_encrypt(CMS_RecipientInfo *ri)
         if (EVP_PKEY_CTX_get_rsa_padding(pkctx, &pad_mode) <= 0)
             return 0;
     }
-    if (pad_mode == RSA_PKCS1_PADDING) {
-        X509_ALGOR_set0(alg, OBJ_nid2obj(NID_rsaEncryption), V_ASN1_NULL, 0);
-        return 1;
-    }
+    if (pad_mode == RSA_PKCS1_PADDING)
+        return X509_ALGOR_set0(alg, OBJ_nid2obj(NID_rsaEncryption),
+                               V_ASN1_NULL, NULL);
+
     /* Not supported */
     if (pad_mode != RSA_PKCS1_OAEP_PADDING)
         return 0;
@@ -147,30 +148,32 @@ static int rsa_cms_encrypt(CMS_RecipientInfo *ri)
     if (!ossl_x509_algor_md_to_mgf1(&oaep->maskGenFunc, mgf1md))
         goto err;
     if (labellen > 0) {
-        ASN1_OCTET_STRING *los;
-
         oaep->pSourceFunc = X509_ALGOR_new();
         if (oaep->pSourceFunc == NULL)
             goto err;
         los = ASN1_OCTET_STRING_new();
         if (los == NULL)
             goto err;
-        if (!ASN1_OCTET_STRING_set(los, label, labellen)) {
-            ASN1_OCTET_STRING_free(los);
+        if (!ASN1_OCTET_STRING_set(los, label, labellen))
             goto err;
-        }
-        X509_ALGOR_set0(oaep->pSourceFunc, OBJ_nid2obj(NID_pSpecified),
-                        V_ASN1_OCTET_STRING, los);
+
+        if (!X509_ALGOR_set0(oaep->pSourceFunc, OBJ_nid2obj(NID_pSpecified),
+                        V_ASN1_OCTET_STRING, los))
+            goto err;
+
+        los = NULL;
     }
-    /* create string with pss parameter encoding. */
+    /* create string with oaep parameter encoding. */
     if (!ASN1_item_pack(oaep, ASN1_ITEM_rptr(RSA_OAEP_PARAMS), &os))
-         goto err;
-    X509_ALGOR_set0(alg, OBJ_nid2obj(NID_rsaesOaep), V_ASN1_SEQUENCE, os);
+        goto err;
+    if (!X509_ALGOR_set0(alg, OBJ_nid2obj(NID_rsaesOaep), V_ASN1_SEQUENCE, os))
+        goto err;
     os = NULL;
     rv = 1;
  err:
     RSA_OAEP_PARAMS_free(oaep);
     ASN1_STRING_free(os);
+    ASN1_OCTET_STRING_free(los);
     return rv;
 }