Ensure CI is run in FIPS 140 approved only mode by jkakavas · Pull Request #64024 · elastic/elasticsearch

jkakavas · 2020-10-21T21:36:58Z

We were depending on the BouncyCastle FIPS own mechanics to set
itself in approved only mode since we run with the Security
Manager enabled. The check during startup seems to happen before we
set our restrictive SecurityManager though in
org.elasticsearch.bootstrap.Elasticsearch , and this means that
BCFIPS would not be in approved only mode, unless explicitly
configured so.

This commit sets the appropriate JVM property to explicitly set
BCFIPS in approved only mode in CI and adds tests to ensure that we
will be running with BCFIPS in approved only mode when we expect to.
It also sets xpack.security.fips_mode.enabled to true for all test clusters
used in fips mode and sets the distribution to the default one. It adds a
password to the elasticsearch keystore for all test clusters that run in fips
mode.
Moreover, it changes a few unit tests where we would use bcrypt even in
FIPS 140 mode. These would still pass since we are bundling our own
bcrypt implementation, but are now changed to use FIPS 140 approved
algorithms instead for better coverage.

It also addresses a number of tests that would fail in approved only mode
Mainly:

Tests that use PBKDF2 with a password less than 112 bits (14char). We
elected to change the passwords used everywhere to be at least 14
characters long instead of mandating
the use of pbkdf2_stretch because both pbkdf2 and
pbkdf2_stretch are supported and allowed in fips mode and it makes sense
to test with both. We could possibly figure out the password algorithm used
for each test and adjust password length accordingly only for pbkdf2 but
there is little value in that. It's good practice to use strong passwords so if
our docs and tests use longer passwords, then it's for the best. The approach
is brittle as there is no guarantee that the next test that will be added won't
use a short password, so we add some testing documentation too.
This leaves us with a possible coverage gap since we do support passwords
as short as 6 characters but we only test with > 14 chars but the
validation itself was not tested even before. Tests can be added in a followup,
outside of fips related context.
Tests that use a PKCS12 keystore and were not already muted.
Tests that depend on running test clusters with a basic license or
using the OSS distribution as FIPS 140 support is not available in
neither of these.

Finally, it adds some information around FIPS 140 testing in our testing
documentation reference so that developers can hopefully keep in
mind fips 140 related intricacies when writing/changing docs.

We were depending on the BouncyCastle FIPS own mechanics to set itself in approved only mode since we run with the Security Manager enabled. The check during startup seems to happen before we set our restrictive SecurityManager though in org.elasticsearch.bootstrap.Elasticsearch , and this means that BCFIPS would not be in approved onlu mode, unless explicitly configured so. This commit sets the appropriate JVM property to explicitly set BCFIPS in approved only mode and addresses a number of tests that would fail in approved mode. Mainly: - Tests that use PBKDF2 with a password less than 112 bits (14char) - Tests that use a PKCS12 keystore and were not already muted. It also changes a few unit tests where we would use bcrypt even in FIPS 140 mode. These would still pass since we are bundling our own bcrypt implementation, but are now changed to use FIPS 140 approved algorithms instead for better coverage.

elasticmachine · 2020-10-21T21:37:00Z

Pinging @elastic/es-security (:Security/Security)

ywangd

I left a few comments. Woud like to have another round later. Thanks!

...plugin/core/src/test/java/org/elasticsearch/xpack/core/Fips140ProviderVerificationTests.java

...internalClusterTest/java/org/elasticsearch/integration/ClusterPrivilegeIntegrationTests.java

ywangd · 2020-10-22T00:17:28Z

...rc/internalClusterTest/java/org/elasticsearch/integration/CreateDocsIndexPrivilegeTests.java

            "    - names: '*'\n" +
            "      privileges: [ all ]\n" +
-        "create_doc_role:\n" +
+            "create_doc_role:\n" +


Nit: The indentation is now inconsistent with all_indices_role. I do think the change here looks better. But I'd suggest we make the same change to all_indices_role as well.

...rc/internalClusterTest/java/org/elasticsearch/integration/CreateDocsIndexPrivilegeTests.java

...erTest/java/org/elasticsearch/xpack/security/authz/store/NativePrivilegeStoreCacheTests.java

