Implement FIPS functions, adding OpenSSL FIPS mode case on CI. #608

junaruga · 2023-03-17T12:02:52Z

This PR includes the 2 commits below.

CI: Add OpenSSL FIPS mode case.
Fix the fips_mode_get on OpenSSL 3.

The 1st commit is to add the latest stable OpenSSL version 3.0.8 FIPS mode to the CI. I was able to create the FIPS mode environment. That means you can create the environment in your local environment too!

The 2nd commit fixes the issue #605 related to the OpenSSL.fips_mode (fips_mode_get). The commit can be also a minimal fix to test the FIPS mode created by the 1st commit.

CI: Add OpenSSL FIPS mode case.

I referred to the following documents.

I prepared the OpenSSL config file to enable the FIPS mode: test/openssl/fixtures/ssl/openssl_fips.cnf.tmpl that was introduced in the document above.

As a reference, if you want to change the default OpenSSL config file (OPENSSL_INSTALL_DIR/ssl/openssl.cnf) to enable the FIPS mode, the difference is below for the OpenSSL 3.0.8 config file.

$ diff -u openssl.cnf openssl_fips2.cnf 
--- openssl.cnf	2023-03-16 17:04:39.238327719 +0100
+++ openssl_fips2.cnf	2023-03-16 17:13:29.319381845 +0100
@@ -49,16 +49,18 @@
 # referenced from the [provider_sect] below.
 # Refer to the OpenSSL security policy for more information.
 # .include fipsmodule.cnf
+.include /home/jaruga/.local/openssl-3.0.8-fips/ssl/fipsmodule.cnf
 
 [openssl_init]
 providers = provider_sect
+alg_section = algorithm_sect
 
 # List of providers to load
 [provider_sect]
 default = default_sect
 # The fips section name should match the section name inside the
 # included fipsmodule.cnf.
-# fips = fips_sect
+fips = fips_sect
 
 # If no providers are activated explicitly, the default one is activated implicitly.
 # See man 7 OSSL_PROVIDER-default for more details.
@@ -69,8 +71,10 @@
 # OpenSSL may not work correctly which could lead to significant system
 # problems including inability to remotely access the system.
 [default_sect]
-# activate = 1
+activate = 1
 
+[algorithm_sect]
+default_properties = fips=yes
 
 ####################################################################
 [ ca ]

Fix the fips_mode_get on OpenSSL 3.

The commit only fixes OpenSSL.fips_mode (fips_mode_get). I think this can be a good start to fixing other issues for OpenSSL 3 FIPS mode in other PRs. The use of the TEST_RUBY_OPENSSL_FIPS_ENABLED is not the best way. However, I don't know a better way than that right now.

The test test_fips_mode_get previously passed unintentionally because the OpenSSL::OPENSSL_FIPS returns false in the FIPS mode-enabled environment. So, I added the pending message.

What do you think? Review, please. Thanks.

junaruga · 2023-03-17T12:12:19Z

I created a repository of testing OpenSSL APIs as a proof of concept. You can check the OpenSSL fips_mode_get and fips_mode_set APIs.
https://github.com/junaruga/openssl-test

junaruga · 2023-03-17T12:23:13Z

.github/workflows/test.yml

@@ -112,3 +135,10 @@ jobs:
      - name: test
        run:  rake test TESTOPTS="-v --no-show-detail-immediately" OSSL_MDEBUG=1
        timeout-minutes: 5
+        if: ${{ !matrix.fips_enabled }}


This condition is a temporary workaround. If you remove the if, you can reproduce the issue #603 on the fips mode case on CI.

voxik · 2023-03-17T16:58:17Z

test/openssl/test_fips.rb

+      assert OpenSSL.fips_mode == false, ".fips_mode returns false on FIPS mode disabled"
+    end;
+  end
+


Shouldn't this test cases be enabled permanently, also outside of CI?

Or shouldn't there rather be one test case, which will compare the expected result based on the env variables?

