Merge pull request #397 from rhenium/ky/pkey-refactor-generate

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

10 files changed, 239 insertions, 488 deletions

diff --git a/ext/openssl/extconf.rb b/ext/openssl/extconf.rb
index bd7c5770..5cb28f30 100644
--- a/ext/openssl/extconf.rb
+++ b/ext/openssl/extconf.rb
@@ -130,9 +130,6 @@ engines.each { |name|
 if !have_struct_member("SSL", "ctx", "openssl/ssl.h") || is_libressl
   $defs.push("-DHAVE_OPAQUE_OPENSSL")
 end
-have_func("BN_GENCB_new")
-have_func("BN_GENCB_free")
-have_func("BN_GENCB_get_arg")
 have_func("EVP_MD_CTX_new")
 have_func("EVP_MD_CTX_free")
 have_func("EVP_MD_CTX_pkey_ctx")
diff --git a/ext/openssl/openssl_missing.h b/ext/openssl/openssl_missing.h
index 5586f965..4d9b8801 100644
--- a/ext/openssl/openssl_missing.h
+++ b/ext/openssl/openssl_missing.h
@@ -13,18 +13,6 @@
 #include "ruby/config.h"
 
 /* added in 1.1.0 */
-#if !defined(HAVE_BN_GENCB_NEW)
-#  define BN_GENCB_new() ((BN_GENCB *)OPENSSL_malloc(sizeof(BN_GENCB)))
-#endif
-
-#if !defined(HAVE_BN_GENCB_FREE)
-#  define BN_GENCB_free(cb) OPENSSL_free(cb)
-#endif
-
-#if !defined(HAVE_BN_GENCB_GET_ARG)
-#  define BN_GENCB_get_arg(cb) (cb)->arg
-#endif
-
 #if !defined(HAVE_EVP_MD_CTX_NEW)
 #  define EVP_MD_CTX_new EVP_MD_CTX_create
 #endif
diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c
index cce9f5dd..593788e1 100644
--- a/ext/openssl/ossl_pkey.c
+++ b/ext/openssl/ossl_pkey.c
@@ -17,64 +17,6 @@ VALUE cPKey;
 VALUE ePKeyError;
 static ID id_private_q;
 
-/*
- * callback for generating keys
- */
-static VALUE
-call_check_ints0(VALUE arg)
-{
-    rb_thread_check_ints();
-    return Qnil;
-}
-
-static void *
-call_check_ints(void *arg)
-{
-    int state;
-    rb_protect(call_check_ints0, Qnil, &state);
-    return (void *)(VALUE)state;
-}
-
-int
-ossl_generate_cb_2(int p, int n, BN_GENCB *cb)
-{
-    VALUE ary;
-    struct ossl_generate_cb_arg *arg;
-    int state;
-
-    arg = (struct ossl_generate_cb_arg *)BN_GENCB_get_arg(cb);
-    if (arg->yield) {
-	ary = rb_ary_new2(2);
-	rb_ary_store(ary, 0, INT2NUM(p));
-	rb_ary_store(ary, 1, INT2NUM(n));
-
-	/*
-	* can be break by raising exception or 'break'
-	*/
-	rb_protect(rb_yield, ary, &state);
-	if (state) {
-	    arg->state = state;
-	    return 0;
-	}
-    }
-    if (arg->interrupted) {
-	arg->interrupted = 0;
-	state = (int)(VALUE)rb_thread_call_with_gvl(call_check_ints, NULL);
-	if (state) {
-	    arg->state = state;
-	    return 0;
-	}
-    }
-    return 1;
-}
-
-void
-ossl_generate_cb_stop(void *ptr)
-{
-    struct ossl_generate_cb_arg *arg = (struct ossl_generate_cb_arg *)ptr;
-    arg->interrupted = 1;
-}
-
 static void
 ossl_evp_pkey_free(void *ptr)
 {
@@ -239,7 +181,7 @@ struct pkey_blocking_generate_arg {
     int state;
     int yield: 1;
     int genparam: 1;
-    int stop: 1;
+    int interrupted: 1;
 };
 
 static VALUE
@@ -257,27 +199,50 @@ pkey_gen_cb_yield(VALUE ctx_v)
     return rb_yield_values2(info_num, argv);
 }
 
+static VALUE
+call_check_ints0(VALUE arg)
+{
+    rb_thread_check_ints();
+    return Qnil;
+}
+
+static void *
+call_check_ints(void *arg)
+{
+    int state;
+    rb_protect(call_check_ints0, Qnil, &state);
+    return (void *)(VALUE)state;
+}
+
 static int
 pkey_gen_cb(EVP_PKEY_CTX *ctx)
 {
     struct pkey_blocking_generate_arg *arg = EVP_PKEY_CTX_get_app_data(ctx);
+    int state;
 
     if (arg->yield) {
-        int state;
         rb_protect(pkey_gen_cb_yield, (VALUE)ctx, &state);
         if (state) {
-            arg->stop = 1;
             arg->state = state;
+            return 0;
         }
     }
-    return !arg->stop;
+    if (arg->interrupted) {
+        arg->interrupted = 0;
+        state = (int)(VALUE)rb_thread_call_with_gvl(call_check_ints, NULL);
+        if (state) {
+            arg->state = state;
+            return 0;
+        }
+    }
+    return 1;
 }
 
 static void
 pkey_blocking_gen_stop(void *ptr)
 {
     struct pkey_blocking_generate_arg *arg = ptr;
-    arg->stop = 1;
+    arg->interrupted = 1;
 }
 
 static void *
diff --git a/ext/openssl/ossl_pkey.h b/ext/openssl/ossl_pkey.h
index 7dbaed47..629c16ae 100644
--- a/ext/openssl/ossl_pkey.h
+++ b/ext/openssl/ossl_pkey.h
@@ -35,14 +35,6 @@ extern const rb_data_type_t ossl_evp_pkey_type;
     } \
 } while (0)
 
