From c1655427f1ffc4edb92d83009c75e8174765cade Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 31 Jul 2018 07:33:00 +0300 Subject: [PATCH] Ensure KeyStoreWrapper decryption exceptions are handled (#32472) * Ensure decryption related exceptions are handled This commit ensures that all possible Exceptions in KeyStoreWrapper#decrypt() are handled. More specifically, in the case that a wrong password is used for secure settings, calling readX on the DataInputStream that wraps the CipherInputStream can throw an IOException. It also adds a test for loading a KeyStoreWrapper with a wrong password. This is a backport of #32464 --- .../elasticsearch/common/settings/KeyStoreWrapper.java | 2 +- .../action/admin/ReloadSecureSettingsIT.java | 8 -------- .../common/settings/KeyStoreWrapperTests.java | 9 +++++++++ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java index eee45743ee32e..e017e9e7ca93f 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -359,7 +359,7 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio if (input.read() != -1) { throw new SecurityException("Keystore has been corrupted or tampered with"); } - } catch (EOFException e) { + } catch (IOException e) { throw new SecurityException("Keystore has been corrupted or tampered with", e); } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java index c8503603f665c..7952758240544 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java @@ -32,7 +32,6 @@ import org.elasticsearch.plugins.ReloadablePlugin; import org.elasticsearch.test.ESIntegTestCase; -import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; import java.nio.file.StandardCopyOption; @@ -205,14 +204,7 @@ public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { assertThat(nodesMap.size(), equalTo(cluster().size())); for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { assertThat(nodeResponse.reloadException(), notNullValue()); - // Running in a JVM with a BouncyCastle FIPS Security Provider, decrypting the Keystore with the wrong - // password returns a SecurityException if the DataInputStream can't be fully consumed - if (inFipsJvm()) { assertThat(nodeResponse.reloadException(), instanceOf(SecurityException.class)); - } else { - assertThat(nodeResponse.reloadException(), instanceOf(IOException.class)); - } - } } catch (final AssertionError e) { reloadSettingsError.set(e); diff --git a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java index fe7b02d63ecce..bb2b1df7f8c03 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java @@ -99,6 +99,15 @@ public void testCreate() throws Exception { assertTrue(keystore.getSettingNames().contains(KeyStoreWrapper.SEED_SETTING.getKey())); } + public void testDecryptKeyStoreWithWrongPassword() throws Exception { + KeyStoreWrapper keystore = KeyStoreWrapper.create(); + keystore.save(env.configFile(), new char[0]); + final KeyStoreWrapper loadedkeystore = KeyStoreWrapper.load(env.configFile()); + final SecurityException exception = expectThrows(SecurityException.class, + () -> loadedkeystore.decrypt(new char[]{'i', 'n', 'v', 'a', 'l', 'i', 'd'})); + assertThat(exception.getMessage(), containsString("Keystore has been corrupted or tampered with")); + } + public void testCannotReadStringFromClosedKeystore() throws Exception { KeyStoreWrapper keystore = KeyStoreWrapper.create(); assertThat(keystore.getSettingNames(), Matchers.hasItem(KeyStoreWrapper.SEED_SETTING.getKey()));