Merge pull request #848 from rhenium/ky/pkey-disallow-empty-init

pkey: disallow {DH,DSA,EC,RSA}.new without arguments on OpenSSL 3.0

Author: rhenium

ext/openssl/ossl.h

@@ -74,6 +74,10 @@
 # include <openssl/provider.h>
 #endif
 
+#if OSSL_OPENSSL_PREREQ(3, 0, 0)
+# define OSSL_HAVE_IMMUTABLE_PKEY
+#endif
+
 /*
  * Common Module
  */

ext/openssl/ossl_pkey.c

@@ -508,7 +508,7 @@ ossl_pkey_s_generate_key(int argc, VALUE *argv, VALUE self)
 void
 ossl_pkey_check_public_key(const EVP_PKEY *pkey)
 {
-#if OSSL_OPENSSL_PREREQ(3, 0, 0)
+#ifdef OSSL_HAVE_IMMUTABLE_PKEY
     if (EVP_PKEY_missing_parameters(pkey))
         ossl_raise(ePKeyError, "parameters missing");
 #else

ext/openssl/ossl_pkey.h

@@ -105,7 +105,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)
+#ifndef OSSL_HAVE_IMMUTABLE_PKEY
 #define OSSL_PKEY_BN_DEF_SETTER3(_keytype, _type, _group, a1, a2, a3)	\
 /*									\
  *  call-seq:								\

ext/openssl/ossl_pkey_dh.c

@@ -43,6 +43,7 @@ static VALUE eDHError;
  * If called without arguments, an empty instance without any parameter or key
  * components is created. Use #set_pqg to manually set the parameters afterwards
  * (and optionally #set_key to set private and public key components).
+ * This form is not compatible with OpenSSL 3.0 or later.
  *
  * If a String is given, tries to parse it as a DER- or PEM- encoded parameters.
  * See also OpenSSL::PKey.read which can parse keys of any kinds.
@@ -58,14 +59,15 @@ static VALUE eDHError;
  *
  * Examples:
  *   # Creating an instance from scratch
- *   # Note that this is deprecated and will not work on OpenSSL 3.0 or later.
+ *   # Note that this is deprecated and will result in ArgumentError when
+ *   # using OpenSSL 3.0 or later.
  *   dh = OpenSSL::PKey::DH.new
  *   dh.set_pqg(bn_p, nil, bn_g)
  *
  *   # Generating a parameters and a key pair
  *   dh = OpenSSL::PKey::DH.new(2048) # An alias of OpenSSL::PKey::DH.generate(2048)
  *
- *   # Reading DH parameters
+ *   # Reading DH parameters from a PEM-encoded string
  *   dh_params = OpenSSL::PKey::DH.new(File.read('parameters.pem')) # loads parameters only
  *   dh = OpenSSL::PKey.generate_key(dh_params) # generates a key pair
  */
@@ -84,10 +86,15 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self)
 
     /* The DH.new(size, generator) form is handled by lib/openssl/pkey.rb */
     if (rb_scan_args(argc, argv, "01", &arg) == 0) {
+#ifdef OSSL_HAVE_IMMUTABLE_PKEY
+        rb_raise(rb_eArgError, "OpenSSL::PKey::DH.new cannot be called " \
+                 "without arguments; pkeys are immutable with OpenSSL 3.0");
+#else
         dh = DH_new();
         if (!dh)
             ossl_raise(eDHError, "DH_new");
         goto legacy;
+#endif
     }
 
     arg = ossl_to_der_if_possible(arg);

ext/openssl/ossl_pkey_dsa.c

@@ -56,6 +56,7 @@ static VALUE eDSAError;
  *
  * If called without arguments, creates a new instance with no key components
  * set. They can be set individually by #set_pqg and #set_key.
+ * This form is not compatible with OpenSSL 3.0 or later.
  *
  * If called with a String, tries to parse as DER or PEM encoding of a \DSA key.
  * See also OpenSSL::PKey.read which can parse keys of any kinds.
@@ -96,10 +97,15 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self)
     /* The DSA.new(size, generator) form is handled by lib/openssl/pkey.rb */
     rb_scan_args(argc, argv, "02", &arg, &pass);
     if (argc == 0) {
+#ifdef OSSL_HAVE_IMMUTABLE_PKEY
+        rb_raise(rb_eArgError, "OpenSSL::PKey::DSA.new cannot be called " \
+                 "without arguments; pkeys are immutable with OpenSSL 3.0");
+#else
         dsa = DSA_new();
         if (!dsa)
             ossl_raise(eDSAError, "DSA_new");
         goto legacy;
+#endif
     }
 
     pass = ossl_pem_passwd_value(pass);

ext/openssl/ossl_pkey_ec.c

@@ -147,9 +147,14 @@ static VALUE ossl_ec_key_initialize(int argc, VALUE *argv, VALUE self)
 
     rb_scan_args(argc, argv, "02", &arg, &pass);
     if (NIL_P(arg)) {
+#ifdef OSSL_HAVE_IMMUTABLE_PKEY
+        rb_raise(rb_eArgError, "OpenSSL::PKey::EC.new cannot be called " \
+                 "without arguments; pkeys are immutable with OpenSSL 3.0");
+#else
         if (!(ec = EC_KEY_new()))
             ossl_raise(eECError, "EC_KEY_new");
         goto legacy;
+#endif
     }
     else if (rb_obj_is_kind_of(arg, cEC_GROUP)) {
         ec = ec_key_new_from_group(arg);
@@ -246,7 +251,7 @@ 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)
+#ifdef OSSL_HAVE_IMMUTABLE_PKEY
     rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0");
 #else
     EC_KEY *ec;