-struct ossl_generate_cb_arg {
-    int yield;
-    int interrupted;
-    int state;
-};
-int ossl_generate_cb_2(int p, int n, BN_GENCB *cb);
-void ossl_generate_cb_stop(void *ptr);
-
 VALUE ossl_pkey_new(EVP_PKEY *);
 void ossl_pkey_check_public_key(const EVP_PKEY *);
 EVP_PKEY *ossl_pkey_read_generic(BIO *, VALUE);
diff --git a/ext/openssl/ossl_pkey_dh.c b/ext/openssl/ossl_pkey_dh.c
index 5bc1c49c..6b477b07 100644
--- a/ext/openssl/ossl_pkey_dh.c
+++ b/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);
@@ -450,33 +359,6 @@ ossl_dh_check_params(VALUE self)
 }
 
 /*
- *  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:
  *   dh.set_pqg(p, q, g) -> self
@@ -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);
diff --git a/ext/openssl/ossl_pkey_dsa.c b/ext/openssl/ossl_pkey_dsa.c
index 0e68f7f2..1c5a8a73 100644
--- a/ext/openssl/ossl_pkey_dsa.c
+++ b/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
- * * _size_ is an integer representing the desired key size.
- * * _string_ contains a DER or PEM encoded key.
- * * _pass_ is a string that contains an optional password.
+ * If called without arguments, creates a new instance with no key components
+ * set. They can be set individually by #set_pqg and #set_key.
  *
- * === Examples
- *  DSA.new -> dsa
- *  DSA.new(1024) -> dsa
- *  DSA.new(File.read('dsa.pem')) -> dsa
- *  DSA.new(File.read('dsa.pem'), 'mypassword') -> dsa
+ * 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.
+ *
+ * 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);
 
diff --git a/ext/openssl/ossl_pkey_rsa.c b/ext/openssl/ossl_pkey_rsa.c
index 3c298a2a..43f82cb2 100644
--- a/ext/openssl/ossl_pkey_rsa.c
+++ b/ext/openssl/ossl_pkey_rsa.c
@@ -47,125 +47,28 @@ VALUE eRSAError;
 /*
  * Private
  */
