summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKazuki Yamaguchi <k@rhe.jp>2021-03-24 13:23:09 +0900
committerKazuki Yamaguchi <k@rhe.jp>2021-03-31 18:05:07 +0900
commite2bf3659e184088d00d099a49e3263724f43ece2 (patch)
treea6ccdd73438d26b680734f37611702bbbab3e4fb
parent66d2fc7989d741bf5a73286233139901cecb4fc2 (diff)
[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
-rw-r--r--ext/openssl/ossl_pkcs7.c81
-rw-r--r--test/openssl/test_pkcs7.rb18
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 = {
* (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)
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)
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
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
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
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)