Right now I don't find a good way to check if the OpenSSL is running on the FIPS mode enabled or disabled. Because the OpenSSL.fips_mode itself is a logic to check if the OpenSSL is running on the FIPS mode enabled or disabled.

Perhaps, one possible way to check the FIPS mode or not with the OpenSSL 3 is, to add a source code of the command fips_enabled just to return the exit status 0 or 1 using the EVP_default_properties_is_fips_enabled(NULL) such as (the example in my proof of concept repository), build it and use in the testing process. The code can be below. A challenge of this way is when OpenSSL is not set up properly for the FIPS mode unintentionally, the EVP_default_properties_is_fips_enabled(NULL) returns 0 (disabled), and the test is skipped unintentionally. Perhaps with keeping the current tests, and in addition, we can add the new test with the command fips_enabled.

fips_enabled.c

#include <stdio.h> #include <openssl/evp.h> int main(int argc, char *argv[]) { int status = EXIT_SUCCESS; int fips_enabled = 0; fips_enabled = EVP_default_properties_is_fips_enabled(NULL); status = fips_enabled ? EXIT_SUCCESS : EXIT_FAILURE; exit(status); }

If you guys like this way, I can add the source code to this repository and can add new tests. But I would like that it is in another PR. Because the current tests running on CI can test the new implementation on the CI.

The FIPS mode is enabled in CI, so it is in know state. Or are you afraid that somebody who would by a chance run the test suite on their FIPS enabled system would differ from default expectation? I don't think this is very likely scenario.

IOW I believe it is safe to assume that by default, FIPS is disabled and the OpenSSL.fips_mode == false condition is true.

The FIPS mode is enabled in CI, so it is in know state. Or are you afraid that somebody who would by a chance run the test suite on their FIPS enabled system would differ from default expectation? I don't think this is very likely scenario.

Yes. Maybe in my understanding, that is the case I am afraid of. I noticed that OpenSSL.fips_mode returns false on FIPS enabled system on OpenSSL 3, then I opened the issue ticket. Let's say that I want to notice by seeing the failure of the unit test when the OpenSSL.fips_mode will unintentionally return false on FIPS enabled system on OpenSSL "4".

IOW I believe it is safe to assume that by default, FIPS is disabled and the OpenSSL.fips_mode == false condition is true.

I still cannot imagine how do you want to change the condition. Could you share your suggestion for the change with the result of the git diff or diff -u <original_file> <new_file> for the test/openssl/test_fips.rb by doing checkout the branch used for this PR in my forked repository? How can the change look like?

My another suggestion is below, removing the ENV["CI"] from the condition. It can test the assert OpenSSL.fips_mode == false on local environment outside CI.

