Merge pull request #923 from rhenium/ky/pkcs7-fix-add-recipient

pkcs7: make PKCS7#add_recipient actually useful

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
@@ -840,30 +848,38 @@ ossl_pkcs7_add_data(VALUE self, VALUE data)
     PKCS7 *pkcs7;
     BIO *out, *in;
     char buf[4096];
-    int len;
+    int len, ret;
 
     GetPKCS7(self, pkcs7);
-    if(PKCS7_type_is_signed(pkcs7)){
-	if(!PKCS7_content_new(pkcs7, NID_pkcs7_data))
-	    ossl_raise(ePKCS7Error, NULL);
+    if (PKCS7_type_is_signed(pkcs7)) {
+        if (!PKCS7_content_new(pkcs7, NID_pkcs7_data))
+            ossl_raise(ePKCS7Error, "PKCS7_content_new");
     }
     in = ossl_obj2bio(&data);
-    if(!(out = PKCS7_dataInit(pkcs7, NULL))) goto err;
-    for(;;){
-	if((len = BIO_read(in, buf, sizeof(buf))) <= 0)
-	    break;
-	if(BIO_write(out, buf, len) != len)
-	    goto err;
+    if (!(out = PKCS7_dataInit(pkcs7, NULL))) {
+        BIO_free(in);
+        ossl_raise(ePKCS7Error, "PKCS7_dataInit");
     }
-    if(!PKCS7_dataFinal(pkcs7, out)) goto err;
-    ossl_pkcs7_set_data(self, Qnil);
-
- err:
+    for (;;) {
+        if ((len = BIO_read(in, buf, sizeof(buf))) <= 0)
+            break;
+        if (BIO_write(out, buf, len) != len) {
+            BIO_free_all(out);
+            BIO_free(in);
+            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);
-    if(ERR_peek_error()){
-	ossl_raise(ePKCS7Error, NULL);
-    }
+    if (!ret)
+        ossl_raise(ePKCS7Error, "PKCS7_dataFinal");
+    ossl_pkcs7_set_data(self, Qnil);
 
     return data;
 }

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"),
@@ -318,12 +340,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"