From 09edb69791651fd35977c6f0901bd797689c71a8 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sat, 25 Jan 2025 15:50:03 +0900 Subject: [PATCH 1/2] pkey: define and use OSSL_HAVE_IMMUTABLE_PKEY macro Introduce a useful macro to avoid scattering *_PREREQ() checks everywhere. Currently, the macro is defined for OpenSSL 3.0 or later only. I think it is possible that LibreSSL may adopt it in the future. --- ext/openssl/ossl.h | 4 ++++ ext/openssl/ossl_pkey.c | 2 +- ext/openssl/ossl_pkey.h | 2 +- ext/openssl/ossl_pkey_ec.c | 10 +++++----- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/ext/openssl/ossl.h b/ext/openssl/ossl.h index 9b20829b3..43e05ad3e 100644 --- a/ext/openssl/ossl.h +++ b/ext/openssl/ossl.h @@ -73,6 +73,10 @@ # define OSSL_USE_PROVIDER #endif +#if OSSL_OPENSSL_PREREQ(3, 0, 0) +# define OSSL_HAVE_IMMUTABLE_PKEY 1 +#endif + /* * Common Module */ diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index 03cb85979..9e1ba399c 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -507,7 +507,7 @@ ossl_pkey_s_generate_key(int argc, VALUE *argv, VALUE self) void ossl_pkey_check_public_key(const EVP_PKEY *pkey) { -#if OSSL_OPENSSL_PREREQ(3, 0, 0) +#ifdef OSSL_HAVE_IMMUTABLE_PKEY if (EVP_PKEY_missing_parameters(pkey)) ossl_raise(ePKeyError, "parameters missing"); #else diff --git a/ext/openssl/ossl_pkey.h b/ext/openssl/ossl_pkey.h index fdc5e94ae..a063cb2ec 100644 --- a/ext/openssl/ossl_pkey.h +++ b/ext/openssl/ossl_pkey.h @@ -105,7 +105,7 @@ static VALUE ossl_##_keytype##_get_##_name(VALUE self) \ OSSL_PKEY_BN_DEF_GETTER0(_keytype, _type, a2, \ _type##_get0_##_group(obj, NULL, &bn)) -#if !OSSL_OPENSSL_PREREQ(3, 0, 0) +#ifndef OSSL_HAVE_IMMUTABLE_PKEY #define OSSL_PKEY_BN_DEF_SETTER3(_keytype, _type, _group, a1, a2, a3) \ /* \ * call-seq: \ diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c index 810440f3d..b252c767f 100644 --- a/ext/openssl/ossl_pkey_ec.c +++ b/ext/openssl/ossl_pkey_ec.c @@ -248,7 +248,7 @@ ossl_ec_key_get_group(VALUE self) static VALUE ossl_ec_key_set_group(VALUE self, VALUE group_v) { -#if OSSL_OPENSSL_PREREQ(3, 0, 0) +#ifdef OSSL_HAVE_IMMUTABLE_PKEY rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0"); #else EC_KEY *ec; @@ -290,7 +290,7 @@ static VALUE ossl_ec_key_get_private_key(VALUE self) */ static VALUE ossl_ec_key_set_private_key(VALUE self, VALUE private_key) { -#if OSSL_OPENSSL_PREREQ(3, 0, 0) +#ifdef OSSL_HAVE_IMMUTABLE_PKEY rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0"); #else EC_KEY *ec; @@ -341,7 +341,7 @@ static VALUE ossl_ec_key_get_public_key(VALUE self) */ static VALUE ossl_ec_key_set_public_key(VALUE self, VALUE public_key) { -#if OSSL_OPENSSL_PREREQ(3, 0, 0) +#ifdef OSSL_HAVE_IMMUTABLE_PKEY rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0"); #else EC_KEY *ec; @@ -513,7 +513,7 @@ ossl_ec_key_to_der(VALUE self) */ static VALUE ossl_ec_key_generate_key(VALUE self) { -#if OSSL_OPENSSL_PREREQ(3, 0, 0) +#ifdef OSSL_HAVE_IMMUTABLE_PKEY rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0"); #else EC_KEY *ec; @@ -1367,7 +1367,7 @@ static VALUE ossl_ec_point_make_affine(VALUE self) GetECPointGroup(self, group); rb_warn("OpenSSL::PKey::EC::Point#make_affine! is deprecated"); -#if !OSSL_OPENSSL_PREREQ(3, 0, 0) +#ifndef OSSL_HAVE_IMMUTABLE_PKEY if (EC_POINT_make_affine(group, point, ossl_bn_ctx) != 1) ossl_raise(eEC_POINT, "EC_POINT_make_affine"); #endif From 43aa5ca2e8d2ab625eef0df0d62173f8a7db2339 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Thu, 30 Jan 2025 02:26:41 +0900 Subject: [PATCH 2/2] pkey: disallow {DH,DSA,EC,RSA}.new without arguments on OpenSSL 3.0 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. --- ext/openssl/ossl_pkey_dh.c | 6 ++++++ ext/openssl/ossl_pkey_dsa.c | 6 ++++++ ext/openssl/ossl_pkey_ec.c | 5 +++++ ext/openssl/ossl_pkey_rsa.c | 6 ++++++ test/openssl/test_pkey_dh.rb | 10 ++++++++-- test/openssl/test_pkey_dsa.rb | 5 +++++ test/openssl/test_pkey_ec.rb | 30 +++++++++++++++++++----------- test/openssl/test_pkey_rsa.rb | 13 +++++++++++++ 8 files changed, 68 insertions(+), 13 deletions(-) diff --git a/ext/openssl/ossl_pkey_dh.c b/ext/openssl/ossl_pkey_dh.c index 7bf589d5f..1a37f8ce3 100644 --- a/ext/openssl/ossl_pkey_dh.c +++ b/ext/openssl/ossl_pkey_dh.c @@ -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. @@ -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); diff --git a/ext/openssl/ossl_pkey_dsa.c b/ext/openssl/ossl_pkey_dsa.c index a1f0f9d77..81c3a9a98 100644 --- a/ext/openssl/ossl_pkey_dsa.c +++ b/ext/openssl/ossl_pkey_dsa.c @@ -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. @@ -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); diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c index b252c767f..b969114f2 100644 --- a/ext/openssl/ossl_pkey_ec.c +++ b/ext/openssl/ossl_pkey_ec.c @@ -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); diff --git a/ext/openssl/ossl_pkey_rsa.c b/ext/openssl/ossl_pkey_rsa.c index ceda6708a..3614d1b60 100644 --- a/ext/openssl/ossl_pkey_rsa.c +++ b/ext/openssl/ossl_pkey_rsa.c @@ -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 @@ -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); diff --git a/test/openssl/test_pkey_dh.rb b/test/openssl/test_pkey_dh.rb index 686c9b97d..1ce19919b 100644 --- a/test/openssl/test_pkey_dh.rb +++ b/test/openssl/test_pkey_dh.rb @@ -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 diff --git a/test/openssl/test_pkey_dsa.rb b/test/openssl/test_pkey_dsa.rb index a8578daf5..99e1a5ed1 100644 --- a/test/openssl/test_pkey_dsa.rb +++ b/test/openssl/test_pkey_dsa.rb @@ -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 } diff --git a/test/openssl/test_pkey_ec.rb b/test/openssl/test_pkey_ec.rb index 5a15c5441..aa512f5c9 100644 --- a/test/openssl/test_pkey_ec.rb +++ b/test/openssl/test_pkey_ec.rb @@ -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 @@ -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 diff --git a/test/openssl/test_pkey_rsa.rb b/test/openssl/test_pkey_rsa.rb index 360309b47..22b2491d1 100644 --- a/test/openssl/test_pkey_rsa.rb +++ b/test/openssl/test_pkey_rsa.rb @@ -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 @@ -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")