$ git diff diff --git a/test/openssl/test_fips.rb b/test/openssl/test_fips.rb index dc92bde..e12f1d2 100644 --- a/test/openssl/test_fips.rb +++ b/test/openssl/test_fips.rb @@ -5,22 +5,25 @@ if defined?(OpenSSL) class OpenSSL::TestFIPS < OpenSSL::TestCase def test_fips_mode_get_is_true_on_fips_mode_enabled - unless ENV["CI"] && ENV["TEST_RUBY_OPENSSL_FIPS_ENABLED"] + unless ENV["TEST_RUBY_OPENSSL_FIPS_ENABLED"] omit "Only for on FIPS mode environment on CI" end assert_separately([{ "OSSL_MDEBUG" => nil }, "-ropenssl"], <<~"end;") - assert OpenSSL.fips_mode == true, ".fips_mode returns true on FIPS mode enabled" + assert OpenSSL.fips_mode == true, ".fips_mode should return true on FIPS mode enabled" end; end def test_fips_mode_get_is_false_on_fips_mode_disabled - unless ENV["CI"] && !ENV["TEST_RUBY_OPENSSL_FIPS_ENABLED"] + if ENV["TEST_RUBY_OPENSSL_FIPS_ENABLED"] omit "Only for non-FIPS mode environment on CI" end assert_separately([{ "OSSL_MDEBUG" => nil }, "-ropenssl"], <<~"end;") - assert OpenSSL.fips_mode == false, ".fips_mode returns false on FIPS mode disabled" + message = ".fips_mode should return false on FIPS mode disabled. " \ + "If you run the test on FIPS mode, please set " \ + "TEST_RUBY_OPENSSL_FIPS_ENABLED=true" + assert OpenSSL.fips_mode == false, message end; end @@ -35,10 +38,10 @@ class OpenSSL::TestFIPS < OpenSSL::TestCase assert_separately([{ "OSSL_MDEBUG" => nil }, "-ropenssl"], <<~"end;") begin OpenSSL.fips_mode = true - assert OpenSSL.fips_mode == true, ".fips_mode returns true when .fips_mode=true" + assert OpenSSL.fips_mode == true, ".fips_mode should return true when .fips_mode=true" OpenSSL.fips_mode = false - assert OpenSSL.fips_mode == false, ".fips_mode returns false when .fips_mode=false" + assert OpenSSL.fips_mode == false, ".fips_mode should return false when .fips_mode=false" rescue OpenSSL::OpenSSLError pend "Could not set FIPS mode (OpenSSL::OpenSSLError: \#$!); skipping" end

test/openssl/test_fips.rb

junaruga · 2023-03-17T22:32:09Z

I rebased the 2nd commit on this PR. I was able to implement OpenSSL::OPENSSL_FIPS, ossl_fips_mode_get and ossl_fips_mode_set to pass the test test/openssl/test_fips.rb.

Note that the OpenSSL::OPENSSL_FIPS returns always true in OpenSSL 3 because the used functions EVP_default_properties_enable_fips and EVP_default_properties_is_fips_enabled work with the OpenSSL installed without FIPS option.

When I run the fips_mode_set command in my repo for the OpenSSL 3.0.8 installed without FIPS option, I was able to confirm the functions works as expected.

The steps I confirmed are below.

Install OpenSSL without FIPS option.

$ ./Configure --prefix=$HOME/.local/openssl-3.0.8 --libdir=lib shared linux-x86_64
$ make -j4
$ make install

Run the ./fips_mode_set to check the FIPS functions. And I confirmed the value of the FIPS enable is changed.

$ LD_LIBRARY_PATH=/home/jaruga/.local/openssl-3.0.8/lib/ ./fips_mode_set
FIPS mode: 0
FIPS mode on.
FIPS mode: 1
FIPS mode on 2nd time.
FIPS mode: 1
FIPS mode off.
FIPS mode: 0
FIPS mode off 2nd time.
FIPS mode: 0

junaruga · 2023-03-18T08:42:57Z

test/openssl/test_fips.rb

-  def test_fips_mode_get
-    return unless OpenSSL::OPENSSL_FIPS
+  def test_fips_mode_get_with_fips_mode_set
+    omit('OpenSSL is not FIPS-capable') unless OpenSSL::OPENSSL_FIPS


Note that I used the message OpenSSL is not FIPS-capable, because the word "OpenSSL" in the message is used as a meaning of the SSL Library in this context including LibreSSL cases, as well as the "openssl" in this repository name "OpenSSL for Ruby". The word "FIPS-capable" comes from the OpenSSL::OPENSSL_FIPS's comment: "Boolean indicating whether OpenSSL is FIPS-capable or not".

I see the following error message in the ext/openssl/ossl.c. So, another candidate of the message is "OpenSSL does not support FIPS mode" to align with the error message.

ossl_raise(eOSSLError, "This version of OpenSSL does not support FIPS mode");

voxik · 2023-03-18T16:53:26Z

test/openssl/test_fips.rb

+    end
+
+    assert_separately([{ "OSSL_MDEBUG" => nil }, "-ropenssl"], <<~"end;")
+      assert OpenSSL.fips_mode == true, ".fips_mode returns true on FIPS mode enabled"


