pkey/dh: use high level EVP interface to generate parameters and keys

Implement PKey::DH.new(size, gen), PKey::DH.generate(size, gen), and
PKey::DH#generate_key! using OpenSSL::PKey.generate_parameters and
.generate_key instead of the low level DH functions.

Note that the EVP interface can enforce additional restrictions - for
example, DH key shorter than 2048 bits is no longer supported by
default in OpenSSL 3.0. The test code is updated as such.

Author: rhenium

ext/openssl/ossl_pkey_dh.c

@@ -32,147 +32,56 @@ VALUE eDHError;
 /*
  * Private
  */
-struct dh_blocking_gen_arg {
-    DH *dh;
-    int size;
-    int gen;
-    BN_GENCB *cb;
-    int result;
-};
-
-static void *
-dh_blocking_gen(void *arg)
-{
-    struct dh_blocking_gen_arg *gen = (struct dh_blocking_gen_arg *)arg;
-    gen->result = DH_generate_parameters_ex(gen->dh, gen->size, gen->gen, gen->cb);
-    return 0;
-}
-
-static DH *
-dh_generate(int size, int gen)
-{
-    struct ossl_generate_cb_arg cb_arg = { 0 };
-    struct dh_blocking_gen_arg gen_arg;
-    DH *dh = DH_new();
-    BN_GENCB *cb = BN_GENCB_new();
-
-    if (!dh || !cb) {
-	DH_free(dh);
-	BN_GENCB_free(cb);
-        ossl_raise(eDHError, "malloc failure");
-    }
-
-    if (rb_block_given_p())
-	cb_arg.yield = 1;
-    BN_GENCB_set(cb, ossl_generate_cb_2, &cb_arg);
-    gen_arg.dh = dh;
-    gen_arg.size = size;
-    gen_arg.gen = gen;
-    gen_arg.cb = cb;
-    if (cb_arg.yield == 1) {
-	/* we cannot release GVL when callback proc is supplied */
-	dh_blocking_gen(&gen_arg);
-    } else {
-	/* there's a chance to unblock */
-	rb_thread_call_without_gvl(dh_blocking_gen, &gen_arg, ossl_generate_cb_stop, &cb_arg);
-    }
-
-    BN_GENCB_free(cb);
-    if (!gen_arg.result) {
-	DH_free(dh);
-	if (cb_arg.state) {
-	    /* Clear OpenSSL error queue before re-raising. */
-	    ossl_clear_error();
-	    rb_jump_tag(cb_arg.state);
-	}
-        ossl_raise(eDHError, "DH_generate_parameters_ex");
-    }
-
-    if (!DH_generate_key(dh)) {
-        DH_free(dh);
-        ossl_raise(eDHError, "DH_generate_key");
-    }
-
-    return dh;
-}
-
-/*
- *  call-seq:
- *     DH.generate(size [, generator]) -> dh
- *
- * Creates a new DH instance from scratch by generating the private and public
- * components alike.
- *
- * === Parameters
- * * _size_ is an integer representing the desired key size. Keys smaller than 1024 bits should be considered insecure.
- * * _generator_ is a small number > 1, typically 2 or 5.
- *
- */
-static VALUE
-ossl_dh_s_generate(int argc, VALUE *argv, VALUE klass)
-{
-    EVP_PKEY *pkey;
-    DH *dh ;
-    int g = 2;
-    VALUE size, gen, obj;
-
-    if (rb_scan_args(argc, argv, "11", &size, &gen) == 2) {
-	g = NUM2INT(gen);
-    }
-    obj = rb_obj_alloc(klass);
-    GetPKey(obj, pkey);
-
-    dh = dh_generate(NUM2INT(size), g);
-    if (!EVP_PKEY_assign_DH(pkey, dh)) {
-        DH_free(dh);
-        ossl_raise(eDHError, "EVP_PKEY_assign_DH");
-    }
-    return obj;
-}
-
 /*
  * call-seq:
  *   DH.new -> dh
  *   DH.new(string) -> dh
  *   DH.new(size [, generator]) -> dh
  *
- * Either generates a DH instance from scratch or by reading already existing
- * DH parameters from _string_. Note that when reading a DH instance from
- * data that was encoded from a DH instance by using DH#to_pem or DH#to_der
- * the result will *not* contain a public/private key pair yet. This needs to
- * be generated using DH#generate_key! first.
+ * Creates a new instance of OpenSSL::PKey::DH.
+ *
+ * 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).
+ *
+ * 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.
+ *
+ * The DH.new(size [, generator]) form is an alias of DH.generate.
  *
- * === Parameters
- * * _size_ is an integer representing the desired key size. Keys smaller than 1024 bits should be considered insecure.
- * * _generator_ is a small number > 1, typically 2 or 5.
- * * _string_ contains the DER or PEM encoded key.
+ * string::
+ *   A String that contains the DER or PEM encoded key.
+ * size::
+ *   See DH.generate.
+ * generator::
+ *   See DH.generate.
  *
- * === Examples
- *  DH.new # -> dh
- *  DH.new(1024) # -> dh
- *  DH.new(1024, 5) # -> dh
- *  #Reading DH parameters
- *  dh = DH.new(File.read('parameters.pem')) # -> dh, but no public/private key yet
- *  dh.generate_key! # -> dh with public and private key
+ * Examples:
+ *   # Creating an instance from scratch
+ *   dh = DH.new
+ *   dh.set_pqg(bn_p, nil, bn_g)
+ *
+ *   # Generating a parameters and a key pair
+ *   dh = DH.new(2048) # An alias of DH.generate(2048)
+ *
+ *   # Reading DH parameters
+ *   dh = DH.new(File.read('parameters.pem')) # -> dh, but no public/private key yet
+ *   dh.generate_key! # -> dh with public and private key
  */
 static VALUE
 ossl_dh_initialize(int argc, VALUE *argv, VALUE self)
 {
     EVP_PKEY *pkey;
     DH *dh;
-    int g = 2;
     BIO *in;
-    VALUE arg, gen;
+    VALUE arg;
 
     GetPKey(self, pkey);
-    if(rb_scan_args(argc, argv, "02", &arg, &gen) == 0) {
-      dh = DH_new();
-    }
-    else if (RB_INTEGER_TYPE_P(arg)) {
-	if (!NIL_P(gen)) {
-	    g = NUM2INT(gen);
-	}
-        dh = dh_generate(NUM2INT(arg), g);
+    /* The DH.new(size, generator) form is handled by lib/openssl/pkey.rb */
+    if (rb_scan_args(argc, argv, "01", &arg) == 0) {
+        dh = DH_new();
+        if (!dh)
+            ossl_raise(eDHError, "DH_new");
     }
     else {
 	arg = ossl_to_der_if_possible(arg);
@@ -449,33 +358,6 @@ ossl_dh_check_params(VALUE self)
     return codes == 0 ? Qtrue : Qfalse;
 }
 
-/*
- *  call-seq:
- *     dh.generate_key! -> self
- *
- * Generates a private and public key unless a private key already exists.
- * If this DH instance was generated from public DH parameters (e.g. by
- * encoding the result of DH#public_key), then this method needs to be
- * called first in order to generate the per-session keys before performing
- * the actual key exchange.
- *
- * === Example
- *   dh = OpenSSL::PKey::DH.new(2048)
- *   public_key = dh.public_key #contains no private/public key yet
- *   public_key.generate_key!
- *   puts public_key.private? # => true
- */
-static VALUE
-ossl_dh_generate_key(VALUE self)
-{
-    DH *dh;
-
-    GetDH(self, dh);
-    if (!DH_generate_key(dh))
-	ossl_raise(eDHError, "Failed to generate key");
-    return self;
-}
-
 /*
  * Document-method: OpenSSL::PKey::DH#set_pqg
  * call-seq:
@@ -540,7 +422,6 @@ Init_ossl_dh(void)
      *  puts symm_key1 == symm_key2 # => true
      */
     cDH = rb_define_class_under(mPKey, "DH", cPKey);
-    rb_define_singleton_method(cDH, "generate", ossl_dh_s_generate, -1);
     rb_define_method(cDH, "initialize", ossl_dh_initialize, -1);
     rb_define_method(cDH, "initialize_copy", ossl_dh_initialize_copy, 1);
     rb_define_method(cDH, "public?", ossl_dh_is_public, 0);
@@ -552,7 +433,6 @@ Init_ossl_dh(void)
     rb_define_method(cDH, "to_der", ossl_dh_to_der, 0);
     rb_define_method(cDH, "public_key", ossl_dh_to_public_key, 0);
     rb_define_method(cDH, "params_ok?", ossl_dh_check_params, 0);
-    rb_define_method(cDH, "generate_key!", ossl_dh_generate_key, 0);
 
     DEF_OSSL_PKEY_BN(cDH, dh, p);
     DEF_OSSL_PKEY_BN(cDH, dh, q);

lib/openssl/pkey.rb

@@ -27,6 +27,63 @@ def compute_key(pub_bn)
       peer.set_key(pub_bn, nil)
       derive(peer)
     end
+
+    # :call-seq:
+    #    dh.generate_key! -> self
+    #
+    # Generates a private and public key unless a private key already exists.
+    # If this DH instance was generated from public DH parameters (e.g. by
+    # encoding the result of DH#public_key), then this method needs to be
+    # called first in order to generate the per-session keys before performing
+    # the actual key exchange.
+    #
+    # See also OpenSSL::PKey.generate_key.
+    #
+    # Example:
+    #   dh = OpenSSL::PKey::DH.new(2048)
+    #   public_key = dh.public_key #contains no private/public key yet
+    #   public_key.generate_key!
+    #   puts public_key.private? # => true
+    def generate_key!
+      unless priv_key
+        tmp = OpenSSL::PKey.generate_key(self)
+        set_key(tmp.pub_key, tmp.priv_key)
+      end
+      self
+    end
+
+    class << self
+      # :call-seq:
+      #    DH.generate(size [, generator = 2]) -> dh
+      #
+      # Creates a new DH instance from scratch by generating the private and
+      # public components alike.
+      #
+      # See also OpenSSL::PKey.generate_parameters and
+      # OpenSSL::PKey.generate_key.
+      #
+      # size::
+      #   An Integer representing the desired key size.
+      # generator::
+      #   A small Integer > 1, typically 2 or 5.
+      def generate(size, generator = 2, &blk)
+        dhparams = OpenSSL::PKey.generate_parameters("DH", {
+          "dh_paramgen_prime_len" => size,
+          "dh_paramgen_generator" => generator,
+        }, &blk)
+        OpenSSL::PKey.generate_key(dhparams)
+      end
+
+      # Handle DH.new(size, generator) form here; new(str) and new() forms
+      # are handled by #initialize
+      def new(*args, &blk) # :nodoc:
+        if args[0].is_a?(Integer)
+          generate(*args, &blk)
+        else
+          super
+        end
+      end
+    end
   end
 
   class DSA

test/openssl/test_pkey_dh.rb

@@ -4,12 +4,19 @@
 if defined?(OpenSSL) && defined?(OpenSSL::PKey::DH)
 
 class OpenSSL::TestPKeyDH < OpenSSL::PKeyTestCase
-  NEW_KEYLEN = 256
+  NEW_KEYLEN = 2048
 
-  def test_new
+  def test_new_empty
+    dh = OpenSSL::PKey::DH.new
+    assert_equal nil, dh.p
+    assert_equal nil, dh.priv_key
+  end
+
+  def test_new_generate
+    # This test is slow
     dh = OpenSSL::PKey::DH.new(NEW_KEYLEN)
     assert_key(dh)
-  end
+  end if ENV["OSSL_TEST_ALL"]
 
   def test_new_break
     assert_nil(OpenSSL::PKey::DH.new(NEW_KEYLEN) { break })
@@ -80,7 +87,7 @@ def test_key_exchange
   end
 
   def test_dup
-    dh = OpenSSL::PKey::DH.new(NEW_KEYLEN)
+    dh = Fixtures.pkey("dh1024")
     dh2 = dh.dup
     assert_equal dh.to_der, dh2.to_der # params
     assert_equal_params dh, dh2 # keys