From 75e3d2dc93bb346beddf63f4bc2a6c9913c023cf Mon Sep 17 00:00:00 2001 From: Dirkjan Bussink Date: Tue, 23 Aug 2022 09:29:48 +0000 Subject: [PATCH 1/2] Fix parsing EC private keys with EC parameters The logic used with OpenSSL 1.1.1 allows for parsing private keys with EC parameters in the PEM format. The OpenSSL 3.0 logic doesn't allow for this and breaks loading these types of private keys. The parsing loop here is changed to continue parsing PEM blocks also if it succeeds to parse a PEM block, like for EC parameters. We want to ensure that all valid PEM blocks are loaded, not only the first one. Combined with it also trying to parse non valid PEM bits, it means we always want to continue parsing until we hit EOF on the BIO, or if we no longer see progress. These two checks were already implemented, so all that's needed is to unconditionally try to grab all PEM blocks. --- ext/openssl/ossl_pkey.c | 3 ++- test/openssl/test_pkey_ec.rb | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index 24d0da468..ce41b6dbf 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -104,7 +104,8 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass) OSSL_BIO_reset(bio); if (OSSL_DECODER_CTX_set_input_type(dctx, "PEM") != 1) goto out; - while (OSSL_DECODER_from_bio(dctx, bio) != 1) { + for (;;) { + OSSL_DECODER_from_bio(dctx, bio); if (BIO_eof(bio)) goto out; pos2 = BIO_tell(bio); diff --git a/test/openssl/test_pkey_ec.rb b/test/openssl/test_pkey_ec.rb index ffe5a94e5..877243016 100644 --- a/test/openssl/test_pkey_ec.rb +++ b/test/openssl/test_pkey_ec.rb @@ -199,6 +199,45 @@ def test_ECPrivateKey assert_equal pem, p256.export end + def test_ECPrivateKey_with_parameters + p256 = Fixtures.pkey("p256") + asn1 = OpenSSL::ASN1::Sequence([ + OpenSSL::ASN1::Integer(1), + OpenSSL::ASN1::OctetString(p256.private_key.to_s(2)), + OpenSSL::ASN1::ObjectId("prime256v1", 0, :EXPLICIT), + OpenSSL::ASN1::BitString(p256.public_key.to_octet_string(:uncompressed), + 1, :EXPLICIT) + ]) + key = OpenSSL::PKey::EC.new(asn1.to_der) + assert_predicate key, :private? + assert_same_ec p256, key + + in_pem = <<~EOF + -----BEGIN EC PARAMETERS----- + BggqhkjOPQMBBw== + -----END EC PARAMETERS----- + -----BEGIN EC PRIVATE KEY----- + MHcCAQEEIID49FDqcf1O1eO8saTgG70UbXQw9Fqwseliit2aWhH1oAoGCCqGSM49 + AwEHoUQDQgAEFglk2c+oVUIKQ64eZG9bhLNPWB7lSZ/ArK41eGy5wAzU/0G51Xtt + CeBUl+MahZtn9fO1JKdF4qJmS39dXnpENg== + -----END EC PRIVATE KEY----- + EOF + + out_pem = <<~EOF + -----BEGIN EC PRIVATE KEY----- + MHcCAQEEIID49FDqcf1O1eO8saTgG70UbXQw9Fqwseliit2aWhH1oAoGCCqGSM49 + AwEHoUQDQgAEFglk2c+oVUIKQ64eZG9bhLNPWB7lSZ/ArK41eGy5wAzU/0G51Xtt + CeBUl+MahZtn9fO1JKdF4qJmS39dXnpENg== + -----END EC PRIVATE KEY----- + EOF + + key = OpenSSL::PKey::EC.new(in_pem) + assert_same_ec p256, key + + assert_equal asn1.to_der, p256.to_der + assert_equal out_pem, p256.export + end + def test_ECPrivateKey_encrypted p256 = Fixtures.pkey("p256") # key = abcdef From de170c0658177e15461d9de7858dd8ab4e723eef Mon Sep 17 00:00:00 2001 From: Dirkjan Bussink Date: Tue, 23 Aug 2022 15:58:26 +0000 Subject: [PATCH 2/2] Add check for parsed EC key It's possible that an EC key is parsed with just parameters, but it's not usable at that point as a valid public or private key. So we error in that case for now. --- ext/openssl/ossl_pkey.c | 11 +++++++++++ test/openssl/test_pkey.rb | 10 ++++++++++ 2 files changed, 21 insertions(+) diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index ce41b6dbf..1a8bc0294 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -116,6 +116,17 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass) out: OSSL_DECODER_CTX_free(dctx); + +#if !defined(OPENSSL_NO_EC) + if (pkey && EVP_PKEY_base_id(pkey) == EVP_PKEY_EC) { + const EC_KEY *ec_key = EVP_PKEY_get0_EC_KEY(pkey); + if (!EC_KEY_get0_public_key(ec_key) && !EC_KEY_get0_private_key(ec_key)) { + EVP_PKEY_free(pkey); + pkey = NULL; + } + } +#endif + return pkey; } #else diff --git a/test/openssl/test_pkey.rb b/test/openssl/test_pkey.rb index 544340e37..ca91ba6f7 100644 --- a/test/openssl/test_pkey.rb +++ b/test/openssl/test_pkey.rb @@ -147,6 +147,16 @@ def test_x25519 assert_equal [shared_secret].pack("H*"), alice.derive(bob) end + def test_invalid_pkey_valid_ecparam + priv_pem = <<~EOF + -----BEGIN EC PARAMETERS----- + BggqhkjOPQMBBw== + -----END EC PARAMETERS----- + EOF + + assert_raise(OpenSSL::PKey::PKeyError) { OpenSSL::PKey.read(priv_pem) } + end + def test_compare? key1 = Fixtures.pkey("rsa1024") key2 = Fixtures.pkey("rsa1024")