Support keystore tests on FIPS JVM#66846
Conversation
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
|
Pinging @elastic/es-security (Team:Security) |
| String password = randomFrom("", "keystorepassword"); | ||
| terminal.addSecretInput(password); | ||
| terminal.addSecretInput(password); | ||
| execute(); |
There was a problem hiding this comment.
This actually never tested with the password, because it didn't pass --password on the command line.
When testing with an incorrect password, make sure it is longer than the original password so that we trigger "incorrect password" behaviour rather than "password too short" behaviour.
| terminal.addSecretInput("the-better-password"); | ||
| terminal.addSecretInput("the-better-password"); | ||
| // Prompted thrice: Once for the existing and twice for the new password | ||
| execute(); |
There was a problem hiding this comment.
Do we need to execute(randomFrom("-p", "--password")) here as well?
There was a problem hiding this comment.
No, it's calling the ChangeKeyStorePasswordCommand (elasticsearch-keystore passwd) which doesn 't have a "--password" option (because it always deals with passwords).
| } | ||
|
|
||
| public void testDefaultNotPromptForPassword() throws Exception { | ||
| assumeFalse("Cannot open unprotected keystore on FIPS JVM", inFipsJvm()); |
There was a problem hiding this comment.
Ideally, when not firefighting, I think we should assert the behavior from the FIPS JVM in circumstances such as this one. For regular JVMs, the tests assert all kinds of behaviors, but since some are skipped in the FIPS JVM we condone unspecified behavior.
@jkakavas Is #65464 supposed to handle some/all cases like this one?
Granted, this assumes the FIPS provider implementation (different ones might fail differently), and maybe the JVMs as well. I see this as an argument for more clearly specifying and documenting the FIPS environments we test (and support).
Should I open a separate GH issue regarding negative testing in the FIPS JVM?
There was a problem hiding this comment.
This is not covered in 65454. We can add tests for certain cases with assumeTrue in fips mode, but only for ones we can anticipate the behavior, i.e. short passwords. As you say its tricky.
I see this as an argument for more clearly specifying and documenting the FIPS environments we test (and support).
Agreed, there is an ongoing separate discussion around this
There was a problem hiding this comment.
If there are specific cases in this PR where it feels like we're skipping a test for something that should work on FIPS, then definitely point it out because I tried hard to resolve all of those (which increased coverage on non-FIPS as well, since there were scenarios that we never tested with a password).
However, in this case, this is simply something that is not supported on FIPS. The test is that we don't prompt for a password if there isn't one, and that's not a valid scenario to test of FIPS. We simply cannot read a keystore without a password.
I could write a test to check that unprotected keystores fail on FIPS, except that's not really a requirement per se, it's just a consequence of the JVM being in FIPS mode.
One could make an argument that we should have a test testThatReadingUnprotectedKeystoreInFipsModeFailsWithUsefulMessageAndDoesntCorruptKeystore (maybe a shorter test name) but it's probably not a super high priority.
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
|
The failures no longer exist since this PR is merged. I am going to merge the pending backport to 7.x and also backport it to 7.11. |
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>
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:
passwords)
Resolves: #66845