Skip to content

Commit

Permalink
pkey: disallow {DH,DSA,EC,RSA}.new without arguments on OpenSSL 3.0
Browse files Browse the repository at this point in the history
When OpenSSL::PKey::{DH,DSA,EC,RSA}.new is called without any arguments,
it sets up an empty corresponding low-level struct and wraps it in an
EVP_PKEY. This form has been supported so that users can fill the fields
later using low-level setter methods such as OpenSSL::PKey::RSA#set_key.

Such setter methods are not compatible with OpenSSL 3.0 or later, where
pkeys are immutable once created. This means that the ability to create
an empty instance is useless. Let's remove it and raise ArgumentError if
attempted.
  • Loading branch information
rhenium committed Jan 30, 2025
1 parent 09edb69 commit 43aa5ca
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 13 deletions.
6 changes: 6 additions & 0 deletions ext/openssl/ossl_pkey_dh.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ static VALUE eDHError;
* 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).
* This form is deprecated and will not work on OpenSSL 3.0 or later.
*
* 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.
Expand Down Expand Up @@ -84,10 +85,15 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self)

/* The DH.new(size, generator) form is handled by lib/openssl/pkey.rb */
if (rb_scan_args(argc, argv, "01", &arg) == 0) {
#ifdef OSSL_HAVE_IMMUTABLE_PKEY
rb_raise(rb_eArgError, "OpenSSL::PKey::DH.new without arguments is " \
"not supported in this version of OpenSSL");
#else
dh = DH_new();
if (!dh)
ossl_raise(eDHError, "DH_new");
goto legacy;
#endif
}

arg = ossl_to_der_if_possible(arg);
Expand Down
6 changes: 6 additions & 0 deletions ext/openssl/ossl_pkey_dsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ static VALUE eDSAError;
*
* If called without arguments, creates a new instance with no key components
* set. They can be set individually by #set_pqg and #set_key.
* This form is deprecated and will not work on OpenSSL 3.0 or later.
*
* 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.
Expand Down Expand Up @@ -96,10 +97,15 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self)
/* The DSA.new(size, generator) form is handled by lib/openssl/pkey.rb */
rb_scan_args(argc, argv, "02", &arg, &pass);
if (argc == 0) {
#ifdef OSSL_HAVE_IMMUTABLE_PKEY
rb_raise(rb_eArgError, "OpenSSL::PKey::DSA.new without arguments is " \
"not supported in this version of OpenSSL");
#else
dsa = DSA_new();
if (!dsa)
ossl_raise(eDSAError, "DSA_new");
goto legacy;
#endif
}

pass = ossl_pem_passwd_value(pass);
Expand Down
5 changes: 5 additions & 0 deletions ext/openssl/ossl_pkey_ec.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,14 @@ static VALUE ossl_ec_key_initialize(int argc, VALUE *argv, VALUE self)

rb_scan_args(argc, argv, "02", &arg, &pass);
if (NIL_P(arg)) {
#ifdef OSSL_HAVE_IMMUTABLE_PKEY
rb_raise(rb_eArgError, "OpenSSL::PKey::EC.new without arguments is " \
"not supported in this version of OpenSSL");
#else
if (!(ec = EC_KEY_new()))
ossl_raise(eECError, "EC_KEY_new");
goto legacy;
#endif
}
else if (rb_obj_is_kind_of(arg, cEC_GROUP)) {
ec = ec_key_new_from_group(arg);
Expand Down
6 changes: 6 additions & 0 deletions ext/openssl/ossl_pkey_rsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ static VALUE eRSAError;
* 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.
* This form is deprecated and will not work on OpenSSL 3.0 or later.
*
* If called with a String, tries to parse as DER or PEM encoding of an \RSA key.
* Note that if _password_ is not specified, but the key is encrypted with a
Expand Down Expand Up @@ -89,10 +90,15 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self)
/* The RSA.new(size, generator) form is handled by lib/openssl/pkey.rb */
rb_scan_args(argc, argv, "02", &arg, &pass);
if (argc == 0) {
#ifdef OSSL_HAVE_IMMUTABLE_PKEY
rb_raise(rb_eArgError, "OpenSSL::PKey::RSA.new without arguments is " \
"not supported in this version of OpenSSL");
#else
rsa = RSA_new();
if (!rsa)
ossl_raise(eRSAError, "RSA_new");
goto legacy;
#endif
}