-struct rsa_blocking_gen_arg {
-    RSA *rsa;
-    BIGNUM *e;
-    int size;
-    BN_GENCB *cb;
-    int result;
-};
-
-static void *
-rsa_blocking_gen(void *arg)
-{
-    struct rsa_blocking_gen_arg *gen = (struct rsa_blocking_gen_arg *)arg;
-    gen->result = RSA_generate_key_ex(gen->rsa, gen->size, gen->e, gen->cb);
-    return 0;
-}
-
-static RSA *
-rsa_generate(int size, unsigned long exp)
-{
-    int i;
-    struct ossl_generate_cb_arg cb_arg = { 0 };
-    struct rsa_blocking_gen_arg gen_arg;
-    RSA *rsa = RSA_new();
-    BIGNUM *e = BN_new();
-    BN_GENCB *cb = BN_GENCB_new();
-
-    if (!rsa || !e || !cb) {
-	RSA_free(rsa);
-	BN_free(e);
-	BN_GENCB_free(cb);
-        ossl_raise(eRSAError, "malloc failure");
-    }
-    for (i = 0; i < (int)sizeof(exp) * 8; ++i) {
-	if (exp & (1UL << i)) {
-	    if (BN_set_bit(e, i) == 0) {
-		BN_free(e);
-		RSA_free(rsa);
-		BN_GENCB_free(cb);
-                ossl_raise(eRSAError, "BN_set_bit");
-	    }
-	}
-    }
-
-    if (rb_block_given_p())
-	cb_arg.yield = 1;
-    BN_GENCB_set(cb, ossl_generate_cb_2, &cb_arg);
-    gen_arg.rsa = rsa;
-    gen_arg.e = e;
-    gen_arg.size = size;
-    gen_arg.cb = cb;
-    if (cb_arg.yield == 1) {
-	/* we cannot release GVL when callback proc is supplied */
-	rsa_blocking_gen(&gen_arg);
-    } else {
-	/* there's a chance to unblock */
-	rb_thread_call_without_gvl(rsa_blocking_gen, &gen_arg, ossl_generate_cb_stop, &cb_arg);
-    }
-
-    BN_GENCB_free(cb);
-    BN_free(e);
-    if (!gen_arg.result) {
-	RSA_free(rsa);
-	if (cb_arg.state) {
-	    /* must clear OpenSSL error stack */
-	    ossl_clear_error();
-	    rb_jump_tag(cb_arg.state);
-	}
-        ossl_raise(eRSAError, "RSA_generate_key_ex");
-    }
-
-    return rsa;
-}
-
 /*
  * call-seq:
- *   RSA.generate(size)           => RSA instance
- *   RSA.generate(size, exponent) => RSA instance
+ *   RSA.new -> rsa
+ *   RSA.new(encoded_key [, passphrase]) -> rsa
+ *   RSA.new(encoded_key) { passphrase } -> rsa
+ *   RSA.new(size [, exponent]) -> rsa
  *
- * Generates an RSA keypair.  _size_ is an integer representing the desired key
- * size.  Keys smaller than 1024 should be considered insecure.  _exponent_ is
- * an odd number normally 3, 17, or 65537.
- */
-static VALUE
-ossl_rsa_s_generate(int argc, VALUE *argv, VALUE klass)
-{
-/* why does this method exist?  why can't initialize take an optional exponent? */
-    EVP_PKEY *pkey;
-    RSA *rsa;
-    VALUE size, exp;
-    VALUE obj;
-
-    rb_scan_args(argc, argv, "11", &size, &exp);
-    obj = rb_obj_alloc(klass);
-    GetPKey(obj, pkey);
-
-    rsa = rsa_generate(NUM2INT(size), NIL_P(exp) ? RSA_F4 : NUM2ULONG(exp));
-    if (!EVP_PKEY_assign_RSA(pkey, rsa)) {
-        RSA_free(rsa);
-        ossl_raise(eRSAError, "EVP_PKEY_assign_RSA");
-    }
-    return obj;
-}
-
-/*
- * call-seq:
- *   RSA.new(size [, exponent])        => RSA instance
- *   RSA.new(encoded_key)              => RSA instance
- *   RSA.new(encoded_key, pass_phrase) => RSA instance
+ * Generates or loads an \RSA keypair.
  *
- * Generates or loads an RSA keypair.  If an integer _key_size_ is given it
- * represents the desired key size.  Keys less than 1024 bits should be
- * considered insecure.
+ * 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.
  *
- * A key can instead be loaded from an _encoded_key_ which must be PEM or DER
- * encoded.  A _pass_phrase_ can be used to decrypt the key.  If none is given
- * OpenSSL will prompt for the pass phrase.
+ * If called with a String, tries to parse as DER or PEM encoding of an \RSA key.
+ * Note that, if _passphrase_ is not specified but the key is encrypted with a
+ * passphrase, \OpenSSL will prompt for it.
+ * See also OpenSSL::PKey.read which can parse keys of any kinds.
  *
- * = Examples
+ * If called with a number, generates a new key pair. This form works as an
+ * alias of RSA.generate.
  *
+ * Examples:
  *   OpenSSL::PKey::RSA.new 2048
  *   OpenSSL::PKey::RSA.new File.read 'rsa.pem'
  *   OpenSSL::PKey::RSA.new File.read('rsa.pem'), 'my pass phrase'
@@ -179,15 +82,13 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self)
     VALUE arg, pass;
 
     GetPKey(self, pkey);
+    /* The RSA.new(size, generator) form is handled by lib/openssl/pkey.rb */
     rb_scan_args(argc, argv, "02", &arg, &pass);
     if (argc == 0) {
 	rsa = RSA_new();
         if (!rsa)
             ossl_raise(eRSAError, "RSA_new");
     }
