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.
ruby · Dec 16, 2021 · 0d84f10 · 0d84f10
1 parent 22adc68
commit 0d84f10
Show file tree

Hide file tree

Showing 6 changed files with 161 additions and 74 deletions.
diff --git a/ext/openssl/ossl_pkey.h b/ext/openssl/ossl_pkey.h
@@ -124,6 +124,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:								\
@@ -136,6 +137,7 @@ static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2, VALU
 	BIGNUM *bn2 = NULL, *orig_bn2 = NIL_P(v2) ? NULL : GetBNPtr(v2);\
 	BIGNUM *bn3 = NULL, *orig_bn3 = NIL_P(v3) ? NULL : GetBNPtr(v3);\
 									\
+        rb_warning(#_keytype"#set_"#_group"= is incompatible with OpenSSL 3.0"); \
 	Get##_type(self, obj);						\
         if ((orig_bn1 && !(bn1 = BN_dup(orig_bn1))) ||			\
             (orig_bn2 && !(bn2 = BN_dup(orig_bn2))) ||			\
@@ -166,6 +168,7 @@ static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2) \
 	BIGNUM *bn1 = NULL, *orig_bn1 = NIL_P(v1) ? NULL : GetBNPtr(v1);\
 	BIGNUM *bn2 = NULL, *orig_bn2 = NIL_P(v2) ? NULL : GetBNPtr(v2);\
 									\
+        rb_warning(#_keytype"#set_"#_group"= is incompatible with OpenSSL 3.0"); \
 	Get##_type(self, obj);						\
         if ((orig_bn1 && !(bn1 = BN_dup(orig_bn1))) ||			\
             (orig_bn2 && !(bn2 = BN_dup(orig_bn2)))) {			\
@@ -181,6 +184,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
@@ -237,6 +237,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;
 
@@ -247,6 +250,7 @@ ossl_ec_key_set_group(VALUE self, VALUE group_v)
         ossl_raise(eECError, "EC_KEY_set_group");
 
     return group_v;
+#endif
 }
 
 /*
@@ -275,6 +279,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;
 
@@ -294,6 +301,7 @@ static VALUE ossl_ec_key_set_private_key(VALUE self, VALUE private_key)
     }
 
     return private_key;
+#endif
 }
 
 /*
@@ -322,6 +330,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;
 
@@ -341,6 +352,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
@@ -108,13 +108,32 @@ def test_params_ok?
   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
+    EnvUtil.suppress_warning do
+      dh2.set_pqg(dh2.p + 1, nil, dh2.g)
+      assert_not_equal dh2.p, dh1.p
+    end if !openssl?(3, 0, 0)
+
+    # 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
@@ -126,11 +145,6 @@ def test_marshal
 
   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
@@ -208,8 +208,12 @@ def test_dup
     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
+    EnvUtil.suppress_warning do
+      key2.set_pqg(key2.p + 1, key2.q, key2.g)
+      assert_not_equal key.params, key2.params
+    end if !openssl?(3, 0, 0)
   end
 
   def test_marshal

diff --git a/test/openssl/test_pkey_ec.rb b/test/openssl/test_pkey_ec.rb
@@ -21,11 +21,15 @@ def test_ec_key
 
     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
+    EnvUtil.suppress_warning do
+      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 if !openssl?(3, 0, 0)
 
     key3 = OpenSSL::PKey::EC.new(key1)
     assert_equal key1.to_der, key3.to_der
@@ -35,10 +39,14 @@ def test_ec_key
 
     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
+    EnvUtil.suppress_warning do
+      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 if !openssl?(3, 0, 0)
   end
 
   def test_generate
@@ -65,22 +73,28 @@ def test_marshal
   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)
+      EnvUtil.suppress_warning do
+        key2.private_key += 1
+        assert_raise(OpenSSL::PKey::ECError) { key2.check_key }
+      end
+    end
   end
 
   def test_sign_verify
@@ -112,7 +126,7 @@ def test_derive_key
     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
@@ -5,12 +5,14 @@
 
 class OpenSSL::TestPKeyRSA < OpenSSL::PKeyTestCase
   def test_no_private_exp
-    key = OpenSSL::PKey::RSA.new
-    rsa = Fixtures.pkey("rsa2048")
-    key.set_key(rsa.n, rsa.e, nil)
-    key.set_factors(rsa.p, rsa.q)
-    assert_raise(OpenSSL::PKey::RSAError){ key.private_encrypt("foo") }
-    assert_raise(OpenSSL::PKey::RSAError){ key.private_decrypt("foo") }
+    EnvUtil.suppress_warning do
+      key = OpenSSL::PKey::RSA.new
+      rsa = Fixtures.pkey("rsa2048")
+      key.set_key(rsa.n, rsa.e, nil)
+      key.set_factors(rsa.p, rsa.q)
+      assert_raise(OpenSSL::PKey::RSAError){ key.private_encrypt("foo") }
+      assert_raise(OpenSSL::PKey::RSAError){ key.private_decrypt("foo") }
+    end
   end if !openssl?(3, 0, 0) # Impossible state in OpenSSL 3.0
 
   def test_private
@@ -31,15 +33,18 @@ def test_private
     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?)
+    EnvUtil.suppress_warning do
+      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 if !openssl?(3, 0, 0) # PKey is immutable in OpenSSL >= 3.0
   end
 
   def test_new
@@ -235,36 +240,52 @@ def test_encrypt_decrypt_legacy
 
   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
+    EnvUtil.suppress_warning do
+      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
 
-    # 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, 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
+      # 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 if !openssl?(3, 0, 0)
   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
+    EnvUtil.suppress_warning do
+      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 if !openssl?(3, 0, 0)
   end
 
   def test_RSAPrivateKey
@@ -495,8 +516,12 @@ def test_dup
     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
+    EnvUtil.suppress_warning do
+      key2.set_key(key2.n, 3, key2.d)
+      assert_not_equal key.params, key2.params
+    end if !openssl?(3, 0, 0)
   end
 
   def test_marshal