pkcs7: make PKCS7#add_recipient actually useful

Add a simple test case that creates an enveloped-data structure without
using the shorthand method, and fix two issues preventing this from
working correctly.

First, OpenSSL::PKey::PKCS7#add_recipient currently inserts an
incomplete PKCS7_RECIP_INFO object into the PKCS7 object. When
duplicating an unfinalized PKCS7_RECIP_INFO, the internal X509 reference
must also be copied, as it is later used by #add_data to fill the rest.

A similar issue with #add_signer was fixed in commit 20ca7a2
(pkcs7: keep private key when duplicating PKCS7_SIGNER_INFO,
2021-03-24).

Second, #add_data calls PKCS7_dataFinal(), which for enveloped-data
appears to require the BIO to be flushed explicitly with BIO_flush().
Without this, the last block of the encrypted data would be missing.

Author: rhenium

ext/openssl/ossl_pkcs7.c

@@ -143,11 +143,19 @@ ossl_PKCS7_SIGNER_INFO_dup(PKCS7_SIGNER_INFO *si)
 }
 
 static PKCS7_RECIP_INFO *
-ossl_PKCS7_RECIP_INFO_dup(PKCS7_RECIP_INFO *si)
+ossl_PKCS7_RECIP_INFO_dup(PKCS7_RECIP_INFO *ri)
 {
-    return ASN1_dup((i2d_of_void *)i2d_PKCS7_RECIP_INFO,
-                    (d2i_of_void *)d2i_PKCS7_RECIP_INFO,
-                    si);
+    PKCS7_RECIP_INFO *ri_new = ASN1_dup((i2d_of_void *)i2d_PKCS7_RECIP_INFO,
+                                        (d2i_of_void *)d2i_PKCS7_RECIP_INFO,
+                                        ri);
+    if (ri_new && ri->cert) {
+        if (!X509_up_ref(ri->cert)) {
+            PKCS7_RECIP_INFO_free(ri_new);
+            return NULL;
+        }
+        ri_new->cert = ri->cert;
+    }
+    return ri_new;
 }
 
 static VALUE
@@ -859,6 +867,11 @@ ossl_pkcs7_add_data(VALUE self, VALUE data)
             ossl_raise(ePKCS7Error, "BIO_write");
         }
     }
+    if (BIO_flush(out) <= 0) {
+        BIO_free_all(out);
+        BIO_free(in);
+        ossl_raise(ePKCS7Error, "BIO_flush");
+    }
     ret = PKCS7_dataFinal(pkcs7, out);
     BIO_free_all(out);
     BIO_free(in);

test/openssl/test_pkcs7.rb

@@ -250,6 +250,28 @@ def test_enveloped
     }
   end
 
+  def test_enveloped_add_recipient
+    omit_on_fips # PKCS #1 v1.5 padding
+
+    data = "aaaaa\nbbbbb\nccccc\n"
+    ktri_ee1 = OpenSSL::PKCS7::RecipientInfo.new(@ee1_cert)
+    ktri_ee2 = OpenSSL::PKCS7::RecipientInfo.new(@ee2_cert)
+
+    tmp = OpenSSL::PKCS7.new
+    tmp.type = :enveloped
+    tmp.cipher = "AES-128-CBC"
+    tmp.add_recipient(ktri_ee1)
+    tmp.add_recipient(ktri_ee2)
+    tmp.add_data(data)
+
+    p7 = OpenSSL::PKCS7.new(tmp.to_der)
+    assert_equal(:enveloped, p7.type)
+    assert_equal(data, p7.decrypt(@ee1_key, @ee1_cert))
+    assert_equal(data, p7.decrypt(@ee2_key, @ee2_cert))
+    assert_equal([@ee1_cert.serial, @ee2_cert.serial].sort,
+                 p7.recipients.map(&:serial).sort)
+  end
+
   def test_data
     asn1 = OpenSSL::ASN1::Sequence([
       OpenSSL::ASN1::ObjectId("pkcs7-data"),
@@ -317,12 +339,6 @@ def test_set_type_signed_and_enveloped
     assert_equal(:signedAndEnveloped, p7.type)
   end
 
-  def test_set_type_enveloped
-    p7 = OpenSSL::PKCS7.new
-    p7.type = "enveloped"
-    assert_equal(:enveloped, p7.type)
-  end
-
   def test_set_type_encrypted
     p7 = OpenSSL::PKCS7.new
     p7.type = "encrypted"