ruby / openssl

Provides SSL, TLS and general purpose cryptography.
Other
240 stars 167 forks source link

Fix test_pkey_rsa.rb in FIPS. #790

Closed junaruga closed 2 months ago

junaruga commented 3 months ago

This is a draft PR to fix the test_pkey_rsa.rb in FIPS. The 1st commit is a temporary workaround to fix another test failure that is same with the https://github.com/ruby/openssl/pull/789 .

junaruga commented 3 months ago

@rhenium I have a question. I am trying to fix the test_sign_verify_pss in the test/openssl/test_pkey_rsa.rb.

I had to change the RSA bits from 1024 to 2048 to support FIPS. And I need to adjust the value of the salt_length: option. I still don't understand how to use the option after I debugged. Could you tell me how to adjust it? Thanks.

-    key = Fixtures.pkey("rsa1024")
+    key = Fixtures.pkey("rsa2048")

openssl-master case (non-FIPS),

https://github.com/ruby/openssl/actions/runs/10422253508/job/28866328411?pr=790#step:10:637

1) Failure: test_sign_verify_pss(OpenSSL::TestPKeyRSA)
/home/runner/work/openssl/openssl/test/openssl/test_pkey_rsa.rb:213:in `test_sign_verify_pss'
     210:       key.verify_pss("SHA256", signature, data, salt_length: 20, mgf1_hash: "SHA1")
     211: 
     212:     signature = key.sign_pss("SHA256", data, salt_length: :max, mgf1_hash: "SHA1")
  => 213:     assert_equal true,
     214:       key.verify_pss("SHA256", signature, data, salt_length: 94, mgf1_hash: "SHA1")
     215:     assert_equal true,
     216:       key.verify_pss("SHA256", signature, data, salt_length: :auto, mgf1_hash: "SHA1")
<true> expected but was
<false>

openssl-master with fips provider

https://github.com/ruby/openssl/actions/runs/10422253508/job/28866333229?pr=790#step:11:218

9) Error: test_sign_verify_pss(OpenSSL::TestPKeyRSA): OpenSSL::PKey::RSAError: invalid salt length
/home/runner/work/openssl/openssl/test/openssl/test_pkey_rsa.rb:212:in `sign_pss'
/home/runner/work/openssl/openssl/test/openssl/test_pkey_rsa.rb:212:in `test_sign_verify_pss'
     209:     assert_equal false,
     210:       key.verify_pss("SHA256", signature, data, salt_length: 20, mgf1_hash: "SHA1")
     211: 
  => 212:     signature = key.sign_pss("SHA256", data, salt_length: :max, mgf1_hash: "SHA1")
     213:     assert_equal true,
     214:       key.verify_pss("SHA256", signature, data, salt_length: 94, mgf1_hash: "SHA1")
     215:     assert_equal true,
rhenium commented 3 months ago

The test case is for the salt length chosen with salt_length: :max, which depends on the key size. When using a 2048-bit key with SHA-256, it will be 256 - 1 - 1 - 32 = 222 octets.

Please see the EMSA-PSS-ENCODE definition in https://datatracker.ietf.org/doc/html/rfc8017: sLen (salt length) must be equal to or lower than emLen - 1 - 1 - hLen, where emLen is the modulus size in octets and hLen is the hash function's output size in octets.

junaruga commented 3 months ago

This is a draft PR to fix the test_pkey_rsa.rb in FIPS. The 1st commit is a temporary workaround to fix another test failure that is same with the #789 .

I rebased this PR on the latest master branch, as the PR #789 was marged.

junaruga commented 3 months ago

The test case is for the salt length chosen with salt_length: :max, which depends on the key size. When using a 2048-bit key with SHA-256, it will be 256 - 1 - 1 - 32 = 222 octets.

Please see the EMSA-PSS-ENCODE definition in https://datatracker.ietf.org/doc/html/rfc8017: sLen (salt length) must be equal to or lower than emLen - 1 - 1 - hLen, where emLen is the modulus size in octets and hLen is the hash function's output size in octets.

Thanks for your info with the reference of the document. I was able to fix the test_sign_verify_pss in non-FIPS cases by the following modification. The CI result is here.

diff --git a/test/openssl/test_pkey_rsa.rb b/test/openssl/test_pkey_rsa.rb
index 2230a47..15f5672 100644
--- a/test/openssl/test_pkey_rsa.rb
+++ b/test/openssl/test_pkey_rsa.rb
@@ -211,12 +211,12 @@ class OpenSSL::TestPKeyRSA < OpenSSL::PKeyTestCase

     signature = key.sign_pss("SHA256", data, salt_length: :max, mgf1_hash: "SHA1")
     assert_equal true,
-      key.verify_pss("SHA256", signature, data, salt_length: 94, mgf1_hash: "SHA1")
+      key.verify_pss("SHA256", signature, data, salt_length: 222, mgf1_hash: "SHA1")
     assert_equal true,
       key.verify_pss("SHA256", signature, data, salt_length: :auto, mgf1_hash: "SHA1")

     assert_raise(OpenSSL::PKey::RSAError) {
-      key.sign_pss("SHA256", data, salt_length: 95, mgf1_hash: "SHA1")
+      key.sign_pss("SHA256", data, salt_length: 223, mgf1_hash: "SHA1")
     }
   end

I still don't understand the calculation for the salt length (sLen) is like that.

sLen <= emLen - 1 - 1 - hLen = 256 - 1 - 1 - 32 = 222

According to the reference of the document.

https://datatracker.ietf.org/doc/html/rfc8017#section-9.1.1

Steps: ...

  1. If emLen < hLen + sLen + 2, output "encoding error" and stop.

That means if sLen > emLen - 2 - hLen, output "encoding error" and stop.

What I don't understand are below. Could you explain more about these things?

And how about the following error in FIPS cases?

9) Error: test_sign_verify_pss(OpenSSL::TestPKeyRSA): OpenSSL::PKey::RSAError: invalid salt length
/home/runner/work/openssl/openssl/test/openssl/test_pkey_rsa.rb:212:in `sign_pss'
/home/runner/work/openssl/openssl/test/openssl/test_pkey_rsa.rb:212:in `test_sign_verify_pss'
junaruga commented 3 months ago

And how about the following error in FIPS cases?

9) Error: test_sign_verify_pss(OpenSSL::TestPKeyRSA): OpenSSL::PKey::RSAError: invalid salt length
/home/runner/work/openssl/openssl/test/openssl/test_pkey_rsa.rb:212:in `sign_pss'
/home/runner/work/openssl/openssl/test/openssl/test_pkey_rsa.rb:212:in `test_sign_verify_pss'

I was able to debug this issue by the following minimal script with gdb.

$ OPENSSL_CONF=$HOME/.local/openssl-3.4.0-dev-fips-debug-d550d2aae5/ssl/openssl_fips.cnf \
  gdb --args ruby -I./lib -ropenssl -e '
    key = OpenSSL::PKey.read(File.read("test/openssl/fixtures/pkey/rsa2048.pem"))
    signature = key.sign_pss("SHA256", "Sign me!", salt_length: :max, mgf1_hash: "SHA1")
'

At the following part, the approved is 0 (= non-approved), because the saltlen (222) is larger than mdsize (32). But I am not sure how to fix the test signature = key.sign_pss("SHA256", data, salt_length: :max, mgf1_hash: "SHA1") (test/openssl/test_pkey_rsa.rb:212). Maybe we need to change the SHA1 to larger one such as SHA256?

https://github.com/openssl/openssl/blob/d550d2aae531c6fa2e10b1a30d2acdf373663889/providers/implementations/signature/rsa_sig.c#L580-L597

(gdb) f
#0  rsa_pss_saltlen_check_passed (ctx=0x68f9b0,
    algoname=0x7fffcde03673 "RSA Sign", saltlen=222)
    at providers/implementations/signature/rsa_sig.c:589
589     if (!approved) {

(gdb) p saltlen
$15 = 222
(gdb) p mdsize
$16 = 32
(gdb) p approved
$17 = 0

(gdb) bt
#0  rsa_pss_saltlen_check_passed (ctx=0x68f9b0, algoname=0x7fffcde03673 "RSA Sign", saltlen=222) at providers/implementations/signature/rsa_sig.c:589
#1  0x00007fffcdc55936 in rsa_sign (vprsactx=0x68f9b0, sig=0x7fffe9c8e4d8 "", siglen=0x7fffffffc420, sigsize=256,
    tbs=0x7fffffffc300 "I\240N\f\335d\245\2007\262\020ei\nK\377\232\354\003\032\374r#\vO{%\b\vE\372i@\303\377\377\377\177", tbslen=32) at providers/implementations/signature/rsa_sig.c:734
#2  0x00007fffcdc56608 in rsa_digest_sign_final (vprsactx=0x68f9b0, sig=0x7fffe9c8e4d8 "", siglen=0x7fffffffc420, sigsize=256) at providers/implementations/signature/rsa_sig.c:1041
#3  0x00007fffce22cb5a in EVP_DigestSignFinal (ctx=0x69f020, sigret=0x7fffe9c8e4d8 "", siglen=0x7fffffffc420) at crypto/evp/m_sigver.c:520
#4  0x00007fffd06d61c1 in ossl_rsa_sign_pss (argc=3, argv=0x7fffe9cff058, self=140736690994560) at ../../../../ext/openssl/ossl_pkey_rsa.c:391
rhenium commented 3 months ago

That means if sLen > emLen - 2 - hLen, output "encoding error" and stop.

What I don't understand are below. Could you explain more about these things?

  • Does emLen: 256 comes from the used SHA256?

No, it comes from the RSA key size. emLen = \ceil(emBits/8), where emBits is the RSA key's modulus size (the max size of data it can sign) in bits.

  • Why is hLen 32?

SHA-256's output is 32 octets long.


https://github.com/openssl/openssl/blob/d550d2aae531c6fa2e10b1a30d2acdf373663889/providers/implementations/signature/rsa_sig.c#L580-L597

According to the comment and https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-5.pdf, the NIST standard introduces additional constraints to RFC 8017. It sounds like that salt_length: :max tests need to be skipped in the FIPS mode.

junaruga commented 3 months ago

That means if sLen > emLen - 2 - hLen, output "encoding error" and stop. What I don't understand are below. Could you explain more about these things?

  • Does emLen: 256 comes from the used SHA256?

No, it comes from the RSA key size. emLen = \ceil(emBits/8), where emBits is the RSA key's modulus size (the max size of data it can sign) in bits.

  • Why is hLen 32?

SHA-256's output is 32 octets long.

All right! So, the calculation of the salt_length is like this.

sLen <= emLen (octat) - 2 - hLen (octet) = 2048 / 8 - 2 - 256 / 8 = 222

https://github.com/openssl/openssl/blob/d550d2aae531c6fa2e10b1a30d2acdf373663889/providers/implementations/signature/rsa_sig.c#L580-L597

According to the comment and https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-5.pdf, the NIST standard introduces additional constraints to RFC 8017. It sounds like that salt_length: :max tests need to be skipped in the FIPS mode.

All right. I couldn't find the written part in the https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-5.pdf. However, I will skip the assertion in FIPS with adding a comment.

junaruga commented 3 months ago

@rhenium I fixed all the tests in test_pkey_rsa.rb for FIPS. Please view. Thank you.

junaruga commented 2 months ago

@rhenium I fixed all the items that you mentioned, except for the one thing whether we can drop the key2 tests in test_new and test_s_generate. Please review again. Thank you.

junaruga commented 2 months ago

I fixed all the items that you mentioned!

rhenium commented 2 months ago

Sorry for the delay - it looks good to me. Thank you!

junaruga commented 2 months ago

Thank you for reviewing my PR!