From 5a437e0c7a55961150f79709d72c695a8e6d7bcb Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Tue, 26 Jul 2022 02:20:21 +0900 Subject: [PATCH 1/7] ossl.h: add OSSL_HAVE_PROVIDER macro With providers, pkeys are immutable and we should avoid using low-level types such as RSA or EC_KEY. Use this special macro instead of version numbers to make the intention clear, and also to make it easier to update when LibreSSL gains OpenSSL 3.0 providers support. --- ext/openssl/ossl.h | 4 ++++ ext/openssl/ossl_pkey.c | 4 ++-- ext/openssl/ossl_pkey.h | 6 +++--- ext/openssl/ossl_pkey_ec.c | 8 ++++---- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/ext/openssl/ossl.h b/ext/openssl/ossl.h index 2ab8aeaeb..e64ea8805 100644 --- a/ext/openssl/ossl.h +++ b/ext/openssl/ossl.h @@ -56,6 +56,10 @@ # define OSSL_USE_ENGINE #endif +#if OSSL_OPENSSL_PREREQ(3, 0, 0) +# define OSSL_HAVE_PROVIDER +#endif + /* * Common Module */ diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index ec39e8bd7..b38f185bb 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -79,7 +79,7 @@ ossl_pkey_new(EVP_PKEY *pkey) return obj; } -#if OSSL_OPENSSL_PREREQ(3, 0, 0) +#ifdef OSSL_HAVE_PROVIDER # include EVP_PKEY * @@ -484,7 +484,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_PROVIDER 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 38fb9fad1..6194fbb6b 100644 --- a/ext/openssl/ossl_pkey.h +++ b/ext/openssl/ossl_pkey.h @@ -116,7 +116,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_PROVIDER #define OSSL_PKEY_BN_DEF_SETTER3(_keytype, _type, _group, a1, a2, a3) \ /* \ * call-seq: \ @@ -174,7 +174,7 @@ static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2) \ } \ return self; \ } -#else +#else /* OSSL_HAVE_PROVIDER */ #define OSSL_PKEY_BN_DEF_SETTER3(_keytype, _type, _group, a1, a2, a3) \ static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2, VALUE v3) \ { \ @@ -188,7 +188,7 @@ static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2) \ rb_raise(ePKeyError, \ #_keytype"#set_"#_group"= is incompatible with OpenSSL 3.0"); \ } -#endif +#endif /* OSSL_HAVE_PROVIDER */ #define OSSL_PKEY_BN_DEF3(_keytype, _type, _group, a1, a2, a3) \ OSSL_PKEY_BN_DEF_GETTER3(_keytype, _type, _group, a1, a2, a3) \ diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c index 06d59c2a4..85f55c66d 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_PROVIDER 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_PROVIDER 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_PROVIDER rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0"); #else EC_KEY *ec; @@ -457,7 +457,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_PROVIDER rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0"); #else EC_KEY *ec; From 8c5cd1d8fcc76899370cba1cfff253e7ca765e9d Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Tue, 26 Jul 2022 16:36:22 +0900 Subject: [PATCH 2/7] test/openssl/test_pkey_*: skip tests for empty instances These don't work and make no sense on OpenSSL 3.0, since PKey instances are immutable once initialized. --- test/openssl/test_pkey_dh.rb | 2 +- test/openssl/test_pkey_rsa.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/openssl/test_pkey_dh.rb b/test/openssl/test_pkey_dh.rb index 161af1897..726ec768a 100644 --- a/test/openssl/test_pkey_dh.rb +++ b/test/openssl/test_pkey_dh.rb @@ -10,7 +10,7 @@ def test_new_empty dh = OpenSSL::PKey::DH.new assert_equal nil, dh.p assert_equal nil, dh.priv_key - end + end if !openssl?(3, 0, 0) def test_new_generate # This test is slow diff --git a/test/openssl/test_pkey_rsa.rb b/test/openssl/test_pkey_rsa.rb index fa84b76f4..2ffb3c9f4 100644 --- a/test/openssl/test_pkey_rsa.rb +++ b/test/openssl/test_pkey_rsa.rb @@ -172,7 +172,7 @@ def test_verify_empty_rsa assert_raise(OpenSSL::PKey::PKeyError, "[Bug #12783]") { rsa.verify("SHA1", "a", "b") } - end + end if !openssl?(3, 0, 0) def test_sign_verify_pss key = Fixtures.pkey("rsa1024") @@ -395,7 +395,7 @@ def test_RSAPublicKey EOF key = OpenSSL::PKey::RSA.new(pem) assert_same_rsa rsa1024pub, key - end + end if !openssl?(3, 0, 0) # RSAPublicKey is not available on OpenSSL 3.0 def test_PUBKEY rsa1024 = Fixtures.pkey("rsa1024") From 5892958ccae32edb0a5d0f99daff2e162b823e2f Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Tue, 26 Jul 2022 02:20:40 +0900 Subject: [PATCH 3/7] pkey/dh: do not create legacy DH pkey Avoid using DH_new() or d2i_DHparams_bio(). The path calling DH_new() creates an empty pkey. This is totally useless on OpenSSL 3.0 because pkey instances are immutable. Calling d2i_DHparams_bio() is not needed on OpenSSL 3.0. This format should be handled by ossl_pkey_read_generic() already. Besides, it creates a "legacy" pkey instance which prevents EVP_PKEY_todata() from working properly. --- ext/openssl/ossl_pkey_dh.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ext/openssl/ossl_pkey_dh.c b/ext/openssl/ossl_pkey_dh.c index 696455dcf..3892f6dc5 100644 --- a/ext/openssl/ossl_pkey_dh.c +++ b/ext/openssl/ossl_pkey_dh.c @@ -74,7 +74,9 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self) { EVP_PKEY *pkey; int type; +#ifndef OSSL_HAVE_PROVIDER DH *dh; +#endif BIO *in = NULL; VALUE arg; @@ -84,15 +86,20 @@ 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_PROVIDER + rb_raise(eDHError, "empty DH cannot be created"); +#else dh = DH_new(); if (!dh) ossl_raise(eDHError, "DH_new"); goto legacy; +#endif } arg = ossl_to_der_if_possible(arg); in = ossl_obj2bio(&arg); +#ifndef OSSL_HAVE_PROVIDER /* * On OpenSSL <= 1.1.1 and current versions of LibreSSL, the generic * routine does not support DER-encoded parameters @@ -101,6 +108,7 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self) if (dh) goto legacy; OSSL_BIO_reset(in); +#endif pkey = ossl_pkey_read_generic(in, Qnil); BIO_free(in); @@ -115,6 +123,7 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self) RTYPEDDATA_DATA(self) = pkey; return self; +#ifndef OSSL_HAVE_PROVIDER legacy: BIO_free(in); pkey = EVP_PKEY_new(); @@ -125,6 +134,7 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self) } RTYPEDDATA_DATA(self) = pkey; return self; +#endif } #ifndef HAVE_EVP_PKEY_DUP From 9d0718ca017c9c2f2c2ec85c8d12e56935ff9635 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 30 Sep 2022 21:00:33 +0900 Subject: [PATCH 4/7] pkey/dsa: do not create legacy DSA pkey --- ext/openssl/ossl_pkey_dsa.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ext/openssl/ossl_pkey_dsa.c b/ext/openssl/ossl_pkey_dsa.c index 25404aa7f..138cc04c6 100644 --- a/ext/openssl/ossl_pkey_dsa.c +++ b/ext/openssl/ossl_pkey_dsa.c @@ -84,7 +84,9 @@ static VALUE ossl_dsa_initialize(int argc, VALUE *argv, VALUE self) { EVP_PKEY *pkey; +#ifndef OSSL_HAVE_PROVIDER DSA *dsa; +#endif BIO *in = NULL; VALUE arg, pass; int type; @@ -96,16 +98,21 @@ 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_PROVIDER + rb_raise(eDHError, "empty DSA cannot be created"); +#else dsa = DSA_new(); if (!dsa) ossl_raise(eDSAError, "DSA_new"); goto legacy; +#endif } pass = ossl_pem_passwd_value(pass); arg = ossl_to_der_if_possible(arg); in = ossl_obj2bio(&arg); +#ifndef OSSL_HAVE_PROVIDER /* DER-encoded DSAPublicKey format isn't supported by the generic routine */ dsa = (DSA *)PEM_ASN1_read_bio((d2i_of_void *)d2i_DSAPublicKey, PEM_STRING_DSA_PUBLIC, @@ -113,6 +120,7 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self) if (dsa) goto legacy; OSSL_BIO_reset(in); +#endif pkey = ossl_pkey_read_generic(in, pass); BIO_free(in); @@ -127,6 +135,7 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self) RTYPEDDATA_DATA(self) = pkey; return self; +#ifndef OSSL_HAVE_PROVIDER legacy: BIO_free(in); pkey = EVP_PKEY_new(); @@ -137,6 +146,7 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self) } RTYPEDDATA_DATA(self) = pkey; return self; +#endif } #ifndef HAVE_EVP_PKEY_DUP From 341d72b1c1fe9b6571a9a6f52907de7234e7dbff Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 30 Sep 2022 21:02:17 +0900 Subject: [PATCH 5/7] pkey/rsa: do not create legacy RSA pkey --- ext/openssl/ossl_pkey_rsa.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ext/openssl/ossl_pkey_rsa.c b/ext/openssl/ossl_pkey_rsa.c index 4d66010f4..cb77d083e 100644 --- a/ext/openssl/ossl_pkey_rsa.c +++ b/ext/openssl/ossl_pkey_rsa.c @@ -77,7 +77,9 @@ static VALUE ossl_rsa_initialize(int argc, VALUE *argv, VALUE self) { EVP_PKEY *pkey; +#ifndef OSSL_HAVE_PROVIDER RSA *rsa; +#endif BIO *in = NULL; VALUE arg, pass; int type; @@ -89,16 +91,21 @@ 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_PROVIDER + rb_raise(eRSAError, "empty RSA cannot be created"); +#else rsa = RSA_new(); if (!rsa) ossl_raise(eRSAError, "RSA_new"); goto legacy; +#endif } pass = ossl_pem_passwd_value(pass); arg = ossl_to_der_if_possible(arg); in = ossl_obj2bio(&arg); +#ifndef OSSL_HAVE_PROVIDER /* First try RSAPublicKey format */ rsa = d2i_RSAPublicKey_bio(in, NULL); if (rsa) @@ -108,6 +115,7 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self) if (rsa) goto legacy; OSSL_BIO_reset(in); +#endif /* Use the generic routine */ pkey = ossl_pkey_read_generic(in, pass); @@ -123,6 +131,7 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self) RTYPEDDATA_DATA(self) = pkey; return self; +#ifndef OSSL_HAVE_PROVIDER legacy: BIO_free(in); pkey = EVP_PKEY_new(); @@ -133,6 +142,7 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self) } RTYPEDDATA_DATA(self) = pkey; return self; +#endif } #ifndef HAVE_EVP_PKEY_DUP From 0ab4e676d63cbb80e2e3a0d19a6e116da0039791 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Tue, 26 Jul 2022 16:47:35 +0900 Subject: [PATCH 6/7] pkey: give input_type to OSSL_DECODER_CTX DER encoding may be ambiguous because it doesn't contain information about the key type or format. For example, DHParameters and RSAPublicKey look identical on the DER encoding, which is a SEQUENCE with two INTEGER inside. --- ext/openssl/ossl_pkey.c | 33 +++++++++++++++++++++++++++++---- ext/openssl/ossl_pkey.h | 2 +- ext/openssl/ossl_pkey_dh.c | 9 +-------- ext/openssl/ossl_pkey_dsa.c | 9 +-------- ext/openssl/ossl_pkey_ec.c | 9 +-------- ext/openssl/ossl_pkey_rsa.c | 9 +-------- 6 files changed, 34 insertions(+), 37 deletions(-) diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index b38f185bb..7ab1bc700 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -83,14 +83,14 @@ ossl_pkey_new(EVP_PKEY *pkey) # include EVP_PKEY * -ossl_pkey_read_generic(BIO *bio, VALUE pass) +ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type) { void *ppass = (void *)pass; OSSL_DECODER_CTX *dctx; EVP_PKEY *pkey = NULL; int pos = 0, pos2; - dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "DER", NULL, NULL, 0, NULL, NULL); + dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "DER", NULL, input_type, 0, NULL, NULL); if (!dctx) goto out; if (OSSL_DECODER_CTX_set_pem_password_cb(dctx, ossl_pem_passwd_cb, ppass) != 1) @@ -158,7 +158,7 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass) } #else EVP_PKEY * -ossl_pkey_read_generic(BIO *bio, VALUE pass) +ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type) { void *ppass = (void *)pass; EVP_PKEY *pkey; @@ -183,6 +183,31 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass) goto out; out: + /* This is to mimic OSSL_DECODER_CTX's input_type parameter */ + if (pkey && input_type) { + switch (EVP_PKEY_base_id(pkey)) { + case EVP_PKEY_RSA: + if (!strcmp(input_type, "RSA")) + return pkey; + break; + case EVP_PKEY_DSA: + if (!strcmp(input_type, "DSA")) + return pkey; + break; + case EVP_PKEY_DH: + if (!strcmp(input_type, "DH")) + return pkey; + break; +#if !defined(OPENSSL_NO_EC) + case EVP_PKEY_EC: + if (!strcmp(input_type, "EC")) + return pkey; + break; +#endif + } + EVP_PKEY_free(pkey); + return NULL; + } return pkey; } #endif @@ -212,7 +237,7 @@ ossl_pkey_new_from_data(int argc, VALUE *argv, VALUE self) rb_scan_args(argc, argv, "11", &data, &pass); bio = ossl_obj2bio(&data); - pkey = ossl_pkey_read_generic(bio, ossl_pem_passwd_value(pass)); + pkey = ossl_pkey_read_generic(bio, ossl_pem_passwd_value(pass), NULL); BIO_free(bio); if (!pkey) ossl_raise(ePKeyError, "Could not parse PKey"); diff --git a/ext/openssl/ossl_pkey.h b/ext/openssl/ossl_pkey.h index 6194fbb6b..db54dfd21 100644 --- a/ext/openssl/ossl_pkey.h +++ b/ext/openssl/ossl_pkey.h @@ -29,7 +29,7 @@ extern const rb_data_type_t ossl_evp_pkey_type; /* Takes ownership of the EVP_PKEY */ VALUE ossl_pkey_new(EVP_PKEY *); void ossl_pkey_check_public_key(const EVP_PKEY *); -EVP_PKEY *ossl_pkey_read_generic(BIO *, VALUE); +EVP_PKEY *ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type); EVP_PKEY *GetPKeyPtr(VALUE); EVP_PKEY *DupPKeyPtr(VALUE); EVP_PKEY *GetPrivPKeyPtr(VALUE); diff --git a/ext/openssl/ossl_pkey_dh.c b/ext/openssl/ossl_pkey_dh.c index 3892f6dc5..f9ff45087 100644 --- a/ext/openssl/ossl_pkey_dh.c +++ b/ext/openssl/ossl_pkey_dh.c @@ -73,7 +73,6 @@ static VALUE ossl_dh_initialize(int argc, VALUE *argv, VALUE self) { EVP_PKEY *pkey; - int type; #ifndef OSSL_HAVE_PROVIDER DH *dh; #endif @@ -110,16 +109,10 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self) OSSL_BIO_reset(in); #endif - pkey = ossl_pkey_read_generic(in, Qnil); + pkey = ossl_pkey_read_generic(in, Qnil, "DH"); BIO_free(in); if (!pkey) ossl_raise(eDHError, "could not parse pkey"); - - type = EVP_PKEY_base_id(pkey); - if (type != EVP_PKEY_DH) { - EVP_PKEY_free(pkey); - rb_raise(eDHError, "incorrect pkey type: %s", OBJ_nid2sn(type)); - } RTYPEDDATA_DATA(self) = pkey; return self; diff --git a/ext/openssl/ossl_pkey_dsa.c b/ext/openssl/ossl_pkey_dsa.c index 138cc04c6..d50f46be6 100644 --- a/ext/openssl/ossl_pkey_dsa.c +++ b/ext/openssl/ossl_pkey_dsa.c @@ -89,7 +89,6 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self) #endif BIO *in = NULL; VALUE arg, pass; - int type; TypedData_Get_Struct(self, EVP_PKEY, &ossl_evp_pkey_type, pkey); if (pkey) @@ -122,16 +121,10 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self) OSSL_BIO_reset(in); #endif - pkey = ossl_pkey_read_generic(in, pass); + pkey = ossl_pkey_read_generic(in, pass, "DSA"); BIO_free(in); if (!pkey) ossl_raise(eDSAError, "Neither PUB key nor PRIV key"); - - type = EVP_PKEY_base_id(pkey); - if (type != EVP_PKEY_DSA) { - EVP_PKEY_free(pkey); - rb_raise(eDSAError, "incorrect pkey type: %s", OBJ_nid2sn(type)); - } RTYPEDDATA_DATA(self) = pkey; return self; diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c index 85f55c66d..6bab54573 100644 --- a/ext/openssl/ossl_pkey_ec.c +++ b/ext/openssl/ossl_pkey_ec.c @@ -142,7 +142,6 @@ static VALUE ossl_ec_key_initialize(int argc, VALUE *argv, VALUE self) EC_KEY *ec; BIO *in; VALUE arg, pass; - int type; TypedData_Get_Struct(self, EVP_PKEY, &ossl_evp_pkey_type, pkey); if (pkey) @@ -163,19 +162,13 @@ static VALUE ossl_ec_key_initialize(int argc, VALUE *argv, VALUE self) arg = ossl_to_der_if_possible(arg); in = ossl_obj2bio(&arg); - pkey = ossl_pkey_read_generic(in, pass); + pkey = ossl_pkey_read_generic(in, pass, "EC"); BIO_free(in); if (!pkey) { ossl_clear_error(); ec = ec_key_new_from_group(arg); goto legacy; } - - type = EVP_PKEY_base_id(pkey); - if (type != EVP_PKEY_EC) { - EVP_PKEY_free(pkey); - rb_raise(eDSAError, "incorrect pkey type: %s", OBJ_nid2sn(type)); - } RTYPEDDATA_DATA(self) = pkey; return self; diff --git a/ext/openssl/ossl_pkey_rsa.c b/ext/openssl/ossl_pkey_rsa.c index cb77d083e..036762625 100644 --- a/ext/openssl/ossl_pkey_rsa.c +++ b/ext/openssl/ossl_pkey_rsa.c @@ -82,7 +82,6 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self) #endif BIO *in = NULL; VALUE arg, pass; - int type; TypedData_Get_Struct(self, EVP_PKEY, &ossl_evp_pkey_type, pkey); if (pkey) @@ -118,16 +117,10 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self) #endif /* Use the generic routine */ - pkey = ossl_pkey_read_generic(in, pass); + pkey = ossl_pkey_read_generic(in, pass, "RSA"); BIO_free(in); if (!pkey) ossl_raise(eRSAError, "Neither PUB key nor PRIV key"); - - type = EVP_PKEY_base_id(pkey); - if (type != EVP_PKEY_RSA) { - EVP_PKEY_free(pkey); - rb_raise(eRSAError, "incorrect pkey type: %s", OBJ_nid2sn(type)); - } RTYPEDDATA_DATA(self) = pkey; return self; From dfbfc024ed61480d8531d529838a8c332a7fcf04 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Tue, 26 Jul 2022 16:59:44 +0900 Subject: [PATCH 7/7] pkey: track whether pkey is private key or not There are multiple places where it's necessary to know whether a pkey is a private key, a public key, or just key parameters. This is due to two reasons: 1. It's currently a responsibility of the caller to give a properly populated pkey instance to _some_ OpenSSL functions. For example, calling EVP_PKEY_sign() with an RSA pkey instance without the necessary components is known to cause a segfault. 2. OpenSSL::PKey::{RSA,DSA,EC}#to_der behaves differently depending on it: they use the X.509 SubjectPublicKeyInfo structure instead of private key structures if the receiver pkey is a public key. Currently, whenever this is necessary, we check the backing object, such as RSA, and see if the fields are set or not. This approach won't always work on OpenSSL 3.0 because of the architecture change. Unfortunately, OpenSSL doesn't expose an API for this purpose (even though it has one for its internal use). As a workaround, let's manually track this information in an instance variable. This has been partly done for ENGINE-backed pkeys. Now all pkeys get this flag. This is straightforward on OpenSSL 3.0 as pkeys are immutable once instantiated. On OpenSSL 1.1 or before (and current versions of LibreSSL), it must be updated whenever a modification is made to the object. This commit comes with a slight behavior change. Pkeys returned by following method will be explicitly marked as "public", even if it happens to point at an EVP_PKEY struct containing private key components. I expect the effect is minimum since these methods explicitly say "public key". - OpenSSL::X509::Certificate#public_key - OpenSSL::X509::Request#public_key - OpenSSL::Netscape::SPKI#public_key --- ext/openssl/openssl_missing.h | 1 + ext/openssl/ossl_engine.c | 5 +- ext/openssl/ossl_ns_spki.c | 2 +- ext/openssl/ossl_pkcs12.c | 2 +- ext/openssl/ossl_pkey.c | 158 +++++++++++++++++++++++----------- ext/openssl/ossl_pkey.h | 35 ++++++-- ext/openssl/ossl_pkey_dh.c | 67 +++++--------- ext/openssl/ossl_pkey_dsa.c | 103 +++++++++------------- ext/openssl/ossl_pkey_ec.c | 57 +++++++++--- ext/openssl/ossl_pkey_rsa.c | 91 +++++++------------- ext/openssl/ossl_ssl.c | 4 +- ext/openssl/ossl_x509cert.c | 2 +- ext/openssl/ossl_x509req.c | 2 +- 13 files changed, 288 insertions(+), 241 deletions(-) diff --git a/ext/openssl/openssl_missing.h b/ext/openssl/openssl_missing.h index 8629bfe50..c2a4464c7 100644 --- a/ext/openssl/openssl_missing.h +++ b/ext/openssl/openssl_missing.h @@ -140,6 +140,7 @@ IMPL_KEY_ACCESSOR3(RSA, crt_params, dmp1, dmq1, iqmp, (dmp1 == obj->dmp1 || dmq1 IMPL_PKEY_GETTER(DSA, dsa) IMPL_KEY_ACCESSOR2(DSA, key, pub_key, priv_key, (pub_key == obj->pub_key || (obj->priv_key && priv_key == obj->priv_key))) IMPL_KEY_ACCESSOR3(DSA, pqg, p, q, g, (p == obj->p || q == obj->q || g == obj->g)) +static inline ENGINE *DSA_get0_engine(DSA *dsa) { return dsa->engine; } #endif #if !defined(OPENSSL_NO_DH) diff --git a/ext/openssl/ossl_engine.c b/ext/openssl/ossl_engine.c index 1abde7f76..3041fa8e5 100644 --- a/ext/openssl/ossl_engine.c +++ b/ext/openssl/ossl_engine.c @@ -373,8 +373,7 @@ ossl_engine_load_privkey(int argc, VALUE *argv, VALUE self) GetEngine(self, e); pkey = ENGINE_load_private_key(e, sid, NULL, sdata); if (!pkey) ossl_raise(eEngineError, NULL); - obj = ossl_pkey_new(pkey); - OSSL_PKEY_SET_PRIVATE(obj); + obj = ossl_pkey_new(pkey, OSSL_PKEY_HAS_PRIVATE); return obj; } @@ -403,7 +402,7 @@ ossl_engine_load_pubkey(int argc, VALUE *argv, VALUE self) pkey = ENGINE_load_public_key(e, sid, NULL, sdata); if (!pkey) ossl_raise(eEngineError, NULL); - return ossl_pkey_new(pkey); + return ossl_pkey_new(pkey, OSSL_PKEY_HAS_PUBLIC); } /* diff --git a/ext/openssl/ossl_ns_spki.c b/ext/openssl/ossl_ns_spki.c index 9b1147367..80da905dc 100644 --- a/ext/openssl/ossl_ns_spki.c +++ b/ext/openssl/ossl_ns_spki.c @@ -190,7 +190,7 @@ ossl_spki_get_public_key(VALUE self) ossl_raise(eSPKIError, NULL); } - return ossl_pkey_new(pkey); /* NO DUP - OK */ + return ossl_pkey_new(pkey, OSSL_PKEY_HAS_PUBLIC); /* NO DUP - OK */ } /* diff --git a/ext/openssl/ossl_pkcs12.c b/ext/openssl/ossl_pkcs12.c index fb947df1d..9f8e2d51a 100644 --- a/ext/openssl/ossl_pkcs12.c +++ b/ext/openssl/ossl_pkcs12.c @@ -152,7 +152,7 @@ ossl_pkcs12_s_create(int argc, VALUE *argv, VALUE self) static VALUE ossl_pkey_new_i(VALUE arg) { - return ossl_pkey_new((EVP_PKEY *)arg); + return ossl_pkey_new((EVP_PKEY *)arg, OSSL_PKEY_HAS_PRIVATE); } static VALUE diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index 7ab1bc700..260af4465 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -19,7 +19,7 @@ VALUE mPKey; VALUE cPKey; VALUE ePKeyError; -static ID id_private_q; +ID ossl_pkey_feature_id; static void ossl_evp_pkey_free(void *ptr) @@ -65,7 +65,7 @@ pkey_new0(VALUE arg) } VALUE -ossl_pkey_new(EVP_PKEY *pkey) +ossl_pkey_new(EVP_PKEY *pkey, enum ossl_pkey_feature ps) { VALUE obj; int status; @@ -75,6 +75,7 @@ ossl_pkey_new(EVP_PKEY *pkey) EVP_PKEY_free(pkey); rb_jump_tag(status); } + ossl_pkey_set(obj, ps); return obj; } @@ -83,12 +84,13 @@ ossl_pkey_new(EVP_PKEY *pkey) # include EVP_PKEY * -ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type) +ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type, enum ossl_pkey_feature *ps) { void *ppass = (void *)pass; OSSL_DECODER_CTX *dctx; EVP_PKEY *pkey = NULL; int pos = 0, pos2; + size_t i; dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "DER", NULL, input_type, 0, NULL, NULL); if (!dctx) @@ -96,16 +98,14 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type) if (OSSL_DECODER_CTX_set_pem_password_cb(dctx, ossl_pem_passwd_cb, ppass) != 1) goto out; - /* First check DER */ - if (OSSL_DECODER_from_bio(dctx, bio) == 1) - goto out; - OSSL_BIO_reset(bio); - - /* Then check PEM; multiple OSSL_DECODER_from_bio() calls may be needed */ - if (OSSL_DECODER_CTX_set_input_type(dctx, "PEM") != 1) - goto out; /* - * First check for private key formats. This is to keep compatibility with + * This is very inefficient as it will try to decode the same DER/PEM + * encoding repeatedly, but OpenSSL 3.0 doesn't provide an API to return + * what kind of information an EVP_PKEY is holding. + * OpenSSL issue: https://github.com/openssl/openssl/issues/9467 + * + * The attempt order of selections is important: private key formats are + * checked first. This is to keep compatibility with * ruby/openssl < 3.0 which decoded the following as a private key. * * $ openssl ecparam -name prime256v1 -genkey -outform PEM @@ -125,59 +125,94 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type) * Note that normally, the input is supposed to contain a single decodable * PEM block only, so this special handling should not create a new problem. */ - OSSL_DECODER_CTX_set_selection(dctx, EVP_PKEY_KEYPAIR); - while (1) { + struct { int selection; enum ossl_pkey_feature ps; } selections[] = { +#if OSSL_OPENSSL_PREREQ(3, 0, 0) && !OSSL_OPENSSL_PREREQ(3, 0, 3) + /* + * Public key formats are checked last, with 0 (any) selection to avoid + * segfault in OpenSSL <= 3.0.3. + * Fixed by https://github.com/openssl/openssl/pull/18462 + * + * This workaround is mainly for Ubuntu 22.04, which currently has yet + * to backport it (as of 2022-07-26). + */ + { EVP_PKEY_KEYPAIR, OSSL_PKEY_HAS_PRIVATE }, + { EVP_PKEY_KEY_PARAMETERS, OSSL_PKEY_HAS_NONE }, + { 0, OSSL_PKEY_HAS_PUBLIC }, +#else + { EVP_PKEY_KEYPAIR, OSSL_PKEY_HAS_PRIVATE }, + { EVP_PKEY_PUBLIC_KEY, OSSL_PKEY_HAS_PUBLIC }, + { EVP_PKEY_KEY_PARAMETERS, OSSL_PKEY_HAS_NONE }, +#endif + }; + + /* First check DER */ + for (i = 0; i < sizeof(selections)/sizeof(selections[0]); i++) { + OSSL_DECODER_CTX_set_selection(dctx, selections[i].selection); + *ps = selections[i].ps; if (OSSL_DECODER_from_bio(dctx, bio) == 1) goto out; - if (BIO_eof(bio)) - break; - pos2 = BIO_tell(bio); - if (pos2 < 0 || pos2 <= pos) - break; - ossl_clear_error(); - pos = pos2; + OSSL_BIO_reset(bio); } - OSSL_BIO_reset(bio); - OSSL_DECODER_CTX_set_selection(dctx, 0); - while (1) { - if (OSSL_DECODER_from_bio(dctx, bio) == 1) - goto out; - if (BIO_eof(bio)) - break; - pos2 = BIO_tell(bio); - if (pos2 < 0 || pos2 <= pos) - break; - ossl_clear_error(); - pos = pos2; + /* Then check PEM; multiple OSSL_DECODER_from_bio() calls may be needed */ + if (OSSL_DECODER_CTX_set_input_type(dctx, "PEM") != 1) + goto out; + for (i = 0; i < sizeof(selections)/sizeof(selections[0]); i++) { + OSSL_DECODER_CTX_set_selection(dctx, selections[i].selection); + *ps = selections[i].ps; + while (1) { + if (OSSL_DECODER_from_bio(dctx, bio) == 1) + goto out; + if (BIO_eof(bio)) + break; + pos2 = BIO_tell(bio); + if (pos2 < 0 || pos2 <= pos) + break; + ossl_clear_error(); + pos = pos2; + } + OSSL_BIO_reset(bio); } out: OSSL_DECODER_CTX_free(dctx); +#if OSSL_OPENSSL_PREREQ(3, 0, 0) && !OSSL_OPENSSL_PREREQ(3, 0, 3) + /* It also leaks an error queue entry in the success path */ + if (pkey) ossl_clear_error(); +#endif return pkey; } #else EVP_PKEY * -ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type) +ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type, enum ossl_pkey_feature *ps) { void *ppass = (void *)pass; EVP_PKEY *pkey; + *ps = OSSL_PKEY_HAS_PRIVATE; if ((pkey = d2i_PrivateKey_bio(bio, NULL))) goto out; OSSL_BIO_reset(bio); if ((pkey = d2i_PKCS8PrivateKey_bio(bio, NULL, ossl_pem_passwd_cb, ppass))) goto out; + + *ps = OSSL_PKEY_HAS_PUBLIC; OSSL_BIO_reset(bio); if ((pkey = d2i_PUBKEY_bio(bio, NULL))) goto out; - OSSL_BIO_reset(bio); + /* PEM_read_bio_PrivateKey() also parses PKCS #8 formats */ + *ps = OSSL_PKEY_HAS_PRIVATE; + OSSL_BIO_reset(bio); if ((pkey = PEM_read_bio_PrivateKey(bio, NULL, ossl_pem_passwd_cb, ppass))) goto out; + + *ps = OSSL_PKEY_HAS_PUBLIC; OSSL_BIO_reset(bio); if ((pkey = PEM_read_bio_PUBKEY(bio, NULL, NULL, NULL))) goto out; + + *ps = OSSL_PKEY_HAS_NONE; OSSL_BIO_reset(bio); if ((pkey = PEM_read_bio_Parameters(bio, NULL))) goto out; @@ -234,14 +269,15 @@ ossl_pkey_new_from_data(int argc, VALUE *argv, VALUE self) EVP_PKEY *pkey; BIO *bio; VALUE data, pass; + enum ossl_pkey_feature ps; rb_scan_args(argc, argv, "11", &data, &pass); bio = ossl_obj2bio(&data); - pkey = ossl_pkey_read_generic(bio, ossl_pem_passwd_value(pass), NULL); + pkey = ossl_pkey_read_generic(bio, ossl_pem_passwd_value(pass), NULL, &ps); BIO_free(bio); if (!pkey) ossl_raise(ePKeyError, "Could not parse PKey"); - return ossl_pkey_new(pkey); + return ossl_pkey_new(pkey, ps); } static VALUE @@ -445,7 +481,7 @@ pkey_generate(int argc, VALUE *argv, VALUE self, int genparam) } } - return ossl_pkey_new(gen_arg.pkey); + return ossl_pkey_new(gen_arg.pkey, OSSL_PKEY_HAS_PRIVATE); } /* @@ -567,18 +603,9 @@ GetPrivPKeyPtr(VALUE obj) EVP_PKEY *pkey; GetPKey(obj, pkey); - if (OSSL_PKEY_IS_PRIVATE(obj)) - return pkey; - /* - * The EVP API does not provide a way to check if the EVP_PKEY has private - * components. Assuming it does... - */ - if (!rb_respond_to(obj, id_private_q)) - return pkey; - if (RTEST(rb_funcallv(obj, id_private_q, 0, NULL))) - return pkey; - - rb_raise(rb_eArgError, "private key is needed"); + if (!ossl_pkey_has(obj, OSSL_PKEY_HAS_PRIVATE)) + rb_raise(rb_eArgError, "private key is needed"); + return pkey; } EVP_PKEY * @@ -654,6 +681,33 @@ ossl_pkey_oid(VALUE self) return rb_str_new_cstr(OBJ_nid2sn(nid)); } +/* + * call-seq: + * pkey.public? -> true | false + * + * Indicates whether this PKey instance has a public key associated with it or + * not. + */ +static VALUE +ossl_pkey_is_public(VALUE self) +{ + return ossl_pkey_has(self, OSSL_PKEY_HAS_PUBLIC) ? Qtrue : Qfalse; +} + +/* + * call-seq: + * pkey.private? -> true | false + * + * Indicates whether this PKey instance has a private key associated with it or + * not. + */ +static VALUE +ossl_pkey_is_private(VALUE self) +{ + return ossl_pkey_has(self, OSSL_PKEY_HAS_PRIVATE) ? Qtrue : Qfalse; +} + + /* * call-seq: * pkey.inspect -> string @@ -1620,6 +1674,8 @@ Init_ossl_pkey(void) rb_undef_method(cPKey, "initialize_copy"); #endif rb_define_method(cPKey, "oid", ossl_pkey_oid, 0); + rb_define_method(cPKey, "public?", ossl_pkey_is_public, 0); + rb_define_method(cPKey, "private?", ossl_pkey_is_private, 0); rb_define_method(cPKey, "inspect", ossl_pkey_inspect, 0); rb_define_method(cPKey, "to_text", ossl_pkey_to_text, 0); rb_define_method(cPKey, "private_to_der", ossl_pkey_private_to_der, -1); @@ -1637,7 +1693,7 @@ Init_ossl_pkey(void) rb_define_method(cPKey, "encrypt", ossl_pkey_encrypt, -1); rb_define_method(cPKey, "decrypt", ossl_pkey_decrypt, -1); - id_private_q = rb_intern("private?"); + ossl_pkey_feature_id = rb_intern_const("state"); /* * INIT rsa, dsa, dh, ec diff --git a/ext/openssl/ossl_pkey.h b/ext/openssl/ossl_pkey.h index db54dfd21..74793351a 100644 --- a/ext/openssl/ossl_pkey.h +++ b/ext/openssl/ossl_pkey.h @@ -14,10 +14,7 @@ extern VALUE mPKey; extern VALUE cPKey; extern VALUE ePKeyError; extern const rb_data_type_t ossl_evp_pkey_type; - -/* For ENGINE */ -#define OSSL_PKEY_SET_PRIVATE(obj) rb_ivar_set((obj), rb_intern("private"), Qtrue) -#define OSSL_PKEY_IS_PRIVATE(obj) (rb_attr_get((obj), rb_intern("private")) == Qtrue) +extern ID ossl_pkey_feature_id; #define GetPKey(obj, pkey) do {\ TypedData_Get_Struct((obj), EVP_PKEY, &ossl_evp_pkey_type, (pkey)); \ @@ -26,10 +23,34 @@ extern const rb_data_type_t ossl_evp_pkey_type; } \ } while (0) +/* + * Store whether pkey contains public key, private key, or neither of them. + * This is ugly, but OpenSSL currently (3.0) doesn't provide a public API for + * this purpose. + */ +enum ossl_pkey_feature { + OSSL_PKEY_HAS_NONE, /* or parameters only */ + OSSL_PKEY_HAS_PUBLIC, + OSSL_PKEY_HAS_PRIVATE, +}; + +static inline void +ossl_pkey_set(VALUE self, enum ossl_pkey_feature feature) +{ + rb_ivar_set(self, ossl_pkey_feature_id, INT2FIX(feature)); +} + +static inline int +ossl_pkey_has(VALUE self, enum ossl_pkey_feature feature) +{ + return FIX2INT(rb_attr_get(self, ossl_pkey_feature_id)) >= (int)feature; +} + /* Takes ownership of the EVP_PKEY */ -VALUE ossl_pkey_new(EVP_PKEY *); +VALUE ossl_pkey_new(EVP_PKEY *pkey, enum ossl_pkey_feature ps); void ossl_pkey_check_public_key(const EVP_PKEY *); -EVP_PKEY *ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type); +EVP_PKEY *ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type, + enum ossl_pkey_feature *ps); EVP_PKEY *GetPKeyPtr(VALUE); EVP_PKEY *DupPKeyPtr(VALUE); EVP_PKEY *GetPrivPKeyPtr(VALUE); @@ -145,6 +166,7 @@ static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2, VALU BN_clear_free(bn3); \ ossl_raise(ePKeyError, #_type"_set0_"#_group); \ } \ + _keytype##_fix_selection(self, obj); \ return self; \ } @@ -172,6 +194,7 @@ static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2) \ BN_clear_free(bn2); \ ossl_raise(ePKeyError, #_type"_set0_"#_group); \ } \ + _keytype##_fix_selection(self, obj); \ return self; \ } #else /* OSSL_HAVE_PROVIDER */ diff --git a/ext/openssl/ossl_pkey_dh.c b/ext/openssl/ossl_pkey_dh.c index f9ff45087..784fe202c 100644 --- a/ext/openssl/ossl_pkey_dh.c +++ b/ext/openssl/ossl_pkey_dh.c @@ -78,6 +78,7 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self) #endif BIO *in = NULL; VALUE arg; + enum ossl_pkey_feature ps; TypedData_Get_Struct(self, EVP_PKEY, &ossl_evp_pkey_type, pkey); if (pkey) @@ -88,6 +89,7 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self) #ifdef OSSL_HAVE_PROVIDER rb_raise(eDHError, "empty DH cannot be created"); #else + ps = OSSL_PKEY_HAS_NONE; dh = DH_new(); if (!dh) ossl_raise(eDHError, "DH_new"); @@ -103,17 +105,19 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self) * On OpenSSL <= 1.1.1 and current versions of LibreSSL, the generic * routine does not support DER-encoded parameters */ + ps = OSSL_PKEY_HAS_NONE; dh = d2i_DHparams_bio(in, NULL); if (dh) goto legacy; OSSL_BIO_reset(in); #endif - pkey = ossl_pkey_read_generic(in, Qnil, "DH"); + pkey = ossl_pkey_read_generic(in, Qnil, "DH", &ps); BIO_free(in); if (!pkey) ossl_raise(eDHError, "could not parse pkey"); RTYPEDDATA_DATA(self) = pkey; + ossl_pkey_set(self, ps); return self; #ifndef OSSL_HAVE_PROVIDER @@ -126,6 +130,7 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self) ossl_raise(eDHError, "EVP_PKEY_assign_DH"); } RTYPEDDATA_DATA(self) = pkey; + ossl_pkey_set(self, ps); return self; #endif } @@ -171,48 +176,6 @@ ossl_dh_initialize_copy(VALUE self, VALUE other) } #endif -/* - * call-seq: - * dh.public? -> true | false - * - * Indicates whether this DH instance has a public key associated with it or - * not. The public key may be retrieved with DH#pub_key. - */ -static VALUE -ossl_dh_is_public(VALUE self) -{ - DH *dh; - const BIGNUM *bn; - - GetDH(self, dh); - DH_get0_key(dh, &bn, NULL); - - return bn ? Qtrue : Qfalse; -} - -/* - * call-seq: - * dh.private? -> true | false - * - * Indicates whether this DH instance has a private key associated with it or - * not. The private key may be retrieved with DH#priv_key. - */ -static VALUE -ossl_dh_is_private(VALUE self) -{ - DH *dh; - const BIGNUM *bn; - - GetDH(self, dh); - DH_get0_key(dh, NULL, &bn); - -#if !defined(OPENSSL_NO_ENGINE) - return (bn || DH_get0_engine(dh)) ? Qtrue : Qfalse; -#else - return bn ? Qtrue : Qfalse; -#endif -} - /* * call-seq: * dh.export -> aString @@ -342,6 +305,22 @@ ossl_dh_check_params(VALUE self) } } +#ifndef OSSL_HAVE_PROVIDER /* Used by OSSL_PKEY_BN_DEF* */ +static void dh_fix_selection(VALUE self, DH *dh) +{ + const BIGNUM *pub_key, *priv_key; + + DH_get0_key(dh, &pub_key, &priv_key); + + if (priv_key) + ossl_pkey_set(self, OSSL_PKEY_HAS_PRIVATE); + else if (pub_key) + ossl_pkey_set(self, OSSL_PKEY_HAS_PUBLIC); + else + ossl_pkey_set(self, OSSL_PKEY_HAS_NONE); +} +#endif + /* * Document-method: OpenSSL::PKey::DH#set_pqg * call-seq: @@ -416,8 +395,6 @@ Init_ossl_dh(void) #ifndef HAVE_EVP_PKEY_DUP rb_define_method(cDH, "initialize_copy", ossl_dh_initialize_copy, 1); #endif - rb_define_method(cDH, "public?", ossl_dh_is_public, 0); - rb_define_method(cDH, "private?", ossl_dh_is_private, 0); rb_define_method(cDH, "export", ossl_dh_export, 0); rb_define_alias(cDH, "to_pem", "export"); rb_define_alias(cDH, "to_s", "export"); diff --git a/ext/openssl/ossl_pkey_dsa.c b/ext/openssl/ossl_pkey_dsa.c index d50f46be6..8cbeec7a3 100644 --- a/ext/openssl/ossl_pkey_dsa.c +++ b/ext/openssl/ossl_pkey_dsa.c @@ -23,20 +23,6 @@ (dsa) = EVP_PKEY_get0_DSA(_pkey); \ } while (0) -static inline int -DSA_HAS_PRIVATE(DSA *dsa) -{ - const BIGNUM *bn; - DSA_get0_key(dsa, NULL, &bn); - return !!bn; -} - -static inline int -DSA_PRIVATE(VALUE obj, DSA *dsa) -{ - return DSA_HAS_PRIVATE(dsa) || OSSL_PKEY_IS_PRIVATE(obj); -} - /* * Classes */ @@ -89,6 +75,7 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self) #endif BIO *in = NULL; VALUE arg, pass; + enum ossl_pkey_feature ps; TypedData_Get_Struct(self, EVP_PKEY, &ossl_evp_pkey_type, pkey); if (pkey) @@ -100,6 +87,7 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self) #ifdef OSSL_HAVE_PROVIDER rb_raise(eDHError, "empty DSA cannot be created"); #else + ps = OSSL_PKEY_HAS_NONE; dsa = DSA_new(); if (!dsa) ossl_raise(eDSAError, "DSA_new"); @@ -113,6 +101,7 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self) #ifndef OSSL_HAVE_PROVIDER /* DER-encoded DSAPublicKey format isn't supported by the generic routine */ + ps = OSSL_PKEY_HAS_PUBLIC; dsa = (DSA *)PEM_ASN1_read_bio((d2i_of_void *)d2i_DSAPublicKey, PEM_STRING_DSA_PUBLIC, in, NULL, NULL, NULL); @@ -121,11 +110,12 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self) OSSL_BIO_reset(in); #endif - pkey = ossl_pkey_read_generic(in, pass, "DSA"); + pkey = ossl_pkey_read_generic(in, pass, "DSA", &ps); BIO_free(in); if (!pkey) ossl_raise(eDSAError, "Neither PUB key nor PRIV key"); RTYPEDDATA_DATA(self) = pkey; + ossl_pkey_set(self, ps); return self; #ifndef OSSL_HAVE_PROVIDER @@ -138,6 +128,7 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self) ossl_raise(eDSAError, "EVP_PKEY_assign_DSA"); } RTYPEDDATA_DATA(self) = pkey; + ossl_pkey_set(self, ps); return self; #endif } @@ -172,42 +163,6 @@ ossl_dsa_initialize_copy(VALUE self, VALUE other) } #endif -/* - * call-seq: - * dsa.public? -> true | false - * - * Indicates whether this DSA instance has a public key associated with it or - * not. The public key may be retrieved with DSA#public_key. - */ -static VALUE -ossl_dsa_is_public(VALUE self) -{ - DSA *dsa; - const BIGNUM *bn; - - GetDSA(self, dsa); - DSA_get0_key(dsa, &bn, NULL); - - return bn ? Qtrue : Qfalse; -} - -/* - * call-seq: - * dsa.private? -> true | false - * - * Indicates whether this DSA instance has a private key associated with it or - * not. The private key may be retrieved with DSA#private_key. - */ -static VALUE -ossl_dsa_is_private(VALUE self) -{ - DSA *dsa; - - GetDSA(self, dsa); - - return DSA_PRIVATE(self, dsa) ? Qtrue : Qfalse; -} - /* * call-seq: * dsa.export([cipher, password]) -> aString @@ -228,13 +183,19 @@ ossl_dsa_is_private(VALUE self) static VALUE ossl_dsa_export(int argc, VALUE *argv, VALUE self) { + EVP_PKEY *pkey; +#ifndef OSSL_HAVE_PROVIDER DSA *dsa; - GetDSA(self, dsa); - if (DSA_HAS_PRIVATE(dsa)) +#endif + + GetPKey(self, pkey); + if (ossl_pkey_has(self, OSSL_PKEY_HAS_PRIVATE)) +#ifndef OSSL_HAVE_PROVIDER + if (!DSA_get0_engine(dsa)) +#endif return ossl_pkey_export_traditional(argc, argv, self, 0); - else - return ossl_pkey_export_spki(self, 0); + return ossl_pkey_export_spki(self, 0); } /* @@ -247,13 +208,19 @@ ossl_dsa_export(int argc, VALUE *argv, VALUE self) static VALUE ossl_dsa_to_der(VALUE self) { + EVP_PKEY *pkey; +#ifndef OSSL_HAVE_PROVIDER DSA *dsa; - GetDSA(self, dsa); - if (DSA_HAS_PRIVATE(dsa)) +#endif + + GetPKey(self, pkey); + if (ossl_pkey_has(self, OSSL_PKEY_HAS_PRIVATE)) +#ifndef OSSL_HAVE_PROVIDER + if (!DSA_get0_engine(dsa)) +#endif return ossl_pkey_export_traditional(0, NULL, self, 1); - else - return ossl_pkey_export_spki(self, 1); + return ossl_pkey_export_spki(self, 1); } @@ -286,6 +253,22 @@ ossl_dsa_get_params(VALUE self) return hash; } +#ifndef OSSL_HAVE_PROVIDER /* Used by OSSL_PKEY_BN_DEF* */ +static void dsa_fix_selection(VALUE self, DSA *dsa) +{ + const BIGNUM *pub_key, *priv_key; + + DSA_get0_key(dsa, &pub_key, &priv_key); + + if (priv_key) + ossl_pkey_set(self, OSSL_PKEY_HAS_PRIVATE); + else if (pub_key) + ossl_pkey_set(self, OSSL_PKEY_HAS_PUBLIC); + else + ossl_pkey_set(self, OSSL_PKEY_HAS_NONE); +} +#endif + /* * Document-method: OpenSSL::PKey::DSA#set_pqg * call-seq: @@ -336,8 +319,6 @@ Init_ossl_dsa(void) rb_define_method(cDSA, "initialize_copy", ossl_dsa_initialize_copy, 1); #endif - rb_define_method(cDSA, "public?", ossl_dsa_is_public, 0); - rb_define_method(cDSA, "private?", ossl_dsa_is_private, 0); rb_define_method(cDSA, "export", ossl_dsa_export, -1); rb_define_alias(cDSA, "to_pem", "export"); rb_define_alias(cDSA, "to_s", "export"); diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c index 6bab54573..9ce205326 100644 --- a/ext/openssl/ossl_pkey_ec.c +++ b/ext/openssl/ossl_pkey_ec.c @@ -118,6 +118,7 @@ ossl_ec_key_s_generate(VALUE klass, VALUE arg) ossl_raise(eECError, "EVP_PKEY_assign_EC_KEY"); } RTYPEDDATA_DATA(obj) = pkey; + ossl_pkey_set(obj, OSSL_PKEY_HAS_PRIVATE); if (!EC_KEY_generate_key(ec)) ossl_raise(eECError, "EC_KEY_generate_key"); @@ -142,6 +143,7 @@ static VALUE ossl_ec_key_initialize(int argc, VALUE *argv, VALUE self) EC_KEY *ec; BIO *in; VALUE arg, pass; + enum ossl_pkey_feature ps; TypedData_Get_Struct(self, EVP_PKEY, &ossl_evp_pkey_type, pkey); if (pkey) @@ -149,11 +151,13 @@ static VALUE ossl_ec_key_initialize(int argc, VALUE *argv, VALUE self) rb_scan_args(argc, argv, "02", &arg, &pass); if (NIL_P(arg)) { + ps = OSSL_PKEY_HAS_NONE; if (!(ec = EC_KEY_new())) ossl_raise(eECError, "EC_KEY_new"); goto legacy; } else if (rb_obj_is_kind_of(arg, cEC_GROUP)) { + ps = OSSL_PKEY_HAS_NONE; ec = ec_key_new_from_group(arg); goto legacy; } @@ -162,14 +166,16 @@ static VALUE ossl_ec_key_initialize(int argc, VALUE *argv, VALUE self) arg = ossl_to_der_if_possible(arg); in = ossl_obj2bio(&arg); - pkey = ossl_pkey_read_generic(in, pass, "EC"); + pkey = ossl_pkey_read_generic(in, pass, "EC", &ps); BIO_free(in); if (!pkey) { ossl_clear_error(); + ps = OSSL_PKEY_HAS_NONE; ec = ec_key_new_from_group(arg); goto legacy; } RTYPEDDATA_DATA(self) = pkey; + ossl_pkey_set(self, ps); return self; legacy: @@ -180,6 +186,7 @@ static VALUE ossl_ec_key_initialize(int argc, VALUE *argv, VALUE self) ossl_raise(eECError, "EVP_PKEY_assign_EC_KEY"); } RTYPEDDATA_DATA(self) = pkey; + ossl_pkey_set(self, ps); return self; } @@ -257,6 +264,18 @@ ossl_ec_key_set_group(VALUE self, VALUE group_v) #endif } +#ifndef OSSL_HAVE_PROVIDER +static void ec_key_fix_selection(VALUE self, EC_KEY *ec) +{ + if (EC_KEY_get0_private_key(ec)) + ossl_pkey_set(self, OSSL_PKEY_HAS_PRIVATE); + else if (EC_KEY_get0_public_key(ec)) + ossl_pkey_set(self, OSSL_PKEY_HAS_PUBLIC); + else + ossl_pkey_set(self, OSSL_PKEY_HAS_NONE); +} +#endif + /* * call-seq: * key.private_key => OpenSSL::BN @@ -294,15 +313,16 @@ static VALUE ossl_ec_key_set_private_key(VALUE self, VALUE private_key) bn = GetBNPtr(private_key); switch (EC_KEY_set_private_key(ec, bn)) { - case 1: + case 1: break; - case 0: + case 0: if (bn == NULL) break; - /* fallthrough */ - default: + /* fallthrough */ + default: ossl_raise(eECError, "EC_KEY_set_private_key"); } + ec_key_fix_selection(self, ec); return private_key; #endif @@ -354,6 +374,7 @@ static VALUE ossl_ec_key_set_public_key(VALUE self, VALUE public_key) default: ossl_raise(eECError, "EC_KEY_set_public_key"); } + ec_key_fix_selection(self, ec); return public_key; #endif @@ -404,15 +425,21 @@ static VALUE ossl_ec_key_is_private(VALUE self) static VALUE ossl_ec_key_export(int argc, VALUE *argv, VALUE self) { + EVP_PKEY *pkey; +#ifndef OSSL_HAVE_PROVIDER EC_KEY *ec; - GetEC(self, ec); if (EC_KEY_get0_public_key(ec) == NULL) ossl_raise(eECError, "can't export - no public key set"); +#endif + + GetPKey(self, pkey); + if (ossl_pkey_has(self, OSSL_PKEY_HAS_PRIVATE)) +#ifndef OSSL_HAVE_PROVIDER if (EC_KEY_get0_private_key(ec)) +#endif return ossl_pkey_export_traditional(argc, argv, self, 0); - else - return ossl_pkey_export_spki(self, 0); + return ossl_pkey_export_spki(self, 0); } /* @@ -424,16 +451,23 @@ ossl_ec_key_export(int argc, VALUE *argv, VALUE self) static VALUE ossl_ec_key_to_der(VALUE self) { + EVP_PKEY *pkey; +#ifndef OSSL_HAVE_PROVIDER EC_KEY *ec; - GetEC(self, ec); if (EC_KEY_get0_public_key(ec) == NULL) ossl_raise(eECError, "can't export - no public key set"); +#endif + + GetPKey(self, pkey); + if (ossl_pkey_has(self, OSSL_PKEY_HAS_PRIVATE)) +#ifndef OSSL_HAVE_PROVIDER if (EC_KEY_get0_private_key(ec)) +#endif return ossl_pkey_export_traditional(0, NULL, self, 1); - else - return ossl_pkey_export_spki(self, 1); + return ossl_pkey_export_spki(self, 1); } + /* * call-seq: * key.generate_key! => self @@ -458,6 +492,7 @@ static VALUE ossl_ec_key_generate_key(VALUE self) GetEC(self, ec); if (EC_KEY_generate_key(ec) != 1) ossl_raise(eECError, "EC_KEY_generate_key"); + ossl_pkey_set(self, OSSL_PKEY_HAS_PRIVATE); return self; #endif diff --git a/ext/openssl/ossl_pkey_rsa.c b/ext/openssl/ossl_pkey_rsa.c index 036762625..2ac80bbdf 100644 --- a/ext/openssl/ossl_pkey_rsa.c +++ b/ext/openssl/ossl_pkey_rsa.c @@ -23,21 +23,6 @@ (rsa) = EVP_PKEY_get0_RSA(_pkey); \ } while (0) -static inline int -RSA_HAS_PRIVATE(RSA *rsa) -{ - const BIGNUM *e, *d; - - RSA_get0_key(rsa, NULL, &e, &d); - return e && d; -} - -static inline int -RSA_PRIVATE(VALUE obj, RSA *rsa) -{ - return RSA_HAS_PRIVATE(rsa) || OSSL_PKEY_IS_PRIVATE(obj); -} - /* * Classes */ @@ -82,6 +67,7 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self) #endif BIO *in = NULL; VALUE arg, pass; + enum ossl_pkey_feature ps; TypedData_Get_Struct(self, EVP_PKEY, &ossl_evp_pkey_type, pkey); if (pkey) @@ -106,6 +92,7 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self) #ifndef OSSL_HAVE_PROVIDER /* First try RSAPublicKey format */ + ps = OSSL_PKEY_HAS_PUBLIC; rsa = d2i_RSAPublicKey_bio(in, NULL); if (rsa) goto legacy; @@ -117,11 +104,12 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self) #endif /* Use the generic routine */ - pkey = ossl_pkey_read_generic(in, pass, "RSA"); + pkey = ossl_pkey_read_generic(in, pass, "RSA", &ps); BIO_free(in); if (!pkey) ossl_raise(eRSAError, "Neither PUB key nor PRIV key"); RTYPEDDATA_DATA(self) = pkey; + ossl_pkey_set(self, ps); return self; #ifndef OSSL_HAVE_PROVIDER @@ -134,6 +122,7 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self) ossl_raise(eRSAError, "EVP_PKEY_assign_RSA"); } RTYPEDDATA_DATA(self) = pkey; + ossl_pkey_set(self, ps); return self; #endif } @@ -167,55 +156,25 @@ ossl_rsa_initialize_copy(VALUE self, VALUE other) } #endif -/* - * call-seq: - * rsa.public? => true - * - * The return value is always +true+ since every private key is also a public - * key. - */ -static VALUE -ossl_rsa_is_public(VALUE self) -{ - RSA *rsa; - - GetRSA(self, rsa); - /* - * This method should check for n and e. BUG. - */ - (void)rsa; - return Qtrue; -} - -/* - * call-seq: - * rsa.private? => true | false - * - * Does this keypair contain a private key? - */ -static VALUE -ossl_rsa_is_private(VALUE self) -{ - RSA *rsa; - - GetRSA(self, rsa); - - return RSA_PRIVATE(self, rsa) ? Qtrue : Qfalse; -} - static int can_export_rsaprivatekey(VALUE self) { +#ifndef OSSL_HAVE_PROVIDER RSA *rsa; - const BIGNUM *n, *e, *d, *p, *q, *dmp1, *dmq1, *iqmp; + const BIGNUM *p, *q, *dmp1, *dmq1, *iqmp; GetRSA(self, rsa); - - RSA_get0_key(rsa, &n, &e, &d); RSA_get0_factors(rsa, &p, &q); RSA_get0_crt_params(rsa, &dmp1, &dmq1, &iqmp); +#endif - return n && e && d && p && q && dmp1 && dmq1 && iqmp; + if (ossl_pkey_has(self, OSSL_PKEY_HAS_PRIVATE)) +#ifndef OSSL_HAVE_PROVIDER + /* OSSL_PKEY_HAS_PRIVATE implies n/e/d are present */ + if (p && q && dmp1 && dmq1 && iqmp) +#endif + return 1; + return 0; } /* @@ -478,6 +437,24 @@ ossl_rsa_get_params(VALUE self) return hash; } +#ifndef OSSL_HAVE_PROVIDER /* Used by OSSL_PKEY_BN_DEF* */ +static void rsa_fix_selection(VALUE self, RSA *rsa) +{ + const BIGNUM *n, *e, *d, *p, *q, *dmp1, *dmq1, *iqmp; + + RSA_get0_key(rsa, &n, &e, &d); + RSA_get0_factors(rsa, &p, &q); + RSA_get0_crt_params(rsa, &dmp1, &dmq1, &iqmp); + + if (n && e && d) + ossl_pkey_set(self, OSSL_PKEY_HAS_PRIVATE); + else if (n && e) + ossl_pkey_set(self, OSSL_PKEY_HAS_PUBLIC); + else + ossl_pkey_set(self, OSSL_PKEY_HAS_NONE); +} +#endif + /* * Document-method: OpenSSL::PKey::RSA#set_key * call-seq: @@ -544,8 +521,6 @@ Init_ossl_rsa(void) rb_define_method(cRSA, "initialize_copy", ossl_rsa_initialize_copy, 1); #endif - rb_define_method(cRSA, "public?", ossl_rsa_is_public, 0); - rb_define_method(cRSA, "private?", ossl_rsa_is_private, 0); rb_define_method(cRSA, "export", ossl_rsa_export, -1); rb_define_alias(cRSA, "to_pem", "export"); rb_define_alias(cRSA, "to_s", "export"); diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 478ff869a..1aedae246 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -2541,7 +2541,7 @@ ossl_ssl_export_keying_material(int argc, VALUE *argv, VALUE self) * call-seq: * ssl.tmp_key => PKey or nil * - * Returns the ephemeral key used in case of forward secrecy cipher. + * Returns the temporary key provided by the peer and used during key exchange. */ static VALUE ossl_ssl_tmp_key(VALUE self) @@ -2552,7 +2552,7 @@ ossl_ssl_tmp_key(VALUE self) GetSSL(self, ssl); if (!SSL_get_server_tmp_key(ssl, &key)) return Qnil; - return ossl_pkey_new(key); + return ossl_pkey_new(key, OSSL_PKEY_HAS_PUBLIC); } #endif /* !defined(OPENSSL_NO_SOCK) */ diff --git a/ext/openssl/ossl_x509cert.c b/ext/openssl/ossl_x509cert.c index 944354164..f291d18f4 100644 --- a/ext/openssl/ossl_x509cert.c +++ b/ext/openssl/ossl_x509cert.c @@ -506,7 +506,7 @@ ossl_x509_get_public_key(VALUE self) ossl_raise(eX509CertError, NULL); } - return ossl_pkey_new(pkey); /* NO DUP - OK */ + return ossl_pkey_new(pkey, OSSL_PKEY_HAS_PUBLIC); /* NO DUP - OK */ } /* diff --git a/ext/openssl/ossl_x509req.c b/ext/openssl/ossl_x509req.c index 77a7d3f2f..5be163c32 100644 --- a/ext/openssl/ossl_x509req.c +++ b/ext/openssl/ossl_x509req.c @@ -286,7 +286,7 @@ ossl_x509req_get_public_key(VALUE self) ossl_raise(eX509ReqError, NULL); } - return ossl_pkey_new(pkey); /* NO DUP - OK */ + return ossl_pkey_new(pkey, OSSL_PKEY_HAS_PUBLIC); /* NO DUP - OK */ } static VALUE