[ruby/openssl] pkey/dsa: use high level EVP interface to generate parameters and keys

Implement PKey::DSA.new(size) and PKey::DSA.generate using
OpenSSL::PKey.generate_parameters and .generate_key instead of the low
level DSA functions.

https://github.com/ruby/openssl/commit/1800a8d5eb

ext/openssl/lib/openssl/pkey.rb

@@ -88,6 +88,36 @@ module OpenSSL::PKey
 
   class DSA
     include OpenSSL::Marshal
+
+    class << self
+      # :call-seq:
+      #    DSA.generate(size) -> dsa
+      #
+      # Creates a new DSA instance by generating a private/public key pair
+      # from scratch.
+      #
+      # See also OpenSSL::PKey.generate_parameters and
+      # OpenSSL::PKey.generate_key.
+      #
+      # +size+::
+      #   The desired key size in bits.
+      def generate(size, &blk)
+        dsaparams = OpenSSL::PKey.generate_parameters("DSA", {
+          "dsa_paramgen_bits" => size,
+        }, &blk)
+        OpenSSL::PKey.generate_key(dsaparams)
+      end
+
+      # Handle DSA.new(size) 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
 
   if defined?(EC)

ext/openssl/ossl_pkey_dsa.c

@@ -46,126 +46,39 @@ VALUE eDSAError;
 /*
  * Private
  */
-struct dsa_blocking_gen_arg {
-    DSA *dsa;
-    int size;
-    int *counter;
-    unsigned long *h;
-    BN_GENCB *cb;
-    int result;
-};
-
-static void *
-dsa_blocking_gen(void *arg)
-{
-    struct dsa_blocking_gen_arg *gen = (struct dsa_blocking_gen_arg *)arg;
-    gen->result = DSA_generate_parameters_ex(gen->dsa, gen->size, NULL, 0,
-					     gen->counter, gen->h, gen->cb);
-    return 0;
-}
-
-static DSA *
-dsa_generate(int size)
-{
-    struct ossl_generate_cb_arg cb_arg = { 0 };
-    struct dsa_blocking_gen_arg gen_arg;
-    DSA *dsa = DSA_new();
-    BN_GENCB *cb = BN_GENCB_new();
-    int counter;
-    unsigned long h;
-
-    if (!dsa || !cb) {
-        DSA_free(dsa);
-        BN_GENCB_free(cb);
-        ossl_raise(eDSAError, "malloc failure");
-    }
-
-    if (rb_block_given_p())
-	cb_arg.yield = 1;
-    BN_GENCB_set(cb, ossl_generate_cb_2, &cb_arg);
-    gen_arg.dsa = dsa;
-    gen_arg.size = size;
-    gen_arg.counter = &counter;
-    gen_arg.h = &h;
-    gen_arg.cb = cb;
-    if (cb_arg.yield == 1) {
-	/* we cannot release GVL when callback proc is supplied */
-	dsa_blocking_gen(&gen_arg);
-    } else {
-	/* there's a chance to unblock */
-	rb_thread_call_without_gvl(dsa_blocking_gen, &gen_arg, ossl_generate_cb_stop, &cb_arg);
-    }
-
-    BN_GENCB_free(cb);
-    if (!gen_arg.result) {
-	DSA_free(dsa);
-	if (cb_arg.state) {
-	    /* Clear OpenSSL error queue before re-raising. By the way, the
-	     * documentation of DSA_generate_parameters_ex() says the error code
-	     * can be obtained by ERR_get_error(), but the default
-	     * implementation, dsa_builtin_paramgen() doesn't put any error... */
-	    ossl_clear_error();
-	    rb_jump_tag(cb_arg.state);
-	}
-        ossl_raise(eDSAError, "DSA_generate_parameters_ex");
-    }
-
-    if (!DSA_generate_key(dsa)) {
-        DSA_free(dsa);
-        ossl_raise(eDSAError, "DSA_generate_key");
-    }
-
-    return dsa;
-}
-
-/*
- *  call-seq:
- *    DSA.generate(size) -> dsa
- *
- * Creates a new DSA instance by generating a private/public key pair
- * from scratch.
- *
- * === Parameters
- * * _size_ is an integer representing the desired key size.
- *
- */
-static VALUE
-ossl_dsa_s_generate(VALUE klass, VALUE size)
-{
-    EVP_PKEY *pkey;
-    DSA *dsa;
-    VALUE obj;
-
-    obj = rb_obj_alloc(klass);
-    GetPKey(obj, pkey);
-
-    dsa = dsa_generate(NUM2INT(size));
-    if (!EVP_PKEY_assign_DSA(pkey, dsa)) {
-        DSA_free(dsa);
-        ossl_raise(eDSAError, "EVP_PKEY_assign_DSA");
-    }
-    return obj;
-}
-
 /*
  *  call-seq:
  *    DSA.new -> dsa
- *    DSA.new(size) -> dsa
  *    DSA.new(string [, pass]) -> dsa
+ *    DSA.new(size) -> dsa
  *
  * Creates a new DSA instance by reading an existing key from _string_.
  *
- * === Parameters
+ * If called without arguments, creates a new instance with no key components
- * * _size_ is an integer representing the desired key size.
+ * set. They can be set individually by #set_pqg and #set_key.
- * * _string_ contains a DER or PEM encoded key.
- * * _pass_ is a string that contains an optional password.
  *
- * === Examples
+ * If called with a String, tries to parse as DER or PEM encoding of a \DSA key.
- *  DSA.new -> dsa
+ * See also OpenSSL::PKey.read which can parse keys of any kinds.
- *  DSA.new(1024) -> dsa
- *  DSA.new(File.read('dsa.pem')) -> dsa
- *  DSA.new(File.read('dsa.pem'), 'mypassword') -> dsa
  *
+ * If called with a number, generates random parameters and a key pair. This
+ * form works as an alias of DSA.generate.
+ *
+ * +string+::
+ *   A String that contains a DER or PEM encoded key.
+ * +pass+::
+ *   A String that contains an optional password.
+ * +size+::
+ *   See DSA.generate.
+ *
+ * Examples:
+ *   p OpenSSL::PKey::DSA.new(1024)
+ *   #=> #<OpenSSL::PKey::DSA:0x000055a8d6025bf0 oid=DSA>
+ *
+ *   p OpenSSL::PKey::DSA.new(File.read('dsa.pem'))
+ *   #=> #<OpenSSL::PKey::DSA:0x000055555d6b8110 oid=DSA>
+ *
+ *   p OpenSSL::PKey::DSA.new(File.read('dsa.pem'), 'mypassword')
+ *   #=> #<OpenSSL::PKey::DSA:0x0000556f973c40b8 oid=DSA>
  */
 static VALUE
 ossl_dsa_initialize(int argc, VALUE *argv, VALUE self)