@@ -288,7 +293,7 @@ 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)
+#ifdef OSSL_HAVE_IMMUTABLE_PKEY
     rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0");
 #else
     EC_KEY *ec;
@@ -339,7 +344,7 @@ 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)
+#ifdef OSSL_HAVE_IMMUTABLE_PKEY
     rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0");
 #else
     EC_KEY *ec;
@@ -511,7 +516,7 @@ ossl_ec_key_to_der(VALUE self)
  */
 static VALUE ossl_ec_key_generate_key(VALUE self)
 {
-#if OSSL_OPENSSL_PREREQ(3, 0, 0)
+#ifdef OSSL_HAVE_IMMUTABLE_PKEY
     rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0");
 #else
     EC_KEY *ec;
@@ -1368,7 +1373,7 @@ static VALUE ossl_ec_point_make_affine(VALUE self)
     GetECPointGroup(self, group);
 
     rb_warn("OpenSSL::PKey::EC::Point#make_affine! is deprecated");
-#if !OSSL_OPENSSL_PREREQ(3, 0, 0) && !defined(OPENSSL_IS_AWSLC)
+#if !defined(OSSL_HAVE_IMMUTABLE_PKEY) && !defined(OPENSSL_IS_AWSLC)
     if (EC_POINT_make_affine(group, point, ossl_bn_ctx) != 1)
         ossl_raise(eEC_POINT, "EC_POINT_make_affine");
 #endif

ext/openssl/ossl_pkey_rsa.c

@@ -59,6 +59,7 @@ static VALUE eRSAError;
  * If called without arguments, creates a new instance with no key components
  * set. They can be set individually by #set_key, #set_factors, and
  * #set_crt_params.
+ * This form is not compatible with OpenSSL 3.0 or later.
  *
  * If called with a String, tries to parse as DER or PEM encoding of an \RSA key.
  * Note that if _password_ is not specified, but the key is encrypted with a
@@ -89,10 +90,15 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self)
     /* The RSA.new(size, generator) form is handled by lib/openssl/pkey.rb */
     rb_scan_args(argc, argv, "02", &arg, &pass);
     if (argc == 0) {
+#ifdef OSSL_HAVE_IMMUTABLE_PKEY
+        rb_raise(rb_eArgError, "OpenSSL::PKey::RSA.new cannot be called " \
+                 "without arguments; pkeys are immutable with OpenSSL 3.0");
+#else
 	rsa = RSA_new();
         if (!rsa)
             ossl_raise(eRSAError, "RSA_new");
         goto legacy;
+#endif
     }
 
     pass = ossl_pem_passwd_value(pass);

test/openssl/test_pkey_dh.rb

@@ -7,9 +7,14 @@ class OpenSSL::TestPKeyDH < OpenSSL::PKeyTestCase
   NEW_KEYLEN = 2048
 
   def test_new_empty
-    dh = OpenSSL::PKey::DH.new
-    assert_equal nil, dh.p
-    assert_equal nil, dh.priv_key
+    # pkeys are immutable with OpenSSL >= 3.0
+    if openssl?(3, 0, 0)
+      assert_raise(ArgumentError) { OpenSSL::PKey::DH.new }
+    else
+      dh = OpenSSL::PKey::DH.new
+      assert_nil(dh.p)
+      assert_nil(dh.priv_key)
+    end
   end
 
   def test_new_generate

test/openssl/test_pkey_dsa.rb

@@ -34,9 +34,14 @@ def test_new_break
   end
 
   def test_new_empty
-    key = OpenSSL::PKey::DSA.new
-    assert_nil(key.p)
-    assert_raise(OpenSSL::PKey::PKeyError) { key.to_der }
+    # pkeys are immutable with OpenSSL >= 3.0
+    if openssl?(3, 0, 0)
+      assert_raise(ArgumentError) { OpenSSL::PKey::DSA.new }
+    else
+      key = OpenSSL::PKey::DSA.new
+      assert_nil(key.p)
+      assert_raise(OpenSSL::PKey::PKeyError) { key.to_der }
+    end
   end
 
   def test_generate

test/openssl/test_pkey_ec.rb

@@ -4,19 +4,9 @@
 if defined?(OpenSSL)
 
 class OpenSSL::TestEC < OpenSSL::PKeyTestCase
-  def test_ec_key
+  def test_ec_key_new
     key1 = OpenSSL::PKey::EC.generate("prime256v1")
 
-    # 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,6 +25,23 @@ def test_ec_key
     end
   end
 
+  def test_ec_key_new_empty
+    # pkeys are immutable with OpenSSL >= 3.0; constructing an empty EC object is
+    # disallowed
+    if openssl?(3, 0, 0)
+      assert_raise(ArgumentError) { OpenSSL::PKey::EC.new }
+    else
+      key = OpenSSL::PKey::EC.new
+      assert_nil(key.group)
+
+      p256 = Fixtures.pkey("p256")
+      key.group = p256.group
+      key.private_key = p256.private_key
+      key.public_key = p256.public_key
+      assert_equal(p256.to_der, key.to_der)
+    end
+  end
+
   def test_builtin_curves
     builtin_curves = OpenSSL::PKey::EC.builtin_curves
     assert_not_empty builtin_curves