From e8504c6248c4b0e5e961f57f004e1133c20c88a5 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Mon, 5 Apr 2021 00:30:01 +0900 Subject: [PATCH 1/5] pkey: fix interrupt handling in OpenSSL::PKey.generate_key rb_thread_call_without_gvl() can be interrupted, but it may be able to resume the operation. Call rb_thread_check_ints() to see if it raises an exception or not. --- ext/openssl/ossl_pkey.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index 22e9f19982..d76f0600d1 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -239,7 +239,7 @@ struct pkey_blocking_generate_arg { int state; int yield: 1; int genparam: 1; - int stop: 1; + int interrupted: 1; }; static VALUE @@ -261,23 +261,31 @@ 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; + } + } + 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 !arg->stop; + 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 * -- 2.32.0 From f433d1b680e7ac5ef13fc15b0844267222438cf3 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sun, 17 May 2020 20:48:23 +0900 Subject: [PATCH 2/5] 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 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 accepted by default in OpenSSL 3.0. The test code is updated accordingly. --- ext/openssl/lib/openssl/pkey.rb | 57 ++++++++++ ext/openssl/ossl_pkey_dh.c | 186 ++++++-------------------------- test/openssl/test_pkey_dh.rb | 15 ++- 3 files changed, 101 insertions(+), 157 deletions(-) diff --git a/ext/openssl/lib/openssl/pkey.rb b/ext/openssl/lib/openssl/pkey.rb index be60ac2beb..5a3d0ed1ef 100644 --- a/ext/openssl/lib/openssl/pkey.rb +++ b/ext/openssl/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 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 diff --git a/ext/openssl/ossl_pkey_dh.c b/ext/openssl/ossl_pkey_dh.c index 5bc1c49ca1..6b477b077c 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); @@ -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); diff --git a/test/openssl/test_pkey_dh.rb b/test/openssl/test_pkey_dh.rb index 9efc3ba68d..279ce1984c 100644 --- a/test/openssl/test_pkey_dh.rb +++ b/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 -- 2.32.0 From ba1d1d68ac2b489691eb3fe2052e77b3e57a372b Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sun, 17 May 2020 20:48:23 +0900 Subject: [PATCH 3/5] pkey/rsa: use high level EVP interface to generate parameters and keys Implement PKey::RSA.new(size, exponent) and PKey::RSA.generate using OpenSSL::PKey.generate_key instead of the low level RSA functions. --- ext/openssl/lib/openssl/pkey.rb | 30 ++++++++ ext/openssl/ossl_pkey_rsa.c | 132 ++++---------------------------- 2 files changed, 46 insertions(+), 116 deletions(-) diff --git a/ext/openssl/lib/openssl/pkey.rb b/ext/openssl/lib/openssl/pkey.rb index 5a3d0ed1ef..3bef06e3b3 100644 --- a/ext/openssl/lib/openssl/pkey.rb +++ b/ext/openssl/lib/openssl/pkey.rb @@ -128,5 +128,35 @@ def to_bn(conversion_form = group.point_conversion_form) 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/ext/openssl/ossl_pkey_rsa.c b/ext/openssl/ossl_pkey_rsa.c index 3c298a2aea..43f82cb29e 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); -- 2.32.0 From a6c4a8116c09243c39cc8d1e7ececcd8be0cfaf2 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sun, 17 May 2020 22:14:03 +0900 Subject: [PATCH 4/5] 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. --- ext/openssl/lib/openssl/pkey.rb | 30 +++++++ ext/openssl/ossl_pkey_dsa.c | 140 ++++++-------------------------- test/openssl/test_pkey_dsa.rb | 23 ++---- 3 files changed, 64 insertions(+), 129 deletions(-) diff --git a/ext/openssl/lib/openssl/pkey.rb b/ext/openssl/lib/openssl/pkey.rb index 3bef06e3b3..53ee52f98b 100644 --- a/ext/openssl/lib/openssl/pkey.rb +++ b/ext/openssl/lib/openssl/pkey.rb @@ -88,6 +88,36 @@ def new(*args, &blk) # :nodoc: 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) diff --git a/ext/openssl/ossl_pkey_dsa.c b/ext/openssl/ossl_pkey_dsa.c index 0e68f7f27f..1c5a8a737e 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) + * #=> # + * + * p OpenSSL::PKey::DSA.new(File.read('dsa.pem')) + * #=> # + * + * p OpenSSL::PKey::DSA.new(File.read('dsa.pem'), 'mypassword') + * #=> # */ 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/test/openssl/test_pkey_dsa.rb b/test/openssl/test_pkey_dsa.rb index 4bf8a7b374..85bb6ec0ae 100644 --- a/test/openssl/test_pkey_dsa.rb +++ b/test/openssl/test_pkey_dsa.rb @@ -5,31 +5,26 @@ 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 @@ def test_read_DSAPublicKey_pem 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) -- 2.32.0 From ba5a3a5c3eabf969f5cd2232b022e440af803b5b Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Mon, 5 Apr 2021 00:39:04 +0900 Subject: [PATCH 5/5] pkey: remove unused ossl_generate_cb_2() helper function The previous series of commits re-implemented key generation with the low level API with the EVP API. The BN_GENCB-based callback function is no longer used. --- ext/openssl/extconf.rb | 3 -- ext/openssl/openssl_missing.h | 12 ------ ext/openssl/ossl_pkey.c | 73 +++++++---------------------------- ext/openssl/ossl_pkey.h | 8 ---- 4 files changed, 15 insertions(+), 81 deletions(-) diff --git a/ext/openssl/extconf.rb b/ext/openssl/extconf.rb index 693e55cd97..b3c6647faf 100644 --- a/ext/openssl/extconf.rb +++ b/ext/openssl/extconf.rb @@ -136,9 +136,6 @@ def find_openssl_library $defs.push("-DHAVE_OPAQUE_OPENSSL") end have_func("CRYPTO_lock") || $defs.push("-DHAVE_OPENSSL_110_THREADING_API") -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 7d218f86f5..e575415f49 100644 --- a/ext/openssl/openssl_missing.h +++ b/ext/openssl/openssl_missing.h @@ -34,18 +34,6 @@ int ossl_EC_curve_nist2nid(const char *); #endif /* 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 d76f0600d1..f9282b9417 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) { @@ -257,6 +199,21 @@ 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) { diff --git a/ext/openssl/ossl_pkey.h b/ext/openssl/ossl_pkey.h index 7dbaed47bc..629c16ae1f 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); -- 2.32.0