From 5ff4a3162182c6adf1c780ccaf9ce32895f87a6c Mon Sep 17 00:00:00 2001 From: Jun Aruga Date: Wed, 12 Apr 2023 17:15:21 +0200 Subject: [PATCH 1/3] Workaround: Fix OpenSSL::PKey.read that cannot parse PKey in the FIPS mode. This commit is a workaround to avoid the error below that the `OpenSSL::PKey.read` fails with the OpenSSL 3.0 FIPS mode. ``` $ openssl genrsa -out key.pem 4096 $ ruby -e "require 'openssl'; OpenSSL::PKey.read(File.read('key.pem'))" -e:1:in `read': Could not parse PKey (OpenSSL::PKey::PKeyError) from -e:1:in `
' ``` The root cause is on the OpenSSL side. The `OSSL_DECODER_CTX_set_selection` doesn't apply the selection value properly if there are multiple providers, and a provider (e.g. "base" provider) handles the decoder implementation, and another provider (e.g. "fips" provider) handles the keys. The workaround is to create `OSSL_DECODER_CTX` variable each time without using the `OSSL_DECODER_CTX_set_selection`. --- ext/openssl/ossl_pkey.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index 476256679..a8e97d0ba 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -101,10 +101,9 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass) 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; /* + * Then check PEM; multiple OSSL_DECODER_from_bio() calls may be needed. + * * First check for private key formats. This is to keep compatibility with * ruby/openssl < 3.0 which decoded the following as a private key. * @@ -124,8 +123,19 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass) * * 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. + * + * Note that we need to create the OSSL_DECODER_CTX variable each time when + * we use the different selection as a workaround. + * https://github.com/openssl/openssl/issues/20657 */ - OSSL_DECODER_CTX_set_selection(dctx, EVP_PKEY_KEYPAIR); + OSSL_DECODER_CTX_free(dctx); + dctx = NULL; + dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "PEM", NULL, NULL, + EVP_PKEY_KEYPAIR, NULL, NULL); + if (!dctx) + goto out; + if (OSSL_DECODER_CTX_set_pem_password_cb(dctx, ossl_pem_passwd_cb, ppass) != 1) + goto out; while (1) { if (OSSL_DECODER_from_bio(dctx, bio) == 1) goto out; @@ -139,7 +149,13 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass) } OSSL_BIO_reset(bio); - OSSL_DECODER_CTX_set_selection(dctx, 0); + OSSL_DECODER_CTX_free(dctx); + dctx = NULL; + dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "PEM", NULL, NULL, 0, NULL, NULL); + if (!dctx) + goto out; + if (OSSL_DECODER_CTX_set_pem_password_cb(dctx, ossl_pem_passwd_cb, ppass) != 1) + goto out; while (1) { if (OSSL_DECODER_from_bio(dctx, bio) == 1) goto out; From ab92baff34097fbb9e74815eab1b72185b250cc6 Mon Sep 17 00:00:00 2001 From: Jun Aruga Date: Thu, 13 Apr 2023 17:28:27 +0200 Subject: [PATCH 2/3] Drop a common logic disabling the FIPS mode in the tests. We want to run the unit tests in the FIPS mode too. --- test/openssl/utils.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/openssl/utils.rb b/test/openssl/utils.rb index e474fccaf..f00084ffd 100644 --- a/test/openssl/utils.rb +++ b/test/openssl/utils.rb @@ -1,11 +1,6 @@ # frozen_string_literal: true begin require "openssl" - - # Disable FIPS mode for tests for installations - # where FIPS mode would be enabled by default. - # Has no effect on all other installations. - OpenSSL.fips_mode=false rescue LoadError end From 8149cdf6e874214f9349f1f236b003d9239228f9 Mon Sep 17 00:00:00 2001 From: Jun Aruga Date: Mon, 17 Apr 2023 19:05:48 +0200 Subject: [PATCH 3/3] CI: Add the test/openssl/test_pkey.rb on the FIPS mode case. It's to test the `OpenSSL::PKey.read` in the `test/openssl/test_pkey.rb`. I added the pending status to the following tests failing on the FIPS mode case in the `test/openssl/test_pkey.rb`. * `test_ed25519` * `test_x25519` * `test_compare?` --- .github/workflows/test.yml | 4 +++- test/openssl/test_pkey.rb | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index becf99029..f60f99bea 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -149,5 +149,7 @@ jobs: # Run only the passing tests on the FIPS mode as a temporary workaround. # TODO Fix other tests, and run all the tests on FIPS mode. - name: test on fips mode - run: ruby -Ilib test/openssl/test_fips.rb + run: | + ruby -I./lib -ropenssl \ + -e 'Dir.glob "./test/openssl/{test_fips.rb,test_pkey.rb}", &method(:require)' if: matrix.fips_enabled diff --git a/test/openssl/test_pkey.rb b/test/openssl/test_pkey.rb index 2b99e8f37..2cd5290f4 100644 --- a/test/openssl/test_pkey.rb +++ b/test/openssl/test_pkey.rb @@ -82,6 +82,9 @@ def test_hmac_sign_verify end def test_ed25519 + # https://github.com/openssl/openssl/issues/20758 + pend('Not supported on FIPS mode enabled') if OpenSSL.fips_mode + # Test vector from RFC 8032 Section 7.1 TEST 2 priv_pem = <<~EOF -----BEGIN PRIVATE KEY----- @@ -127,6 +130,8 @@ def test_ed25519 end def test_x25519 + pend('Not supported on FIPS mode enabled') if OpenSSL.fips_mode + # Test vector from RFC 7748 Section 6.1 alice_pem = <<~EOF -----BEGIN PRIVATE KEY----- @@ -153,6 +158,8 @@ def test_x25519 end def test_compare? + pend('Not supported on FIPS mode enabled') if OpenSSL.fips_mode + key1 = Fixtures.pkey("rsa1024") key2 = Fixtures.pkey("rsa1024") key3 = Fixtures.pkey("rsa2048")