ruby-changes:65717
From: Kazuki <ko1@a...>
Date: Wed, 31 Mar 2021 18:07:22 +0900 (JST)
Subject: [ruby-changes:65717] e2bf3659e1 (master): [ruby/openssl] pkcs7: keep private key when duplicating PKCS7_SIGNER_INFO
https://git.ruby-lang.org/ruby.git/commit/?id=e2bf3659e1 From e2bf3659e184088d00d099a49e3263724f43ece2 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi <k@r...> Date: Wed, 24 Mar 2021 13:23:09 +0900 Subject: [ruby/openssl] pkcs7: keep private key when duplicating PKCS7_SIGNER_INFO ASN1_dup() will not copy the 'pkey' field of a PKCS7_SIGNER_INFO object by design; it is a temporary field kept until the PKCS7 structure is finalized. Let's bump reference counter of the pkey in the original object and use it in the new object, too. This commit also removes PKCS7#add_signer's routine to add the content-type attribute as a signed attribute automatically. This behavior was not documented or tested. This change should not break any working user code since the method was completely useless without the change above. https://github.com/ruby/openssl/commit/20ca7a27a8 --- ext/openssl/ossl_pkcs7.c | 81 +++++++++++++++++++--------------------------- test/openssl/test_pkcs7.rb | 18 +++++++++++ 2 files changed, 51 insertions(+), 48 deletions(-) diff --git a/ext/openssl/ossl_pkcs7.c b/ext/openssl/ossl_pkcs7.c index ea8e92d..0bcc76a 100644 --- a/ext/openssl/ossl_pkcs7.c +++ b/ext/openssl/ossl_pkcs7.c @@ -101,19 +101,24 @@ static const rb_data_type_t ossl_pkcs7_recip_info_type = { https://github.com/ruby/ruby/blob/trunk/ext/openssl/ossl_pkcs7.c#L101 * (MADE PRIVATE UNTIL SOMEBODY WILL NEED THEM) */ static PKCS7_SIGNER_INFO * -ossl_PKCS7_SIGNER_INFO_dup(const PKCS7_SIGNER_INFO *si) +ossl_PKCS7_SIGNER_INFO_dup(PKCS7_SIGNER_INFO *si) { - return (PKCS7_SIGNER_INFO *)ASN1_dup((i2d_of_void *)i2d_PKCS7_SIGNER_INFO, - (d2i_of_void *)d2i_PKCS7_SIGNER_INFO, - (char *)si); + PKCS7_SIGNER_INFO *si_new = ASN1_dup((i2d_of_void *)i2d_PKCS7_SIGNER_INFO, + (d2i_of_void *)d2i_PKCS7_SIGNER_INFO, + si); + if (si_new && si->pkey) { + EVP_PKEY_up_ref(si->pkey); + si_new->pkey = si->pkey; + } + return si_new; } static PKCS7_RECIP_INFO * -ossl_PKCS7_RECIP_INFO_dup(const PKCS7_RECIP_INFO *si) +ossl_PKCS7_RECIP_INFO_dup(PKCS7_RECIP_INFO *si) { - return (PKCS7_RECIP_INFO *)ASN1_dup((i2d_of_void *)i2d_PKCS7_RECIP_INFO, - (d2i_of_void *)d2i_PKCS7_RECIP_INFO, - (char *)si); + return ASN1_dup((i2d_of_void *)i2d_PKCS7_RECIP_INFO, + (d2i_of_void *)d2i_PKCS7_RECIP_INFO, + si); } static VALUE @@ -130,19 +135,6 @@ ossl_pkcs7si_new(PKCS7_SIGNER_INFO *p7si) https://github.com/ruby/ruby/blob/trunk/ext/openssl/ossl_pkcs7.c#L135 return obj; } -static PKCS7_SIGNER_INFO * -DupPKCS7SignerPtr(VALUE obj) -{ - PKCS7_SIGNER_INFO *p7si, *pkcs7; - - GetPKCS7si(obj, p7si); - if (!(pkcs7 = ossl_PKCS7_SIGNER_INFO_dup(p7si))) { - ossl_raise(ePKCS7Error, NULL); - } - - return pkcs7; -} - static VALUE ossl_pkcs7ri_new(PKCS7_RECIP_INFO *p7ri) { @@ -157,19 +149,6 @@ ossl_pkcs7ri_new(PKCS7_RECIP_INFO *p7ri) https://github.com/ruby/ruby/blob/trunk/ext/openssl/ossl_pkcs7.c#L149 return obj; } -static PKCS7_RECIP_INFO * -DupPKCS7RecipientPtr(VALUE obj) -{ - PKCS7_RECIP_INFO *p7ri, *pkcs7; - - GetPKCS7ri(obj, p7ri); - if (!(pkcs7 = ossl_PKCS7_RECIP_INFO_dup(p7ri))) { - ossl_raise(ePKCS7Error, NULL); - } - - return pkcs7; -} - /* * call-seq: * PKCS7.read_smime(string) => pkcs7 @@ -521,17 +500,18 @@ static VALUE https://github.com/ruby/ruby/blob/trunk/ext/openssl/ossl_pkcs7.c#L500 ossl_pkcs7_add_signer(VALUE self, VALUE signer) { PKCS7 *pkcs7; - PKCS7_SIGNER_INFO *p7si; + PKCS7_SIGNER_INFO *si, *si_new; - p7si = DupPKCS7SignerPtr(signer); /* NEED TO DUP */ GetPKCS7(self, pkcs7); - if (!PKCS7_add_signer(pkcs7, p7si)) { - PKCS7_SIGNER_INFO_free(p7si); - ossl_raise(ePKCS7Error, "Could not add signer."); - } - if (PKCS7_type_is_signed(pkcs7)){ - PKCS7_add_signed_attribute(p7si, NID_pkcs9_contentType, - V_ASN1_OBJECT, OBJ_nid2obj(NID_pkcs7_data)); + GetPKCS7si(signer, si); + + si_new = ossl_PKCS7_SIGNER_INFO_dup(si); + if (!si_new) + ossl_raise(ePKCS7Error, "PKCS7_SIGNER_INFO_dup"); + + if (PKCS7_add_signer(pkcs7, si_new) != 1) { + PKCS7_SIGNER_INFO_free(si_new); + ossl_raise(ePKCS7Error, "PKCS7_add_signer"); } return self; @@ -567,13 +547,18 @@ static VALUE https://github.com/ruby/ruby/blob/trunk/ext/openssl/ossl_pkcs7.c#L547 ossl_pkcs7_add_recipient(VALUE self, VALUE recip) { PKCS7 *pkcs7; - PKCS7_RECIP_INFO *ri; + PKCS7_RECIP_INFO *ri, *ri_new; - ri = DupPKCS7RecipientPtr(recip); /* NEED TO DUP */ GetPKCS7(self, pkcs7); - if (!PKCS7_add_recipient_info(pkcs7, ri)) { - PKCS7_RECIP_INFO_free(ri); - ossl_raise(ePKCS7Error, "Could not add recipient."); + GetPKCS7ri(recip, ri); + + ri_new = ossl_PKCS7_RECIP_INFO_dup(ri); + if (!ri_new) + ossl_raise(ePKCS7Error, "PKCS7_RECIP_INFO_dup"); + + if (PKCS7_add_recipient_info(pkcs7, ri_new) != 1) { + PKCS7_RECIP_INFO_free(ri_new); + ossl_raise(ePKCS7Error, "PKCS7_add_recipient_info"); } return self; diff --git a/test/openssl/test_pkcs7.rb b/test/openssl/test_pkcs7.rb index d0d9dca..ba8b93d 100644 --- a/test/openssl/test_pkcs7.rb +++ b/test/openssl/test_pkcs7.rb @@ -89,6 +89,24 @@ class OpenSSL::TestPKCS7 < OpenSSL::TestCase https://github.com/ruby/ruby/blob/trunk/test/openssl/test_pkcs7.rb#L89 assert_equal(@ee2_cert.issuer.to_s, signers[1].issuer.to_s) end + def test_signed_add_signer + data = "aaaaa\nbbbbb\nccccc\n" + psi = OpenSSL::PKCS7::SignerInfo.new(@ee1_cert, @rsa1024, "sha256") + p7 = OpenSSL::PKCS7.new + p7.type = :signed + p7.add_signer(psi) + p7.add_certificate(@ee1_cert) + p7.add_certificate(@ca_cert) + p7.add_data(data) + + store = OpenSSL::X509::Store.new + store.add_cert(@ca_cert) + + assert_equal(true, p7.verify([], store)) + assert_equal(true, OpenSSL::PKCS7.new(p7.to_der).verify([], store)) + assert_equal(1, p7.signers.size) + end + def test_detached_sign store = OpenSSL::X509::Store.new store.add_cert(@ca_cert) -- cgit v1.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/