-    else if (RB_INTEGER_TYPE_P(arg)) {
-	rsa = rsa_generate(NUM2INT(arg), NIL_P(pass) ? RSA_F4 : NUM2ULONG(pass));
-    }
     else {
 	pass = ossl_pem_passwd_value(pass);
 	arg = ossl_to_der_if_possible(arg);
@@ -832,7 +733,6 @@ Init_ossl_rsa(void)
      */
     cRSA = rb_define_class_under(mPKey, "RSA", cPKey);
 
-    rb_define_singleton_method(cRSA, "generate", ossl_rsa_s_generate, -1);
     rb_define_method(cRSA, "initialize", ossl_rsa_initialize, -1);
     rb_define_method(cRSA, "initialize_copy", ossl_rsa_initialize_copy, 1);
 
diff --git a/lib/openssl/pkey.rb b/lib/openssl/pkey.rb
index be60ac2b..53ee52f9 100644
--- a/lib/openssl/pkey.rb
+++ b/lib/openssl/pkey.rb
@@ -27,10 +27,97 @@ module OpenSSL::PKey
       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 random parameters
+      # and a key pair.
+      #
+      # See also OpenSSL::PKey.generate_parameters and
+      # OpenSSL::PKey.generate_key.
+      #
+      # +size+::
+      #   The desired key size in bits.
+      # +generator+::
+      #   The generator.
+      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
     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)
@@ -71,5 +158,35 @@ module OpenSSL::PKey
 
   class RSA
     include OpenSSL::Marshal
+
+    class << self
+      # :call-seq:
+      #    RSA.generate(size, exponent = 65537) -> RSA
+      #
+      # Generates an \RSA keypair.
+      #
+      # See also OpenSSL::PKey.generate_key.
+      #
+      # +size+::
+      #   The desired key size in bits.
+      # +exponent+::
+      #   An odd Integer, normally 3, 17, or 65537.
+      def generate(size, exp = 0x10001, &blk)
+        OpenSSL::PKey.generate_key("RSA", {
+          "rsa_keygen_bits" => size,
+          "rsa_keygen_pubexp" => exp,
+        }, &blk)
+      end
+
+      # Handle RSA.new(size, exponent) 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
 end
diff --git a/test/openssl/test_pkey_dh.rb b/test/openssl/test_pkey_dh.rb
index 9efc3ba6..279ce198 100644
--- a/test/openssl/test_pkey_dh.rb
+++ b/test/openssl/test_pkey_dh.rb
@@ -4,12 +4,19 @@ require_relative 'utils'
 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 @@ class OpenSSL::TestPKeyDH < OpenSSL::PKeyTestCase
   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
diff --git a/test/openssl/test_pkey_dsa.rb b/test/openssl/test_pkey_dsa.rb
index 4bf8a7b3..85bb6ec0 100644
--- a/test/openssl/test_pkey_dsa.rb
+++ b/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)
-    assert(key.private?)
+    key = Fixtures.pkey("dsa1024")
+    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)