From 8ebf5978852e22358cbcdf74c0eb506f22e2c73f Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Tue, 21 Sep 2021 18:29:59 +0900 Subject: [ruby/openssl] pkey: deprecate PKey#set_* methods OpenSSL 3.0 made EVP_PKEY immutable. This means we can only have a const pointer of the low level struct and the following methods can no longer be provided when linked against OpenSSL 3.0: - OpenSSL::PKey::RSA#set_key - OpenSSL::PKey::RSA#set_factors - OpenSSL::PKey::RSA#set_crt_params - OpenSSL::PKey::DSA#set_pqg - OpenSSL::PKey::DSA#set_key - OpenSSL::PKey::DH#set_pqg - OpenSSL::PKey::DH#set_key - OpenSSL::PKey::EC#group= - OpenSSL::PKey::EC#private_key= - OpenSSL::PKey::EC#public_key= There is no direct replacement for this functionality at the moment. I plan to introduce a wrapper around EVP_PKEY_fromdata(), which takes all key components at once to construct an EVP_PKEY. https://github.com/ruby/openssl/commit/6848d2d969 --- ext/openssl/ossl_pkey.h | 16 ++++++++ ext/openssl/ossl_pkey_ec.c | 12 ++++++ test/openssl/test_pkey_dh.rb | 38 +++++++++++++------ test/openssl/test_pkey_dsa.rb | 8 +++- test/openssl/test_pkey_ec.rb | 58 +++++++++++++++++------------ test/openssl/test_pkey_rsa.rb | 85 +++++++++++++++++++++++++++---------------- 6 files changed, 149 insertions(+), 68 deletions(-) diff --git a/ext/openssl/ossl_pkey.h b/ext/openssl/ossl_pkey.h index ed18bc6927..38fb9fad10 100644 --- a/ext/openssl/ossl_pkey.h +++ b/ext/openssl/ossl_pkey.h @@ -116,6 +116,7 @@ static VALUE ossl_##_keytype##_get_##_name(VALUE self) \ OSSL_PKEY_BN_DEF_GETTER0(_keytype, _type, a2, \ _type##_get0_##_group(obj, NULL, &bn)) +#if !OSSL_OPENSSL_PREREQ(3, 0, 0) #define OSSL_PKEY_BN_DEF_SETTER3(_keytype, _type, _group, a1, a2, a3) \ /* \ * call-seq: \ @@ -173,6 +174,21 @@ static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2) \ } \ return self; \ } +#else +#define OSSL_PKEY_BN_DEF_SETTER3(_keytype, _type, _group, a1, a2, a3) \ +static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2, VALUE v3) \ +{ \ + rb_raise(ePKeyError, \ + #_keytype"#set_"#_group"= is incompatible with OpenSSL 3.0"); \ +} + +#define OSSL_PKEY_BN_DEF_SETTER2(_keytype, _type, _group, a1, a2) \ +static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2) \ +{ \ + rb_raise(ePKeyError, \ + #_keytype"#set_"#_group"= is incompatible with OpenSSL 3.0"); \ +} +#endif #define OSSL_PKEY_BN_DEF3(_keytype, _type, _group, a1, a2, a3) \ OSSL_PKEY_BN_DEF_GETTER3(_keytype, _type, _group, a1, a2, a3) \ diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c index ff3150dac0..dee215447d 100644 --- a/ext/openssl/ossl_pkey_ec.c +++ b/ext/openssl/ossl_pkey_ec.c @@ -248,6 +248,9 @@ ossl_ec_key_get_group(VALUE self) static VALUE ossl_ec_key_set_group(VALUE self, VALUE group_v) { +#if OSSL_OPENSSL_PREREQ(3, 0, 0) + rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0"); +#else EC_KEY *ec; EC_GROUP *group; @@ -258,6 +261,7 @@ ossl_ec_key_set_group(VALUE self, VALUE group_v) ossl_raise(eECError, "EC_KEY_set_group"); return group_v; +#endif } /* @@ -286,6 +290,9 @@ static VALUE ossl_ec_key_get_private_key(VALUE self) */ static VALUE ossl_ec_key_set_private_key(VALUE self, VALUE private_key) { +#if OSSL_OPENSSL_PREREQ(3, 0, 0) + rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0"); +#else EC_KEY *ec; BIGNUM *bn = NULL; @@ -305,6 +312,7 @@ static VALUE ossl_ec_key_set_private_key(VALUE self, VALUE private_key) } return private_key; +#endif } /* @@ -333,6 +341,9 @@ static VALUE ossl_ec_key_get_public_key(VALUE self) */ static VALUE ossl_ec_key_set_public_key(VALUE self, VALUE public_key) { +#if OSSL_OPENSSL_PREREQ(3, 0, 0) + rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0"); +#else EC_KEY *ec; EC_POINT *point = NULL; @@ -352,6 +363,7 @@ static VALUE ossl_ec_key_set_public_key(VALUE self, VALUE public_key) } return public_key; +#endif } /* diff --git a/test/openssl/test_pkey_dh.rb b/test/openssl/test_pkey_dh.rb index ac11af382a..161af1897b 100644 --- a/test/openssl/test_pkey_dh.rb +++ b/test/openssl/test_pkey_dh.rb @@ -107,13 +107,32 @@ class OpenSSL::TestPKeyDH < OpenSSL::PKeyTestCase end def test_dup - dh = Fixtures.pkey("dh1024") - dh2 = dh.dup - assert_equal dh.to_der, dh2.to_der # params - assert_equal_params dh, dh2 # keys - dh2.set_pqg(dh2.p + 1, nil, dh2.g) - assert_not_equal dh2.p, dh.p - assert_equal dh2.g, dh.g + # Parameters only + dh1 = Fixtures.pkey("dh1024") + dh2 = dh1.dup + assert_equal dh1.to_der, dh2.to_der + assert_not_equal nil, dh1.p + assert_not_equal nil, dh1.g + assert_equal [dh1.p, dh1.g], [dh2.p, dh2.g] + assert_equal nil, dh1.pub_key + assert_equal nil, dh1.priv_key + assert_equal [dh1.pub_key, dh1.priv_key], [dh2.pub_key, dh2.priv_key] + + # PKey is immutable in OpenSSL >= 3.0 + if !openssl?(3, 0, 0) + dh2.set_pqg(dh2.p + 1, nil, dh2.g) + assert_not_equal dh2.p, dh1.p + end + + # With a key pair + dh3 = OpenSSL::PKey.generate_key(Fixtures.pkey("dh1024")) + dh4 = dh3.dup + assert_equal dh3.to_der, dh4.to_der + assert_equal dh1.to_der, dh4.to_der # encodes parameters only + assert_equal [dh1.p, dh1.g], [dh4.p, dh4.g] + assert_not_equal nil, dh3.pub_key + assert_not_equal nil, dh3.priv_key + assert_equal [dh3.pub_key, dh3.priv_key], [dh4.pub_key, dh4.priv_key] end def test_marshal @@ -125,11 +144,6 @@ class OpenSSL::TestPKeyDH < OpenSSL::PKeyTestCase private - def assert_equal_params(dh1, dh2) - assert_equal(dh1.g, dh2.g) - assert_equal(dh1.p, dh2.p) - end - def assert_no_key(dh) assert_equal(false, dh.public?) assert_equal(false, dh.private?) diff --git a/test/openssl/test_pkey_dsa.rb b/test/openssl/test_pkey_dsa.rb index 0994607f21..726b7dbf7e 100644 --- a/test/openssl/test_pkey_dsa.rb +++ b/test/openssl/test_pkey_dsa.rb @@ -208,8 +208,12 @@ fWLOqqkzFeRrYMDzUpl36XktY6Yq8EJYlW9pCMmBVNy/dQ== key = Fixtures.pkey("dsa1024") key2 = key.dup assert_equal key.params, key2.params - key2.set_pqg(key2.p + 1, key2.q, key2.g) - assert_not_equal key.params, key2.params + + # PKey is immutable in OpenSSL >= 3.0 + if !openssl?(3, 0, 0) + key2.set_pqg(key2.p + 1, key2.q, key2.g) + assert_not_equal key.params, key2.params + end end def test_marshal diff --git a/test/openssl/test_pkey_ec.rb b/test/openssl/test_pkey_ec.rb index 33f78a4c77..ffe5a94e5b 100644 --- a/test/openssl/test_pkey_ec.rb +++ b/test/openssl/test_pkey_ec.rb @@ -21,11 +21,15 @@ class OpenSSL::TestEC < OpenSSL::PKeyTestCase key1 = OpenSSL::PKey::EC.generate("prime256v1") - key2 = OpenSSL::PKey::EC.new - key2.group = key1.group - key2.private_key = key1.private_key - key2.public_key = key1.public_key - assert_equal key1.to_der, key2.to_der + # PKey is immutable in OpenSSL >= 3.0; constructing an empty EC object is + # deprecated + if !openssl?(3, 0, 0) + key2 = OpenSSL::PKey::EC.new + key2.group = key1.group + key2.private_key = key1.private_key + key2.public_key = key1.public_key + assert_equal key1.to_der, key2.to_der + end key3 = OpenSSL::PKey::EC.new(key1) assert_equal key1.to_der, key3.to_der @@ -35,10 +39,14 @@ class OpenSSL::TestEC < OpenSSL::PKeyTestCase key5 = key1.dup assert_equal key1.to_der, key5.to_der - key_tmp = OpenSSL::PKey::EC.new("prime256v1").generate_key! - key5.private_key = key_tmp.private_key - key5.public_key = key_tmp.public_key - assert_not_equal key1.to_der, key5.to_der + + # PKey is immutable in OpenSSL >= 3.0; EC object should not be modified + if !openssl?(3, 0, 0) + key_tmp = OpenSSL::PKey::EC.generate("prime256v1") + key5.private_key = key_tmp.private_key + key5.public_key = key_tmp.public_key + assert_not_equal key1.to_der, key5.to_der + end end def test_generate @@ -65,22 +73,26 @@ class OpenSSL::TestEC < OpenSSL::PKeyTestCase end def test_check_key - key = OpenSSL::PKey::EC.new("prime256v1").generate_key! - assert_equal(true, key.check_key) - assert_equal(true, key.private?) - assert_equal(true, key.public?) - key2 = OpenSSL::PKey::EC.new(key.group) - assert_equal(false, key2.private?) - assert_equal(false, key2.public?) - key2.public_key = key.public_key - assert_equal(false, key2.private?) - assert_equal(true, key2.public?) - key2.private_key = key.private_key + key0 = Fixtures.pkey("p256") + assert_equal(true, key0.check_key) + assert_equal(true, key0.private?) + assert_equal(true, key0.public?) + + key1 = OpenSSL::PKey.read(key0.public_to_der) + assert_equal(true, key1.check_key) + assert_equal(false, key1.private?) + assert_equal(true, key1.public?) + + key2 = OpenSSL::PKey.read(key0.private_to_der) assert_equal(true, key2.private?) assert_equal(true, key2.public?) assert_equal(true, key2.check_key) - key2.private_key += 1 - assert_raise(OpenSSL::PKey::ECError) { key2.check_key } + + # EC#private_key= is deprecated in 3.0 and won't work on OpenSSL 3.0 + if !openssl?(3, 0, 0) + key2.private_key += 1 + assert_raise(OpenSSL::PKey::ECError) { key2.check_key } + end end def test_sign_verify @@ -112,7 +124,7 @@ class OpenSSL::TestEC < OpenSSL::PKeyTestCase assert_equal [zIUT].pack("H*"), a.derive(b) assert_equal a.derive(b), a.dh_compute_key(b.public_key) - end + end if !openssl?(3, 0, 0) # TODO: Test it without using #private_key= def test_sign_verify_raw key = Fixtures.pkey("p256") diff --git a/test/openssl/test_pkey_rsa.rb b/test/openssl/test_pkey_rsa.rb index 7510658df7..4bb39ed4a6 100644 --- a/test/openssl/test_pkey_rsa.rb +++ b/test/openssl/test_pkey_rsa.rb @@ -31,15 +31,18 @@ class OpenSSL::TestPKeyRSA < OpenSSL::PKeyTestCase assert(!key4.private?) rsa1024 = Fixtures.pkey("rsa1024") - # Generated by RSA#set_key - key5 = OpenSSL::PKey::RSA.new - key5.set_key(rsa1024.n, rsa1024.e, rsa1024.d) - assert(key5.private?) - - # Generated by RSA#set_key, without d - key6 = OpenSSL::PKey::RSA.new - key6.set_key(rsa1024.n, rsa1024.e, nil) - assert(!key6.private?) + if !openssl?(3, 0, 0) + key = OpenSSL::PKey::RSA.new + # Generated by RSA#set_key + key5 = OpenSSL::PKey::RSA.new + key5.set_key(rsa1024.n, rsa1024.e, rsa1024.d) + assert(key5.private?) + + # Generated by RSA#set_key, without d + key6 = OpenSSL::PKey::RSA.new + key6.set_key(rsa1024.n, rsa1024.e, nil) + assert(!key6.private?) + end end def test_new @@ -235,36 +238,52 @@ class OpenSSL::TestPKeyRSA < OpenSSL::PKeyTestCase def test_export rsa1024 = Fixtures.pkey("rsa1024") - key = OpenSSL::PKey::RSA.new - # key has only n, e and d - key.set_key(rsa1024.n, rsa1024.e, rsa1024.d) - assert_equal rsa1024.public_key.export, key.export + pub = OpenSSL::PKey.read(rsa1024.public_to_der) + assert_not_equal rsa1024.export, pub.export + assert_equal rsa1024.public_to_pem, pub.export + + # PKey is immutable in OpenSSL >= 3.0 + if !openssl?(3, 0, 0) + key = OpenSSL::PKey::RSA.new - # key has only n, e, d, p and q - key.set_factors(rsa1024.p, rsa1024.q) - assert_equal rsa1024.public_key.export, key.export + # key has only n, e and d + key.set_key(rsa1024.n, rsa1024.e, rsa1024.d) + assert_equal rsa1024.public_key.export, key.export - # key has n, e, d, p, q, dmp1, dmq1 and iqmp - key.set_crt_params(rsa1024.dmp1, rsa1024.dmq1, rsa1024.iqmp) - assert_equal rsa1024.export, key.export + # key has only n, e, d, p and q + key.set_factors(rsa1024.p, rsa1024.q) + assert_equal rsa1024.public_key.export, key.export + + # key has n, e, d, p, q, dmp1, dmq1 and iqmp + key.set_crt_params(rsa1024.dmp1, rsa1024.dmq1, rsa1024.iqmp) + assert_equal rsa1024.export, key.export + end end def test_to_der rsa1024 = Fixtures.pkey("rsa1024") - key = OpenSSL::PKey::RSA.new - # key has only n, e and d - key.set_key(rsa1024.n, rsa1024.e, rsa1024.d) - assert_equal rsa1024.public_key.to_der, key.to_der + pub = OpenSSL::PKey.read(rsa1024.public_to_der) + assert_not_equal rsa1024.to_der, pub.to_der + assert_equal rsa1024.public_to_der, pub.to_der - # key has only n, e, d, p and q - key.set_factors(rsa1024.p, rsa1024.q) - assert_equal rsa1024.public_key.to_der, key.to_der + # PKey is immutable in OpenSSL >= 3.0 + if !openssl?(3, 0, 0) + key = OpenSSL::PKey::RSA.new - # key has n, e, d, p, q, dmp1, dmq1 and iqmp - key.set_crt_params(rsa1024.dmp1, rsa1024.dmq1, rsa1024.iqmp) - assert_equal rsa1024.to_der, key.to_der + # key has only n, e and d + key.set_key(rsa1024.n, rsa1024.e, rsa1024.d) + assert_equal rsa1024.public_key.to_der, key.to_der + + # key has only n, e, d, p and q + key.set_factors(rsa1024.p, rsa1024.q) + assert_equal rsa1024.public_key.to_der, key.to_der + + # key has n, e, d, p, q, dmp1, dmq1 and iqmp + key.set_crt_params(rsa1024.dmp1, rsa1024.dmq1, rsa1024.iqmp) + assert_equal rsa1024.to_der, key.to_der + end end def test_RSAPrivateKey @@ -501,8 +520,12 @@ class OpenSSL::TestPKeyRSA < OpenSSL::PKeyTestCase key = Fixtures.pkey("rsa1024") key2 = key.dup assert_equal key.params, key2.params - key2.set_key(key2.n, 3, key2.d) - assert_not_equal key.params, key2.params + + # PKey is immutable in OpenSSL >= 3.0 + if !openssl?(3, 0, 0) + key2.set_key(key2.n, 3, key2.d) + assert_not_equal key.params, key2.params + end end def test_marshal -- cgit v1.2.3