Isn't the assert message negated? Shouldn't the assert failure report ".fips_mode returns false on FIPS mode enabled" instead, because the message is actually displayed when OpenSSL.fips_mode != true if I am not mistaken.

A good point! The message was partly copied from the exisintg test test_fips_mode_get_with_fips_mode_set. I think we should replace the "returns" with "should return"?

The change is like this. Ignore the line OpenSSL.fips_mode = false # debug to print the error message in the debugging purpose.

$ git diff diff --git a/test/openssl/test_fips.rb b/test/openssl/test_fips.rb index dc92bde..93243dc 100644 --- a/test/openssl/test_fips.rb +++ b/test/openssl/test_fips.rb @@ -10,7 +10,9 @@ class OpenSSL::TestFIPS < OpenSSL::TestCase end assert_separately([{ "OSSL_MDEBUG" => nil }, "-ropenssl"], <<~"end;") - assert OpenSSL.fips_mode == true, ".fips_mode returns true on FIPS mode enabled" + OpenSSL.fips_mode = false # debug + + assert OpenSSL.fips_mode == true, ".fips_mode should return true on FIPS mode enabled" end; end @@ -20,7 +22,7 @@ class OpenSSL::TestFIPS < OpenSSL::TestCase end assert_separately([{ "OSSL_MDEBUG" => nil }, "-ropenssl"], <<~"end;") - assert OpenSSL.fips_mode == false, ".fips_mode returns false on FIPS mode disabled" + assert OpenSSL.fips_mode == false, ".fips_mode should return false on FIPS mode disabled" end; end @@ -35,10 +37,10 @@ class OpenSSL::TestFIPS < OpenSSL::TestCase assert_separately([{ "OSSL_MDEBUG" => nil }, "-ropenssl"], <<~"end;") begin OpenSSL.fips_mode = true - assert OpenSSL.fips_mode == true, ".fips_mode returns true when .fips_mode=true" + assert OpenSSL.fips_mode == true, ".fips_mode should return true when .fips_mode=true" OpenSSL.fips_mode = false - assert OpenSSL.fips_mode == false, ".fips_mode returns false when .fips_mode=false" + assert OpenSSL.fips_mode == false, ".fips_mode should return false when .fips_mode=false" rescue OpenSSL::OpenSSLError pend "Could not set FIPS mode (OpenSSL::OpenSSLError: \#$!); skipping" end

Then the result is below. Does this make sense for you?

