Merge pull request ruby#615 from junaruga/wip/fips-read

Fix OpenSSL::PKey.read that cannot parse PKey in the FIPS mode.
rhenium · Jun 1, 2023 · 7e411b4 · 7e411b4
2 parents 5f505c5 + 8149cdf
commit 7e411b4
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 11 deletions.
diff --git 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/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;

diff --git 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")

diff --git 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