test/openssl/test_pkey.rb: Fix pending tests in FIPS case.

[junaruga]

This PR is to run the previously pended tests by the issues (openssl/openssl#21493 and openssl/openssl#20758) conditionally on OpenSSL versions including the fixes in FIPS case. The fixes were merged to the OpenSSL (openssl/openssl) master, openssl-3.1, and openssl-3.0 branches. See this. So, the coming stable versions should pass the tests.

[rhenium]

I wonder why this is skipped. This test case doesn't seem to be using any FIPS-140 unapproved algorithms.

[rhenium]

I got it. OpenSSL::PKey.read(der_encoded_string) failing is indeed the issue fixed by the OpenSSL change.

[junaruga]

This is really a good point.

When commenting out the pend_on_openssl_issue_21493, CI result is here.

diff --git a/test/openssl/test_pkey.rb b/test/openssl/test_pkey.rb
index da3ae5d..247dd96 100644
--- a/test/openssl/test_pkey.rb
+++ b/test/openssl/test_pkey.rb
@@ -82,7 +82,7 @@ class OpenSSL::TestPKey < OpenSSL::PKeyTestCase
   end
 
   def test_ed25519
-    pend_on_openssl_issue_21493
+    # pend_on_openssl_issue_21493
 
     # Test vector from RFC 8032 Section 7.1 TEST 2
     priv_pem = <<~EOF
@@ -148,7 +148,7 @@ class OpenSSL::TestPKey < OpenSSL::PKeyTestCase
   end
 
   def test_x25519
-    pend_on_openssl_issue_21493
+    # pend_on_openssl_issue_21493
 
     # Test vector from RFC 7748 Section 6.1
     alice_pem = <<~EOF
@@ -202,7 +202,7 @@ class OpenSSL::TestPKey < OpenSSL::PKeyTestCase
   end
 
   def test_compare?
-    pend_on_openssl_issue_21493
+    # pend_on_openssl_issue_21493
 
     key1 = Fixtures.pkey("rsa1024")
     key2 = Fixtures.pkey("rsa1024")

The result test_compare? with error looks quite different from the result test_x25519 (and test_ed25519 on openssl-3.1.10 fips case). Actually what I added the pend_on_openssl_issue_21493 was close to my mistake. I just missed why the test_compare? failed. But actually these test_ed25519, test_x25519 and test_compare? failures can be fixed on the same one commit openssl/openssl@2acb0d3. Note that PR's commits were merged as 3 commits to the openssl/openssl's master, openssl-3.1 and openssl-3.0 branches.

After I saw your comment here, I checked which commit actually fixed by running git bisect with test_pkey_test_compare.rb including only test_compare? test, and test_pkey_test_x25519.rb including only test_x25519 test.

I executed the git-bisect below with the script git-bisect-ruby-test.sh to find which commit fixes the test.

$ pwd
/home/jaruga/git/openssl2 # openssl/openssl repository, master branch

$ git bisect start --term-new=fixed --term-old=unfixed
$ git bisect fixed 4c50610bdadbcf7aa6bbd968df67b8874234677b
$ git bisect unfixed cb8e64131e7ce230a9268bdd7cc4664868ff0dc9

$ git bisect run ~/git/report-openssl-fips-base-decoding-corruption/git-bisect-ruby-test.sh
...
2acb0d363c0032b5b97c4f6596609f40bd7d842f is the first fixed commit
commit 2acb0d363c0032b5b97c4f6596609f40bd7d842f
Author: Tomas Mraz <tomas@openssl.org>
Date:   Fri Jul 21 17:40:31 2023 +0200

    When exporting/importing decoded keys do not use 0 as selection
    
    When decoding 0 as the selection means to decode anything
    you get.
     
    However when exporting and then importing the key data 0 as
    selection is not meaningful.
    So we set it to OSSL_KEYMGMT_SELECT_ALL to make the export/import
    function export/import everything that we have decoded.
    
    Fixes #21493
    
    Reviewed-by: Matt Caswell <matt@openssl.org>
    Reviewed-by: Paul Dale <pauli@openssl.org>
    Reviewed-by: Todd Short <todd.short@me.com>
    (Merged from https://github.com/openssl/openssl/pull/21519)

 crypto/encode_decode/decoder_pkey.c                         | 6 +++++-
 providers/implementations/encode_decode/decode_der2key.c    | 6 +++++-
 providers/implementations/encode_decode/decode_msblob2key.c | 6 +++++-
 providers/implementations/encode_decode/decode_pvk2key.c    | 6 +++++-
 4 files changed, 20 insertions(+), 4 deletions(-)
bisect found first bad commit

$ git bisect log
git bisect start '--term-new=fixed' '--term-old=unfixed'
# status: waiting for both good and bad commits
# fixed: [4c50610bdadbcf7aa6bbd968df67b8874234677b] endecode_test.c: Add tests for decoding with 0 selection
git bisect fixed 4c50610bdadbcf7aa6bbd968df67b8874234677b
# status: waiting for good commit(s), bad commit known
# unfixed: [cb8e64131e7ce230a9268bdd7cc4664868ff0dc9] no_autoload: make the no-autoload-config option work again.
git bisect unfixed cb8e64131e7ce230a9268bdd7cc4664868ff0dc9
# fixed: [2acb0d363c0032b5b97c4f6596609f40bd7d842f] When exporting/importing decoded keys do not use 0 as selection
git bisect fixed 2acb0d363c0032b5b97c4f6596609f40bd7d842f
# unfixed: [1ae4678cebaa13604c0f31bdf2c64cd28bdaf287] Avoid exporting bogus (empty) data if empty selection is used
git bisect unfixed 1ae4678cebaa13604c0f31bdf2c64cd28bdaf287
# first fixed commit: [2acb0d363c0032b5b97c4f6596609f40bd7d842f] When exporting/importing decoded keys do not use 0 as selection

And the openssl/openssl@2acb0d3 was the first fixed commit. I am not sure why this commit fixed the test_compare?. I have not debugged the case yet.

I will adjust or refactor the test/openssl/utils.rb#pend_on_openssl_issue_21493, and comment in the commit by this PR.

[junaruga]

After debugging this case, I can see that the OpenSSL::PKey.read(der_encoded_string) is failing. So, I am planning to apply a workaround conditionally to the c code ossl_pkey_read_generic, rather than pending tests. The workaround should be only executed in the affected OpenSSL versions with 2 or more providers (number of providers >= 2). Because this issue would happen if an encoding/decoding provider (e.g. "base") is different from the encryption provider (e.g. "fips").