@@ -176,15 +89,13 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self)
     VALUE arg, pass;
 
     GetPKey(self, pkey);
+    /* The DSA.new(size, generator) form is handled by lib/openssl/pkey.rb */
     rb_scan_args(argc, argv, "02", &arg, &pass);
     if (argc == 0) {
         dsa = DSA_new();
         if (!dsa)
             ossl_raise(eDSAError, "DSA_new");
     }
-    else if (argc == 1 && RB_INTEGER_TYPE_P(arg)) {
-        dsa = dsa_generate(NUM2INT(arg));
-    }
     else {
 	pass = ossl_pem_passwd_value(pass);
 	arg = ossl_to_der_if_possible(arg);
@@ -553,7 +464,6 @@ Init_ossl_dsa(void)
      */
     cDSA = rb_define_class_under(mPKey, "DSA", cPKey);
 
-    rb_define_singleton_method(cDSA, "generate", ossl_dsa_s_generate, 1);
     rb_define_method(cDSA, "initialize", ossl_dsa_initialize, -1);
     rb_define_method(cDSA, "initialize_copy", ossl_dsa_initialize_copy, 1);
 

test/openssl/test_pkey_dsa.rb

@@ -5,31 +5,26 @@ if defined?(OpenSSL) && defined?(OpenSSL::PKey::DSA)
 
 class OpenSSL::TestPKeyDSA < OpenSSL::PKeyTestCase
   def test_private
-    key = OpenSSL::PKey::DSA.new(256)
+    key = Fixtures.pkey("dsa1024")
-    assert(key.private?)
+    assert_equal true, key.private?
     key2 = OpenSSL::PKey::DSA.new(key.to_der)
-    assert(key2.private?)
+    assert_equal true, key2.private?
     key3 = key.public_key
-    assert(!key3.private?)
+    assert_equal false, key3.private?
     key4 = OpenSSL::PKey::DSA.new(key3.to_der)
-    assert(!key4.private?)
+    assert_equal false, key4.private?
   end
 
   def test_new
-    key = OpenSSL::PKey::DSA.new 256
+    key = OpenSSL::PKey::DSA.new(2048)
     pem  = key.public_key.to_pem
     OpenSSL::PKey::DSA.new pem
-    if $0 == __FILE__
-      assert_nothing_raised {
-        key = OpenSSL::PKey::DSA.new 2048
-      }
-    end
   end
 
   def test_new_break
-    assert_nil(OpenSSL::PKey::DSA.new(512) { break })
+    assert_nil(OpenSSL::PKey::DSA.new(2048) { break })
     assert_raise(RuntimeError) do
-      OpenSSL::PKey::DSA.new(512) { raise }
+      OpenSSL::PKey::DSA.new(2048) { raise }
     end
   end
 
@@ -184,7 +179,7 @@ fWLOqqkzFeRrYMDzUpl36XktY6Yq8EJYlW9pCMmBVNy/dQ==
   end
 
   def test_dup
-    key = OpenSSL::PKey::DSA.new(256)
+    key = Fixtures.pkey("dsa1024")
     key2 = key.dup
     assert_equal key.params, key2.params
     key2.set_pqg(key2.p + 1, key2.q, key2.g)