Review changes

ruby · Sep 7, 2024 · b05603b · b05603b
1 parent c4e1946
commit b05603b
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 34 deletions.
diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c
@@ -478,7 +478,8 @@ struct pkey_from_data_arg {
 };
 
 static int
-add_data_to_builder(VALUE key, VALUE value, VALUE arg) {
+add_data_to_builder(VALUE key, VALUE value, VALUE arg)
+{
     if(NIL_P(value))
         return ST_CONTINUE;
 
@@ -521,6 +522,9 @@ add_data_to_builder(VALUE key, VALUE value, VALUE arg) {
             case OSSL_PARAM_OCTET_PTR:
                 ossl_raise(ePKeyError, "Unsupported parameter \"%s\", handling of OSSL_PARAM_UTF8_PTR and OSSL_PARAM_OCTET_PTR not implemented", key_ptr);
                 break;
+            default:
+                ossl_raise(ePKeyError, "Unsupported parameter \"%s\"", key_ptr);
+                break;
             }
 
             return ST_CONTINUE;
@@ -612,7 +616,6 @@ pkey_from_data(int argc, VALUE *argv, VALUE self)
 
     if (EVP_PKEY_fromdata(ctx, &pkey, EVP_PKEY_KEYPAIR, params) <= 0) {
         EVP_PKEY_CTX_free(ctx);
-        EVP_PKEY_free(pkey);
         ossl_raise(ePKeyError, "EVP_PKEY_fromdata");
     }
 
@@ -690,18 +693,15 @@ ossl_pkey_s_generate_key(int argc, VALUE *argv, VALUE self)
  * == Example
  *   pkey = OpenSSL::PKey.from_data("RSA", n: 3161751493, e: 65537, d: 2064855961)
  *   pkey.private? #=> true
- *   pkey.public_key #=> #<OpenSSL::PKey::RSA...
+ *   pkey.n #=> #<OpenSSL::BN 3161751493>
  */
+#if OSSL_OPENSSL_PREREQ(3, 0, 0)
 static VALUE
 ossl_pkey_s_from_data(int argc, VALUE *argv, VALUE self)
 {
-#if OSSL_OPENSSL_PREREQ(3, 0, 0)
-    return pkey_from_data(argc, argv, self);
-#else
-    rb_raise(ePKeyError, "OpenSSL::PKey.from_data requires OpenSSL 3.0 or later");
-#endif
+  return pkey_from_data(argc, argv, self);
 }
-
+#endif
 /*
  * TODO: There is no convenient way to check the presence of public key
  * components on OpenSSL 3.0. But since keys are immutable on 3.0, pkeys without
@@ -1955,8 +1955,9 @@ Init_ossl_pkey(void)
     rb_define_module_function(mPKey, "read", ossl_pkey_new_from_data, -1);
     rb_define_module_function(mPKey, "generate_parameters", ossl_pkey_s_generate_parameters, -1);
     rb_define_module_function(mPKey, "generate_key", ossl_pkey_s_generate_key, -1);
+#if OSSL_OPENSSL_PREREQ(3, 0, 0)
     rb_define_module_function(mPKey, "from_data", ossl_pkey_s_from_data, -1);
-
+#endif
 #ifdef HAVE_EVP_PKEY_NEW_RAW_PRIVATE_KEY
     rb_define_module_function(mPKey, "new_raw_private_key", ossl_pkey_new_raw_private_key, 2);
     rb_define_module_function(mPKey, "new_raw_public_key", ossl_pkey_new_raw_public_key, 2);

diff --git a/test/openssl/fixtures/pkey/ec-prime256v1.pem b/test/openssl/fixtures/pkey/ec-prime256v1.pem
diff --git a/test/openssl/fixtures/pkey/ec-secp384r1.pem b/test/openssl/fixtures/pkey/ec-secp384r1.pem
diff --git a/test/openssl/fixtures/pkey/ec-secp521r1.pem b/test/openssl/fixtures/pkey/ec-secp521r1.pem
diff --git a/test/openssl/test_pkey.rb b/test/openssl/test_pkey.rb
@@ -318,7 +318,7 @@ def test_s_from_data_rsa_with_invalid_parameter
     end
 
     def test_s_from_data_ec_pub_given_as_string
-      source  = Fixtures.pkey("ec-prime256v1")
+      source  = OpenSSL::PKey::EC.generate("prime256v1")
       new_key = OpenSSL::PKey.from_data("EC", "group" => source.group.curve_name,
                                               "pub" => source.public_key.to_bn.to_s(2))
       assert_instance_of OpenSSL::PKey::EC, new_key
@@ -328,7 +328,7 @@ def test_s_from_data_ec_pub_given_as_string
     end
 
     def test_s_from_data_ec_priv_given_as_bn
-      source  = Fixtures.pkey("ec-prime256v1")
+      source  = OpenSSL::PKey::EC.generate("prime256v1")
       new_key = OpenSSL::PKey.from_data("EC", "group" => source.group.curve_name,
                                               "priv" => source.private_key.to_bn)
       assert_instance_of OpenSSL::PKey::EC, new_key
@@ -338,7 +338,7 @@ def test_s_from_data_ec_priv_given_as_bn
     end
 
     def test_s_from_data_ec_priv_given_as_integer
-      source  = Fixtures.pkey("ec-prime256v1")
+      source  = OpenSSL::PKey::EC.generate("prime256v1")
       new_key = OpenSSL::PKey.from_data("EC", "group" => source.group.curve_name,
                                               "priv" => source.private_key.to_i)
       assert_instance_of OpenSSL::PKey::EC, new_key
@@ -348,9 +348,9 @@ def test_s_from_data_ec_priv_given_as_integer
     end
 
     def test_s_from_data_ec_priv_and_pub_given_for_different_curves
-      [Fixtures.pkey("ec-prime256v1"),
-       Fixtures.pkey("ec-secp384r1"),
-       Fixtures.pkey("ec-secp521r1")].each do |source|
+      [OpenSSL::PKey::EC.generate("prime256v1"),
+       OpenSSL::PKey::EC.generate("secp384r1"),
+       OpenSSL::PKey::EC.generate("secp521r1")].each do |source|
         new_key = OpenSSL::PKey.from_data("EC", "group" => source.group.curve_name,
                                                 "pub" => source.public_key.to_bn.to_s(2),
                                                 "priv" => source.private_key.to_i)