$ CI=true TEST_RUBY_OPENSSL_FIPS_ENABLED=true LD_LIBRARY_PATH=/home/jaruga/.local/openssl-3.0.8-fips/lib/ OPENSSL_CONF=/home/jaruga/.local/openssl-3.0.8-fips/ssl/openssl_fips.cnf bundle exec ruby test/openssl/test_fips.rb Loaded suite test/openssl/test_fips Started O ================================================================================================== Omission: Only for non-FIPS mode environment on CI [test_fips_mode_get_is_false_on_fips_mode_disabled(OpenSSL::TestFIPS)] test/openssl/test_fips.rb:21:in `test_fips_mode_get_is_false_on_fips_mode_disabled' ================================================================================================== F ================================================================================================== Failure: test_fips_mode_get_is_true_on_fips_mode_enabled(OpenSSL::TestFIPS): .fips_mode should return true on FIPS mode enabled. <false> is not true. /home/jaruga/var/git/ruby/openssl/test/lib/core_assertions.rb:490:in `assert' test/openssl/test_fips.rb:15:in `<main>' test/openssl/test_fips.rb:12:in `test_fips_mode_get_is_true_on_fips_mode_enabled' 9: omit "Only for on FIPS mode environment on CI" 10: end 11: => 12: assert_separately([{ "OSSL_MDEBUG" => nil }, "-ropenssl"], <<~"end;") 13: OpenSSL.fips_mode = false # debug 14: 15: assert OpenSSL.fips_mode == true, ".fips_mode should return true on FIPS mode enabled" ================================================================================================== .. Finished in 0.470415204 seconds. -------------------------------------------------------------------------------------------------- 4 tests, 8 assertions, 1 failures, 0 errors, 0 pendings, 1 omissions, 0 notifications 66.6667% passed -------------------------------------------------------------------------------------------------- 8.50 tests/s, 17.01 assertions/s

junaruga · 2023-03-18T20:04:31Z

I rebased adding the 3rd commit that fixes issues in the review. If it is ok, I will do squash the 3rd commit with the 2nd commit.

junaruga · 2023-03-22T09:51:37Z

I squashed the 3rd commit with the 2nd commit, updating the commit message.

junaruga · 2023-04-12T10:11:40Z

Hello, @rhenium, can you review this PR? Perhaps, can I merge this PR by myself?

test/openssl/fixtures/ssl/openssl_fips.cnf.tmpl: I referred to the following document for the openssl config file for FIPS mode. <https://www.openssl.org/docs/manmaster/man7/fips_module.html> - Making all applications use the FIPS module by default It seems that the `.include` syntax only requires the absolute path. So, the placeholder OPENSSL_DIR in the template file is replaced with the actual OpenSSL directory. .github/workflows/test.yml: The `TEST_RUBY_OPENSSL_FIPS_ENABLED` environment variable is set in the FIPS mode CI case. It can be used in the unit tests.

This commit is to implement the `OpenSSL::OPENSSL_FIPS`, `ossl_fips_mode_get` and `ossl_fips_mode_set` to pass the test `test/openssl/test_fips.rb`. It seems that the `OPENSSL_FIPS` macro is not used on the FIPS mode case any more, and some FIPS related APIs also were removed in OpenSSL 3. See the document <https://github.com/openssl/openssl/blob/master/doc/man7/migration_guide.pod#removed-fips_mode-and-fips_mode_set> the section OPENSSL 3.0 > Main Changes from OpenSSL 1.1.1 > Other notable deprecations and changes - Removed FIPS_mode() and FIPS_mode_set() . The `OpenSSL::OPENSSL_FIPS` returns always true in OpenSSL 3 because the used functions `EVP_default_properties_enable_fips` and `EVP_default_properties_is_fips_enabled` works with the OpenSSL installed without FIPS option. The `TEST_RUBY_OPENSSL_FIPS_ENABLED` is set on the FIPS mode case on the CI. Because I want to test that the `OpenSSL.fips_mode` returns the `true` or 'false' surely in the CI. You can test the FIPS mode case by setting `TEST_RUBY_OPENSSL_FIPS_ENABLED` on local too. Right now I don't find a better way to get the status of the FIPS mode enabled or disabled for this purpose. I am afraid of the possibility that the FIPS test case is unintentionally skipped. I also replaced the ambiguous "returns" with "should return" in the tests.

junaruga · 2023-05-15T11:15:18Z

According to this Ruby ticket where I didn't see any disagreements, I will start to work as a co-maintainer of the ruby/openssl for FIPS mode.

junaruga mentioned this pull request Mar 17, 2023

The FIPS enabled environment for developers #601

Closed

junaruga force-pushed the wip/fips branch from 77cdc9f to 71c2e81 Compare March 17, 2023 12:16

junaruga commented Mar 17, 2023

View reviewed changes

junaruga mentioned this pull request Mar 17, 2023

Support OpenSSL 3 FIPS #603

Open

junaruga requested a review from rhenium March 17, 2023 12:26

voxik reviewed Mar 17, 2023

View reviewed changes

test/openssl/test_fips.rb Show resolved Hide resolved

junaruga force-pushed the wip/fips branch 2 times, most recently from 9acb9b5 to 00fb72f Compare March 17, 2023 22:24

junaruga changed the title ~~Fix fips_mode_get, adding OpenSSL FIPS mode case on CI.~~ Implement the FIPS functions, adding OpenSSL FIPS mode case on CI. Mar 17, 2023

junaruga commented Mar 18, 2023

View reviewed changes

junaruga force-pushed the wip/fips branch from 00fb72f to 3ffff4c Compare March 18, 2023 10:54

junaruga changed the title ~~Implement the FIPS functions, adding OpenSSL FIPS mode case on CI.~~ Implement FIPS functions, adding OpenSSL FIPS mode case on CI. Mar 18, 2023

voxik reviewed Mar 18, 2023

View reviewed changes

junaruga force-pushed the wip/fips branch 2 times, most recently from aed2e77 to c21ddfa Compare March 22, 2023 09:50

junaruga mentioned this pull request Mar 23, 2023

Support OpenSSL 3.0 #369

Closed

junaruga added 2 commits April 12, 2023 21:06

junaruga force-pushed the wip/fips branch from c21ddfa to 285baec Compare April 12, 2023 19:07

junaruga mentioned this pull request Apr 17, 2023

Fix OpenSSL::PKey.read that cannot parse PKey in the FIPS mode. #615

Merged

junaruga merged commit c5b2bc1 into ruby:master May 15, 2023

junaruga deleted the wip/fips branch May 15, 2023 11:16

junaruga mentioned this pull request May 17, 2023

OpenSSL 3: OpenSSL.fips_mode returns false in FIPS enabled environment #605

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement FIPS functions, adding OpenSSL FIPS mode case on CI. #608

Implement FIPS functions, adding OpenSSL FIPS mode case on CI. #608

junaruga commented Mar 17, 2023 •

edited

Loading

junaruga commented Mar 17, 2023 •

edited

Loading

junaruga Mar 17, 2023

voxik Mar 17, 2023

junaruga Mar 17, 2023 •

edited

Loading

voxik Mar 18, 2023

junaruga Mar 18, 2023

junaruga Mar 18, 2023 •

edited

Loading

junaruga commented Mar 17, 2023

junaruga Mar 18, 2023

voxik Mar 18, 2023

junaruga Mar 18, 2023

junaruga commented Mar 18, 2023

junaruga commented Mar 22, 2023 •

edited

Loading

junaruga commented Apr 12, 2023 •

edited

Loading

junaruga commented May 15, 2023

Implement FIPS functions, adding OpenSSL FIPS mode case on CI. #608

Implement FIPS functions, adding OpenSSL FIPS mode case on CI. #608

Conversation

junaruga commented Mar 17, 2023 • edited Loading

CI: Add OpenSSL FIPS mode case.

Fix the fips_mode_get on OpenSSL 3.

junaruga commented Mar 17, 2023 • edited Loading

junaruga Mar 17, 2023

Choose a reason for hiding this comment

voxik Mar 17, 2023

Choose a reason for hiding this comment

junaruga Mar 17, 2023 • edited Loading

Choose a reason for hiding this comment

voxik Mar 18, 2023

Choose a reason for hiding this comment

junaruga Mar 18, 2023

Choose a reason for hiding this comment

junaruga Mar 18, 2023 • edited Loading

Choose a reason for hiding this comment

junaruga commented Mar 17, 2023

junaruga Mar 18, 2023

Choose a reason for hiding this comment

voxik Mar 18, 2023

Choose a reason for hiding this comment

junaruga Mar 18, 2023

Choose a reason for hiding this comment

junaruga commented Mar 18, 2023

junaruga commented Mar 22, 2023 • edited Loading

junaruga commented Apr 12, 2023 • edited Loading

junaruga commented May 15, 2023

junaruga commented Mar 17, 2023 •

edited

Loading

junaruga commented Mar 17, 2023 •

edited

Loading

junaruga Mar 17, 2023 •

edited

Loading

junaruga Mar 18, 2023 •

edited

Loading

junaruga commented Mar 22, 2023 •

edited

Loading

junaruga commented Apr 12, 2023 •

edited

Loading