Ensure KeyStoreWrapper decryption exceptions are handled (#32464)

* 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.

Resolves #32411
This commit is contained in:
Ioannis Kakavas 2018-07-30 22:15:59 +03:00 committed by GitHub
parent 34d006f82a
commit c2e3bebab9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 10 additions and 9 deletions

View File

@ -359,7 +359,7 @@ public class KeyStoreWrapper implements SecureSettings {
if (input.read() != -1) { if (input.read() != -1) {
throw new SecurityException("Keystore has been corrupted or tampered with"); 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); throw new SecurityException("Keystore has been corrupted or tampered with", e);
} }
} }

View File

@ -32,7 +32,6 @@ import org.elasticsearch.plugins.PluginsService;
import org.elasticsearch.plugins.ReloadablePlugin; import org.elasticsearch.plugins.ReloadablePlugin;
import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.ESIntegTestCase;
import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.StandardCopyOption; import java.nio.file.StandardCopyOption;
@ -205,14 +204,7 @@ public class ReloadSecureSettingsIT extends ESIntegTestCase {
assertThat(nodesMap.size(), equalTo(cluster().size())); assertThat(nodesMap.size(), equalTo(cluster().size()));
for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) {
assertThat(nodeResponse.reloadException(), notNullValue()); 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)); assertThat(nodeResponse.reloadException(), instanceOf(SecurityException.class));
} else {
assertThat(nodeResponse.reloadException(), instanceOf(IOException.class));
}
} }
} catch (final AssertionError e) { } catch (final AssertionError e) {
reloadSettingsError.set(e); reloadSettingsError.set(e);

View File

@ -99,6 +99,15 @@ public class KeyStoreWrapperTests extends ESTestCase {
assertTrue(keystore.getSettingNames().contains(KeyStoreWrapper.SEED_SETTING.getKey())); 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 { public void testCannotReadStringFromClosedKeystore() throws Exception {
KeyStoreWrapper keystore = KeyStoreWrapper.create(); KeyStoreWrapper keystore = KeyStoreWrapper.create();
assertThat(keystore.getSettingNames(), Matchers.hasItem(KeyStoreWrapper.SEED_SETTING.getKey())); assertThat(keystore.getSettingNames(), Matchers.hasItem(KeyStoreWrapper.SEED_SETTING.getKey()));