Merge pull request #681 from junaruga/wip/fix-test-pkey-ec

Fix test_pkey_ec.rb on FIPS.

Author: rhenium

.github/workflows/test.yml

@@ -148,10 +148,6 @@ jobs:
         run: echo "OPENSSL_CONF=$(pwd)/test/openssl/fixtures/ssl/openssl_fips.cnf" >> $GITHUB_ENV
         if: matrix.fips-enabled
 
-      - name: set fips environment variable for testing.
-        run: echo "TEST_RUBY_OPENSSL_FIPS_ENABLED=true" >> $GITHUB_ENV
-        if: matrix.fips-enabled
-
       - name: load ruby
         uses: ruby/setup-ruby@v1
         with:
@@ -180,7 +176,5 @@ jobs:
       # TODO Fix other tests, and run all the tests on FIPS module.
       - name: test on fips module
         run:  |
-          bundle exec rake debug &&
-            ruby -I./lib -ropenssl \
-            -e 'Dir.glob "./test/openssl/{test_fips.rb,test_pkey.rb}", &method(:require)'
+          rake test_fips TESTOPTS="-v --no-show-detail-immediately"
         if: matrix.fips-enabled

Rakefile

@@ -18,12 +18,29 @@ Rake::TestTask.new do |t|
   t.warning = true
 end
 
+desc 'Run tests for fips'
+task :test_fips do
+  ENV['TEST_RUBY_OPENSSL_FIPS_ENABLED'] = 'true'
+  Rake::Task['test_fips_internal'].invoke
+end
+
+Rake::TestTask.new(:test_fips_internal) do |t|
+  t.libs << 'test/openssl'
+  t.test_files = FileList[
+    'test/openssl/test_fips.rb',
+    'test/openssl/test_pkey.rb',
+    'test/openssl/test_pkey_ec.rb',
+  ]
+  t.warning = true
+end
+
 RDoc::Task.new do |rdoc|
   rdoc.main = "README.md"
   rdoc.rdoc_files.include("*.md", "lib/**/*.rb", "ext/**/*.c")
 end
 
 task :test => [:compile, :debug]
+task :test_fips => [:compile, :debug]
 
 # Print Ruby and compiler info for debugging purpose.
 task :debug_compiler do

test/openssl/test_fips.rb

@@ -28,8 +28,10 @@ def test_fips_mode_get_is_false_on_fips_mode_disabled
   end
 
   def test_fips_mode_is_reentrant
-    OpenSSL.fips_mode = false
-    OpenSSL.fips_mode = false
+    assert_separately(["-ropenssl"], <<~"end;")
+      OpenSSL.fips_mode = false
+      OpenSSL.fips_mode = false
+    end;
   end
 
   def test_fips_mode_get_with_fips_mode_set

test/openssl/test_pkey.rb

@@ -82,6 +82,9 @@ def test_hmac_sign_verify
   end
 
   def test_ed25519
+    # Ed25519 is not FIPS-approved.
+    omit_on_fips
+
     # Test vector from RFC 8032 Section 7.1 TEST 2
     priv_pem = <<~EOF
     -----BEGIN PRIVATE KEY-----
@@ -96,15 +99,11 @@ def test_ed25519
     begin
       priv = OpenSSL::PKey.read(priv_pem)
       pub = OpenSSL::PKey.read(pub_pem)
-    rescue OpenSSL::PKey::PKeyError
+    rescue OpenSSL::PKey::PKeyError => e
       # OpenSSL < 1.1.1
-      if !openssl?(1, 1, 1)
-        pend "Ed25519 is not implemented"
-      elsif OpenSSL.fips_mode && openssl?(3, 1, 0, 0)
-        # See OpenSSL providers/fips/fipsprov.c PROV_NAMES_ED25519 entries
-        # with FIPS_UNAPPROVED_PROPERTIES in OpenSSL 3.1+.
-        pend "Ed25519 is not approved in OpenSSL 3.1+ FIPS code"
-      end
+      pend "Ed25519 is not implemented" unless openssl?(1, 1, 1)
+
+      raise e
     end
     assert_instance_of OpenSSL::PKey::PKey, priv
     assert_instance_of OpenSSL::PKey::PKey, pub
@@ -145,6 +144,32 @@ def test_ed25519
     assert_raise(OpenSSL::PKey::PKeyError) { priv.derive(pub) }
   end
 
+  def test_ed25519_not_approved_on_fips
+    omit_on_non_fips
+    # Ed25519 is technically allowed in the OpenSSL 3.0 code as a kind of bug.
+    # So, we need to omit OpenSSL 3.0.
+    #
+    # See OpenSSL providers/fips/fipsprov.c PROV_NAMES_ED25519 entries with
+    # FIPS_DEFAULT_PROPERTIES on openssl-3.0 branch and
+    # FIPS_UNAPPROVED_PROPERTIES on openssl-3.1 branch.
+    #
+    # See also
+    # https://github.com/openssl/openssl/issues/20758#issuecomment-1639658102
+    # for details.
+    unless openssl?(3, 1, 0, 0)
+      omit 'Ed25519 is allowed in the OpenSSL 3.0 FIPS code as a kind of bug'
+    end
+
+    priv_pem = <<~EOF
+    -----BEGIN PRIVATE KEY-----
+    MC4CAQAwBQYDK2VwBCIEIEzNCJso/5banbbDRuwRTg9bijGfNaumJNqM9u1PuKb7
+    -----END PRIVATE KEY-----
+    EOF
+    assert_raise(OpenSSL::PKey::PKeyError) do
+      OpenSSL::PKey.read(priv_pem)
+    end
+  end
+
   def test_x25519
     # Test vector from RFC 7748 Section 6.1
     alice_pem = <<~EOF

test/openssl/test_pkey_ec.rb

@@ -229,6 +229,8 @@ def test_ECPrivateKey_with_parameters
   end
 
   def test_ECPrivateKey_encrypted
+    omit_on_fips
+
     p256 = Fixtures.pkey("p256")
     # key = abcdef
     pem = <<~EOF

test/openssl/utils.rb

@@ -139,6 +139,26 @@ def teardown
     # OpenSSL error stack must be empty
     assert_equal([], OpenSSL.errors)
   end
+
+  # Omit the tests in FIPS.
+  #
+  # For example, the password based encryption used in the PEM format uses MD5
+  # for deriving the encryption key from the password, and MD5 is not
+  # FIPS-approved.
+  #
+  # See https://github.com/openssl/openssl/discussions/21830#discussioncomment-6865636
+  # for details.
+  def omit_on_fips
+    return unless OpenSSL.fips_mode
+
+    omit 'An encryption used in the test is not FIPS-approved'
+  end
+
+  def omit_on_non_fips
+    return if OpenSSL.fips_mode
+
+    omit "Only for OpenSSL FIPS"
+  end
 end
 
 class OpenSSL::SSLTestCase < OpenSSL::TestCase