From cd76116e49e29a40e2073f58fa5fa41f39b1dd4c Mon Sep 17 00:00:00 2001 From: Jun Aruga Date: Tue, 7 Nov 2023 14:36:17 +0100 Subject: [PATCH] Fix test_pkey_dh.rb in FIPS. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [ This is a backport to the 3.2 branch. ] We use dh2048_ffdhe2048.pem file (DH 2048 bits) instead of dh1024.pem file in both non-FIPS and FIPS cases. Because the following command fails to generate the pem file with 1024 bits. And the OpenSSL FIPS 140-2 security policy document explains the DH public keys are allowed from 2048 bits.[1] ``` $ OPENSSL_CONF=/home/jaruga/.local/openssl-3.3.0-dev-fips-debug-1aa08644ec/ssl/openssl_fips.cnf \ /home/jaruga/.local/openssl-3.3.0-dev-fips-debug-1aa08644ec/bin/openssl \ dhparam -out dh1024.pem 1024 Generating DH parameters, 1024 bit long safe prime dhparam: Generating DH key parameters failed ``` The dh2048_ffdhe2048.pem file was created by the following command with the OpenSSL FIPS configuration file. The logic to generate the DH pem file is different between non-FIPS and FIPS cases. In FIPS, it seems that the command always returns the text defined as ffdhe2048 in the FFDHE groups in RFC 7919 unlike non-FIPS.[2] As the generated pem file is a normal and valid PKCS#3-style group parameter, we use the file for the non-FIPS case too. ``` $ OPENSSL_CONF=/home/jaruga/.local/openssl-3.3.0-dev-fips-debug-1aa08644ec/ssl/openssl_fips.cnf \ /home/jaruga/.local/openssl-3.3.0-dev-fips-debug-1aa08644ec/bin/openssl \ dhparam -out dh2048_ffdhe2048.pem 2048 ``` Note that the hard-coded PEM-encoded string in the `test_DHparams` is intentional to avoid modifying the content unintentionally. * [1] https://www.openssl.org/source/ - OpenSSL 3.0.8 FIPS 140-2 security policy document page 25, Table 10 – Public Keys - DH Public - DH (2048/3072/4096/6144/8192) public key agreement key * [2] RFC7919 - Appendix A.1: ffdhe2048 https://www.rfc-editor.org/rfc/rfc7919#appendix-A.1 (cherry picked from commit 6a4ff26475adbbd70a1df430f314f03544172b15) --- Rakefile | 1 + test/openssl/fixtures/pkey/dh1024.pem | 5 -- .../fixtures/pkey/dh2048_ffdhe2048.pem | 8 +++ test/openssl/test_pkey_dh.rb | 64 +++++++++++++------ test/openssl/utils.rb | 6 +- 5 files changed, 58 insertions(+), 26 deletions(-) delete mode 100644 test/openssl/fixtures/pkey/dh1024.pem create mode 100644 test/openssl/fixtures/pkey/dh2048_ffdhe2048.pem diff --git a/Rakefile b/Rakefile index c673a8bc1..5d087ea14 100644 --- a/Rakefile +++ b/Rakefile @@ -29,6 +29,7 @@ Rake::TestTask.new(:test_fips_internal) do |t| t.test_files = FileList[ 'test/openssl/test_fips.rb', 'test/openssl/test_pkey.rb', + 'test/openssl/test_pkey_dh.rb', 'test/openssl/test_pkey_ec.rb', ] t.warning = true diff --git a/test/openssl/fixtures/pkey/dh1024.pem b/test/openssl/fixtures/pkey/dh1024.pem deleted file mode 100644 index f99c757f2..000000000 --- a/test/openssl/fixtures/pkey/dh1024.pem +++ /dev/null @@ -1,5 +0,0 @@ ------BEGIN DH PARAMETERS----- -MIGHAoGBAKnKQ8MNK6nYZzLrrcuTsLxuiJGXoOO5gT+tljOTbHBuiktdMTITzIY0 -pFxIvjG05D7HoBZQfrR0c92NGWPkAiCkhQKB8JCbPVzwNLDy6DZ0pmofDKrEsYHG -AQjjxMXhwULlmuR/K+WwlaZPiLIBYalLAZQ7ZbOPeVkJ8ePao0eLAgEC ------END DH PARAMETERS----- diff --git a/test/openssl/fixtures/pkey/dh2048_ffdhe2048.pem b/test/openssl/fixtures/pkey/dh2048_ffdhe2048.pem new file mode 100644 index 000000000..9b182b720 --- /dev/null +++ b/test/openssl/fixtures/pkey/dh2048_ffdhe2048.pem @@ -0,0 +1,8 @@ +-----BEGIN DH PARAMETERS----- +MIIBCAKCAQEA//////////+t+FRYortKmq/cViAnPTzx2LnFg84tNpWp4TZBFGQz ++8yTnc4kmz75fS/jY2MMddj2gbICrsRhetPfHtXV/WVhJDP1H18GbtCFY2VVPe0a +87VXE15/V8k1mE8McODmi3fipona8+/och3xWKE2rec1MKzKT0g6eXq8CrGCsyT7 +YdEIqUuyyOP7uWrat2DX9GgdT0Kj3jlN9K5W7edjcrsZCwenyO4KbXCeAvzhzffi +7MA0BM0oNC9hkXL+nOmFg/+OTxIy7vKBg8P+OxtMb61zO7X8vC7CIAXFjvGDfRaD +ssbzSibBsu/6iGtCOGEoXJf//////////wIBAg== +-----END DH PARAMETERS----- diff --git a/test/openssl/test_pkey_dh.rb b/test/openssl/test_pkey_dh.rb index 161af1897..d32ffaf6b 100644 --- a/test/openssl/test_pkey_dh.rb +++ b/test/openssl/test_pkey_dh.rb @@ -18,15 +18,26 @@ def test_new_generate assert_key(dh) end if ENV["OSSL_TEST_ALL"] - def test_new_break + def test_new_break_on_non_fips + omit_on_fips + assert_nil(OpenSSL::PKey::DH.new(NEW_KEYLEN) { break }) assert_raise(RuntimeError) do OpenSSL::PKey::DH.new(NEW_KEYLEN) { raise } end end + def test_new_break_on_fips + omit_on_non_fips + + # The block argument is not executed in FIPS case. + # See https://github.com/ruby/openssl/issues/692 for details. + assert(OpenSSL::PKey::DH.new(NEW_KEYLEN) { break }) + assert(OpenSSL::PKey::DH.new(NEW_KEYLEN) { raise }) + end + def test_derive_key - params = Fixtures.pkey("dh1024") + params = Fixtures.pkey("dh2048_ffdhe2048") dh1 = OpenSSL::PKey.generate_key(params) dh2 = OpenSSL::PKey.generate_key(params) dh1_pub = OpenSSL::PKey.read(dh1.public_to_der) @@ -44,34 +55,38 @@ def test_derive_key end def test_DHparams - dh1024 = Fixtures.pkey("dh1024") - dh1024params = dh1024.public_key + dh = Fixtures.pkey("dh2048_ffdhe2048") + dh_params = dh.public_key asn1 = OpenSSL::ASN1::Sequence([ - OpenSSL::ASN1::Integer(dh1024.p), - OpenSSL::ASN1::Integer(dh1024.g) + OpenSSL::ASN1::Integer(dh.p), + OpenSSL::ASN1::Integer(dh.g) ]) key = OpenSSL::PKey::DH.new(asn1.to_der) - assert_same_dh dh1024params, key + assert_same_dh dh_params, key pem = <<~EOF -----BEGIN DH PARAMETERS----- - MIGHAoGBAKnKQ8MNK6nYZzLrrcuTsLxuiJGXoOO5gT+tljOTbHBuiktdMTITzIY0 - pFxIvjG05D7HoBZQfrR0c92NGWPkAiCkhQKB8JCbPVzwNLDy6DZ0pmofDKrEsYHG - AQjjxMXhwULlmuR/K+WwlaZPiLIBYalLAZQ7ZbOPeVkJ8ePao0eLAgEC + MIIBCAKCAQEA//////////+t+FRYortKmq/cViAnPTzx2LnFg84tNpWp4TZBFGQz + +8yTnc4kmz75fS/jY2MMddj2gbICrsRhetPfHtXV/WVhJDP1H18GbtCFY2VVPe0a + 87VXE15/V8k1mE8McODmi3fipona8+/och3xWKE2rec1MKzKT0g6eXq8CrGCsyT7 + YdEIqUuyyOP7uWrat2DX9GgdT0Kj3jlN9K5W7edjcrsZCwenyO4KbXCeAvzhzffi + 7MA0BM0oNC9hkXL+nOmFg/+OTxIy7vKBg8P+OxtMb61zO7X8vC7CIAXFjvGDfRaD + ssbzSibBsu/6iGtCOGEoXJf//////////wIBAg== -----END DH PARAMETERS----- EOF + key = OpenSSL::PKey::DH.new(pem) - assert_same_dh dh1024params, key + assert_same_dh dh_params, key key = OpenSSL::PKey.read(pem) - assert_same_dh dh1024params, key + assert_same_dh dh_params, key - assert_equal asn1.to_der, dh1024.to_der - assert_equal pem, dh1024.export + assert_equal asn1.to_der, dh.to_der + assert_equal pem, dh.export end def test_public_key - dh = Fixtures.pkey("dh1024") + dh = Fixtures.pkey("dh2048_ffdhe2048") public_key = dh.public_key assert_no_key(public_key) #implies public_key.public? is false! assert_equal(dh.to_der, public_key.to_der) @@ -80,7 +95,8 @@ def test_public_key def test_generate_key # Deprecated in v3.0.0; incompatible with OpenSSL 3.0 - dh = Fixtures.pkey("dh1024").public_key # creates a copy with params only + # Creates a copy with params only + dh = Fixtures.pkey("dh2048_ffdhe2048").public_key assert_no_key(dh) dh.generate_key! assert_key(dh) @@ -91,7 +107,15 @@ def test_generate_key end if !openssl?(3, 0, 0) def test_params_ok? - dh0 = Fixtures.pkey("dh1024") + # Skip the tests in old OpenSSL version 1.1.1c or early versions before + # applying the following commits in OpenSSL 1.1.1d to make `DH_check` + # function pass the RFC 7919 FFDHE group texts. + # https://github.com/openssl/openssl/pull/9435 + unless openssl?(1, 1, 1, 4) + pend 'DH check for RFC 7919 FFDHE group texts is not implemented' + end + + dh0 = Fixtures.pkey("dh2048_ffdhe2048") dh1 = OpenSSL::PKey::DH.new(OpenSSL::ASN1::Sequence([ OpenSSL::ASN1::Integer(dh0.p), @@ -108,7 +132,7 @@ def test_params_ok? def test_dup # Parameters only - dh1 = Fixtures.pkey("dh1024") + dh1 = Fixtures.pkey("dh2048_ffdhe2048") dh2 = dh1.dup assert_equal dh1.to_der, dh2.to_der assert_not_equal nil, dh1.p @@ -125,7 +149,7 @@ def test_dup end # With a key pair - dh3 = OpenSSL::PKey.generate_key(Fixtures.pkey("dh1024")) + dh3 = OpenSSL::PKey.generate_key(Fixtures.pkey("dh2048_ffdhe2048")) dh4 = dh3.dup assert_equal dh3.to_der, dh4.to_der assert_equal dh1.to_der, dh4.to_der # encodes parameters only @@ -136,7 +160,7 @@ def test_dup end def test_marshal - dh = Fixtures.pkey("dh1024") + dh = Fixtures.pkey("dh2048_ffdhe2048") deserialized = Marshal.load(Marshal.dump(dh)) assert_equal dh.to_der, deserialized.to_der diff --git a/test/openssl/utils.rb b/test/openssl/utils.rb index cd70d4886..f6c84eef6 100644 --- a/test/openssl/utils.rb +++ b/test/openssl/utils.rb @@ -151,7 +151,11 @@ def teardown def omit_on_fips return unless OpenSSL.fips_mode - omit 'An encryption used in the test is not FIPS-approved' + omit <<~MESSAGE + Only for OpenSSL non-FIPS with the following possible reasons: + * A testing logic is non-FIPS specific. + * An encryption used in the test is not FIPS-approved. + MESSAGE end def omit_on_non_fips