From 29afe47d728baf62329eb4d3c5ef405f5e77b6cb Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 11 Jul 2025 00:56:07 +0900 Subject: [PATCH 1/5] Rakefile: do not add test/openssl to the load paths The directory used to contain a copy of envutil.rb. It is now gone since we started using the test-unit-ruby-core gem. The top-level lib directory is automatically added by Rake::TestTask. --- Rakefile | 2 -- 1 file changed, 2 deletions(-) diff --git a/Rakefile b/Rakefile index c92c8f534..237b634b5 100644 --- a/Rakefile +++ b/Rakefile @@ -13,7 +13,6 @@ rescue LoadError end Rake::TestTask.new do |t| - t.libs << 'test/openssl' t.test_files = FileList["test/**/test_*.rb"] t.warning = true end @@ -25,7 +24,6 @@ task :test_fips do end Rake::TestTask.new(:test_fips_internal) do |t| - t.libs << 'test/openssl' # Exclude failing test files in FIPS for this task to pass. # TODO: Fix failing test files. t.test_files = FileList['test/**/test_*.rb'] - FileList[ From 77133c6fc2aff9195434bde5a70826a89cb269bf Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 11 Jul 2025 01:06:58 +0900 Subject: [PATCH 2/5] .github/workflows/test.yml: update debug print Changes include: - Remove Rake task dependency to "debug" and "debug_compiler" from "compile" and "test". Let the workflow explicitly run them as necessary. Since we use separate rake invocations for compiling and running tests, the dependency causes repeated debug prints. - Remove printing Ruby version from the debug tasks as it is not necessary. The ruby/setup-ruby action shows the activated version. - Let "debug" rake task call FileUtils#ruby with verbose: false to avoid printing the script itself. --- .github/workflows/test.yml | 11 +++++++++-- Rakefile | 8 +------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 627a7b5d8..328083de6 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -60,7 +60,10 @@ jobs: if: ${{ !matrix.skip-warnings }} - name: rake compile - run: bundle exec rake compile + run: bundle exec rake debug_compiler compile + + - name: rake debug + run: bundle exec rake debug - name: rake test run: bundle exec rake test TESTOPTS="-v --no-show-detail-immediately" @@ -158,7 +161,10 @@ jobs: if: ${{ !matrix.skip-warnings }} - name: rake compile - run: bundle exec rake compile -- --with-openssl-dir=$HOME/openssl + run: bundle exec rake debug_compiler compile -- --with-openssl-dir=$HOME/openssl + + - name: rake debug + run: bundle exec rake debug - name: rake test run: bundle exec rake test TESTOPTS="-v --no-show-detail-immediately" @@ -170,6 +176,7 @@ jobs: run: | sed -e "s|OPENSSL_DIR|$HOME/openssl|" tool/openssl_fips.cnf.tmpl > tmp/openssl_fips.cnf export OPENSSL_CONF=$(pwd)/tmp/openssl_fips.cnf + bundle exec rake debug bundle exec rake test_fips TESTOPTS="-v --no-show-detail-immediately" timeout-minutes: 5 if: ${{ startsWith(matrix.openssl, 'openssl-3') || matrix.openssl == 'openssl-master' }} diff --git a/Rakefile b/Rakefile index 237b634b5..86c95201a 100644 --- a/Rakefile +++ b/Rakefile @@ -5,8 +5,6 @@ require 'bundler/gem_tasks' begin require 'rake/extensiontask' Rake::ExtensionTask.new('openssl') - # Run the debug_compiler task before the compile task. - Rake::Task['compile'].prerequisites.unshift :debug_compiler rescue LoadError warn "rake-compiler not installed. Run 'bundle install' to " \ "install testing dependency gems." @@ -51,12 +49,8 @@ RDoc::Task.new do |rdoc| 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 - ruby '-v' compiler = RbConfig::CONFIG['CC'] case compiler when 'gcc', 'clang' @@ -82,7 +76,7 @@ task :debug do Providers: #{providers_str} MESSAGE EOF - ruby %Q(-I./lib -ropenssl.so -ve'#{ruby_code}') + ruby %Q(-I./lib -ropenssl.so -e'#{ruby_code}'), verbose: false end task :default => :test From 382eca2aec3d1f47c1efc90dccbe09bf6ce27185 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Thu, 10 Jul 2025 21:46:36 +0900 Subject: [PATCH 3/5] Move slow tests to OSSL_TEST_ALL=1 only Update GitHub Actions workflows to set OSSL_TEST_ALL=1. Exclude a few slow tests that are not critical for local development, unless OSSL_TEST_ALL=1 is set. The bindings code paths are still reached by other tests with smaller inputs, and failures in those would likely indicate an issue in OpenSSL rather than in the bindings. Newly excluded tests include generating large DSA keys and measuring CRYPTO_memcmp() timing. These tests currently take nearly half of the total runtime. --- .github/workflows/test.yml | 6 +++--- test/openssl/test_ossl.rb | 2 +- test/openssl/test_pkey_dh.rb | 2 +- test/openssl/test_pkey_dsa.rb | 8 ++++---- test/openssl/test_ssl_session.rb | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 328083de6..f4f16cafc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -66,7 +66,7 @@ jobs: run: bundle exec rake debug - name: rake test - run: bundle exec rake test TESTOPTS="-v --no-show-detail-immediately" + run: bundle exec rake test TESTOPTS="-v --no-show-detail-immediately" OSSL_TEST_ALL=1 timeout-minutes: 5 test-openssls: @@ -167,7 +167,7 @@ jobs: run: bundle exec rake debug - name: rake test - run: bundle exec rake test TESTOPTS="-v --no-show-detail-immediately" + run: bundle exec rake test TESTOPTS="-v --no-show-detail-immediately" OSSL_TEST_ALL=1 timeout-minutes: 5 # Run only the passing tests on the FIPS module as a temporary workaround. @@ -177,6 +177,6 @@ jobs: sed -e "s|OPENSSL_DIR|$HOME/openssl|" tool/openssl_fips.cnf.tmpl > tmp/openssl_fips.cnf export OPENSSL_CONF=$(pwd)/tmp/openssl_fips.cnf bundle exec rake debug - bundle exec rake test_fips TESTOPTS="-v --no-show-detail-immediately" + bundle exec rake test_fips TESTOPTS="-v --no-show-detail-immediately" OSSL_TEST_ALL=1 timeout-minutes: 5 if: ${{ startsWith(matrix.openssl, 'openssl-3') || matrix.openssl == 'openssl-master' }} diff --git a/test/openssl/test_ossl.rb b/test/openssl/test_ossl.rb index 9f4b39d4f..2e06203ec 100644 --- a/test/openssl/test_ossl.rb +++ b/test/openssl/test_ossl.rb @@ -63,7 +63,7 @@ def test_memcmp_timing end assert_operator(a_b_time, :<, a_c_time * 10, "fixed_length_secure_compare timing test failed") assert_operator(a_c_time, :<, a_b_time * 10, "fixed_length_secure_compare timing test failed") - end + end if ENV["OSSL_TEST_ALL"] == "1" def test_error_data # X509V3_EXT_nconf_nid() called from OpenSSL::X509::ExtensionFactory#create_ext is a function diff --git a/test/openssl/test_pkey_dh.rb b/test/openssl/test_pkey_dh.rb index cf56032cb..c82f642c0 100644 --- a/test/openssl/test_pkey_dh.rb +++ b/test/openssl/test_pkey_dh.rb @@ -16,7 +16,7 @@ def test_new_generate # This test is slow dh = OpenSSL::PKey::DH.new(NEW_KEYLEN) assert_key(dh) - end if ENV["OSSL_TEST_ALL"] + end if ENV["OSSL_TEST_ALL"] == "1" def test_new_break unless openssl? && OpenSSL.fips_mode diff --git a/test/openssl/test_pkey_dsa.rb b/test/openssl/test_pkey_dsa.rb index b88247634..f3324b04a 100644 --- a/test/openssl/test_pkey_dsa.rb +++ b/test/openssl/test_pkey_dsa.rb @@ -47,11 +47,11 @@ def test_generate assert_equal 1024, key1024.p.num_bits assert_equal 160, key1024.q.num_bits - key2048 = OpenSSL::PKey::DSA.generate(2048) - assert_equal 2048, key2048.p.num_bits - assert_equal 256, key2048.q.num_bits - if ENV["OSSL_TEST_ALL"] == "1" # slow + key2048 = OpenSSL::PKey::DSA.generate(2048) + assert_equal 2048, key2048.p.num_bits + assert_equal 256, key2048.q.num_bits + key3072 = OpenSSL::PKey::DSA.generate(3072) assert_equal 3072, key3072.p.num_bits assert_equal 256, key3072.q.num_bits diff --git a/test/openssl/test_ssl_session.rb b/test/openssl/test_ssl_session.rb index f453f5865..37874ca27 100644 --- a/test/openssl/test_ssl_session.rb +++ b/test/openssl/test_ssl_session.rb @@ -222,7 +222,7 @@ def test_server_session_cache # Skipping tests that use session_remove_cb by default because it may cause # deadlock. - TEST_SESSION_REMOVE_CB = ENV["OSSL_TEST_ALL"] == "1" + TEST_SESSION_REMOVE_CB = ENV["OSSL_TEST_UNSAFE"] == "1" def test_ctx_client_session_cb_tls12 start_server do |port| From 3d9938ed406018ee2b5754340dba56b2e8e97d48 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 11 Jul 2025 01:14:43 +0900 Subject: [PATCH 4/5] test/openssl/test_ossl.rb: fix style issues Use OpenSSL::TestCase instead of OpenSSL::SSLTestCase. Prefer assert_true and assert_false over the bare assert and refute. OpenSSL.fixed_length_secure_compare and OpenSSL.secure_compare will only return true or false, and it should be checked. --- test/openssl/test_ossl.rb | 40 +++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/test/openssl/test_ossl.rb b/test/openssl/test_ossl.rb index 2e06203ec..554038bbd 100644 --- a/test/openssl/test_ossl.rb +++ b/test/openssl/test_ossl.rb @@ -3,42 +3,42 @@ if defined?(OpenSSL) -class OpenSSL::OSSL < OpenSSL::SSLTestCase +class OpenSSL::TestOSSL < OpenSSL::TestCase def test_fixed_length_secure_compare assert_raise(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "a") } assert_raise(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "aa") } - assert OpenSSL.fixed_length_secure_compare("aaa", "aaa") - assert OpenSSL.fixed_length_secure_compare( + assert_true(OpenSSL.fixed_length_secure_compare("aaa", "aaa")) + assert_true(OpenSSL.fixed_length_secure_compare( OpenSSL::Digest.digest('SHA256', "aaa"), OpenSSL::Digest::SHA256.digest("aaa") - ) + )) assert_raise(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "aaaa") } - refute OpenSSL.fixed_length_secure_compare("aaa", "baa") - refute OpenSSL.fixed_length_secure_compare("aaa", "aba") - refute OpenSSL.fixed_length_secure_compare("aaa", "aab") + assert_false(OpenSSL.fixed_length_secure_compare("aaa", "baa")) + assert_false(OpenSSL.fixed_length_secure_compare("aaa", "aba")) + assert_false(OpenSSL.fixed_length_secure_compare("aaa", "aab")) assert_raise(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "aaab") } assert_raise(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "b") } assert_raise(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "bb") } - refute OpenSSL.fixed_length_secure_compare("aaa", "bbb") + assert_false(OpenSSL.fixed_length_secure_compare("aaa", "bbb")) assert_raise(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "bbbb") } end def test_secure_compare - refute OpenSSL.secure_compare("aaa", "a") - refute OpenSSL.secure_compare("aaa", "aa") + assert_false(OpenSSL.secure_compare("aaa", "a")) + assert_false(OpenSSL.secure_compare("aaa", "aa")) - assert OpenSSL.secure_compare("aaa", "aaa") + assert_true(OpenSSL.secure_compare("aaa", "aaa")) - refute OpenSSL.secure_compare("aaa", "aaaa") - refute OpenSSL.secure_compare("aaa", "baa") - refute OpenSSL.secure_compare("aaa", "aba") - refute OpenSSL.secure_compare("aaa", "aab") - refute OpenSSL.secure_compare("aaa", "aaab") - refute OpenSSL.secure_compare("aaa", "b") - refute OpenSSL.secure_compare("aaa", "bb") - refute OpenSSL.secure_compare("aaa", "bbb") - refute OpenSSL.secure_compare("aaa", "bbbb") + assert_false(OpenSSL.secure_compare("aaa", "aaaa")) + assert_false(OpenSSL.secure_compare("aaa", "baa")) + assert_false(OpenSSL.secure_compare("aaa", "aba")) + assert_false(OpenSSL.secure_compare("aaa", "aab")) + assert_false(OpenSSL.secure_compare("aaa", "aaab")) + assert_false(OpenSSL.secure_compare("aaa", "b")) + assert_false(OpenSSL.secure_compare("aaa", "bb")) + assert_false(OpenSSL.secure_compare("aaa", "bbb")) + assert_false(OpenSSL.secure_compare("aaa", "bbbb")) end def test_memcmp_timing From dbfcc44b37c5defebf528815be77e5d819fc97ad Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Mon, 7 Jul 2025 18:09:18 +0900 Subject: [PATCH 5/5] test/openssl/test_ts.rb: make assert_raise blocks smaller --- test/openssl/test_ts.rb | 45 ++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/test/openssl/test_ts.rb b/test/openssl/test_ts.rb index ac0469ad5..7b154d1c3 100644 --- a/test/openssl/test_ts.rb +++ b/test/openssl/test_ts.rb @@ -70,15 +70,14 @@ def ts_cert_ee def test_request_mandatory_fields req = OpenSSL::Timestamp::Request.new assert_raise(OpenSSL::Timestamp::TimestampError) do - tmp = req.to_der - pp OpenSSL::ASN1.decode(tmp) + req.to_der end req.algorithm = "sha1" assert_raise(OpenSSL::Timestamp::TimestampError) do req.to_der end req.message_imprint = OpenSSL::Digest.digest('SHA1', "data") - req.to_der + assert_nothing_raised { req.to_der } end def test_request_assignment @@ -371,60 +370,60 @@ def test_no_cert_requested end def test_response_no_policy_defined - assert_raise(OpenSSL::Timestamp::TimestampError) do - req = OpenSSL::Timestamp::Request.new - req.algorithm = "SHA1" - digest = OpenSSL::Digest.digest('SHA1', "test") - req.message_imprint = digest + req = OpenSSL::Timestamp::Request.new + req.algorithm = "SHA1" + digest = OpenSSL::Digest.digest('SHA1', "test") + req.message_imprint = digest - fac = OpenSSL::Timestamp::Factory.new - fac.gen_time = Time.now - fac.serial_number = 1 - fac.allowed_digests = ["sha1"] + fac = OpenSSL::Timestamp::Factory.new + fac.gen_time = Time.now + fac.serial_number = 1 + fac.allowed_digests = ["sha1"] + assert_raise(OpenSSL::Timestamp::TimestampError) do fac.create_timestamp(ee_key, ts_cert_ee, req) end end def test_verify_ee_no_req + ts, _ = timestamp_ee assert_raise(TypeError) do - ts, _ = timestamp_ee ts.verify(nil, ca_cert) end end def test_verify_ee_no_store + ts, req = timestamp_ee assert_raise(TypeError) do - ts, req = timestamp_ee ts.verify(req, nil) end end def test_verify_ee_wrong_root_no_intermediate + ts, req = timestamp_ee assert_raise(OpenSSL::Timestamp::TimestampError) do - ts, req = timestamp_ee ts.verify(req, intermediate_store) end end def test_verify_ee_wrong_root_wrong_intermediate + ts, req = timestamp_ee assert_raise(OpenSSL::Timestamp::TimestampError) do - ts, req = timestamp_ee ts.verify(req, intermediate_store, [ca_cert]) end end def test_verify_ee_nonce_mismatch + ts, req = timestamp_ee + req.nonce = 1 assert_raise(OpenSSL::Timestamp::TimestampError) do - ts, req = timestamp_ee - req.nonce = 1 ts.verify(req, ca_store, [intermediate_cert]) end end def test_verify_ee_intermediate_missing + ts, req = timestamp_ee assert_raise(OpenSSL::Timestamp::TimestampError) do - ts, req = timestamp_ee ts.verify(req, ca_store) end end @@ -472,27 +471,27 @@ def test_verify_direct_unrelated_untrusted end def test_verify_direct_wrong_root + ts, req = timestamp_direct assert_raise(OpenSSL::Timestamp::TimestampError) do - ts, req = timestamp_direct ts.verify(req, intermediate_store) end end def test_verify_direct_no_cert_no_intermediate + ts, req = timestamp_direct_no_cert assert_raise(OpenSSL::Timestamp::TimestampError) do - ts, req = timestamp_direct_no_cert ts.verify(req, ca_store) end end def test_verify_ee_no_cert ts, req = timestamp_ee_no_cert - ts.verify(req, ca_store, [ts_cert_ee, intermediate_cert]) + assert_same(ts, ts.verify(req, ca_store, [ts_cert_ee, intermediate_cert])) end def test_verify_ee_no_cert_no_intermediate + ts, req = timestamp_ee_no_cert assert_raise(OpenSSL::Timestamp::TimestampError) do - ts, req = timestamp_ee_no_cert ts.verify(req, ca_store, [ts_cert_ee]) end end