pass = ossl_pem_passwd_value(pass);
Expand Down
10 changes: 8 additions & 2 deletions test/openssl/test_pkey_dh.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,15 @@ class OpenSSL::TestPKeyDH < OpenSSL::PKeyTestCase
NEW_KEYLEN = 2048

def test_new_empty
# pkeys are immutable in OpenSSL >= 3.0
if openssl?(3, 0, 0)
assert_raise(ArgumentError) { OpenSSL::PKey::DH.new }
return
end

dh = OpenSSL::PKey::DH.new
assert_equal nil, dh.p
assert_equal nil, dh.priv_key
assert_nil(dh.p)
assert_nil(dh.priv_key)
end

def test_new_generate
Expand Down
5 changes: 5 additions & 0 deletions test/openssl/test_pkey_dsa.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ def test_new_break
end

def test_new_empty
# pkeys are immutable in OpenSSL >= 3.0
if openssl?(3, 0, 0)
assert_raise(ArgumentError) { OpenSSL::PKey::DSA.new }
return
end
key = OpenSSL::PKey::DSA.new
assert_nil(key.p)
assert_raise(OpenSSL::PKey::PKeyError) { key.to_der }
Expand Down
30 changes: 19 additions & 11 deletions test/openssl/test_pkey_ec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,9 @@
if defined?(OpenSSL)

class OpenSSL::TestEC < OpenSSL::PKeyTestCase
def test_ec_key
def test_ec_key_new
key1 = OpenSSL::PKey::EC.generate("prime256v1")

# PKey is immutable in OpenSSL >= 3.0; constructing an empty EC object is
# deprecated
if !openssl?(3, 0, 0)
key2 = OpenSSL::PKey::EC.new
key2.group = key1.group
key2.private_key = key1.private_key
key2.public_key = key1.public_key
assert_equal key1.to_der, key2.to_der
end

key3 = OpenSSL::PKey::EC.new(key1)
assert_equal key1.to_der, key3.to_der

Expand All @@ -35,6 +25,24 @@ def test_ec_key
end
end

def test_ec_key_new_empty
# pkeys are immutable in OpenSSL >= 3.0; constructing an empty EC object is
# disallowed
if openssl?(3, 0, 0)
assert_raise(ArgumentError) { OpenSSL::PKey::EC.new }
return
end

key = OpenSSL::PKey::EC.new
assert_nil(key.group)

p256 = Fixtures.pkey("p256")
key.group = p256.group
key.private_key = p256.private_key
key.public_key = p256.public_key
assert_equal(p256.to_der, key.to_der)
end

def test_builtin_curves
builtin_curves = OpenSSL::PKey::EC.builtin_curves
assert_not_empty builtin_curves
Expand Down
13 changes: 13 additions & 0 deletions test/openssl/test_pkey_rsa.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ def test_new_public_exponent
assert_equal 3, key.e
end

def test_new_empty
# pkeys are immutable in OpenSSL >= 3.0
if openssl?(3, 0, 0)
assert_raise(ArgumentError) { OpenSSL::PKey::RSA.new }
return
end

key = OpenSSL::PKey::RSA.new
assert_nil(key.n)
end

def test_s_generate
key1 = OpenSSL::PKey::RSA.generate(2048)
assert_equal 2048, key1.n.num_bits
Expand Down Expand Up @@ -177,6 +188,8 @@ def test_sign_verify_raw_legacy


def test_verify_empty_rsa
# pkeys are immutable in OpenSSL >= 3.0; empty RSA instance is disallowed
return if openssl?(3, 0, 0)
rsa = OpenSSL::PKey::RSA.new
assert_raise(OpenSSL::PKey::PKeyError, "[Bug #12783]") {
rsa.verify("SHA1", "a", "b")
Expand Down

0 comments on commit 43aa5ca

Please sign in to comment.