...plugin/core/src/test/java/org/elasticsearch/xpack/core/Fips140ProviderVerificationTests.java

tvernum · 2020-10-22T02:25:33Z

...plugin/core/src/test/java/org/elasticsearch/xpack/core/Fips140ProviderVerificationTests.java

+
+public class Fips140ProviderVerificationTests extends ESTestCase {
+
+    public void testBcFipsProviderInUse(){


Nit

Suggested change

public void testBcFipsProviderInUse(){

public void testBcFipsProviderInUse() {

...plugin/core/src/test/java/org/elasticsearch/xpack/core/Fips140ProviderVerificationTests.java

...rity/src/test/java/org/elasticsearch/xpack/security/authc/file/FileUserPasswdStoreTests.java

.../plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlTestCase.java

tvernum · 2020-10-22T02:37:45Z

.../plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlTestCase.java

                break;
            default:
-                keySize = randomFrom(1024, 2048);
+                keySize = randomFrom(2048);


Suggested change

keySize = randomFrom(2048);

keySize = 2048;

...t/java/org/elasticsearch/xpack/security/authc/support/CachingUsernamePasswordRealmTests.java

ywangd · 2020-10-26T05:38:20Z

I could be missing something, but is xpack.security.fips_mode.enabled set to true anywhere when -Dtests.fips.enabled=true. It seems to me that we should at least have it enabled for testClusters?

…hing algo for testclusters

…chars

…tests

In certain situations, such as when configured in FIPS 140 mode, the Java security provider in use might throw a subclass of java.lang.Error. We currently do not catch these and as a result the JVM exits, shutting down elasticsearch. This commit attempts to address this by catching subclasses of Error that might be thrown for instance when a PBKDF2 implementation is used from a Security Provider in FIPS 140 mode, with the password input being less than 14 bytes (112 bits). - In our PBKDF2 family of hashers, we catch the Error and throw an ElasticsearchException while creating or verifying the hash. We throw on verification instead of simply returning false on purpose so that the message bubbles up and the cause becomes obvious (otherwise it would be indistinguishable from a wrong password). - In KeyStoreWrapper, we catch the Error in order to wrap and re-throw a GeneralSecurityException with a helpful message. This can happen when using any of the keystore CLI commands, when the node starts or when we attempt to reload secure settings. - In the `elasticsearch-users` tool, we catch the ElasticsearchException that the Hasher class re-throws and throw an appropriate UserException. Tests are missing because it's not trivial to set CI in fips approved mode right now, and thus any tests would need to be muted. There is a parallel effort in #64024 to enable that and tests will be added in a followup.

In certain situations, such as when configured in FIPS 140 mode, the Java security provider in use might throw a subclass of java.lang.Error. We currently do not catch these and as a result the JVM exits, shutting down elasticsearch. This commit attempts to address this by catching subclasses of Error that might be thrown for instance when a PBKDF2 implementation is used from a Security Provider in FIPS 140 mode, with the password input being less than 14 bytes (112 bits). - In our PBKDF2 family of hashers, we catch the Error and throw an ElasticsearchException while creating or verifying the hash. We throw on verification instead of simply returning false on purpose so that the message bubbles up and the cause becomes obvious (otherwise it would be indistinguishable from a wrong password). - In KeyStoreWrapper, we catch the Error in order to wrap and re-throw a GeneralSecurityException with a helpful message. This can happen when using any of the keystore CLI commands, when the node starts or when we attempt to reload secure settings. - In the `elasticsearch-users` tool, we catch the ElasticsearchException that the Hasher class re-throws and throw an appropriate UserException. Tests are missing because it's not trivial to set CI in fips approved mode right now, and thus any tests would need to be muted. There is a parallel effort in elastic#64024 to enable that and tests will be added in a followup.

In certain situations, such as when configured in FIPS 140 mode, the Java security provider in use might throw a subclass of java.lang.Error. We currently do not catch these and as a result the JVM exits, shutting down elasticsearch. This commit attempts to address this by catching subclasses of Error that might be thrown for instance when a PBKDF2 implementation is used from a Security Provider in FIPS 140 mode, with the password input being less than 14 bytes (112 bits). - In our PBKDF2 family of hashers, we catch the Error and throw an ElasticsearchException while creating or verifying the hash. We throw on verification instead of simply returning false on purpose so that the message bubbles up and the cause becomes obvious (otherwise it would be indistinguishable from a wrong password). - In KeyStoreWrapper, we catch the Error in order to wrap and re-throw a GeneralSecurityException with a helpful message. This can happen when using any of the keystore CLI commands, when the node starts or when we attempt to reload secure settings. - In the `elasticsearch-users` tool, we catch the ElasticsearchException that the Hasher class re-throws and throw an appropriate UserException. Tests are missing because it's not trivial to set CI in fips approved mode right now, and thus any tests would need to be muted. There is a parallel effort in #64024 to enable that and tests will be added in a followup.

…tests

…sic. All the tests that require basic license are already muted in fips mode so they won't be affected

…sions that can be run in a correctly configured FIPS 140-2 JVM and we are lacking granural control of which bwc versions can be used

…tests

…tores so that these can be run in fips mode

…d of keystores so that these can be run in fips mode

…tests

As of elastic#64024 we run FIPS CI on a true, FIPS approved only mode JVM. This mandates that any passwords that are fed into PBKDF2 must have at least 112 bits of entropy (that is, be 14 characters long). This commit updates our Keystore CLI tests so that tests either: 1. Use a 14+ character password when in FIPS mode, _or_ 2. Are skipped on FIPS mode (because they explicitly test empty passwords) Resolves: elastic#66845

As of #64024 we run FIPS CI on a true, FIPS approved only mode JVM. This mandates that any passwords that are fed into PBKDF2 must have at least 112 bits of entropy (that is, be 14 characters long). This commit updates our Keystore CLI tests so that tests either: 1. Use a 14+ character password when in FIPS mode, _or_ 2. Are skipped on FIPS mode (because they explicitly test empty passwords) Resolves: #66845

As of elastic#64024 we run FIPS CI on a true, FIPS approved only mode JVM. This mandates that any passwords that are fed into PBKDF2 must have at least 112 bits of entropy (that is, be 14 characters long). This commit updates our Keystore CLI tests so that tests either: 1. Use a 14+ character password when in FIPS mode, _or_ 2. Are skipped on FIPS mode (because they explicitly test empty passwords) Backport of: elastic#66846

As of #64024 we run FIPS CI on a true, FIPS approved only mode JVM. This mandates that any passwords that are fed into PBKDF2 must have at least 112 bits of entropy (that is, be 14 characters long). This commit updates our Keystore CLI tests so that tests either: 1. Use a 14+ character password when in FIPS mode, _or_ 2. Are skipped on FIPS mode (because they explicitly test empty passwords) Backport of: #66846

As of elastic#64024 we run FIPS CI on a true, FIPS approved only mode JVM. This mandates that any passwords that are fed into PBKDF2 must have at least 112 bits of entropy (that is, be 14 characters long). This commit updates our Keystore CLI tests so that tests either: 1. Use a 14+ character password when in FIPS mode, _or_ 2. Are skipped on FIPS mode (because they explicitly test empty passwords) Backport of: elastic#66846

As of #64024 we run FIPS CI on a true, FIPS approved only mode JVM. This mandates that any passwords that are fed into PBKDF2 must have at least 112 bits of entropy (that is, be 14 characters long). This commit updates our Keystore CLI tests so that tests either: 1. Use a 14+ character password when in FIPS mode, _or_ 2. Are skipped on FIPS mode (because they explicitly test empty passwords) Backport of: #66846 Co-authored-by: Tim Vernum <tim.vernum@elastic.co>

jkakavas added >test Issues or PRs that are addressing/adding tests :Security/Security Security issues without another label labels Oct 21, 2020

jkakavas requested review from tvernum and ywangd October 21, 2020 21:36

elasticmachine added the Team:Security Meta label for security team label Oct 21, 2020

ywangd reviewed Oct 22, 2020

View reviewed changes

tvernum reviewed Oct 22, 2020

View reviewed changes

jkakavas added 4 commits October 22, 2020 12:41

fix passwords for tests

0a2ff00

Re-enable FileUserPasswdStoreTests

1c7fd10

unused import

fabb237

Adjust a couple of more tests

b6cbb68

jkakavas requested review from tvernum and ywangd October 22, 2020 12:38

jkakavas added 3 commits October 26, 2020 12:31

Fix integration tests to use long passwords

9f8c030

Set fips_mode.enabled to true and set a defaut compliant password has…

547028b

…hing algo for testclusters

revert test debugging change

4179e18

jkakavas marked this pull request as draft October 26, 2020 21:41

jkakavas added 2 commits October 27, 2020 22:12

Add password to keystores and make sure passwords are longer than 14 …

2c5ee11

…chars

Merge remote-tracking branch 'origin/master' into fips-approved-mode-…

3531517

…tests

This was referenced Nov 24, 2020

Gracefully handle exceptions from Security Providers #65464

Merged

Introduce an additional hasher (PBKDF2_STRETCH) #65328

Merged

Use long passwords in rest tests

312d394

jkakavas added 3 commits December 19, 2020 11:42

reference issue in muted CCR tests

7ef71b7

Merge remote-tracking branch 'origin/master' into fips-approved-mode-…

22fd8ea

…tests

handle encrypted keystores in SetupPasswordToolIT

f9f18f1

jkakavas mentioned this pull request Dec 23, 2020

Re-enable BWC testing in fips mode #66772

Closed

jkakavas added 10 commits December 23, 2020 09:49

set license to trial for testclusters as we can't use fips mode in ba…

5fa921b

…sic. All the tests that require basic license are already muted in fips mode so they won't be affected

Disable bwc tests in fips mode as there arent' many ( only 7.11 ) ver…

4199001

…sions that can be run in a correctly configured FIPS 140-2 JVM and we are lacking granural control of which bwc versions can be used

use long enough azure.client.key in tests

6028fc0

use long enough ec2 access keys in tests

658e829

another short access key

7eb7204

Merge remote-tracking branch 'origin/master' into fips-approved-mode-…

15e930a

…tests

Change docker integ tests to use PEM key/certificates instead of keys…

0d54d3d

…tores so that these can be run in fips mode

Change remote-clusters integ tests to use PEM key/certificates instea…

b7bdcde

…d of keystores so that these can be run in fips mode

Fix all s3 fixture related tests by using the correct access key

801da6f

Merge remote-tracking branch 'origin/master' into fips-approved-mode-…

f812ca2

…tests

jkakavas merged commit bd87369 into elastic:master Dec 23, 2020

This was referenced Dec 23, 2020

Ensure CI is run in FIPS 140 approved only mode #66804

Merged

Ensure CI is run in FIPS 140 approved only mode #66805

Merged

testIncorrectPasswordHashingAlgorithm failures #66824

Closed

tvernum mentioned this pull request Dec 29, 2020

Support keystore tests on FIPS JVM #66846

Merged

tvernum mentioned this pull request Dec 31, 2020

[Backport] Support keystore tests on FIPS JVM #66902

Merged

ywangd mentioned this pull request Jan 4, 2021

[Backport] Support keystore tests on FIPS JVM #66964

Merged

jkakavas mentioned this pull request Mar 5, 2021

[CI] Use INTEG_TEST distribution in FIPS 140 testing when possible #70005

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure CI is run in FIPS 140 approved only mode#64024

Ensure CI is run in FIPS 140 approved only mode#64024
jkakavas merged 79 commits intoelastic:masterfrom
jkakavas:fips-approved-mode-tests

jkakavas commented Oct 21, 2020 •

edited

Loading

Uh oh!

elasticmachine commented Oct 21, 2020

Uh oh!

ywangd left a comment

Uh oh!

Uh oh!

Uh oh!

ywangd Oct 22, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tvernum Oct 22, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tvernum Oct 22, 2020

Uh oh!

Uh oh!

ywangd commented Oct 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants


		public class Fips140ProviderVerificationTests extends ESTestCase {

		public void testBcFipsProviderInUse(){

	public void testBcFipsProviderInUse(){
	public void testBcFipsProviderInUse() {

Conversation

jkakavas commented Oct 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Oct 21, 2020

Uh oh!

ywangd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ywangd Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tvernum Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tvernum Oct 22, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ywangd commented Oct 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jkakavas commented Oct 21, 2020 •

edited

Loading