Settings: Reimplement keystore format to use FIPS compliant algorithms#28255
Settings: Reimplement keystore format to use FIPS compliant algorithms#28255rjernst merged 4 commits intoelastic:masterfrom
Conversation
This commit switches the internal format of the elasticsearch keystore to no longer use java's KeyStore class, but instead encrypt the binary data of the secrets using AES-GCM. The cipher key is generated using PBKDF2WithHmacSHA512. Tests are also added for backcompat reading the v1 and v2 formats.
|
I added |
jaymode
left a comment
There was a problem hiding this comment.
I left one comment, but other than that LGTM
| /** The algorithm used to store file setting contents. */ | ||
| private static final String NEW_KEYSTORE_FILE_KEY_ALGO = "PBE"; | ||
| /** The number of bits for the cipher key. */ | ||
| private static final int CIPHER_KEY_BITS = 256; |
There was a problem hiding this comment.
I'm all for 256 bit, but the Oracle JDK 8 ships with a limited JCE policy that restricts key length for AES to 128 bits. To prevent friction with using our secret settings, we need to use 128 bit keys until we have Java 9 as the minimum supported version, http://www.oracle.com/technetwork/java/javase/terms/readme/jdk9-readme-3852447.html#jce
jkakavas
left a comment
There was a problem hiding this comment.
LGTM, a minor comment on testing and a mental not for documentation
| output.writeString("file_setting"); | ||
| output.writeString("FILE"); | ||
|
|
||
| SecretKeyFactory secretFactory = SecretKeyFactory.getInstance("PBE"); |
There was a problem hiding this comment.
We would probably want to run these in CI in a JVM with FIPS support in FIPS approved mode only, which means we won't be able to get a PBE instance of the factory. We could try and catch the NoSuchAlgorithmException and succeed the test as we can't offer backwards compatibility in the sense of "ability to read the old keystore format while running under FIPS approved only mode", anyway.
| return bytes.toByteArray(); | ||
| } | ||
|
|
||
| private void decryptLegacyEntries() throws GeneralSecurityException, IOException { |
There was a problem hiding this comment.
This won't work if the JVM is in FIPS approved mode only as PBE is not available and SecretKeyFactory.getInstance("PBE"); will throw an Exception.
I'm thinking of the case where someone wants to upgrade to 6.3.0 and at the same time enable FIPS approved only mode. We can highlight this and document manual steps that will ensure the keystore gets upgraded before FIPS approved mode is enabled.
There was a problem hiding this comment.
We can add an assumeFalse once we are ready to test that.
There was a problem hiding this comment.
👍 to adding an assumption once we get to the point of running tests on a FIPS enabled JVM
#28255) This commit switches the internal format of the elasticsearch keystore to no longer use java's KeyStore class, but instead encrypt the binary data of the secrets using AES-GCM. The cipher key is generated using PBKDF2WithHmacSHA512. Tests are also added for backcompat reading the v1 and v2 formats.
In elastic#28255 the implementation of the elasticsearch.keystore was changed to no longer be built on top of a PKCS#12 keystore. A side effect of that change was that calling getString or getFile on a closed KeyStoreWrapper ceased to throw an exception, and would instead return a value consisting of all 0 bytes. This change restores the previous behaviour as closely as possible. It is possible to retrieve the _keys_ from a closed keystore, but any attempt to get or set the entries will throw an IllegalStateException. The only divergence from the previous behaviour is that "isLoaded" will now return false if the keystore is closed, in keeping with the "loaded and retrievable" description in the parent javadoc.
In #28255 the implementation of the elasticsearch.keystore was changed to no longer be built on top of a PKCS#12 keystore. A side effect of that change was that calling getString or getFile on a closed KeyStoreWrapper ceased to throw an exception, and would instead return a value consisting of all 0 bytes. This change restores the previous behaviour as closely as possible. It is possible to retrieve the _keys_ from a closed keystore, but any attempt to get or set the entries will throw an IllegalStateException.
In #28255 the implementation of the elasticsearch.keystore was changed to no longer be built on top of a PKCS#12 keystore. A side effect of that change was that calling getString or getFile on a closed KeyStoreWrapper ceased to throw an exception, and would instead return a value consisting of all 0 bytes. This change restores the previous behaviour as closely as possible. It is possible to retrieve the _keys_ from a closed keystore, but any attempt to get or set the entries will throw an IllegalStateException.
In #28255 the implementation of the elasticsearch.keystore was changed to no longer be built on top of a PKCS#12 keystore. A side effect of that change was that calling getString or getFile on a closed KeyStoreWrapper ceased to throw an exception, and would instead return a value consisting of all 0 bytes. This change restores the previous behaviour as closely as possible. It is possible to retrieve the _keys_ from a closed keystore, but any attempt to get or set the entries will throw an IllegalStateException.
This commit switches the internal format of the elasticsearch keystore
to no longer use java's KeyStore class, but instead encrypt the binary
data of the secrets using AES-GCM. The cipher key is generated using
PBKDF2WithHmacSHA512. Tests are also added for backcompat reading the v1
and v2 formats.