From c58f3aac144e41f531ddaa8d1b0fec169d142e91 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 15 Jan 2019 13:39:54 +0200 Subject: [PATCH 01/15] Add passphrase support to elasticsearch-keystore - Subcommands of elasticsearch-keystore can handle (open and create) passphrase protected keystores - When reading a keystore, a user is only prompted for a passphrase only if the keystore is passphrase protected. Relates to: https://github.com/elastic/elasticsearch/issues/32691 --- .../settings/AddFileKeyStoreCommand.java | 91 ++++++++------ .../settings/AddStringKeyStoreCommand.java | 89 +++++++++----- .../settings/CreateKeyStoreCommand.java | 46 ++++--- .../common/settings/ListKeyStoreCommand.java | 25 ++-- .../RemoveSettingKeyStoreCommand.java | 28 +++-- .../settings/AddFileKeyStoreCommandTests.java | 113 ++++++++++++++---- .../AddStringKeyStoreCommandTests.java | 99 +++++++++++---- .../settings/CreateKeyStoreCommandTests.java | 21 ++++ .../settings/KeyStoreCommandTestCase.java | 8 +- .../settings/ListKeyStoreCommandTests.java | 28 ++++- .../RemoveSettingKeyStoreCommandTests.java | 41 +++++-- 11 files changed, 430 insertions(+), 159 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java index f5b3cb9cf7104..e696b255f2d47 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java @@ -53,45 +53,68 @@ class AddFileKeyStoreCommand extends EnvironmentAwareCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); - if (keystore == null) { - if (options.has(forceOption) == false && - terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { - terminal.println("Exiting without creating keystore."); - return; + char[] password = null; + char[] passwordVerification = null; + try { + KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); + if (keystore == null) { + if (options.has(forceOption) == false && + terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { + terminal.println("Exiting without creating keystore."); + return; + } + password = terminal.readSecret("Enter passphrase for the elasticsearch keystore (empty for no passphrase): "); + passwordVerification = terminal.readSecret("Enter same passphrase again: "); + if (Arrays.equals(password, passwordVerification) == false) { + throw new UserException(ExitCodes.DATA_ERROR, "Passphrases are not equal, exiting."); + } + keystore = KeyStoreWrapper.create(); + keystore.save(env.configFile(), password); + terminal.println("Created elasticsearch keystore in " + env.configFile()); + } else { + if (keystore.hasPassword()) { + password = terminal.readSecret("Enter passphrase for the elasticsearch keystore: "); + } else { + password = new char[0]; + } + keystore.decrypt(password); } - keystore = KeyStoreWrapper.create(); - keystore.save(env.configFile(), new char[0] /* always use empty passphrase for auto created keystore */); - terminal.println("Created elasticsearch keystore in " + env.configFile()); - } else { - keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); - } - List argumentValues = arguments.values(options); - if (argumentValues.size() == 0) { - throw new UserException(ExitCodes.USAGE, "Missing setting name"); - } - String setting = argumentValues.get(0); - if (keystore.getSettingNames().contains(setting) && options.has(forceOption) == false) { - if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { - terminal.println("Exiting without modifying keystore."); - return; + List argumentValues = arguments.values(options); + if (argumentValues.size() == 0) { + throw new UserException(ExitCodes.USAGE, "Missing setting name"); + } + String setting = argumentValues.get(0); + if (keystore.getSettingNames().contains(setting) && options.has(forceOption) == false) { + if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { + terminal.println("Exiting without modifying keystore."); + return; + } } - } - if (argumentValues.size() == 1) { - throw new UserException(ExitCodes.USAGE, "Missing file name"); - } - Path file = getPath(argumentValues.get(1)); - if (Files.exists(file) == false) { - throw new UserException(ExitCodes.IO_ERROR, "File [" + file.toString() + "] does not exist"); - } - if (argumentValues.size() > 2) { - throw new UserException(ExitCodes.USAGE, "Unrecognized extra arguments [" + - String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath"); + if (argumentValues.size() == 1) { + throw new UserException(ExitCodes.USAGE, "Missing file name"); + } + Path file = getPath(argumentValues.get(1)); + if (Files.exists(file) == false) { + throw new UserException(ExitCodes.IO_ERROR, "File [" + file.toString() + "] does not exist"); + } + if (argumentValues.size() > 2) { + throw new UserException(ExitCodes.USAGE, "Unrecognized extra arguments [" + + String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath"); + } + keystore.setFile(setting, Files.readAllBytes(file)); + keystore.save(env.configFile(), password); + } catch (SecurityException e) { + throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); + } finally { + if (null != password) { + Arrays.fill(password, '\u0000'); + } + if (null != passwordVerification) { + Arrays.fill(passwordVerification, '\u0000'); + } } - keystore.setFile(setting, Files.readAllBytes(file)); - keystore.save(env.configFile(), new char[0]); } @SuppressForbidden(reason="file arg for cli") diff --git a/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java index ee6614618010b..aa4547028768d 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java @@ -56,44 +56,67 @@ InputStream getStdin() { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); - if (keystore == null) { - if (options.has(forceOption) == false && - terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { - terminal.println("Exiting without creating keystore."); - return; + char[] password = null; + char[] passwordVerification = null; + try { + KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); + if (keystore == null) { + if (options.has(forceOption) == false && + terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { + terminal.println("Exiting without creating keystore."); + return; + } + password = terminal.readSecret("Enter passphrase for the elasticsearch keystore (empty for no passphrase): "); + passwordVerification = terminal.readSecret("Enter same passphrase again: "); + if (Arrays.equals(password, passwordVerification) == false) { + throw new UserException(ExitCodes.DATA_ERROR, "Passphrases are not equal, exiting."); + } + keystore = KeyStoreWrapper.create(); + keystore.save(env.configFile(), password); + terminal.println("Created elasticsearch keystore in " + env.configFile()); + } else { + if (keystore.hasPassword()) { + password = terminal.readSecret("Enter passphrase for the elasticsearch keystore: "); + } else { + password = new char[0]; + } + keystore.decrypt(password); } - keystore = KeyStoreWrapper.create(); - keystore.save(env.configFile(), new char[0] /* always use empty passphrase for auto created keystore */); - terminal.println("Created elasticsearch keystore in " + env.configFile()); - } else { - keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); - } - String setting = arguments.value(options); - if (setting == null) { - throw new UserException(ExitCodes.USAGE, "The setting name can not be null"); - } - if (keystore.getSettingNames().contains(setting) && options.has(forceOption) == false) { - if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { - terminal.println("Exiting without modifying keystore."); - return; + String setting = arguments.value(options); + if (setting == null) { + throw new UserException(ExitCodes.USAGE, "The setting name can not be null"); + } + if (keystore.getSettingNames().contains(setting) && options.has(forceOption) == false) { + if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { + terminal.println("Exiting without modifying keystore."); + return; + } } - } - final char[] value; - if (options.has(stdinOption)) { - BufferedReader stdinReader = new BufferedReader(new InputStreamReader(getStdin(), StandardCharsets.UTF_8)); - value = stdinReader.readLine().toCharArray(); - } else { - value = terminal.readSecret("Enter value for " + setting + ": "); - } + final char[] value; + if (options.has(stdinOption)) { + BufferedReader stdinReader = new BufferedReader(new InputStreamReader(getStdin(), StandardCharsets.UTF_8)); + value = stdinReader.readLine().toCharArray(); + } else { + value = terminal.readSecret("Enter value for " + setting + ": "); + } - try { - keystore.setString(setting, value); - } catch (IllegalArgumentException e) { - throw new UserException(ExitCodes.DATA_ERROR, "String value must contain only ASCII"); + try { + keystore.setString(setting, value); + } catch (IllegalArgumentException e) { + throw new UserException(ExitCodes.DATA_ERROR, "String value must contain only ASCII"); + } + keystore.save(env.configFile(), password); + } catch (SecurityException e) { + throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); + } finally { + if (null != password) { + Arrays.fill(password, '\u0000'); + } + if (null != passwordVerification) { + Arrays.fill(passwordVerification, '\u0000'); + } } - keystore.save(env.configFile(), new char[0]); } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index 3529d7f6810bd..1525a4a919869 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -21,10 +21,13 @@ import java.nio.file.Files; import java.nio.file.Path; +import java.util.Arrays; import joptsimple.OptionSet; import org.elasticsearch.cli.EnvironmentAwareCommand; +import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; /** @@ -38,24 +41,33 @@ class CreateKeyStoreCommand extends EnvironmentAwareCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); - if (Files.exists(keystoreFile)) { - if (terminal.promptYesNo("An elasticsearch keystore already exists. Overwrite?", false) == false) { - terminal.println("Exiting without creating keystore."); - return; + char[] password = null; + char[] passwordVerification = null; + try { + Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); + if (Files.exists(keystoreFile)) { + if (terminal.promptYesNo("An elasticsearch keystore already exists. Overwrite?", false) == false) { + terminal.println("Exiting without creating keystore."); + return; + } + } + password = terminal.readSecret("Enter passphrase (empty for no passphrase): "); + passwordVerification = terminal.readSecret("Enter same passphrase again: "); + if (Arrays.equals(password, passwordVerification) == false) { + throw new UserException(ExitCodes.DATA_ERROR, "Passphrases are not equal, exiting."); + } + KeyStoreWrapper keystore = KeyStoreWrapper.create(); + keystore.save(env.configFile(), password); + terminal.println("Created elasticsearch keystore in " + env.configFile()); + } catch (SecurityException e) { + throw new UserException(ExitCodes.IO_ERROR, "Error creating the elasticsearch keystore.", e); + } finally { + if (null != password) { + Arrays.fill(password, '\u0000'); + } + if (null != passwordVerification) { + Arrays.fill(passwordVerification, '\u0000'); } } - - - char[] password = new char[0];// terminal.readSecret("Enter passphrase (empty for no passphrase): "); - /* TODO: uncomment when entering passwords on startup is supported - char[] passwordRepeat = terminal.readSecret("Enter same passphrase again: "); - if (Arrays.equals(password, passwordRepeat) == false) { - throw new UserException(ExitCodes.DATA_ERROR, "Passphrases are not equal, exiting."); - }*/ - - KeyStoreWrapper keystore = KeyStoreWrapper.create(); - keystore.save(env.configFile(), password); - terminal.println("Created elasticsearch keystore in " + env.configFile()); } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java index 8eef02f213189..b8098aa6fc295 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java @@ -21,6 +21,7 @@ import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -46,13 +47,23 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th if (keystore == null) { throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); } - - keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); - - List sortedEntries = new ArrayList<>(keystore.getSettingNames()); - Collections.sort(sortedEntries); - for (String entry : sortedEntries) { - terminal.println(entry); + char[] password; + if (keystore.hasPassword()) { + password = terminal.readSecret("Enter elasticsearch keystore passphrase (empty for no passphrase): "); + } else { + password = new char[0]; + } + try { + keystore.decrypt(password); + List sortedEntries = new ArrayList<>(keystore.getSettingNames()); + Collections.sort(sortedEntries); + for (String entry : sortedEntries) { + terminal.println(entry); + } + } catch (SecurityException e) { + throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); + } finally { + Arrays.fill(password, '\u0000'); } } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java index 9a83375e6e01a..fc88b99e1a218 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java @@ -19,6 +19,7 @@ package org.elasticsearch.common.settings; +import java.util.Arrays; import java.util.List; import joptsimple.OptionSet; @@ -52,15 +53,28 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th if (keystore == null) { throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); } + char[] password = null; + try { + if (keystore.hasPassword()) { + password = terminal.readSecret("Enter passphrase for the elasticsearch keystore: "); + } else { + password = new char[0]; + } + keystore.decrypt(password); - keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); - - for (String setting : arguments.values(options)) { - if (keystore.getSettingNames().contains(setting) == false) { - throw new UserException(ExitCodes.CONFIG, "Setting [" + setting + "] does not exist in the keystore."); + for (String setting : arguments.values(options)) { + if (keystore.getSettingNames().contains(setting) == false) { + throw new UserException(ExitCodes.CONFIG, "Setting [" + setting + "] does not exist in the keystore."); + } + keystore.remove(setting); + } + keystore.save(env.configFile(), password); + } catch (SecurityException e) { + throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); + } finally { + if (null != password) { + Arrays.fill(password, '\u0000'); } - keystore.remove(setting); } - keystore.save(env.configFile(), new char[0]); } } diff --git a/server/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java index 6cfa2c1fdf255..16ade3543f322 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java @@ -53,110 +53,171 @@ private Path createRandomFile() throws IOException { return file; } - private void addFile(KeyStoreWrapper keystore, String setting, Path file) throws Exception { + private void addFile(KeyStoreWrapper keystore, String setting, Path file, String password) throws Exception { keystore.setFile(setting, Files.readAllBytes(file)); - keystore.save(env.configFile(), new char[0]); + keystore.save(env.configFile(), password.toCharArray()); } public void testMissingPromptCreate() throws Exception { + String passphrase = "keystorepassphrase"; Path file1 = createRandomFile(); + terminal.addSecretInput(passphrase); + terminal.addSecretInput(passphrase); terminal.addTextInput("y"); execute("foo", file1.toString()); - assertSecureFile("foo", file1); + assertSecureFile("foo", file1, passphrase); + } + + public void testMissingPromptCreateNotMatchingPasswpord() throws Exception { + String passphrase = "keystorepassphrase"; + Path file1 = createRandomFile(); + terminal.addTextInput("y"); + terminal.addSecretInput(passphrase); + terminal.addSecretInput("adifferentpassphase"); + UserException e = expectThrows(UserException.class, () -> execute("foo", file1.toString())); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Passphrases are not equal, exiting")); } public void testMissingForceCreate() throws Exception { Path file1 = createRandomFile(); - terminal.addSecretInput("bar"); + String passphrase = "keystorepassphrase"; + terminal.addSecretInput(passphrase); + terminal.addSecretInput(passphrase); execute("-f", "foo", file1.toString()); - assertSecureFile("foo", file1); + assertSecureFile("foo", file1, passphrase); } public void testMissingNoCreate() throws Exception { + terminal.addSecretInput(randomFrom("", "keystorepassphrase")); terminal.addTextInput("n"); // explicit no execute("foo"); assertNull(KeyStoreWrapper.load(env.configFile())); } public void testOverwritePromptDefault() throws Exception { + String passphrase = "keystorepassphrase"; Path file = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file); + KeyStoreWrapper keystore = createKeystore(passphrase); + addFile(keystore, "foo", file, passphrase); + terminal.addSecretInput(passphrase); + terminal.addSecretInput(passphrase); terminal.addTextInput(""); execute("foo", "path/dne"); - assertSecureFile("foo", file); + assertSecureFile("foo", file, passphrase); } public void testOverwritePromptExplicitNo() throws Exception { + String passphrase = "keystorepassphrase"; Path file = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file); + KeyStoreWrapper keystore = createKeystore(passphrase); + addFile(keystore, "foo", file, passphrase); + terminal.addSecretInput(passphrase); terminal.addTextInput("n"); // explicit no execute("foo", "path/dne"); - assertSecureFile("foo", file); + assertSecureFile("foo", file, passphrase); } public void testOverwritePromptExplicitYes() throws Exception { + String passphrase = "keystorepassphrase"; Path file1 = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file1); + KeyStoreWrapper keystore = createKeystore(passphrase); + addFile(keystore, "foo", file1, passphrase); + terminal.addSecretInput(passphrase); + terminal.addSecretInput(passphrase); terminal.addTextInput("y"); Path file2 = createRandomFile(); execute("foo", file2.toString()); - assertSecureFile("foo", file2); + assertSecureFile("foo", file2, passphrase); } public void testOverwriteForceShort() throws Exception { + String passphrase = "keystorepassphrase"; Path file1 = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file1); + KeyStoreWrapper keystore = createKeystore(passphrase); + addFile(keystore, "foo", file1, passphrase); Path file2 = createRandomFile(); + terminal.addSecretInput(passphrase); + terminal.addSecretInput(passphrase); execute("-f", "foo", file2.toString()); - assertSecureFile("foo", file2); + assertSecureFile("foo", file2, passphrase); } public void testOverwriteForceLong() throws Exception { + String passphrase = "keystorepassphrase"; Path file1 = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file1); + KeyStoreWrapper keystore = createKeystore(passphrase); + addFile(keystore, "foo", file1, passphrase); Path file2 = createRandomFile(); + terminal.addSecretInput(passphrase); execute("--force", "foo", file2.toString()); - assertSecureFile("foo", file2); + assertSecureFile("foo", file2, passphrase); } public void testForceNonExistent() throws Exception { - createKeystore(""); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase); Path file = createRandomFile(); + terminal.addSecretInput(passphrase); execute("--force", "foo", file.toString()); - assertSecureFile("foo", file); + assertSecureFile("foo", file, passphrase); } public void testMissingSettingName() throws Exception { - createKeystore(""); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase); + terminal.addSecretInput(passphrase); UserException e = expectThrows(UserException.class, this::execute); assertEquals(ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("Missing setting name")); } public void testMissingFileName() throws Exception { - createKeystore(""); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase); + terminal.addSecretInput(passphrase); UserException e = expectThrows(UserException.class, () -> execute("foo")); assertEquals(ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("Missing file name")); } public void testFileDNE() throws Exception { - createKeystore(""); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase); + terminal.addSecretInput(passphrase); UserException e = expectThrows(UserException.class, () -> execute("foo", "path/dne")); assertEquals(ExitCodes.IO_ERROR, e.exitCode); assertThat(e.getMessage(), containsString("File [path/dne] does not exist")); } public void testExtraArguments() throws Exception { - createKeystore(""); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase); Path file = createRandomFile(); + terminal.addSecretInput(passphrase); UserException e = expectThrows(UserException.class, () -> execute("foo", file.toString(), "bar")); assertEquals(e.getMessage(), ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("Unrecognized extra arguments [bar]")); } + + public void testIncorrectPassword() throws Exception { + String passphrase = "keystorepassphrase"; + createKeystore(passphrase); + Path file = createRandomFile(); + terminal.addSecretInput("thewrongkeystorepassphrase"); + UserException e = expectThrows(UserException.class, () -> execute("foo", file.toString())); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Please make sure the passphrase was correct")); + } + + public void testAddToUnprotectedKeystore() throws Exception { + String passphrase = ""; + Path file = createRandomFile(); + KeyStoreWrapper keystore = createKeystore(passphrase); + addFile(keystore, "foo", file, passphrase); + terminal.addTextInput(""); + // will not be prompted for a passphrase + execute("foo", "path/dne"); + assertSecureFile("foo", file, passphrase); + } } diff --git a/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java index 07ce84b0b7599..f77a50f3153d0 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java @@ -48,17 +48,44 @@ InputStream getStdin() { }; } + public void testInvalidPassphrease() throws Exception { + String passphrase = "keystorepassphrase"; + createKeystore(passphrase, "foo", "bar"); + terminal.addSecretInput("thewrongpassphrase"); + UserException e = expectThrows(UserException.class, () -> execute("foo2")); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Please make sure the passphrase was correct")); + + } + public void testMissingPromptCreate() throws Exception { + String passphrase = "keystorepassphrase"; terminal.addTextInput("y"); + terminal.addSecretInput(passphrase); + terminal.addSecretInput(passphrase); terminal.addSecretInput("bar"); execute("foo"); - assertSecureString("foo", "bar"); + assertSecureString("foo", "bar", passphrase); + } + + public void testMissingPromptCreateNotMatchingPasswords() throws Exception { + String passphrase = "keystorepassphrase"; + terminal.addTextInput("y"); + terminal.addSecretInput(passphrase); + terminal.addSecretInput("anotherpassphrase"); + terminal.addSecretInput("bar"); + UserException e = expectThrows(UserException.class, () -> execute("foo")); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Passphrases are not equal, exiting")); } public void testMissingForceCreate() throws Exception { + String passphrase = "keystorepassphrase"; + terminal.addSecretInput(passphrase); + terminal.addSecretInput(passphrase); terminal.addSecretInput("bar"); execute("-f", "foo"); - assertSecureString("foo", "bar"); + assertSecureString("foo", "bar", passphrase); } public void testMissingNoCreate() throws Exception { @@ -68,77 +95,107 @@ public void testMissingNoCreate() throws Exception { } public void testOverwritePromptDefault() throws Exception { - createKeystore("", "foo", "bar"); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase, "foo", "bar"); + terminal.addSecretInput(passphrase); terminal.addTextInput(""); execute("foo"); - assertSecureString("foo", "bar"); + assertSecureString("foo", "bar", passphrase); } public void testOverwritePromptExplicitNo() throws Exception { - createKeystore("", "foo", "bar"); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase, "foo", "bar"); + terminal.addSecretInput(passphrase); terminal.addTextInput("n"); // explicit no execute("foo"); - assertSecureString("foo", "bar"); + assertSecureString("foo", "bar", passphrase); } public void testOverwritePromptExplicitYes() throws Exception { - createKeystore("", "foo", "bar"); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase, "foo", "bar"); terminal.addTextInput("y"); + terminal.addSecretInput(passphrase); terminal.addSecretInput("newvalue"); execute("foo"); - assertSecureString("foo", "newvalue"); + assertSecureString("foo", "newvalue", passphrase); } public void testOverwriteForceShort() throws Exception { - createKeystore("", "foo", "bar"); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase, "foo", "bar"); + terminal.addSecretInput(passphrase); terminal.addSecretInput("newvalue"); execute("-f", "foo"); // force - assertSecureString("foo", "newvalue"); + assertSecureString("foo", "newvalue", passphrase); } public void testOverwriteForceLong() throws Exception { - createKeystore("", "foo", "bar"); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase, "foo", "bar"); + terminal.addSecretInput(passphrase); terminal.addSecretInput("and yet another secret value"); execute("--force", "foo"); // force - assertSecureString("foo", "and yet another secret value"); + assertSecureString("foo", "and yet another secret value", passphrase); } public void testForceNonExistent() throws Exception { - createKeystore(""); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase); + terminal.addSecretInput(passphrase); terminal.addSecretInput("value"); execute("--force", "foo"); // force - assertSecureString("foo", "value"); + assertSecureString("foo", "value", passphrase); } public void testPromptForValue() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + String passphrase = "keystorepassphrase"; + KeyStoreWrapper.create().save(env.configFile(), passphrase.toCharArray()); + terminal.addSecretInput(passphrase); terminal.addSecretInput("secret value"); execute("foo"); - assertSecureString("foo", "secret value"); + assertSecureString("foo", "secret value", passphrase); } public void testStdinShort() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + String passphrase = "keystorepassphrase"; + KeyStoreWrapper.create().save(env.configFile(), passphrase.toCharArray()); + terminal.addSecretInput(passphrase); setInput("secret value 1"); execute("-x", "foo"); - assertSecureString("foo", "secret value 1"); + assertSecureString("foo", "secret value 1", passphrase); } public void testStdinLong() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + String passphrase = "keystorepassphrase"; + KeyStoreWrapper.create().save(env.configFile(), passphrase.toCharArray()); + terminal.addSecretInput(passphrase); setInput("secret value 2"); execute("--stdin", "foo"); - assertSecureString("foo", "secret value 2"); + assertSecureString("foo", "secret value 2", passphrase); } public void testMissingSettingName() throws Exception { - createKeystore(""); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase); + terminal.addSecretInput(passphrase); + terminal.addSecretInput(passphrase); terminal.addTextInput(""); UserException e = expectThrows(UserException.class, this::execute); assertEquals(ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("The setting name can not be null")); } + public void testAddToUnprotectedKeystore() throws Exception { + String passphrase = ""; + createKeystore(passphrase, "foo", "bar"); + terminal.addTextInput(""); + // will not be prompted for a passphrase + execute("foo"); + assertSecureString("foo", "bar", passphrase); + } + void setInput(String inputStr) { input = new ByteArrayInputStream(inputStr.getBytes(StandardCharsets.UTF_8)); } diff --git a/server/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java index aefedf86e7761..cf736ad7b0729 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java @@ -25,9 +25,12 @@ import java.util.Map; import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; +import static org.hamcrest.Matchers.containsString; + public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase { @Override @@ -40,13 +43,28 @@ protected Environment createEnv(Map settings) throws UserExcepti }; } + public void testNotMatchingPasswords() throws Exception { + String passphrase = randomFrom("", "keystorepassphrase"); + terminal.addSecretInput(passphrase); + terminal.addSecretInput("notthekeystorepassphraseyouarelookingfor"); + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Passphrases are not equal, exiting")); + } + public void testPosix() throws Exception { + String passphrase = randomFrom("", "keystorepassphrase"); + terminal.addSecretInput(passphrase); + terminal.addSecretInput(passphrase); execute(); Path configDir = env.configFile(); assertNotNull(KeyStoreWrapper.load(configDir)); } public void testNotPosix() throws Exception { + String passphrase = randomFrom("", "keystorepassphrase"); + terminal.addSecretInput(passphrase); + terminal.addSecretInput(passphrase); env = setupEnv(false, fileSystems); execute(); Path configDir = env.configFile(); @@ -54,6 +72,7 @@ public void testNotPosix() throws Exception { } public void testOverwrite() throws Exception { + String passphrase = randomFrom("", "keystorepassphrase"); Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); byte[] content = "not a keystore".getBytes(StandardCharsets.UTF_8); Files.write(keystoreFile, content); @@ -67,6 +86,8 @@ public void testOverwrite() throws Exception { assertArrayEquals(content, Files.readAllBytes(keystoreFile)); terminal.addTextInput("y"); + terminal.addSecretInput(passphrase); + terminal.addSecretInput(passphrase); execute(); assertNotNull(KeyStoreWrapper.load(env.configFile())); } diff --git a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java index 7f8c71889e038..1e5527a1e245b 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java +++ b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java @@ -89,16 +89,16 @@ KeyStoreWrapper loadKeystore(String password) throws Exception { return keystore; } - void assertSecureString(String setting, String value) throws Exception { - assertSecureString(loadKeystore(""), setting, value); + void assertSecureString(String setting, String value, String password) throws Exception { + assertSecureString(loadKeystore(password), setting, value); } void assertSecureString(KeyStoreWrapper keystore, String setting, String value) throws Exception { assertEquals(value, keystore.getString(setting).toString()); } - void assertSecureFile(String setting, Path file) throws Exception { - assertSecureFile(loadKeystore(""), setting, file); + void assertSecureFile(String setting, Path file, String password) throws Exception { + assertSecureFile(loadKeystore(password), setting, file); } void assertSecureFile(KeyStoreWrapper keystore, String setting, Path file) throws Exception { diff --git a/server/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java index 27c30d3aa8f58..8f72b1ea31665 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java @@ -47,20 +47,42 @@ public void testMissing() throws Exception { } public void testEmpty() throws Exception { - createKeystore(""); + String passphrase = randomFrom("", "keystorepassphrase"); + createKeystore(passphrase); + terminal.addSecretInput(passphrase); execute(); assertEquals("keystore.seed\n", terminal.getOutput()); } public void testOne() throws Exception { - createKeystore("", "foo", "bar"); + String passphrase = randomFrom("", "keystorepassphrase"); + createKeystore(passphrase, "foo", "bar"); + terminal.addSecretInput(passphrase); execute(); assertEquals("foo\nkeystore.seed\n", terminal.getOutput()); } public void testMultiple() throws Exception { - createKeystore("", "foo", "1", "baz", "2", "bar", "3"); + String passphrase = randomFrom("", "keystorepassphrase"); + createKeystore(passphrase, "foo", "1", "baz", "2", "bar", "3"); + terminal.addSecretInput(passphrase); execute(); assertEquals("bar\nbaz\nfoo\nkeystore.seed\n", terminal.getOutput()); // sorted } + + public void testListWithIncorrectPassphrase() throws Exception { + String passphrase = "keystorepassphrase"; + createKeystore(passphrase, "foo", "bar"); + terminal.addSecretInput("thewrongkeystorepassphrase"); + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Please make sure the passphrase was correct")); + } + + public void testListWithUnprotectedKeystore() throws Exception { + createKeystore("", "foo", "bar"); + execute(); + // Not prompted for a password + assertEquals("foo\nkeystore.seed\n", terminal.getOutput()); + } } diff --git a/server/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java index 2259dee31a8cb..8969e42cf3fd3 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java @@ -41,39 +41,66 @@ protected Environment createEnv(Map settings) throws UserExcepti }; } - public void testMissing() throws Exception { + public void testMissing() { + String passphrase = "keystorepassphrase"; + terminal.addSecretInput(passphrase); UserException e = expectThrows(UserException.class, () -> execute("foo")); assertEquals(ExitCodes.DATA_ERROR, e.exitCode); assertThat(e.getMessage(), containsString("keystore not found")); } public void testNoSettings() throws Exception { - createKeystore(""); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase); + terminal.addSecretInput(passphrase); UserException e = expectThrows(UserException.class, this::execute); assertEquals(ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("Must supply at least one setting")); } public void testNonExistentSetting() throws Exception { - createKeystore(""); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase); + terminal.addSecretInput(passphrase); UserException e = expectThrows(UserException.class, () -> execute("foo")); assertEquals(ExitCodes.CONFIG, e.exitCode); assertThat(e.getMessage(), containsString("[foo] does not exist")); } public void testOne() throws Exception { - createKeystore("", "foo", "bar"); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase, "foo", "bar"); + terminal.addSecretInput(passphrase); execute("foo"); - assertFalse(loadKeystore("").getSettingNames().contains("foo")); + assertFalse(loadKeystore(passphrase).getSettingNames().contains("foo")); } public void testMany() throws Exception { - createKeystore("", "foo", "1", "bar", "2", "baz", "3"); + String passphrase = "keystorepassphrase"; + createKeystore(passphrase, "foo", "1", "bar", "2", "baz", "3"); + terminal.addSecretInput(passphrase); execute("foo", "baz"); - Set settings = loadKeystore("").getSettingNames(); + Set settings = loadKeystore(passphrase).getSettingNames(); assertFalse(settings.contains("foo")); assertFalse(settings.contains("baz")); assertTrue(settings.contains("bar")); assertEquals(2, settings.size()); // account for keystore.seed too } + + public void testRemoveWithIncorrectPassword() throws Exception { + String passphrase = "keystorepassphrase"; + createKeystore(passphrase, "foo", "bar"); + terminal.addSecretInput("thewrongpassphrase"); + UserException e = expectThrows(UserException.class, () -> execute("foo")); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Please make sure the passphrase was correct")); + } + + public void testRemoveFromUnprotectedKeystore() throws Exception { + String passphrase = ""; + createKeystore(passphrase, "foo", "bar"); + // will not be prompted for a passphrase + execute("foo"); + assertFalse(loadKeystore(passphrase).getSettingNames().contains("foo")); + } } From c74685f853200e3990438505b6590eb79a40d83c Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Wed, 23 Jan 2019 14:02:12 +0200 Subject: [PATCH 02/15] Address feedback - Moves the code to read the passphrase to KeyStoreWrapper - Adds a subcommand to elasticsearch-keystore for setting or changing the keystore's password --- .../settings/AddFileKeyStoreCommand.java | 28 ++---- .../settings/AddStringKeyStoreCommand.java | 28 ++---- .../ChangeKeyStorePassphraseCommand.java | 67 +++++++++++++ .../settings/CreateKeyStoreCommand.java | 18 +--- .../common/settings/KeyStoreCli.java | 1 + .../common/settings/KeyStoreWrapper.java | 26 +++++ .../common/settings/ListKeyStoreCommand.java | 12 +-- .../RemoveSettingKeyStoreCommand.java | 17 ++-- .../ChangeKeyStorePassphraseCommandTests.java | 95 +++++++++++++++++++ 9 files changed, 220 insertions(+), 72 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommand.java create mode 100644 server/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommandTests.java diff --git a/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java index e696b255f2d47..12bbf498a404c 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java @@ -53,8 +53,7 @@ class AddFileKeyStoreCommand extends EnvironmentAwareCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - char[] password = null; - char[] passwordVerification = null; + char[] passphrase = null; try { KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); if (keystore == null) { @@ -63,21 +62,13 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th terminal.println("Exiting without creating keystore."); return; } - password = terminal.readSecret("Enter passphrase for the elasticsearch keystore (empty for no passphrase): "); - passwordVerification = terminal.readSecret("Enter same passphrase again: "); - if (Arrays.equals(password, passwordVerification) == false) { - throw new UserException(ExitCodes.DATA_ERROR, "Passphrases are not equal, exiting."); - } keystore = KeyStoreWrapper.create(); - keystore.save(env.configFile(), password); + passphrase = keystore.readPassphrase(terminal, true); + keystore.save(env.configFile(), passphrase); terminal.println("Created elasticsearch keystore in " + env.configFile()); } else { - if (keystore.hasPassword()) { - password = terminal.readSecret("Enter passphrase for the elasticsearch keystore: "); - } else { - password = new char[0]; - } - keystore.decrypt(password); + passphrase = keystore.hasPassword() ? keystore.readPassphrase(terminal, false) : new char[0]; + keystore.decrypt(passphrase); } List argumentValues = arguments.values(options); @@ -104,15 +95,12 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath"); } keystore.setFile(setting, Files.readAllBytes(file)); - keystore.save(env.configFile(), password); + keystore.save(env.configFile(), passphrase); } catch (SecurityException e) { throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); } finally { - if (null != password) { - Arrays.fill(password, '\u0000'); - } - if (null != passwordVerification) { - Arrays.fill(passwordVerification, '\u0000'); + if (null != passphrase) { + Arrays.fill(passphrase, '\u0000'); } } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java index aa4547028768d..c2c6d0eca2505 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java @@ -56,8 +56,7 @@ InputStream getStdin() { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - char[] password = null; - char[] passwordVerification = null; + char[] passphrase = null; try { KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); if (keystore == null) { @@ -66,21 +65,13 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th terminal.println("Exiting without creating keystore."); return; } - password = terminal.readSecret("Enter passphrase for the elasticsearch keystore (empty for no passphrase): "); - passwordVerification = terminal.readSecret("Enter same passphrase again: "); - if (Arrays.equals(password, passwordVerification) == false) { - throw new UserException(ExitCodes.DATA_ERROR, "Passphrases are not equal, exiting."); - } keystore = KeyStoreWrapper.create(); - keystore.save(env.configFile(), password); + passphrase = keystore.readPassphrase(terminal, true); + keystore.save(env.configFile(), passphrase); terminal.println("Created elasticsearch keystore in " + env.configFile()); } else { - if (keystore.hasPassword()) { - password = terminal.readSecret("Enter passphrase for the elasticsearch keystore: "); - } else { - password = new char[0]; - } - keystore.decrypt(password); + passphrase = keystore.hasPassword() ? keystore.readPassphrase(terminal, false) : new char[0]; + keystore.decrypt(passphrase); } String setting = arguments.value(options); @@ -107,15 +98,12 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } catch (IllegalArgumentException e) { throw new UserException(ExitCodes.DATA_ERROR, "String value must contain only ASCII"); } - keystore.save(env.configFile(), password); + keystore.save(env.configFile(), passphrase); } catch (SecurityException e) { throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); } finally { - if (null != password) { - Arrays.fill(password, '\u0000'); - } - if (null != passwordVerification) { - Arrays.fill(passwordVerification, '\u0000'); + if (null != passphrase) { + Arrays.fill(passphrase, '\u0000'); } } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommand.java b/server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommand.java new file mode 100644 index 0000000000000..7f3736a71bbe6 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommand.java @@ -0,0 +1,67 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.settings; + +import joptsimple.OptionSet; +import org.elasticsearch.cli.EnvironmentAwareCommand; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +import java.util.Arrays; + +public class ChangeKeyStorePassphraseCommand extends EnvironmentAwareCommand { + + + ChangeKeyStorePassphraseCommand() { + super("Changes the passphrase of a keystore"); + } + + @Override + protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { + char[] passphrase = null; + try { + KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); + if (keystore == null) { + if (terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { + terminal.println("Exiting without creating keystore."); + return; + } + keystore = KeyStoreWrapper.create(); + passphrase = keystore.readPassphrase(terminal, true); + keystore.save(env.configFile(), passphrase); + terminal.println("Created elasticsearch keystore in " + env.configFile()); + } else { + passphrase = keystore.hasPassword() ? keystore.readPassphrase(terminal, false) : new char[0]; + keystore.decrypt(passphrase); + passphrase = keystore.readPassphrase(terminal, true); + keystore.save(env.configFile(), passphrase); + terminal.println("Elasticsearch keystore passphrase changed successfully." + env.configFile()); + } + } catch (SecurityException e) { + throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); + } finally { + if (null != passphrase) { + Arrays.fill(passphrase, '\u0000'); + } + } + } +} diff --git a/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index 1525a4a919869..a0a9a6f3c8c0b 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -41,8 +41,7 @@ class CreateKeyStoreCommand extends EnvironmentAwareCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - char[] password = null; - char[] passwordVerification = null; + char[] passphrase = null; try { Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); if (Files.exists(keystoreFile)) { @@ -51,22 +50,15 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th return; } } - password = terminal.readSecret("Enter passphrase (empty for no passphrase): "); - passwordVerification = terminal.readSecret("Enter same passphrase again: "); - if (Arrays.equals(password, passwordVerification) == false) { - throw new UserException(ExitCodes.DATA_ERROR, "Passphrases are not equal, exiting."); - } KeyStoreWrapper keystore = KeyStoreWrapper.create(); - keystore.save(env.configFile(), password); + passphrase = keystore.readPassphrase(terminal, true); + keystore.save(env.configFile(), passphrase); terminal.println("Created elasticsearch keystore in " + env.configFile()); } catch (SecurityException e) { throw new UserException(ExitCodes.IO_ERROR, "Error creating the elasticsearch keystore.", e); } finally { - if (null != password) { - Arrays.fill(password, '\u0000'); - } - if (null != passwordVerification) { - Arrays.fill(passwordVerification, '\u0000'); + if (null != passphrase) { + Arrays.fill(passphrase, '\u0000'); } } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java index 3deb5f19c95fe..439ceed854b93 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java @@ -34,6 +34,7 @@ private KeyStoreCli() { subcommands.put("add", new AddStringKeyStoreCommand()); subcommands.put("add-file", new AddFileKeyStoreCommand()); subcommands.put("remove", new RemoveSettingKeyStoreCommand()); + subcommands.put("passwd", new ChangeKeyStorePassphraseCommand()); } public static void main(String[] args) throws Exception { 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 e017e9e7ca93f..4dea05d59cd21 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -65,6 +65,7 @@ import org.apache.lucene.store.SimpleFSDirectory; import org.apache.lucene.util.SetOnce; import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.UserException; import org.elasticsearch.common.Randomness; @@ -510,6 +511,31 @@ public synchronized void save(Path configDir, char[] password) throws Exception } } + /** + * Reads the keystore passphrase from the {@link Terminal}, prompting for verification where applicable and returns it as a + * {@code char[]}. The caller is responsible to clear the char array after use. + * + * @param terminal the terminal to use for user inputs + * @param withVerification whether the user should be prompted for passphrase verification + * @return a char array with the passphrase the user entered + * @throws UserException If the user is prompted for verification and enters a different passphrase + */ + char[] readPassphrase(Terminal terminal, boolean withVerification) throws UserException { + final char[] passphrase; + if (withVerification) { + passphrase = terminal.readSecret("Enter new passphrase for the elasticsearch keystore (empty for no passphrase): "); + char[] passphraseVerification = terminal.readSecret("Enter same passphrase again: "); + if (Arrays.equals(passphrase, passphraseVerification) == false) { + throw new UserException(ExitCodes.DATA_ERROR, "Passphrases are not equal, exiting."); + } + Arrays.fill(passphraseVerification, '\u0000'); + } else { + passphrase = terminal.readSecret("Enter passphrase for the elasticsearch keystore : "); + } + return passphrase; + } + + /** * It is possible to retrieve the setting names even if the keystore is closed. * This allows {@link SecureSetting} to correctly determine that a entry exists even though it cannot be read. Thus attempting to diff --git a/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java index b8098aa6fc295..7ca96e9044df8 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java @@ -47,14 +47,10 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th if (keystore == null) { throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); } - char[] password; - if (keystore.hasPassword()) { - password = terminal.readSecret("Enter elasticsearch keystore passphrase (empty for no passphrase): "); - } else { - password = new char[0]; - } + char[] passphrase; + passphrase = keystore.hasPassword() ? keystore.readPassphrase(terminal, false) : new char[0]; try { - keystore.decrypt(password); + keystore.decrypt(passphrase); List sortedEntries = new ArrayList<>(keystore.getSettingNames()); Collections.sort(sortedEntries); for (String entry : sortedEntries) { @@ -63,7 +59,7 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } catch (SecurityException e) { throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); } finally { - Arrays.fill(password, '\u0000'); + Arrays.fill(passphrase, '\u0000'); } } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java index fc88b99e1a218..3d7e39232839f 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java @@ -53,27 +53,22 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th if (keystore == null) { throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); } - char[] password = null; + char[] passphrase = null; try { - if (keystore.hasPassword()) { - password = terminal.readSecret("Enter passphrase for the elasticsearch keystore: "); - } else { - password = new char[0]; - } - keystore.decrypt(password); - + passphrase = keystore.hasPassword() ? keystore.readPassphrase(terminal, false) : new char[0]; + keystore.decrypt(passphrase); for (String setting : arguments.values(options)) { if (keystore.getSettingNames().contains(setting) == false) { throw new UserException(ExitCodes.CONFIG, "Setting [" + setting + "] does not exist in the keystore."); } keystore.remove(setting); } - keystore.save(env.configFile(), password); + keystore.save(env.configFile(), passphrase); } catch (SecurityException e) { throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); } finally { - if (null != password) { - Arrays.fill(password, '\u0000'); + if (null != passphrase) { + Arrays.fill(passphrase, '\u0000'); } } } diff --git a/server/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommandTests.java new file mode 100644 index 0000000000000..b630e0f1be7e9 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommandTests.java @@ -0,0 +1,95 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.settings; + +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +import java.util.Map; + +import static org.hamcrest.Matchers.containsString; + +public class ChangeKeyStorePassphraseCommandTests extends KeyStoreCommandTestCase { + @Override + protected Command newCommand() { + return new ChangeKeyStorePassphraseCommand() { + @Override + protected Environment createEnv(Map settings) throws UserException { + return env; + } + }; + } + + public void testSetKeyStorePassword() throws Exception { + createKeystore(""); + loadKeystore(""); + terminal.addSecretInput("thepassphrase"); + terminal.addSecretInput("thepassphrase"); + // Prompted twice for the new password, since we didn't have an existing password + execute(); + loadKeystore("thepassphrase"); + } + + public void testChangeKeyStorePassword() throws Exception { + createKeystore("theoldpassphrase"); + loadKeystore("theoldpassphrase"); + terminal.addSecretInput("theoldpassphrase"); + terminal.addSecretInput("thepassphrase"); + terminal.addSecretInput("thepassphrase"); + // Prompted thrice: Once for the existing and twice for the new password + execute(); + loadKeystore("thepassphrase"); + } + + public void testChangeKeyStorePasswordToEmpty() throws Exception { + createKeystore("theoldpassphrase"); + loadKeystore("theoldpassphrase"); + terminal.addSecretInput("theoldpassphrase"); + terminal.addSecretInput(""); + terminal.addSecretInput(""); + // Prompted thrice: Once for the existing and twice for the new password + execute(); + loadKeystore(""); + } + + public void testChangeKeyStorePasswordWrongVerification() throws Exception { + createKeystore("theoldpassphrase"); + loadKeystore("theoldpassphrase"); + terminal.addSecretInput("theoldpassphrase"); + terminal.addSecretInput("thepassphrase"); + terminal.addSecretInput("themisspelledpassphrase"); + // Prompted thrice: Once for the existing and twice for the new password + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Passphrases are not equal, exiting")); + } + + public void testChangeKeyStorePasswordWrongExistingPassword() throws Exception { + createKeystore("theoldpassphrase"); + loadKeystore("theoldpassphrase"); + terminal.addSecretInput("theoldmisspelledpassphrase"); + // We'll only be prompted once (for the old password) + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Please make sure the passphrase was correct")); + } +} From af3d5e9efbac9bae32dddf70ee3a9f87332c6147 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Wed, 23 Jan 2019 15:11:23 +0200 Subject: [PATCH 03/15] Allow creating obfuscated keystores without prompting for a passphrase --- .../common/settings/CreateKeyStoreCommand.java | 6 +++++- .../common/settings/CreateKeyStoreCommandTests.java | 12 ++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index a0a9a6f3c8c0b..37c655b0089f1 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -24,6 +24,7 @@ import java.util.Arrays; import joptsimple.OptionSet; +import joptsimple.OptionSpec; import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.Terminal; @@ -35,8 +36,11 @@ */ class CreateKeyStoreCommand extends EnvironmentAwareCommand { + private final OptionSpec noPassOption; + CreateKeyStoreCommand() { super("Creates a new elasticsearch keystore"); + this.noPassOption = parser.acceptsAll(Arrays.asList("n", "nopass"), "Creates an obfuscated (not passphrase protected) keystore"); } @Override @@ -51,7 +55,7 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } } KeyStoreWrapper keystore = KeyStoreWrapper.create(); - passphrase = keystore.readPassphrase(terminal, true); + passphrase = options.has(noPassOption) ? new char[0] : keystore.readPassphrase(terminal, true); keystore.save(env.configFile(), passphrase); terminal.println("Created elasticsearch keystore in " + env.configFile()); } catch (SecurityException e) { diff --git a/server/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java index cf736ad7b0729..60f10009b3930 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java @@ -52,6 +52,18 @@ public void testNotMatchingPasswords() throws Exception { assertThat(e.getMessage(), containsString("Passphrases are not equal, exiting")); } + public void testNotPromptForPassword() throws Exception { + execute("-n"); + Path configDir = env.configFile(); + assertNotNull(KeyStoreWrapper.load(configDir)); + } + + public void testNotPromptForPasswordFullOptionName() throws Exception { + execute("--nopass"); + Path configDir = env.configFile(); + assertNotNull(KeyStoreWrapper.load(configDir)); + } + public void testPosix() throws Exception { String passphrase = randomFrom("", "keystorepassphrase"); terminal.addSecretInput(passphrase); From d118da4eba1168d485d0bb223f6445daabb7747e Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Wed, 23 Jan 2019 15:12:35 +0200 Subject: [PATCH 04/15] Create obfuscated keystores for integTestCluster tasks. This can be parameterized in a followup PR so that we can run tests with encrypted keystores --- .../org/elasticsearch/gradle/test/ClusterFormationTasks.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy index d87e33166e74d..ed7f031686294 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy @@ -413,7 +413,7 @@ class ClusterFormationTasks { * getting the short name requiring the path to already exist. */ final Object esKeystoreUtil = "${-> node.binPath().resolve('elasticsearch-keystore').toString()}" - return configureExecTask(name, project, setup, node, esKeystoreUtil, 'create') + return configureExecTask(name, project, setup, node, esKeystoreUtil, 'create', '--nopass') } } From 668c438a04d786d06efe4f0e1253484c38b25ae9 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Wed, 6 Feb 2019 11:25:28 +0200 Subject: [PATCH 05/15] address feedback --- .../settings/AddFileKeyStoreCommand.java | 26 ++------- .../settings/AddStringKeyStoreCommand.java | 26 ++------- .../ChangeKeyStorePassphraseCommand.java | 30 ++++------ .../settings/CreateKeyStoreCommand.java | 2 +- .../common/settings/KeyStoreWrapper.java | 57 +++++++++++++++---- .../settings/KeystoreAndPassphrase.java | 49 ++++++++++++++++ .../common/settings/ListKeyStoreCommand.java | 2 +- .../RemoveSettingKeyStoreCommand.java | 2 +- 8 files changed, 119 insertions(+), 75 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassphrase.java diff --git a/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java index 12bbf498a404c..8d4c03c7ee0cf 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java @@ -53,23 +53,11 @@ class AddFileKeyStoreCommand extends EnvironmentAwareCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - char[] passphrase = null; - try { - KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); - if (keystore == null) { - if (options.has(forceOption) == false && - terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { - terminal.println("Exiting without creating keystore."); - return; - } - keystore = KeyStoreWrapper.create(); - passphrase = keystore.readPassphrase(terminal, true); - keystore.save(env.configFile(), passphrase); - terminal.println("Created elasticsearch keystore in " + env.configFile()); - } else { - passphrase = keystore.hasPassword() ? keystore.readPassphrase(terminal, false) : new char[0]; - keystore.decrypt(passphrase); + try (KeystoreAndPassphrase keyAndPass = KeyStoreWrapper.readOrCreate(terminal, env.configFile(), options.has(forceOption))) { + if (null == keyAndPass) { + return; } + KeyStoreWrapper keystore = keyAndPass.getKeystore(); List argumentValues = arguments.values(options); if (argumentValues.size() == 0) { @@ -95,13 +83,9 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath"); } keystore.setFile(setting, Files.readAllBytes(file)); - keystore.save(env.configFile(), passphrase); + keystore.save(env.configFile(), keyAndPass.getPassphrase()); } catch (SecurityException e) { throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); - } finally { - if (null != passphrase) { - Arrays.fill(passphrase, '\u0000'); - } } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java index c2c6d0eca2505..8faad5c3cf501 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java @@ -56,23 +56,11 @@ InputStream getStdin() { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - char[] passphrase = null; - try { - KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); - if (keystore == null) { - if (options.has(forceOption) == false && - terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { - terminal.println("Exiting without creating keystore."); - return; - } - keystore = KeyStoreWrapper.create(); - passphrase = keystore.readPassphrase(terminal, true); - keystore.save(env.configFile(), passphrase); - terminal.println("Created elasticsearch keystore in " + env.configFile()); - } else { - passphrase = keystore.hasPassword() ? keystore.readPassphrase(terminal, false) : new char[0]; - keystore.decrypt(passphrase); + try (KeystoreAndPassphrase keyAndPass = KeyStoreWrapper.readOrCreate(terminal, env.configFile(), options.has(forceOption))) { + if (null == keyAndPass) { + return; } + KeyStoreWrapper keystore = keyAndPass.getKeystore(); String setting = arguments.value(options); if (setting == null) { @@ -98,13 +86,9 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } catch (IllegalArgumentException e) { throw new UserException(ExitCodes.DATA_ERROR, "String value must contain only ASCII"); } - keystore.save(env.configFile(), passphrase); + keystore.save(env.configFile(), keyAndPass.getPassphrase()); } catch (SecurityException e) { throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); - } finally { - if (null != passphrase) { - Arrays.fill(passphrase, '\u0000'); - } } } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommand.java b/server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommand.java index 7f3736a71bbe6..348b2b03caf43 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommand.java @@ -37,30 +37,20 @@ public class ChangeKeyStorePassphraseCommand extends EnvironmentAwareCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - char[] passphrase = null; - try { - KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); - if (keystore == null) { - if (terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { - terminal.println("Exiting without creating keystore."); - return; - } - keystore = KeyStoreWrapper.create(); - passphrase = keystore.readPassphrase(terminal, true); - keystore.save(env.configFile(), passphrase); - terminal.println("Created elasticsearch keystore in " + env.configFile()); - } else { - passphrase = keystore.hasPassword() ? keystore.readPassphrase(terminal, false) : new char[0]; - keystore.decrypt(passphrase); - passphrase = keystore.readPassphrase(terminal, true); - keystore.save(env.configFile(), passphrase); - terminal.println("Elasticsearch keystore passphrase changed successfully." + env.configFile()); + char[] newPassphrase = null; + try (KeystoreAndPassphrase keyAndPass = KeyStoreWrapper.readOrCreate(terminal, env.configFile(), false)) { + if (null == keyAndPass) { + return; } + KeyStoreWrapper keystore = keyAndPass.getKeystore(); + newPassphrase = KeyStoreWrapper.readPassphrase(terminal, true); + keystore.save(env.configFile(), newPassphrase); + terminal.println("Elasticsearch keystore passphrase changed successfully." + env.configFile()); } catch (SecurityException e) { throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); } finally { - if (null != passphrase) { - Arrays.fill(passphrase, '\u0000'); + if (null != newPassphrase) { + Arrays.fill(newPassphrase, '\u0000'); } } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index 37c655b0089f1..2917ec87f5cd1 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -55,7 +55,7 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } } KeyStoreWrapper keystore = KeyStoreWrapper.create(); - passphrase = options.has(noPassOption) ? new char[0] : keystore.readPassphrase(terminal, true); + passphrase = options.has(noPassOption) ? new char[0] : KeyStoreWrapper.readPassphrase(terminal, true); keystore.save(env.configFile(), passphrase); terminal.println("Created elasticsearch keystore in " + env.configFile()); } catch (SecurityException e) { 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 4dea05d59cd21..ce91329c4146a 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -496,9 +496,9 @@ public synchronized void save(Path configDir, char[] password) throws Exception } catch (final AccessDeniedException e) { final String message = String.format( - Locale.ROOT, - "unable to create temporary keystore at [%s], please check filesystem permissions", - configDir.resolve(tmpFile)); + Locale.ROOT, + "unable to create temporary keystore at [%s], please check filesystem permissions", + configDir.resolve(tmpFile)); throw new UserException(ExitCodes.CONFIG, message, e); } @@ -520,7 +520,7 @@ public synchronized void save(Path configDir, char[] password) throws Exception * @return a char array with the passphrase the user entered * @throws UserException If the user is prompted for verification and enters a different passphrase */ - char[] readPassphrase(Terminal terminal, boolean withVerification) throws UserException { + static char[] readPassphrase(Terminal terminal, boolean withVerification) throws UserException { final char[] passphrase; if (withVerification) { passphrase = terminal.readSecret("Enter new passphrase for the elasticsearch keystore (empty for no passphrase): "); @@ -535,6 +535,37 @@ char[] readPassphrase(Terminal terminal, boolean withVerification) throws UserEx return passphrase; } + /** + * Reads the keystore from disk or attempts to create it if it doesn't already exist. If the keystore is password protected + * it also reads the passphrase from user input and decrypts the keystore. Returns both in a POJO. The caller is responsible to clear + * the char array with the passphrase after use, calling {@link KeystoreAndPassphrase#close()} + * + * @param terminal the terminal to use for user inputs + * @param configFile the Path from where to attempt and read the existing keystore + * @param forceCreate if set, the keystore is created without prompting the user + * @return a POJO with the {@link KeyStoreWrapper} and it's passphrase, null if the user elected to not create a keystore + */ + static KeystoreAndPassphrase readOrCreate(Terminal terminal, Path configFile, boolean forceCreate) throws Exception { + KeyStoreWrapper keystore = load(configFile); + final char[] passphrase; + if (keystore == null) { + if (forceCreate == false && + terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { + terminal.println("Exiting without creating keystore."); + return null; + } else { + passphrase = readPassphrase(terminal, true); + keystore = KeyStoreWrapper.create(); + keystore.save(configFile, passphrase); + terminal.println("Created elasticsearch keystore in " + configFile); + return new KeystoreAndPassphrase(keystore, passphrase); + } + } else { + passphrase = keystore.hasPassword ? readPassphrase(terminal, false) : new char[0]; + keystore.decrypt(passphrase); + return new KeystoreAndPassphrase(keystore, passphrase); + } + } /** * It is possible to retrieve the setting names even if the keystore is closed. @@ -583,7 +614,9 @@ public static void validateSettingName(String setting) { } } - /** Set a string setting. */ + /** + * Set a string setting. + */ synchronized void setString(String setting, char[] value) { ensureOpen(); validateSettingName(setting); @@ -592,27 +625,31 @@ synchronized void setString(String setting, char[] value) { byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); Entry oldEntry = entries.get().put(setting, new Entry(EntryType.STRING, bytes)); if (oldEntry != null) { - Arrays.fill(oldEntry.bytes, (byte)0); + Arrays.fill(oldEntry.bytes, (byte) 0); } } - /** Set a file setting. */ + /** + * Set a file setting. + */ synchronized void setFile(String setting, byte[] bytes) { ensureOpen(); validateSettingName(setting); Entry oldEntry = entries.get().put(setting, new Entry(EntryType.FILE, Arrays.copyOf(bytes, bytes.length))); if (oldEntry != null) { - Arrays.fill(oldEntry.bytes, (byte)0); + Arrays.fill(oldEntry.bytes, (byte) 0); } } - /** Remove the given setting from the keystore. */ + /** + * Remove the given setting from the keystore. + */ void remove(String setting) { ensureOpen(); Entry oldEntry = entries.get().remove(setting); if (oldEntry != null) { - Arrays.fill(oldEntry.bytes, (byte)0); + Arrays.fill(oldEntry.bytes, (byte) 0); } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassphrase.java b/server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassphrase.java new file mode 100644 index 0000000000000..c8130b9d6e138 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassphrase.java @@ -0,0 +1,49 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.settings; + +import java.io.Closeable; +import java.io.IOException; +import java.util.Arrays; + +public class KeystoreAndPassphrase implements Closeable { + private final KeyStoreWrapper keystore; + private final char[] passphrase; + + public KeystoreAndPassphrase(KeyStoreWrapper keystore, char[] passphrase) { + this.keystore = keystore; + this.passphrase = passphrase; + } + + public KeyStoreWrapper getKeystore() { + return keystore; + } + + public char[] getPassphrase() { + return passphrase; + } + + @Override + public void close() throws IOException { + if (null != passphrase) { + Arrays.fill(passphrase, '\u0000'); + } + } +} diff --git a/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java index 7ca96e9044df8..04001f816b879 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java @@ -48,7 +48,7 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); } char[] passphrase; - passphrase = keystore.hasPassword() ? keystore.readPassphrase(terminal, false) : new char[0]; + passphrase = keystore.hasPassword() ? KeyStoreWrapper.readPassphrase(terminal, false) : new char[0]; try { keystore.decrypt(passphrase); List sortedEntries = new ArrayList<>(keystore.getSettingNames()); diff --git a/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java index 3d7e39232839f..d8e74f7250a87 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java @@ -55,7 +55,7 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } char[] passphrase = null; try { - passphrase = keystore.hasPassword() ? keystore.readPassphrase(terminal, false) : new char[0]; + passphrase = keystore.hasPassword() ? KeyStoreWrapper.readPassphrase(terminal, false) : new char[0]; keystore.decrypt(passphrase); for (String setting : arguments.values(options)) { if (keystore.getSettingNames().contains(setting) == false) { From 95e1a78e92be54ca88aacdbcac0e99959a384b53 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Wed, 6 Feb 2019 13:56:46 +0200 Subject: [PATCH 06/15] Handle tests by creating obfuscated keystores by default - At least until we can provide the passphrase on startup --- .../org/elasticsearch/packaging/test/ArchiveTestCase.java | 4 ++-- .../test/resources/packaging/tests/bootstrap_password.bash | 2 +- .../vagrant/src/test/resources/packaging/tests/certgen.bash | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java b/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java index a6aad032baa5d..247cfcd035807 100644 --- a/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java +++ b/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java @@ -125,14 +125,14 @@ public void test40CreateKeystoreManually() { final Installation.Executables bin = installation.executables(); final Shell sh = new Shell(); - Platforms.onLinux(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " create")); + Platforms.onLinux(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " create --nopass")); // this is a hack around the fact that we can't run a command in the same session as the same user but not as administrator. // the keystore ends up being owned by the Administrators group, so we manually set it to be owned by the vagrant user here. // from the server's perspective the permissions aren't really different, this is just to reflect what we'd expect in the tests. // when we run these commands as a role user we won't have to do this Platforms.onWindows(() -> sh.run( - bin.elasticsearchKeystore + " create; " + + bin.elasticsearchKeystore + " create --nopass; " + "$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " + "$acl = Get-Acl '" + installation.config("elasticsearch.keystore") + "'; " + "$acl.SetOwner($account); " + diff --git a/x-pack/qa/vagrant/src/test/resources/packaging/tests/bootstrap_password.bash b/x-pack/qa/vagrant/src/test/resources/packaging/tests/bootstrap_password.bash index 2e62635b6baad..1c5b220d83725 100644 --- a/x-pack/qa/vagrant/src/test/resources/packaging/tests/bootstrap_password.bash +++ b/x-pack/qa/vagrant/src/test/resources/packaging/tests/bootstrap_password.bash @@ -52,7 +52,7 @@ fi run sudo -E -u $ESPLUGIN_COMMAND_USER bash <<"NEW_PASS" if [[ ! -f $ESCONFIG/elasticsearch.keystore ]]; then - $ESHOME/bin/elasticsearch-keystore create + $ESHOME/bin/elasticsearch-keystore create --nopass fi cat /dev/urandom | tr -dc "[a-zA-Z0-9]" | fold -w 20 | head -n 1 > /tmp/bootstrap.password cat /tmp/bootstrap.password | $ESHOME/bin/elasticsearch-keystore add --stdin bootstrap.password diff --git a/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash b/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash index c0ae9aac4db30..9aca78ad43c2f 100644 --- a/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash +++ b/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash @@ -164,7 +164,7 @@ start_node_using_package() { # cacerts imported into a Java keystore. run sudo -E -u $MASTER_USER bash <<"NEW_PASS" if [[ ! -f $ESCONFIG/elasticsearch.keystore ]]; then - $ESHOME/bin/elasticsearch-keystore create + $ESHOME/bin/elasticsearch-keystore create --nopass fi echo "changeme" | $ESHOME/bin/elasticsearch-keystore add --stdin bootstrap.password NEW_PASS From 76ea0662d61c6240a6ea2b56b5553b0cd0198133 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Fri, 14 Jun 2019 10:07:32 +0300 Subject: [PATCH 07/15] fix checkstyl --- .../common/settings/AddStringKeyStoreCommand.java | 1 - .../common/settings/AddStringKeyStoreCommandTests.java | 3 --- .../common/settings/ChangeKeyStorePassphraseCommandTests.java | 0 3 files changed, 4 deletions(-) rename {server => distribution/tools/keystore-cli}/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommandTests.java (100%) diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java index e9ca7fc0c722f..8faad5c3cf501 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java @@ -20,7 +20,6 @@ package org.elasticsearch.common.settings; import java.io.BufferedReader; -import java.io.CharArrayWriter; import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java index b8502077c03c0..447f88069352b 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java @@ -20,10 +20,8 @@ package org.elasticsearch.common.settings; import java.io.ByteArrayInputStream; -import java.io.CharArrayWriter; import java.io.InputStream; import java.nio.charset.StandardCharsets; -import java.util.Locale; import java.util.Map; import org.elasticsearch.cli.Command; @@ -32,7 +30,6 @@ import org.elasticsearch.env.Environment; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.hasToString; public class AddStringKeyStoreCommandTests extends KeyStoreCommandTestCase { InputStream input; diff --git a/server/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommandTests.java similarity index 100% rename from server/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommandTests.java rename to distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommandTests.java From 3bf18308bfb4448dc5c780e4e4a7f168e46edfba Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Fri, 14 Jun 2019 20:04:48 +0300 Subject: [PATCH 08/15] Address feedback - Default behavior when creating a keystore is now to set an empty string as the password (as is the current behavior) - Password can be set with the ChangeKeyStorePassword subcommand - Keystores can be created being password protected, by passing `-p` or `--password`. Users will be prompted to set the password. - Renamed passphrase to password everywhere for consistency --- .../gradle/test/ClusterFormationTasks.groovy | 64 +++---- .../settings/AddFileKeyStoreCommand.java | 8 +- .../settings/AddStringKeyStoreCommand.java | 6 +- .../ChangeKeyStorePasswordCommand.java | 26 +-- .../settings/CreateKeyStoreCommand.java | 16 +- .../common/settings/KeyStoreCli.java | 2 +- .../common/settings/ListKeyStoreCommand.java | 10 +- .../RemoveSettingKeyStoreCommand.java | 14 +- .../settings/UpgradeKeyStoreCommand.java | 18 +- .../settings/AddFileKeyStoreCommandTests.java | 130 +++++++------- .../AddStringKeyStoreCommandTests.java | 120 ++++++------- ...> ChangeKeyStorePasswordCommandTests.java} | 48 +++--- .../settings/CreateKeyStoreCommandTests.java | 38 ++-- .../settings/ListKeyStoreCommandTests.java | 28 +-- .../RemoveSettingKeyStoreCommandTests.java | 48 +++--- .../packaging/test/ArchiveTestCase.java | 34 ++-- .../common/settings/KeyStoreWrapper.java | 162 +++++++++++------- ...ssphrase.java => KeystoreAndPassword.java} | 19 +- .../packaging/tests/bootstrap_password.bash | 2 +- .../resources/packaging/tests/certgen.bash | 2 +- 20 files changed, 424 insertions(+), 371 deletions(-) rename server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommand.java => distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java (66%) rename distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/{ChangeKeyStorePassphraseCommandTests.java => ChangeKeyStorePasswordCommandTests.java} (68%) rename server/src/main/java/org/elasticsearch/common/settings/{KeystoreAndPassphrase.java => KeystoreAndPassword.java} (75%) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy index defdf9ffc83f5..837b076e3d613 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy @@ -44,6 +44,7 @@ import java.nio.charset.StandardCharsets import java.nio.file.Paths import java.util.concurrent.TimeUnit import java.util.stream.Collectors + /** * A helper for creating tasks to build a cluster that is used by a task, and tear down the cluster when the task is finished. */ @@ -64,16 +65,16 @@ class ClusterFormationTasks { * snapshots survive failures / test runs and there is no simple way * today to fix that. */ if (config.cleanShared) { - Task cleanup = project.tasks.create( - name: "${prefix}#prepareCluster.cleanShared", - type: Delete, - dependsOn: startDependencies) { - delete sharedDir - doLast { - sharedDir.mkdirs() - } - } - startDependencies = cleanup + Task cleanup = project.tasks.create( + name: "${prefix}#prepareCluster.cleanShared", + type: Delete, + dependsOn: startDependencies) { + delete sharedDir + doLast { + sharedDir.mkdirs() + } + } + startDependencies = cleanup } List startTasks = [] List nodes = [] @@ -115,8 +116,8 @@ class ClusterFormationTasks { String elasticsearchVersion if (i < config.numBwcNodes) { elasticsearchVersion = config.bwcVersion.toString() - if (project.bwcVersions.unreleased.contains(config.bwcVersion) && - (project.version != elasticsearchVersion)) { + if (project.bwcVersions.unreleased.contains(config.bwcVersion) && + (project.version != elasticsearchVersion)) { elasticsearchVersion += "-SNAPSHOT" } distro = bwcDistro @@ -202,7 +203,7 @@ class ClusterFormationTasks { if (distro.equals("oss")) { snapshotProject = "oss-" + snapshotProject } - + BwcVersions.UnreleasedVersionInfo unreleasedInfo = null if (project.hasProperty('bwcVersions')) { @@ -228,7 +229,7 @@ class ClusterFormationTasks { /** Adds a dependency on a different version of the given plugin, which will be retrieved using gradle's dependency resolution */ static void configureBwcPluginDependency(Project project, Object plugin, Configuration configuration, Version elasticsearchVersion) { if (plugin instanceof Project) { - Project pluginProject = (Project)plugin + Project pluginProject = (Project) plugin verifyProjectHasBuildPlugin(configuration.name, elasticsearchVersion, project, pluginProject) final String pluginName = findPluginName(pluginProject) project.dependencies.add(configuration.name, "org.elasticsearch.plugin:${pluginName}:${elasticsearchVersion}@zip") @@ -329,7 +330,7 @@ class ClusterFormationTasks { start.finalizedBy(stop) for (Object dependency : config.dependencies) { if (dependency instanceof Fixture) { - def depStop = ((Fixture)dependency).stopTask + def depStop = ((Fixture) dependency).stopTask runner.finalizedBy(depStop) start.finalizedBy(depStop) } @@ -366,21 +367,21 @@ class ClusterFormationTasks { /** Adds a task to write elasticsearch.yml for the given node configuration */ static Task configureWriteConfigTask(String name, Project project, Task setup, NodeInfo node, Closure configFilter) { Map esConfig = [ - 'cluster.name' : node.clusterName, - 'node.name' : "node-" + node.nodeNum, - 'pidfile' : node.pidFile, - 'path.repo' : "${node.sharedDir}/repo", - 'path.shared_data' : "${node.sharedDir}/", + 'cluster.name' : node.clusterName, + 'node.name' : "node-" + node.nodeNum, + 'pidfile' : node.pidFile, + 'path.repo' : "${node.sharedDir}/repo", + 'path.shared_data' : "${node.sharedDir}/", // Define a node attribute so we can test that it exists - 'node.attr.testattr' : 'test', + 'node.attr.testattr' : 'test', // Don't wait for state, just start up quickly. This will also allow new and old nodes in the BWC case to become the master - 'discovery.initial_state_timeout' : '0s' + 'discovery.initial_state_timeout': '0s' ] esConfig['http.port'] = node.config.httpPort if (node.nodeVersion.onOrAfter('6.7.0')) { - esConfig['transport.port'] = node.config.transportPort + esConfig['transport.port'] = node.config.transportPort } else { - esConfig['transport.tcp.port'] = node.config.transportPort + esConfig['transport.tcp.port'] = node.config.transportPort } // Default the watermarks to absurdly low to prevent the tests from failing on nodes without enough disk space esConfig['cluster.routing.allocation.disk.watermark.low'] = '1b' @@ -424,7 +425,7 @@ class ClusterFormationTasks { * getting the short name requiring the path to already exist. */ final Object esKeystoreUtil = "${-> node.binPath().resolve('elasticsearch-keystore').toString()}" - return configureExecTask(name, project, setup, node, esKeystoreUtil, 'create', '--nopass') + return configureExecTask(name, project, setup, node, esKeystoreUtil, 'create') } } @@ -488,7 +489,7 @@ class ClusterFormationTasks { Copy copyConfig = project.tasks.create(name: name, type: Copy, dependsOn: setup) File configDir = new File(node.homeDir, 'config') copyConfig.into(configDir) // copy must always have a general dest dir, even though we don't use it - for (Map.Entry extraConfigFile : node.config.extraConfigFiles.entrySet()) { + for (Map.Entry extraConfigFile : node.config.extraConfigFiles.entrySet()) { Object extraConfigFileValue = extraConfigFile.getValue() copyConfig.doFirst { // make sure the copy won't be a no-op or act on a directory @@ -554,7 +555,6 @@ class ClusterFormationTasks { } - pluginFiles.add(configuration) } @@ -681,7 +681,7 @@ class ClusterFormationTasks { // this closure is converted into ant nodes by groovy's AntBuilder Closure antRunner = { AntBuilder ant -> ant.exec(executable: node.executable, spawn: node.config.daemonize, newenvironment: true, - dir: node.cwd, taskname: 'elasticsearch') { + dir: node.cwd, taskname: 'elasticsearch') { node.env.each { key, value -> env(key: key, value: value) } if ((project.isRuntimeJavaHomeSet && node.isBwcNode == false) // runtime Java might not be compatible with old nodes || node.config.distribution == 'integ-test-zip') { @@ -728,7 +728,7 @@ class ClusterFormationTasks { start.doLast(elasticsearchRunner) start.doFirst { // If the node runs in a FIPS 140-2 JVM, the BCFKS default keystore will be password protected - if (project.inFipsJvm){ + if (project.inFipsJvm) { node.config.systemProperties.put('javax.net.ssl.trustStorePassword', 'password') node.config.systemProperties.put('javax.net.ssl.keyStorePassword', 'password') } @@ -866,7 +866,7 @@ class ClusterFormationTasks { node.startLog.eachLine { line -> logger.error("| ${line}") } } if (node.pidFile.exists() && node.failedMarker.exists() == false && - (node.httpPortsFile.exists() == false || node.transportPortsFile.exists() == false)) { + (node.httpPortsFile.exists() == false || node.transportPortsFile.exists() == false)) { logger.error("|\n| [jstack]") String pid = node.pidFile.getText('UTF-8') ByteArrayOutputStream output = new ByteArrayOutputStream() @@ -886,7 +886,7 @@ class ClusterFormationTasks { return project.tasks.create(name: name, type: Exec, dependsOn: depends) { onlyIf { node.pidFile.exists() } // the pid file won't actually be read until execution time, since the read is wrapped within an inner closure of the GString - ext.pid = "${ -> node.pidFile.getText('UTF-8').trim()}" + ext.pid = "${-> node.pidFile.getText('UTF-8').trim()}" File jps if (Os.isFamily(Os.FAMILY_WINDOWS)) { jps = getJpsExecutableByName(project, "jps.exe") @@ -923,7 +923,7 @@ class ClusterFormationTasks { return project.tasks.create(name: name, type: LoggedExec, dependsOn: depends) { onlyIf { node.pidFile.exists() } // the pid file won't actually be read until execution time, since the read is wrapped within an inner closure of the GString - ext.pid = "${ -> node.pidFile.getText('UTF-8').trim()}" + ext.pid = "${-> node.pidFile.getText('UTF-8').trim()}" doFirst { logger.info("Shutting down external node with pid ${pid}") } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java index 8d4c03c7ee0cf..4dee78267bf86 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java @@ -53,7 +53,7 @@ class AddFileKeyStoreCommand extends EnvironmentAwareCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - try (KeystoreAndPassphrase keyAndPass = KeyStoreWrapper.readOrCreate(terminal, env.configFile(), options.has(forceOption))) { + try (KeystoreAndPassword keyAndPass = KeyStoreWrapper.readOrCreate(terminal, env.configFile(), options.has(forceOption))) { if (null == keyAndPass) { return; } @@ -83,13 +83,13 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath"); } keystore.setFile(setting, Files.readAllBytes(file)); - keystore.save(env.configFile(), keyAndPass.getPassphrase()); + keystore.save(env.configFile(), keyAndPass.getPassword()); } catch (SecurityException e) { - throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); + throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the password was correct.", e); } } - @SuppressForbidden(reason="file arg for cli") + @SuppressForbidden(reason = "file arg for cli") private Path getPath(String file) { return PathUtils.get(file); } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java index 8faad5c3cf501..72af659fc38cc 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java @@ -56,7 +56,7 @@ InputStream getStdin() { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - try (KeystoreAndPassphrase keyAndPass = KeyStoreWrapper.readOrCreate(terminal, env.configFile(), options.has(forceOption))) { + try (KeystoreAndPassword keyAndPass = KeyStoreWrapper.readOrCreate(terminal, env.configFile(), options.has(forceOption))) { if (null == keyAndPass) { return; } @@ -86,9 +86,9 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } catch (IllegalArgumentException e) { throw new UserException(ExitCodes.DATA_ERROR, "String value must contain only ASCII"); } - keystore.save(env.configFile(), keyAndPass.getPassphrase()); + keystore.save(env.configFile(), keyAndPass.getPassword()); } catch (SecurityException e) { - throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); + throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the password was correct.", e); } } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java similarity index 66% rename from server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommand.java rename to distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java index 348b2b03caf43..f1daf47fd25cf 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java @@ -28,29 +28,31 @@ import java.util.Arrays; -public class ChangeKeyStorePassphraseCommand extends EnvironmentAwareCommand { - +/** + * A sub-command for the keystore cli which changes the password. + */ +class ChangeKeyStorePasswordCommand extends EnvironmentAwareCommand { - ChangeKeyStorePassphraseCommand() { - super("Changes the passphrase of a keystore"); + ChangeKeyStorePasswordCommand() { + super("Changes the password of a keystore"); } @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - char[] newPassphrase = null; - try (KeystoreAndPassphrase keyAndPass = KeyStoreWrapper.readOrCreate(terminal, env.configFile(), false)) { + char[] newPassword = null; + try (KeystoreAndPassword keyAndPass = KeyStoreWrapper.readOrCreate(terminal, env.configFile(), false)) { if (null == keyAndPass) { return; } KeyStoreWrapper keystore = keyAndPass.getKeystore(); - newPassphrase = KeyStoreWrapper.readPassphrase(terminal, true); - keystore.save(env.configFile(), newPassphrase); - terminal.println("Elasticsearch keystore passphrase changed successfully." + env.configFile()); + newPassword = KeyStoreWrapper.readPassword(terminal, true); + keystore.save(env.configFile(), newPassword); + terminal.println("Elasticsearch keystore password changed successfully." + env.configFile()); } catch (SecurityException e) { - throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); + throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the password was correct.", e); } finally { - if (null != newPassphrase) { - Arrays.fill(newPassphrase, '\u0000'); + if (null != newPassword) { + Arrays.fill(newPassword, '\u0000'); } } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index 2917ec87f5cd1..729cc18433411 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -32,20 +32,20 @@ import org.elasticsearch.env.Environment; /** - * A subcommand for the keystore cli to create a new keystore. + * A sub-command for the keystore cli to create a new keystore. */ class CreateKeyStoreCommand extends EnvironmentAwareCommand { - private final OptionSpec noPassOption; + private final OptionSpec passwordOption; CreateKeyStoreCommand() { super("Creates a new elasticsearch keystore"); - this.noPassOption = parser.acceptsAll(Arrays.asList("n", "nopass"), "Creates an obfuscated (not passphrase protected) keystore"); + this.passwordOption = parser.acceptsAll(Arrays.asList("p", "password"), "Prompt for password to encrypt the keystore"); } @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - char[] passphrase = null; + char[] password = null; try { Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); if (Files.exists(keystoreFile)) { @@ -55,14 +55,14 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } } KeyStoreWrapper keystore = KeyStoreWrapper.create(); - passphrase = options.has(noPassOption) ? new char[0] : KeyStoreWrapper.readPassphrase(terminal, true); - keystore.save(env.configFile(), passphrase); + password = options.has(passwordOption) ? KeyStoreWrapper.readPassword(terminal, true) : new char[0]; + keystore.save(env.configFile(), password); terminal.println("Created elasticsearch keystore in " + env.configFile()); } catch (SecurityException e) { throw new UserException(ExitCodes.IO_ERROR, "Error creating the elasticsearch keystore.", e); } finally { - if (null != passphrase) { - Arrays.fill(passphrase, '\u0000'); + if (null != password) { + Arrays.fill(password, '\u0000'); } } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java index 2d8d35e8a5f40..151c13ad68ebe 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java @@ -35,7 +35,7 @@ private KeyStoreCli() { subcommands.put("add-file", new AddFileKeyStoreCommand()); subcommands.put("remove", new RemoveSettingKeyStoreCommand()); subcommands.put("upgrade", new UpgradeKeyStoreCommand()); - subcommands.put("passwd", new ChangeKeyStorePassphraseCommand()); + subcommands.put("passwd", new ChangeKeyStorePasswordCommand()); } public static void main(String[] args) throws Exception { diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java index 04001f816b879..6ec23f38b56f6 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java @@ -47,19 +47,19 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th if (keystore == null) { throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); } - char[] passphrase; - passphrase = keystore.hasPassword() ? KeyStoreWrapper.readPassphrase(terminal, false) : new char[0]; + char[] password; + password = keystore.hasPassword() ? KeyStoreWrapper.readPassword(terminal, false) : new char[0]; try { - keystore.decrypt(passphrase); + keystore.decrypt(password); List sortedEntries = new ArrayList<>(keystore.getSettingNames()); Collections.sort(sortedEntries); for (String entry : sortedEntries) { terminal.println(entry); } } catch (SecurityException e) { - throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); + throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the password was correct.", e); } finally { - Arrays.fill(passphrase, '\u0000'); + Arrays.fill(password, '\u0000'); } } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java index d8e74f7250a87..604d8aabdf47b 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java @@ -53,22 +53,22 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th if (keystore == null) { throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); } - char[] passphrase = null; + char[] password = null; try { - passphrase = keystore.hasPassword() ? KeyStoreWrapper.readPassphrase(terminal, false) : new char[0]; - keystore.decrypt(passphrase); + password = keystore.hasPassword() ? KeyStoreWrapper.readPassword(terminal, false) : new char[0]; + keystore.decrypt(password); for (String setting : arguments.values(options)) { if (keystore.getSettingNames().contains(setting) == false) { throw new UserException(ExitCodes.CONFIG, "Setting [" + setting + "] does not exist in the keystore."); } keystore.remove(setting); } - keystore.save(env.configFile(), passphrase); + keystore.save(env.configFile(), password); } catch (SecurityException e) { - throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the passphrase was correct."); + throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the password was correct.", e); } finally { - if (null != passphrase) { - Arrays.fill(passphrase, '\u0000'); + if (null != password) { + Arrays.fill(password, '\u0000'); } } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java index 6338f40ea05fa..3421b414fdb2b 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java @@ -26,6 +26,8 @@ import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; +import java.util.Arrays; + /** * A sub-command for the keystore CLI that enables upgrading the keystore format. */ @@ -40,11 +42,19 @@ protected void execute(final Terminal terminal, final OptionSet options, final E final KeyStoreWrapper wrapper = KeyStoreWrapper.load(env.configFile()); if (wrapper == null) { throw new UserException( - ExitCodes.CONFIG, - "keystore does not exist at [" + KeyStoreWrapper.keystorePath(env.configFile()) + "]"); + ExitCodes.CONFIG, + "keystore does not exist at [" + KeyStoreWrapper.keystorePath(env.configFile()) + "]"); + } + char[] password; + password = wrapper.hasPassword() ? KeyStoreWrapper.readPassword(terminal, false) : new char[0]; + try { + wrapper.decrypt(new char[0]); + KeyStoreWrapper.upgrade(wrapper, env.configFile(), new char[0]); + } catch (SecurityException e) { + throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the password was correct.", e); + } finally { + Arrays.fill(password, '\u0000'); } - wrapper.decrypt(new char[0]); - KeyStoreWrapper.upgrade(wrapper, env.configFile(), new char[0]); } } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java index 16ade3543f322..2de4d014c675a 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java @@ -59,165 +59,165 @@ private void addFile(KeyStoreWrapper keystore, String setting, Path file, String } public void testMissingPromptCreate() throws Exception { - String passphrase = "keystorepassphrase"; + String password = "keystorepassword"; Path file1 = createRandomFile(); - terminal.addSecretInput(passphrase); - terminal.addSecretInput(passphrase); + terminal.addSecretInput(password); + terminal.addSecretInput(password); terminal.addTextInput("y"); execute("foo", file1.toString()); - assertSecureFile("foo", file1, passphrase); + assertSecureFile("foo", file1, password); } public void testMissingPromptCreateNotMatchingPasswpord() throws Exception { - String passphrase = "keystorepassphrase"; + String password = "keystorepassword"; Path file1 = createRandomFile(); terminal.addTextInput("y"); - terminal.addSecretInput(passphrase); + terminal.addSecretInput(password); terminal.addSecretInput("adifferentpassphase"); UserException e = expectThrows(UserException.class, () -> execute("foo", file1.toString())); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Passphrases are not equal, exiting")); + assertThat(e.getMessage(), containsString("Passwords are not equal, exiting")); } public void testMissingForceCreate() throws Exception { Path file1 = createRandomFile(); - String passphrase = "keystorepassphrase"; - terminal.addSecretInput(passphrase); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + terminal.addSecretInput(password); + terminal.addSecretInput(password); execute("-f", "foo", file1.toString()); - assertSecureFile("foo", file1, passphrase); + assertSecureFile("foo", file1, password); } public void testMissingNoCreate() throws Exception { - terminal.addSecretInput(randomFrom("", "keystorepassphrase")); + terminal.addSecretInput(randomFrom("", "keystorepassword")); terminal.addTextInput("n"); // explicit no execute("foo"); assertNull(KeyStoreWrapper.load(env.configFile())); } public void testOverwritePromptDefault() throws Exception { - String passphrase = "keystorepassphrase"; + String password = "keystorepassword"; Path file = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(passphrase); - addFile(keystore, "foo", file, passphrase); - terminal.addSecretInput(passphrase); - terminal.addSecretInput(passphrase); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file, password); + terminal.addSecretInput(password); + terminal.addSecretInput(password); terminal.addTextInput(""); execute("foo", "path/dne"); - assertSecureFile("foo", file, passphrase); + assertSecureFile("foo", file, password); } public void testOverwritePromptExplicitNo() throws Exception { - String passphrase = "keystorepassphrase"; + String password = "keystorepassword"; Path file = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(passphrase); - addFile(keystore, "foo", file, passphrase); - terminal.addSecretInput(passphrase); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file, password); + terminal.addSecretInput(password); terminal.addTextInput("n"); // explicit no execute("foo", "path/dne"); - assertSecureFile("foo", file, passphrase); + assertSecureFile("foo", file, password); } public void testOverwritePromptExplicitYes() throws Exception { - String passphrase = "keystorepassphrase"; + String password = "keystorepassword"; Path file1 = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(passphrase); - addFile(keystore, "foo", file1, passphrase); - terminal.addSecretInput(passphrase); - terminal.addSecretInput(passphrase); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file1, password); + terminal.addSecretInput(password); + terminal.addSecretInput(password); terminal.addTextInput("y"); Path file2 = createRandomFile(); execute("foo", file2.toString()); - assertSecureFile("foo", file2, passphrase); + assertSecureFile("foo", file2, password); } public void testOverwriteForceShort() throws Exception { - String passphrase = "keystorepassphrase"; + String password = "keystorepassword"; Path file1 = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(passphrase); - addFile(keystore, "foo", file1, passphrase); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file1, password); Path file2 = createRandomFile(); - terminal.addSecretInput(passphrase); - terminal.addSecretInput(passphrase); + terminal.addSecretInput(password); + terminal.addSecretInput(password); execute("-f", "foo", file2.toString()); - assertSecureFile("foo", file2, passphrase); + assertSecureFile("foo", file2, password); } public void testOverwriteForceLong() throws Exception { - String passphrase = "keystorepassphrase"; + String password = "keystorepassword"; Path file1 = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(passphrase); - addFile(keystore, "foo", file1, passphrase); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file1, password); Path file2 = createRandomFile(); - terminal.addSecretInput(passphrase); + terminal.addSecretInput(password); execute("--force", "foo", file2.toString()); - assertSecureFile("foo", file2, passphrase); + assertSecureFile("foo", file2, password); } public void testForceNonExistent() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase); + String password = "keystorepassword"; + createKeystore(password); Path file = createRandomFile(); - terminal.addSecretInput(passphrase); + terminal.addSecretInput(password); execute("--force", "foo", file.toString()); - assertSecureFile("foo", file, passphrase); + assertSecureFile("foo", file, password); } public void testMissingSettingName() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); UserException e = expectThrows(UserException.class, this::execute); assertEquals(ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("Missing setting name")); } public void testMissingFileName() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); UserException e = expectThrows(UserException.class, () -> execute("foo")); assertEquals(ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("Missing file name")); } public void testFileDNE() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); UserException e = expectThrows(UserException.class, () -> execute("foo", "path/dne")); assertEquals(ExitCodes.IO_ERROR, e.exitCode); assertThat(e.getMessage(), containsString("File [path/dne] does not exist")); } public void testExtraArguments() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase); + String password = "keystorepassword"; + createKeystore(password); Path file = createRandomFile(); - terminal.addSecretInput(passphrase); + terminal.addSecretInput(password); UserException e = expectThrows(UserException.class, () -> execute("foo", file.toString(), "bar")); assertEquals(e.getMessage(), ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("Unrecognized extra arguments [bar]")); } public void testIncorrectPassword() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase); + String password = "keystorepassword"; + createKeystore(password); Path file = createRandomFile(); - terminal.addSecretInput("thewrongkeystorepassphrase"); + terminal.addSecretInput("thewrongkeystorepassword"); UserException e = expectThrows(UserException.class, () -> execute("foo", file.toString())); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Please make sure the passphrase was correct")); + assertThat(e.getMessage(), containsString("Please make sure the password was correct")); } public void testAddToUnprotectedKeystore() throws Exception { - String passphrase = ""; + String password = ""; Path file = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(passphrase); - addFile(keystore, "foo", file, passphrase); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file, password); terminal.addTextInput(""); - // will not be prompted for a passphrase + // will not be prompted for a password execute("foo", "path/dne"); - assertSecureFile("foo", file, passphrase); + assertSecureFile("foo", file, password); } } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java index 447f88069352b..6772662c8302f 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java @@ -50,43 +50,43 @@ InputStream getStdin() { } public void testInvalidPassphrease() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase, "foo", "bar"); - terminal.addSecretInput("thewrongpassphrase"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput("thewrongpassword"); UserException e = expectThrows(UserException.class, () -> execute("foo2")); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Please make sure the passphrase was correct")); + assertThat(e.getMessage(), containsString("Please make sure the password was correct")); } public void testMissingPromptCreate() throws Exception { - String passphrase = "keystorepassphrase"; + String password = "keystorepassword"; terminal.addTextInput("y"); - terminal.addSecretInput(passphrase); - terminal.addSecretInput(passphrase); + terminal.addSecretInput(password); + terminal.addSecretInput(password); terminal.addSecretInput("bar"); execute("foo"); - assertSecureString("foo", "bar", passphrase); + assertSecureString("foo", "bar", password); } public void testMissingPromptCreateNotMatchingPasswords() throws Exception { - String passphrase = "keystorepassphrase"; + String password = "keystorepassword"; terminal.addTextInput("y"); - terminal.addSecretInput(passphrase); - terminal.addSecretInput("anotherpassphrase"); + terminal.addSecretInput(password); + terminal.addSecretInput("anotherpassword"); terminal.addSecretInput("bar"); UserException e = expectThrows(UserException.class, () -> execute("foo")); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Passphrases are not equal, exiting")); + assertThat(e.getMessage(), containsString("Passwords are not equal, exiting")); } public void testMissingForceCreate() throws Exception { - String passphrase = "keystorepassphrase"; - terminal.addSecretInput(passphrase); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + terminal.addSecretInput(password); + terminal.addSecretInput(password); terminal.addSecretInput("bar"); execute("-f", "foo"); - assertSecureString("foo", "bar", passphrase); + assertSecureString("foo", "bar", password); } public void testMissingNoCreate() throws Exception { @@ -96,92 +96,92 @@ public void testMissingNoCreate() throws Exception { } public void testOverwritePromptDefault() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase, "foo", "bar"); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); terminal.addTextInput(""); execute("foo"); - assertSecureString("foo", "bar", passphrase); + assertSecureString("foo", "bar", password); } public void testOverwritePromptExplicitNo() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase, "foo", "bar"); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); terminal.addTextInput("n"); // explicit no execute("foo"); - assertSecureString("foo", "bar", passphrase); + assertSecureString("foo", "bar", password); } public void testOverwritePromptExplicitYes() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase, "foo", "bar"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); terminal.addTextInput("y"); - terminal.addSecretInput(passphrase); + terminal.addSecretInput(password); terminal.addSecretInput("newvalue"); execute("foo"); - assertSecureString("foo", "newvalue", passphrase); + assertSecureString("foo", "newvalue", password); } public void testOverwriteForceShort() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase, "foo", "bar"); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); terminal.addSecretInput("newvalue"); execute("-f", "foo"); // force - assertSecureString("foo", "newvalue", passphrase); + assertSecureString("foo", "newvalue", password); } public void testOverwriteForceLong() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase, "foo", "bar"); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); terminal.addSecretInput("and yet another secret value"); execute("--force", "foo"); // force - assertSecureString("foo", "and yet another secret value", passphrase); + assertSecureString("foo", "and yet another secret value", password); } public void testForceNonExistent() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); terminal.addSecretInput("value"); execute("--force", "foo"); // force - assertSecureString("foo", "value", passphrase); + assertSecureString("foo", "value", password); } public void testPromptForValue() throws Exception { - String passphrase = "keystorepassphrase"; - KeyStoreWrapper.create().save(env.configFile(), passphrase.toCharArray()); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + KeyStoreWrapper.create().save(env.configFile(), password.toCharArray()); + terminal.addSecretInput(password); terminal.addSecretInput("secret value"); execute("foo"); - assertSecureString("foo", "secret value", passphrase); + assertSecureString("foo", "secret value", password); } public void testStdinShort() throws Exception { - String passphrase = "keystorepassphrase"; - KeyStoreWrapper.create().save(env.configFile(), passphrase.toCharArray()); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + KeyStoreWrapper.create().save(env.configFile(), password.toCharArray()); + terminal.addSecretInput(password); setInput("secret value 1"); execute("-x", "foo"); - assertSecureString("foo", "secret value 1", passphrase); + assertSecureString("foo", "secret value 1", password); } public void testStdinLong() throws Exception { - String passphrase = "keystorepassphrase"; - KeyStoreWrapper.create().save(env.configFile(), passphrase.toCharArray()); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + KeyStoreWrapper.create().save(env.configFile(), password.toCharArray()); + terminal.addSecretInput(password); setInput("secret value 2"); execute("--stdin", "foo"); - assertSecureString("foo", "secret value 2", passphrase); + assertSecureString("foo", "secret value 2", password); } public void testMissingSettingName() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase); - terminal.addSecretInput(passphrase); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); + terminal.addSecretInput(password); terminal.addTextInput(""); UserException e = expectThrows(UserException.class, this::execute); assertEquals(ExitCodes.USAGE, e.exitCode); @@ -189,12 +189,12 @@ public void testMissingSettingName() throws Exception { } public void testAddToUnprotectedKeystore() throws Exception { - String passphrase = ""; - createKeystore(passphrase, "foo", "bar"); + String password = ""; + createKeystore(password, "foo", "bar"); terminal.addTextInput(""); - // will not be prompted for a passphrase + // will not be prompted for a password execute("foo"); - assertSecureString("foo", "bar", passphrase); + assertSecureString("foo", "bar", password); } void setInput(String inputStr) { diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java similarity index 68% rename from distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommandTests.java rename to distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java index b630e0f1be7e9..46fcfc830ca3d 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePassphraseCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java @@ -28,10 +28,10 @@ import static org.hamcrest.Matchers.containsString; -public class ChangeKeyStorePassphraseCommandTests extends KeyStoreCommandTestCase { +public class ChangeKeyStorePasswordCommandTests extends KeyStoreCommandTestCase { @Override protected Command newCommand() { - return new ChangeKeyStorePassphraseCommand() { + return new ChangeKeyStorePasswordCommand() { @Override protected Environment createEnv(Map settings) throws UserException { return env; @@ -42,28 +42,28 @@ protected Environment createEnv(Map settings) throws UserExcepti public void testSetKeyStorePassword() throws Exception { createKeystore(""); loadKeystore(""); - terminal.addSecretInput("thepassphrase"); - terminal.addSecretInput("thepassphrase"); + terminal.addSecretInput("thepassword"); + terminal.addSecretInput("thepassword"); // Prompted twice for the new password, since we didn't have an existing password execute(); - loadKeystore("thepassphrase"); + loadKeystore("thepassword"); } public void testChangeKeyStorePassword() throws Exception { - createKeystore("theoldpassphrase"); - loadKeystore("theoldpassphrase"); - terminal.addSecretInput("theoldpassphrase"); - terminal.addSecretInput("thepassphrase"); - terminal.addSecretInput("thepassphrase"); + createKeystore("theoldpassword"); + loadKeystore("theoldpassword"); + terminal.addSecretInput("theoldpassword"); + terminal.addSecretInput("thepassword"); + terminal.addSecretInput("thepassword"); // Prompted thrice: Once for the existing and twice for the new password execute(); - loadKeystore("thepassphrase"); + loadKeystore("thepassword"); } public void testChangeKeyStorePasswordToEmpty() throws Exception { - createKeystore("theoldpassphrase"); - loadKeystore("theoldpassphrase"); - terminal.addSecretInput("theoldpassphrase"); + createKeystore("theoldpassword"); + loadKeystore("theoldpassword"); + terminal.addSecretInput("theoldpassword"); terminal.addSecretInput(""); terminal.addSecretInput(""); // Prompted thrice: Once for the existing and twice for the new password @@ -72,24 +72,24 @@ public void testChangeKeyStorePasswordToEmpty() throws Exception { } public void testChangeKeyStorePasswordWrongVerification() throws Exception { - createKeystore("theoldpassphrase"); - loadKeystore("theoldpassphrase"); - terminal.addSecretInput("theoldpassphrase"); - terminal.addSecretInput("thepassphrase"); - terminal.addSecretInput("themisspelledpassphrase"); + createKeystore("theoldpassword"); + loadKeystore("theoldpassword"); + terminal.addSecretInput("theoldpassword"); + terminal.addSecretInput("thepassword"); + terminal.addSecretInput("themisspelledpassword"); // Prompted thrice: Once for the existing and twice for the new password UserException e = expectThrows(UserException.class, this::execute); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Passphrases are not equal, exiting")); + assertThat(e.getMessage(), containsString("Passwords are not equal, exiting")); } public void testChangeKeyStorePasswordWrongExistingPassword() throws Exception { - createKeystore("theoldpassphrase"); - loadKeystore("theoldpassphrase"); - terminal.addSecretInput("theoldmisspelledpassphrase"); + createKeystore("theoldpassword"); + loadKeystore("theoldpassword"); + terminal.addSecretInput("theoldmisspelledpassword"); // We'll only be prompted once (for the old password) UserException e = expectThrows(UserException.class, this::execute); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Please make sure the passphrase was correct")); + assertThat(e.getMessage(), containsString("Please make sure the password was correct")); } } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java index 60f10009b3930..4fd21a9b61a7f 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java @@ -44,39 +44,33 @@ protected Environment createEnv(Map settings) throws UserExcepti } public void testNotMatchingPasswords() throws Exception { - String passphrase = randomFrom("", "keystorepassphrase"); - terminal.addSecretInput(passphrase); - terminal.addSecretInput("notthekeystorepassphraseyouarelookingfor"); - UserException e = expectThrows(UserException.class, this::execute); + String password = randomFrom("", "keystorepassword"); + terminal.addSecretInput(password); + terminal.addSecretInput("notthekeystorepasswordyouarelookingfor"); + UserException e = expectThrows(UserException.class, () -> execute(randomFrom("-p", "--password"))); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Passphrases are not equal, exiting")); + assertThat(e.getMessage(), containsString("Passwords are not equal, exiting")); } - public void testNotPromptForPassword() throws Exception { - execute("-n"); - Path configDir = env.configFile(); - assertNotNull(KeyStoreWrapper.load(configDir)); - } - - public void testNotPromptForPasswordFullOptionName() throws Exception { - execute("--nopass"); + public void testDefaultNotPromptForPassword() throws Exception { + execute(); Path configDir = env.configFile(); assertNotNull(KeyStoreWrapper.load(configDir)); } public void testPosix() throws Exception { - String passphrase = randomFrom("", "keystorepassphrase"); - terminal.addSecretInput(passphrase); - terminal.addSecretInput(passphrase); + String password = randomFrom("", "keystorepassword"); + terminal.addSecretInput(password); + terminal.addSecretInput(password); execute(); Path configDir = env.configFile(); assertNotNull(KeyStoreWrapper.load(configDir)); } public void testNotPosix() throws Exception { - String passphrase = randomFrom("", "keystorepassphrase"); - terminal.addSecretInput(passphrase); - terminal.addSecretInput(passphrase); + String password = randomFrom("", "keystorepassword"); + terminal.addSecretInput(password); + terminal.addSecretInput(password); env = setupEnv(false, fileSystems); execute(); Path configDir = env.configFile(); @@ -84,7 +78,7 @@ public void testNotPosix() throws Exception { } public void testOverwrite() throws Exception { - String passphrase = randomFrom("", "keystorepassphrase"); + String password = randomFrom("", "keystorepassword"); Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); byte[] content = "not a keystore".getBytes(StandardCharsets.UTF_8); Files.write(keystoreFile, content); @@ -98,8 +92,8 @@ public void testOverwrite() throws Exception { assertArrayEquals(content, Files.readAllBytes(keystoreFile)); terminal.addTextInput("y"); - terminal.addSecretInput(passphrase); - terminal.addSecretInput(passphrase); + terminal.addSecretInput(password); + terminal.addSecretInput(password); execute(); assertNotNull(KeyStoreWrapper.load(env.configFile())); } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java index 8f72b1ea31665..b865f620b02ad 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java @@ -47,36 +47,36 @@ public void testMissing() throws Exception { } public void testEmpty() throws Exception { - String passphrase = randomFrom("", "keystorepassphrase"); - createKeystore(passphrase); - terminal.addSecretInput(passphrase); + String password = randomFrom("", "keystorepassword"); + createKeystore(password); + terminal.addSecretInput(password); execute(); assertEquals("keystore.seed\n", terminal.getOutput()); } public void testOne() throws Exception { - String passphrase = randomFrom("", "keystorepassphrase"); - createKeystore(passphrase, "foo", "bar"); - terminal.addSecretInput(passphrase); + String password = randomFrom("", "keystorepassword"); + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); execute(); assertEquals("foo\nkeystore.seed\n", terminal.getOutput()); } public void testMultiple() throws Exception { - String passphrase = randomFrom("", "keystorepassphrase"); - createKeystore(passphrase, "foo", "1", "baz", "2", "bar", "3"); - terminal.addSecretInput(passphrase); + String password = randomFrom("", "keystorepassword"); + createKeystore(password, "foo", "1", "baz", "2", "bar", "3"); + terminal.addSecretInput(password); execute(); assertEquals("bar\nbaz\nfoo\nkeystore.seed\n", terminal.getOutput()); // sorted } - public void testListWithIncorrectPassphrase() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase, "foo", "bar"); - terminal.addSecretInput("thewrongkeystorepassphrase"); + public void testListWithIncorrectPassword() throws Exception { + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput("thewrongkeystorepassword"); UserException e = expectThrows(UserException.class, this::execute); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Please make sure the passphrase was correct")); + assertThat(e.getMessage(), containsString("Please make sure the password was correct")); } public void testListWithUnprotectedKeystore() throws Exception { diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java index 8969e42cf3fd3..2f09bd700c188 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java @@ -42,45 +42,45 @@ protected Environment createEnv(Map settings) throws UserExcepti } public void testMissing() { - String passphrase = "keystorepassphrase"; - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + terminal.addSecretInput(password); UserException e = expectThrows(UserException.class, () -> execute("foo")); assertEquals(ExitCodes.DATA_ERROR, e.exitCode); assertThat(e.getMessage(), containsString("keystore not found")); } public void testNoSettings() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); UserException e = expectThrows(UserException.class, this::execute); assertEquals(ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("Must supply at least one setting")); } public void testNonExistentSetting() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); UserException e = expectThrows(UserException.class, () -> execute("foo")); assertEquals(ExitCodes.CONFIG, e.exitCode); assertThat(e.getMessage(), containsString("[foo] does not exist")); } public void testOne() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase, "foo", "bar"); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); execute("foo"); - assertFalse(loadKeystore(passphrase).getSettingNames().contains("foo")); + assertFalse(loadKeystore(password).getSettingNames().contains("foo")); } public void testMany() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase, "foo", "1", "bar", "2", "baz", "3"); - terminal.addSecretInput(passphrase); + String password = "keystorepassword"; + createKeystore(password, "foo", "1", "bar", "2", "baz", "3"); + terminal.addSecretInput(password); execute("foo", "baz"); - Set settings = loadKeystore(passphrase).getSettingNames(); + Set settings = loadKeystore(password).getSettingNames(); assertFalse(settings.contains("foo")); assertFalse(settings.contains("baz")); assertTrue(settings.contains("bar")); @@ -88,19 +88,19 @@ public void testMany() throws Exception { } public void testRemoveWithIncorrectPassword() throws Exception { - String passphrase = "keystorepassphrase"; - createKeystore(passphrase, "foo", "bar"); - terminal.addSecretInput("thewrongpassphrase"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput("thewrongpassword"); UserException e = expectThrows(UserException.class, () -> execute("foo")); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Please make sure the passphrase was correct")); + assertThat(e.getMessage(), containsString("Please make sure the password was correct")); } public void testRemoveFromUnprotectedKeystore() throws Exception { - String passphrase = ""; - createKeystore(passphrase, "foo", "bar"); - // will not be prompted for a passphrase + String password = ""; + createKeystore(password, "foo", "bar"); + // will not be prompted for a password execute("foo"); - assertFalse(loadKeystore(passphrase).getSettingNames().contains("foo")); + assertFalse(loadKeystore(password).getSettingNames().contains("foo")); } } diff --git a/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java b/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java index 4b99c83159c0d..25b5899defae8 100644 --- a/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java +++ b/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java @@ -111,14 +111,14 @@ public void test40CreateKeystoreManually() throws Exception { final Installation.Executables bin = installation.executables(); final Shell sh = newShell(); - Platforms.onLinux(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " create --nopass")); + Platforms.onLinux(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " create")); // this is a hack around the fact that we can't run a command in the same session as the same user but not as administrator. // the keystore ends up being owned by the Administrators group, so we manually set it to be owned by the vagrant user here. // from the server's perspective the permissions aren't really different, this is just to reflect what we'd expect in the tests. // when we run these commands as a role user we won't have to do this Platforms.onWindows(() -> sh.run( - bin.elasticsearchKeystore + " create --nopass; " + + bin.elasticsearchKeystore + " create; " + "$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " + "$acl = Get-Acl '" + installation.config("elasticsearch.keystore") + "'; " + "$acl.SetOwner($account); " + @@ -282,21 +282,21 @@ public void test70CustomPathConfAndJvmOptions() throws Exception { // startup since we detect usages of logging before it is configured final String jvmOptions = "-Xms512m\n" + - "-Xmx512m\n" + - "-Dlog4j2.disable.jmx=true\n"; + "-Xmx512m\n" + + "-Dlog4j2.disable.jmx=true\n"; append(tempConf.resolve("jvm.options"), jvmOptions); final Shell sh = newShell(); Platforms.onLinux(() -> sh.run("chown -R elasticsearch:elasticsearch " + tempConf)); Platforms.onWindows(() -> sh.run( "$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " + - "$tempConf = Get-ChildItem '" + tempConf + "' -Recurse; " + - "$tempConf += Get-Item '" + tempConf + "'; " + - "$tempConf | ForEach-Object { " + + "$tempConf = Get-ChildItem '" + tempConf + "' -Recurse; " + + "$tempConf += Get-Item '" + tempConf + "'; " + + "$tempConf | ForEach-Object { " + "$acl = Get-Acl $_.FullName; " + "$acl.SetOwner($account); " + "Set-Acl $_.FullName $acl " + - "}" + "}" )); final Shell serverShell = newShell(); @@ -336,13 +336,13 @@ public void test80RelativePathConf() throws Exception { Platforms.onLinux(() -> sh.run("chown -R elasticsearch:elasticsearch " + temp)); Platforms.onWindows(() -> sh.run( "$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " + - "$tempConf = Get-ChildItem '" + temp + "' -Recurse; " + - "$tempConf += Get-Item '" + temp + "'; " + - "$tempConf | ForEach-Object { " + + "$tempConf = Get-ChildItem '" + temp + "' -Recurse; " + + "$tempConf += Get-Item '" + temp + "'; " + + "$tempConf | ForEach-Object { " + "$acl = Get-Acl $_.FullName; " + "$acl.SetOwner($account); " + "Set-Acl $_.FullName $acl " + - "}" + "}" )); final Shell serverShell = newShell(); @@ -410,7 +410,7 @@ public void test92ElasticsearchNodeCliPackaging() throws Exception { Platforms.PlatformAction action = () -> { final Result result = sh.run(bin.elasticsearchNode + " -h"); assertThat(result.stdout, - containsString("A CLI tool to do unsafe cluster and index manipulations on current node")); + containsString("A CLI tool to do unsafe cluster and index manipulations on current node")); }; if (distribution().equals(Distribution.DEFAULT_LINUX) || distribution().equals(Distribution.DEFAULT_WINDOWS)) { @@ -444,16 +444,16 @@ public void test94ElasticsearchNodeExecuteCliNotEsHomeWorkDir() throws Exception sh.setWorkingDirectory(getTempDir()); Platforms.PlatformAction action = () -> { - Result result = sh.run(bin.elasticsearchCertutil+ " -h"); + Result result = sh.run(bin.elasticsearchCertutil + " -h"); assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack")); - result = sh.run(bin.elasticsearchSyskeygen+ " -h"); + result = sh.run(bin.elasticsearchSyskeygen + " -h"); assertThat(result.stdout, containsString("system key tool")); - result = sh.run(bin.elasticsearchSetupPasswords+ " -h"); + result = sh.run(bin.elasticsearchSetupPasswords + " -h"); assertThat(result.stdout, containsString("Sets the passwords for reserved users")); - result = sh.run(bin.elasticsearchUsers+ " -h"); + result = sh.run(bin.elasticsearchUsers + " -h"); assertThat(result.stdout, containsString("Manages elasticsearch file users")); }; 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 25870ed5e49ae..3372d836452ad 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -73,7 +73,7 @@ /** * A disk based container for sensitive settings in Elasticsearch. - * + *

* Loading a keystore has 2 phases. First, call {@link #load(Path)}. Then call * {@link #decrypt(char[])} with the keystore password, or an empty char array if * {@link #hasPassword()} is {@code false}. Loading and decrypting should happen @@ -81,7 +81,9 @@ */ public class KeyStoreWrapper implements SecureSettings { - /** An identifier for the type of data that may be stored in a keystore entry. */ + /** + * An identifier for the type of data that may be stored in a keystore entry. + */ private enum EntryType { STRING, FILE @@ -94,44 +96,64 @@ private enum EntryType { public static final Setting SEED_SETTING = SecureSetting.secureString("keystore.seed", null); - /** Characters that may be used in the bootstrap seed setting added to all keystores. */ + /** + * Characters that may be used in the bootstrap seed setting added to all keystores. + */ private static final char[] SEED_CHARS = ("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789" + "~!@#$%^&*-_=+?").toCharArray(); - /** The name of the keystore file to read and write. */ + /** + * The name of the keystore file to read and write. + */ private static final String KEYSTORE_FILENAME = "elasticsearch.keystore"; - /** The version of the metadata written before the keystore data. */ + /** + * The version of the metadata written before the keystore data. + */ static final int FORMAT_VERSION = 4; - /** The oldest metadata format version that can be read. */ + /** + * The oldest metadata format version that can be read. + */ private static final int MIN_FORMAT_VERSION = 1; - /** The algorithm used to derive the cipher key from a password. */ + /** + * The algorithm used to derive the cipher key from a password. + */ private static final String KDF_ALGO = "PBKDF2WithHmacSHA512"; - /** The number of iterations to derive the cipher key. */ + /** + * The number of iterations to derive the cipher key. + */ private static final int KDF_ITERS = 10000; /** * The number of bits for the cipher key. - * + *

* Note: The Oracle JDK 8 ships with a limited JCE policy that restricts key length for AES to 128 bits. * This can be increased to 256 bits once minimum java 9 is the minimum java version. * See http://www.oracle.com/technetwork/java/javase/terms/readme/jdk9-readme-3852447.html#jce - * */ + */ private static final int CIPHER_KEY_BITS = 128; - /** The number of bits for the GCM tag. */ + /** + * The number of bits for the GCM tag. + */ private static final int GCM_TAG_BITS = 128; - /** The cipher used to encrypt the keystore data. */ + /** + * The cipher used to encrypt the keystore data. + */ private static final String CIPHER_ALGO = "AES"; - /** The mode used with the cipher algorithm. */ + /** + * The mode used with the cipher algorithm. + */ private static final String CIPHER_MODE = "GCM"; - /** The padding used with the cipher algorithm. */ + /** + * The padding used with the cipher algorithm. + */ private static final String CIPHER_PADDING = "NoPadding"; // format version changelog: @@ -140,16 +162,24 @@ private enum EntryType { // 3: FIPS compliant algos, ES 6.3 // 4: remove distinction between string/files, ES 6.8/7.1 - /** The metadata format version used to read the current keystore wrapper. */ + /** + * The metadata format version used to read the current keystore wrapper. + */ private final int formatVersion; - /** True iff the keystore has a password needed to read. */ + /** + * True iff the keystore has a password needed to read. + */ private final boolean hasPassword; - /** The raw bytes of the encrypted keystore. */ + /** + * The raw bytes of the encrypted keystore. + */ private final byte[] dataBytes; - /** The decrypted secret data. See {@link #decrypt(char[])}. */ + /** + * The decrypted secret data. See {@link #decrypt(char[])}. + */ private final SetOnce> entries = new SetOnce<>(); private volatile boolean closed; @@ -166,12 +196,16 @@ public int getFormatVersion() { return formatVersion; } - /** Returns a path representing the ES keystore in the given config dir. */ + /** + * Returns a path representing the ES keystore in the given config dir. + */ public static Path keystorePath(Path configDir) { return configDir.resolve(KEYSTORE_FILENAME); } - /** Constructs a new keystore with the given password. */ + /** + * Constructs a new keystore with the given password. + */ public static KeyStoreWrapper create() { KeyStoreWrapper wrapper = new KeyStoreWrapper(FORMAT_VERSION, false, null); wrapper.entries.set(new HashMap<>()); @@ -179,7 +213,9 @@ public static KeyStoreWrapper create() { return wrapper; } - /** Add the bootstrap seed setting, which may be used as a unique, secure, random value by the node */ + /** + * Add the bootstrap seed setting, which may be used as a unique, secure, random value by the node + */ public static void addBootstrapSeed(KeyStoreWrapper wrapper) { assert wrapper.getSettingNames().contains(SEED_SETTING.getKey()) == false; SecureRandom random = Randomness.createSecure(); @@ -189,12 +225,12 @@ public static void addBootstrapSeed(KeyStoreWrapper wrapper) { characters[i] = SEED_CHARS[random.nextInt(SEED_CHARS.length)]; } wrapper.setString(SEED_SETTING.getKey(), characters); - Arrays.fill(characters, (char)0); + Arrays.fill(characters, (char) 0); } /** * Loads information about the Elasticsearch keystore from the provided config directory. - * + *

* {@link #decrypt(char[])} must be called before reading or writing any entries. * Returns {@code null} if no keystore exists. */ @@ -264,7 +300,9 @@ public static KeyStoreWrapper load(Path configDir) throws IOException { } } - /** Upgrades the format of the keystore, if necessary. */ + /** + * Upgrades the format of the keystore, if necessary. + */ public static void upgrade(KeyStoreWrapper wrapper, Path configDir, char[] password) throws Exception { if (wrapper.getFormatVersion() == FORMAT_VERSION && wrapper.getSettingNames().contains(SEED_SETTING.getKey())) { return; @@ -281,7 +319,9 @@ public boolean isLoaded() { return entries.get() != null; } - /** Return true iff calling {@link #decrypt(char[])} requires a non-empty password. */ + /** + * Return true iff calling {@link #decrypt(char[])} requires a non-empty password. + */ public boolean hasPassword() { return hasPassword; } @@ -301,7 +341,7 @@ private Cipher createCipher(int opmode, char[] password, byte[] salt, byte[] iv) /** * Decrypts the underlying keystore data. - * + *

* This may only be called once. */ public void decrypt(char[] password) throws GeneralSecurityException, IOException { @@ -362,7 +402,9 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio } } - /** Encrypt the keystore entries and return the encrypted data. */ + /** + * Encrypt the keystore entries and return the encrypted data. + */ private byte[] encrypt(char[] password, byte[] salt, byte[] iv) throws GeneralSecurityException, IOException { assert isLoaded(); @@ -437,16 +479,16 @@ private void decryptLegacyEntries() throws GeneralSecurityException, IOException if (settingType == EntryType.STRING) { ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(CharBuffer.wrap(chars)); bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); - Arrays.fill(byteBuffer.array(), (byte)0); + Arrays.fill(byteBuffer.array(), (byte) 0); } else { assert settingType == EntryType.FILE; // The PBE keyspec gives us chars, we convert to bytes byte[] tmpBytes = new byte[chars.length]; for (int i = 0; i < tmpBytes.length; ++i) { - tmpBytes[i] = (byte)chars[i]; // PBE only stores the lower 8 bits, so this narrowing is ok + tmpBytes[i] = (byte) chars[i]; // PBE only stores the lower 8 bits, so this narrowing is ok } bytes = Base64.getDecoder().decode(tmpBytes); - Arrays.fill(tmpBytes, (byte)0); + Arrays.fill(tmpBytes, (byte) 0); } Arrays.fill(chars, '\0'); @@ -454,7 +496,9 @@ private void decryptLegacyEntries() throws GeneralSecurityException, IOException } } - /** Write the keystore to the given config directory. */ + /** + * Write the keystore to the given config directory. + */ public synchronized void save(Path configDir, char[] password) throws Exception { ensureOpen(); @@ -463,7 +507,7 @@ public synchronized void save(Path configDir, char[] password) throws Exception String tmpFile = KEYSTORE_FILENAME + ".tmp"; try (IndexOutput output = directory.createOutput(tmpFile, IOContext.DEFAULT)) { CodecUtil.writeHeader(output, KEYSTORE_FILENAME, FORMAT_VERSION); - output.writeByte(password.length == 0 ? (byte)0 : (byte)1); + output.writeByte(password.length == 0 ? (byte) 0 : (byte) 1); // new cipher params SecureRandom random = Randomness.createSecure(); @@ -508,58 +552,58 @@ public synchronized void save(Path configDir, char[] password) throws Exception } /** - * Reads the keystore passphrase from the {@link Terminal}, prompting for verification where applicable and returns it as a + * Reads the keystore password from the {@link Terminal}, prompting for verification where applicable and returns it as a * {@code char[]}. The caller is responsible to clear the char array after use. * * @param terminal the terminal to use for user inputs - * @param withVerification whether the user should be prompted for passphrase verification - * @return a char array with the passphrase the user entered - * @throws UserException If the user is prompted for verification and enters a different passphrase + * @param withVerification whether the user should be prompted for password verification + * @return a char array with the password the user entered + * @throws UserException If the user is prompted for verification and enters a different password */ - static char[] readPassphrase(Terminal terminal, boolean withVerification) throws UserException { - final char[] passphrase; + static char[] readPassword(Terminal terminal, boolean withVerification) throws UserException { + final char[] password; if (withVerification) { - passphrase = terminal.readSecret("Enter new passphrase for the elasticsearch keystore (empty for no passphrase): "); - char[] passphraseVerification = terminal.readSecret("Enter same passphrase again: "); - if (Arrays.equals(passphrase, passphraseVerification) == false) { - throw new UserException(ExitCodes.DATA_ERROR, "Passphrases are not equal, exiting."); + password = terminal.readSecret("Enter new password for the elasticsearch keystore (empty for no password): "); + char[] passwordVerification = terminal.readSecret("Enter same password again: "); + if (Arrays.equals(password, passwordVerification) == false) { + throw new UserException(ExitCodes.DATA_ERROR, "Passwords are not equal, exiting."); } - Arrays.fill(passphraseVerification, '\u0000'); + Arrays.fill(passwordVerification, '\u0000'); } else { - passphrase = terminal.readSecret("Enter passphrase for the elasticsearch keystore : "); + password = terminal.readSecret("Enter password for the elasticsearch keystore : "); } - return passphrase; + return password; } /** * Reads the keystore from disk or attempts to create it if it doesn't already exist. If the keystore is password protected - * it also reads the passphrase from user input and decrypts the keystore. Returns both in a POJO. The caller is responsible to clear - * the char array with the passphrase after use, calling {@link KeystoreAndPassphrase#close()} + * it also reads the password from user input and decrypts the keystore. Returns both in a POJO. The caller is responsible to clear + * the char array with the password after use, calling {@link KeystoreAndPassword#close()} * * @param terminal the terminal to use for user inputs * @param configFile the Path from where to attempt and read the existing keystore * @param forceCreate if set, the keystore is created without prompting the user - * @return a POJO with the {@link KeyStoreWrapper} and it's passphrase, null if the user elected to not create a keystore + * @return a POJO with the {@link KeyStoreWrapper} and its password, null if the user elected to not create a keystore */ - static KeystoreAndPassphrase readOrCreate(Terminal terminal, Path configFile, boolean forceCreate) throws Exception { + static KeystoreAndPassword readOrCreate(Terminal terminal, Path configFile, boolean forceCreate) throws Exception { KeyStoreWrapper keystore = load(configFile); - final char[] passphrase; + final char[] password; if (keystore == null) { if (forceCreate == false && terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { terminal.println("Exiting without creating keystore."); return null; } else { - passphrase = readPassphrase(terminal, true); + password = readPassword(terminal, true); keystore = KeyStoreWrapper.create(); - keystore.save(configFile, passphrase); + keystore.save(configFile, password); terminal.println("Created elasticsearch keystore in " + configFile); - return new KeystoreAndPassphrase(keystore, passphrase); + return new KeystoreAndPassword(keystore, password); } } else { - passphrase = keystore.hasPassword ? readPassphrase(terminal, false) : new char[0]; - keystore.decrypt(passphrase); - return new KeystoreAndPassphrase(keystore, passphrase); + password = keystore.hasPassword ? readPassword(terminal, false) : new char[0]; + keystore.decrypt(password); + return new KeystoreAndPassword(keystore, password); } } @@ -615,7 +659,7 @@ synchronized void setString(String setting, char[] value) { byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); byte[] oldEntry = entries.get().put(setting, bytes); if (oldEntry != null) { - Arrays.fill(oldEntry, (byte)0); + Arrays.fill(oldEntry, (byte) 0); } } @@ -628,7 +672,7 @@ synchronized void setFile(String setting, byte[] bytes) { byte[] oldEntry = entries.get().put(setting, Arrays.copyOf(bytes, bytes.length)); if (oldEntry != null) { - Arrays.fill(oldEntry, (byte)0); + Arrays.fill(oldEntry, (byte) 0); } } @@ -639,7 +683,7 @@ void remove(String setting) { ensureOpen(); byte[] oldEntry = entries.get().remove(setting); if (oldEntry != null) { - Arrays.fill(oldEntry, (byte)0); + Arrays.fill(oldEntry, (byte) 0); } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassphrase.java b/server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassword.java similarity index 75% rename from server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassphrase.java rename to server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassword.java index c8130b9d6e138..b0c181fe908cb 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassphrase.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassword.java @@ -23,27 +23,30 @@ import java.io.IOException; import java.util.Arrays; -public class KeystoreAndPassphrase implements Closeable { +/** + * POJO containing the keystore and its password + */ +final class KeystoreAndPassword implements Closeable { private final KeyStoreWrapper keystore; - private final char[] passphrase; + private final char[] password; - public KeystoreAndPassphrase(KeyStoreWrapper keystore, char[] passphrase) { + KeystoreAndPassword(KeyStoreWrapper keystore, char[] password) { this.keystore = keystore; - this.passphrase = passphrase; + this.password = password; } public KeyStoreWrapper getKeystore() { return keystore; } - public char[] getPassphrase() { - return passphrase; + public char[] getPassword() { + return password; } @Override public void close() throws IOException { - if (null != passphrase) { - Arrays.fill(passphrase, '\u0000'); + if (null != password) { + Arrays.fill(password, '\u0000'); } } } diff --git a/x-pack/qa/vagrant/src/test/resources/packaging/tests/bootstrap_password.bash b/x-pack/qa/vagrant/src/test/resources/packaging/tests/bootstrap_password.bash index 1c5b220d83725..2e62635b6baad 100644 --- a/x-pack/qa/vagrant/src/test/resources/packaging/tests/bootstrap_password.bash +++ b/x-pack/qa/vagrant/src/test/resources/packaging/tests/bootstrap_password.bash @@ -52,7 +52,7 @@ fi run sudo -E -u $ESPLUGIN_COMMAND_USER bash <<"NEW_PASS" if [[ ! -f $ESCONFIG/elasticsearch.keystore ]]; then - $ESHOME/bin/elasticsearch-keystore create --nopass + $ESHOME/bin/elasticsearch-keystore create fi cat /dev/urandom | tr -dc "[a-zA-Z0-9]" | fold -w 20 | head -n 1 > /tmp/bootstrap.password cat /tmp/bootstrap.password | $ESHOME/bin/elasticsearch-keystore add --stdin bootstrap.password diff --git a/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash b/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash index a68a7147d6401..6cc6fab3d4d1e 100644 --- a/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash +++ b/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash @@ -164,7 +164,7 @@ start_node_using_package() { # cacerts imported into a Java keystore. run sudo -E -u $MASTER_USER bash <<"NEW_PASS" if [[ ! -f $ESCONFIG/elasticsearch.keystore ]]; then - $ESHOME/bin/elasticsearch-keystore create --nopass + $ESHOME/bin/elasticsearch-keystore create fi echo "changeme" | $ESHOME/bin/elasticsearch-keystore add --stdin bootstrap.password NEW_PASS From ba4a9ba845bc63f9185987a0a065408c23d597d0 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Thu, 27 Jun 2019 12:23:34 +0300 Subject: [PATCH 09/15] revert unnecessary formatting changes --- .../gradle/test/ClusterFormationTasks.groovy | 64 +++++++++---------- .../packaging/test/ArchiveTestCase.java | 32 +++++----- 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy index 837b076e3d613..defdf9ffc83f5 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy @@ -44,7 +44,6 @@ import java.nio.charset.StandardCharsets import java.nio.file.Paths import java.util.concurrent.TimeUnit import java.util.stream.Collectors - /** * A helper for creating tasks to build a cluster that is used by a task, and tear down the cluster when the task is finished. */ @@ -65,16 +64,16 @@ class ClusterFormationTasks { * snapshots survive failures / test runs and there is no simple way * today to fix that. */ if (config.cleanShared) { - Task cleanup = project.tasks.create( - name: "${prefix}#prepareCluster.cleanShared", - type: Delete, - dependsOn: startDependencies) { - delete sharedDir - doLast { - sharedDir.mkdirs() - } - } - startDependencies = cleanup + Task cleanup = project.tasks.create( + name: "${prefix}#prepareCluster.cleanShared", + type: Delete, + dependsOn: startDependencies) { + delete sharedDir + doLast { + sharedDir.mkdirs() + } + } + startDependencies = cleanup } List startTasks = [] List nodes = [] @@ -116,8 +115,8 @@ class ClusterFormationTasks { String elasticsearchVersion if (i < config.numBwcNodes) { elasticsearchVersion = config.bwcVersion.toString() - if (project.bwcVersions.unreleased.contains(config.bwcVersion) && - (project.version != elasticsearchVersion)) { + if (project.bwcVersions.unreleased.contains(config.bwcVersion) && + (project.version != elasticsearchVersion)) { elasticsearchVersion += "-SNAPSHOT" } distro = bwcDistro @@ -203,7 +202,7 @@ class ClusterFormationTasks { if (distro.equals("oss")) { snapshotProject = "oss-" + snapshotProject } - + BwcVersions.UnreleasedVersionInfo unreleasedInfo = null if (project.hasProperty('bwcVersions')) { @@ -229,7 +228,7 @@ class ClusterFormationTasks { /** Adds a dependency on a different version of the given plugin, which will be retrieved using gradle's dependency resolution */ static void configureBwcPluginDependency(Project project, Object plugin, Configuration configuration, Version elasticsearchVersion) { if (plugin instanceof Project) { - Project pluginProject = (Project) plugin + Project pluginProject = (Project)plugin verifyProjectHasBuildPlugin(configuration.name, elasticsearchVersion, project, pluginProject) final String pluginName = findPluginName(pluginProject) project.dependencies.add(configuration.name, "org.elasticsearch.plugin:${pluginName}:${elasticsearchVersion}@zip") @@ -330,7 +329,7 @@ class ClusterFormationTasks { start.finalizedBy(stop) for (Object dependency : config.dependencies) { if (dependency instanceof Fixture) { - def depStop = ((Fixture) dependency).stopTask + def depStop = ((Fixture)dependency).stopTask runner.finalizedBy(depStop) start.finalizedBy(depStop) } @@ -367,21 +366,21 @@ class ClusterFormationTasks { /** Adds a task to write elasticsearch.yml for the given node configuration */ static Task configureWriteConfigTask(String name, Project project, Task setup, NodeInfo node, Closure configFilter) { Map esConfig = [ - 'cluster.name' : node.clusterName, - 'node.name' : "node-" + node.nodeNum, - 'pidfile' : node.pidFile, - 'path.repo' : "${node.sharedDir}/repo", - 'path.shared_data' : "${node.sharedDir}/", + 'cluster.name' : node.clusterName, + 'node.name' : "node-" + node.nodeNum, + 'pidfile' : node.pidFile, + 'path.repo' : "${node.sharedDir}/repo", + 'path.shared_data' : "${node.sharedDir}/", // Define a node attribute so we can test that it exists - 'node.attr.testattr' : 'test', + 'node.attr.testattr' : 'test', // Don't wait for state, just start up quickly. This will also allow new and old nodes in the BWC case to become the master - 'discovery.initial_state_timeout': '0s' + 'discovery.initial_state_timeout' : '0s' ] esConfig['http.port'] = node.config.httpPort if (node.nodeVersion.onOrAfter('6.7.0')) { - esConfig['transport.port'] = node.config.transportPort + esConfig['transport.port'] = node.config.transportPort } else { - esConfig['transport.tcp.port'] = node.config.transportPort + esConfig['transport.tcp.port'] = node.config.transportPort } // Default the watermarks to absurdly low to prevent the tests from failing on nodes without enough disk space esConfig['cluster.routing.allocation.disk.watermark.low'] = '1b' @@ -425,7 +424,7 @@ class ClusterFormationTasks { * getting the short name requiring the path to already exist. */ final Object esKeystoreUtil = "${-> node.binPath().resolve('elasticsearch-keystore').toString()}" - return configureExecTask(name, project, setup, node, esKeystoreUtil, 'create') + return configureExecTask(name, project, setup, node, esKeystoreUtil, 'create', '--nopass') } } @@ -489,7 +488,7 @@ class ClusterFormationTasks { Copy copyConfig = project.tasks.create(name: name, type: Copy, dependsOn: setup) File configDir = new File(node.homeDir, 'config') copyConfig.into(configDir) // copy must always have a general dest dir, even though we don't use it - for (Map.Entry extraConfigFile : node.config.extraConfigFiles.entrySet()) { + for (Map.Entry extraConfigFile : node.config.extraConfigFiles.entrySet()) { Object extraConfigFileValue = extraConfigFile.getValue() copyConfig.doFirst { // make sure the copy won't be a no-op or act on a directory @@ -555,6 +554,7 @@ class ClusterFormationTasks { } + pluginFiles.add(configuration) } @@ -681,7 +681,7 @@ class ClusterFormationTasks { // this closure is converted into ant nodes by groovy's AntBuilder Closure antRunner = { AntBuilder ant -> ant.exec(executable: node.executable, spawn: node.config.daemonize, newenvironment: true, - dir: node.cwd, taskname: 'elasticsearch') { + dir: node.cwd, taskname: 'elasticsearch') { node.env.each { key, value -> env(key: key, value: value) } if ((project.isRuntimeJavaHomeSet && node.isBwcNode == false) // runtime Java might not be compatible with old nodes || node.config.distribution == 'integ-test-zip') { @@ -728,7 +728,7 @@ class ClusterFormationTasks { start.doLast(elasticsearchRunner) start.doFirst { // If the node runs in a FIPS 140-2 JVM, the BCFKS default keystore will be password protected - if (project.inFipsJvm) { + if (project.inFipsJvm){ node.config.systemProperties.put('javax.net.ssl.trustStorePassword', 'password') node.config.systemProperties.put('javax.net.ssl.keyStorePassword', 'password') } @@ -866,7 +866,7 @@ class ClusterFormationTasks { node.startLog.eachLine { line -> logger.error("| ${line}") } } if (node.pidFile.exists() && node.failedMarker.exists() == false && - (node.httpPortsFile.exists() == false || node.transportPortsFile.exists() == false)) { + (node.httpPortsFile.exists() == false || node.transportPortsFile.exists() == false)) { logger.error("|\n| [jstack]") String pid = node.pidFile.getText('UTF-8') ByteArrayOutputStream output = new ByteArrayOutputStream() @@ -886,7 +886,7 @@ class ClusterFormationTasks { return project.tasks.create(name: name, type: Exec, dependsOn: depends) { onlyIf { node.pidFile.exists() } // the pid file won't actually be read until execution time, since the read is wrapped within an inner closure of the GString - ext.pid = "${-> node.pidFile.getText('UTF-8').trim()}" + ext.pid = "${ -> node.pidFile.getText('UTF-8').trim()}" File jps if (Os.isFamily(Os.FAMILY_WINDOWS)) { jps = getJpsExecutableByName(project, "jps.exe") @@ -923,7 +923,7 @@ class ClusterFormationTasks { return project.tasks.create(name: name, type: LoggedExec, dependsOn: depends) { onlyIf { node.pidFile.exists() } // the pid file won't actually be read until execution time, since the read is wrapped within an inner closure of the GString - ext.pid = "${-> node.pidFile.getText('UTF-8').trim()}" + ext.pid = "${ -> node.pidFile.getText('UTF-8').trim()}" doFirst { logger.info("Shutting down external node with pid ${pid}") } diff --git a/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java b/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java index 25b5899defae8..4078dfc4f97db 100644 --- a/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java +++ b/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java @@ -118,7 +118,7 @@ public void test40CreateKeystoreManually() throws Exception { // from the server's perspective the permissions aren't really different, this is just to reflect what we'd expect in the tests. // when we run these commands as a role user we won't have to do this Platforms.onWindows(() -> sh.run( - bin.elasticsearchKeystore + " create; " + + bin.elasticsearchKeystore + " create; " + "$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " + "$acl = Get-Acl '" + installation.config("elasticsearch.keystore") + "'; " + "$acl.SetOwner($account); " + @@ -282,21 +282,21 @@ public void test70CustomPathConfAndJvmOptions() throws Exception { // startup since we detect usages of logging before it is configured final String jvmOptions = "-Xms512m\n" + - "-Xmx512m\n" + - "-Dlog4j2.disable.jmx=true\n"; + "-Xmx512m\n" + + "-Dlog4j2.disable.jmx=true\n"; append(tempConf.resolve("jvm.options"), jvmOptions); final Shell sh = newShell(); Platforms.onLinux(() -> sh.run("chown -R elasticsearch:elasticsearch " + tempConf)); Platforms.onWindows(() -> sh.run( "$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " + - "$tempConf = Get-ChildItem '" + tempConf + "' -Recurse; " + - "$tempConf += Get-Item '" + tempConf + "'; " + - "$tempConf | ForEach-Object { " + + "$tempConf = Get-ChildItem '" + tempConf + "' -Recurse; " + + "$tempConf += Get-Item '" + tempConf + "'; " + + "$tempConf | ForEach-Object { " + "$acl = Get-Acl $_.FullName; " + "$acl.SetOwner($account); " + "Set-Acl $_.FullName $acl " + - "}" + "}" )); final Shell serverShell = newShell(); @@ -336,13 +336,13 @@ public void test80RelativePathConf() throws Exception { Platforms.onLinux(() -> sh.run("chown -R elasticsearch:elasticsearch " + temp)); Platforms.onWindows(() -> sh.run( "$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " + - "$tempConf = Get-ChildItem '" + temp + "' -Recurse; " + - "$tempConf += Get-Item '" + temp + "'; " + - "$tempConf | ForEach-Object { " + + "$tempConf = Get-ChildItem '" + temp + "' -Recurse; " + + "$tempConf += Get-Item '" + temp + "'; " + + "$tempConf | ForEach-Object { " + "$acl = Get-Acl $_.FullName; " + "$acl.SetOwner($account); " + "Set-Acl $_.FullName $acl " + - "}" + "}" )); final Shell serverShell = newShell(); @@ -410,7 +410,7 @@ public void test92ElasticsearchNodeCliPackaging() throws Exception { Platforms.PlatformAction action = () -> { final Result result = sh.run(bin.elasticsearchNode + " -h"); assertThat(result.stdout, - containsString("A CLI tool to do unsafe cluster and index manipulations on current node")); + containsString("A CLI tool to do unsafe cluster and index manipulations on current node")); }; if (distribution().equals(Distribution.DEFAULT_LINUX) || distribution().equals(Distribution.DEFAULT_WINDOWS)) { @@ -444,16 +444,16 @@ public void test94ElasticsearchNodeExecuteCliNotEsHomeWorkDir() throws Exception sh.setWorkingDirectory(getTempDir()); Platforms.PlatformAction action = () -> { - Result result = sh.run(bin.elasticsearchCertutil + " -h"); + Result result = sh.run(bin.elasticsearchCertutil+ " -h"); assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack")); - result = sh.run(bin.elasticsearchSyskeygen + " -h"); + result = sh.run(bin.elasticsearchSyskeygen+ " -h"); assertThat(result.stdout, containsString("system key tool")); - result = sh.run(bin.elasticsearchSetupPasswords + " -h"); + result = sh.run(bin.elasticsearchSetupPasswords+ " -h"); assertThat(result.stdout, containsString("Sets the passwords for reserved users")); - result = sh.run(bin.elasticsearchUsers + " -h"); + result = sh.run(bin.elasticsearchUsers+ " -h"); assertThat(result.stdout, containsString("Manages elasticsearch file users")); }; From 2b74f6403c2c1fd49cf9f931576f1cef913bd92f Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Thu, 27 Jun 2019 12:28:27 +0300 Subject: [PATCH 10/15] remove nopass again --- .../org/elasticsearch/gradle/test/ClusterFormationTasks.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy index defdf9ffc83f5..f70f258b80a7d 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy @@ -424,7 +424,7 @@ class ClusterFormationTasks { * getting the short name requiring the path to already exist. */ final Object esKeystoreUtil = "${-> node.binPath().resolve('elasticsearch-keystore').toString()}" - return configureExecTask(name, project, setup, node, esKeystoreUtil, 'create', '--nopass') + return configureExecTask(name, project, setup, node, esKeystoreUtil, 'create') } } From ecc0ed6574b92067235ce8ce9e58273857fe19af Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Fri, 28 Jun 2019 18:48:26 +0300 Subject: [PATCH 11/15] Addresses feedback - Move common functionality in a base class - Change behavior for -f that was also forcing creation of the keystore without this being documented or explained in the prompt - Allow user to indicate if autogenerated keystores should be password protected --- .../settings/AddFileKeyStoreCommand.java | 57 +++--- .../settings/AddStringKeyStoreCommand.java | 55 +++--- .../common/settings/BaseKeyStoreCommand.java | 107 +++++++++++ .../ChangeKeyStorePasswordCommand.java | 24 +-- .../settings/CreateKeyStoreCommand.java | 11 +- .../common/settings/ListKeyStoreCommand.java | 30 +-- .../RemoveSettingKeyStoreCommand.java | 33 +--- .../settings/UpgradeKeyStoreCommand.java | 27 +-- .../settings/AddFileKeyStoreCommandTests.java | 21 ++- .../AddStringKeyStoreCommandTests.java | 21 ++- .../ChangeKeyStorePasswordCommandTests.java | 2 +- .../common/settings/KeyStoreWrapperTests.java | 6 +- .../settings/ListKeyStoreCommandTests.java | 2 +- .../RemoveSettingKeyStoreCommandTests.java | 2 +- .../settings/UpgradeKeyStoreCommandTests.java | 2 +- .../common/settings/KeyStoreWrapper.java | 176 ++++-------------- .../common/settings/KeystoreAndPassword.java | 52 ------ 17 files changed, 256 insertions(+), 372 deletions(-) create mode 100644 distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java delete mode 100644 server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassword.java diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java index 4dee78267bf86..5619d0fbd438c 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java @@ -26,7 +26,6 @@ import joptsimple.OptionSet; import joptsimple.OptionSpec; -import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.UserException; @@ -37,7 +36,7 @@ /** * A subcommand for the keystore cli which adds a file setting. */ -class AddFileKeyStoreCommand extends EnvironmentAwareCommand { +class AddFileKeyStoreCommand extends BaseKeyStoreCommand { private final OptionSpec forceOption; private final OptionSpec arguments; @@ -52,41 +51,33 @@ class AddFileKeyStoreCommand extends EnvironmentAwareCommand { } @Override - protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - try (KeystoreAndPassword keyAndPass = KeyStoreWrapper.readOrCreate(terminal, env.configFile(), options.has(forceOption))) { - if (null == keyAndPass) { - return; - } - KeyStoreWrapper keystore = keyAndPass.getKeystore(); + protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception { - List argumentValues = arguments.values(options); - if (argumentValues.size() == 0) { - throw new UserException(ExitCodes.USAGE, "Missing setting name"); - } - String setting = argumentValues.get(0); - if (keystore.getSettingNames().contains(setting) && options.has(forceOption) == false) { - if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { - terminal.println("Exiting without modifying keystore."); - return; - } + List argumentValues = arguments.values(options); + if (argumentValues.size() == 0) { + throw new UserException(ExitCodes.USAGE, "Missing setting name"); + } + String setting = argumentValues.get(0); + if (keyStore.getSettingNames().contains(setting) && options.has(forceOption) == false) { + if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { + terminal.println("Exiting without modifying keystore."); + return; } + } - if (argumentValues.size() == 1) { - throw new UserException(ExitCodes.USAGE, "Missing file name"); - } - Path file = getPath(argumentValues.get(1)); - if (Files.exists(file) == false) { - throw new UserException(ExitCodes.IO_ERROR, "File [" + file.toString() + "] does not exist"); - } - if (argumentValues.size() > 2) { - throw new UserException(ExitCodes.USAGE, "Unrecognized extra arguments [" + - String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath"); - } - keystore.setFile(setting, Files.readAllBytes(file)); - keystore.save(env.configFile(), keyAndPass.getPassword()); - } catch (SecurityException e) { - throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the password was correct.", e); + if (argumentValues.size() == 1) { + throw new UserException(ExitCodes.USAGE, "Missing file name"); + } + Path file = getPath(argumentValues.get(1)); + if (Files.exists(file) == false) { + throw new UserException(ExitCodes.IO_ERROR, "File [" + file.toString() + "] does not exist"); + } + if (argumentValues.size() > 2) { + throw new UserException(ExitCodes.USAGE, "Unrecognized extra arguments [" + + String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath"); } + keyStore.setFile(setting, Files.readAllBytes(file)); + keyStore.save(env.configFile(), keyStorePassword.getChars()); } @SuppressForbidden(reason = "file arg for cli") diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java index 72af659fc38cc..06204bb46e959 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java @@ -27,7 +27,6 @@ import joptsimple.OptionSet; import joptsimple.OptionSpec; -import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.UserException; @@ -36,7 +35,7 @@ /** * A subcommand for the keystore cli which adds a string setting. */ -class AddStringKeyStoreCommand extends EnvironmentAwareCommand { +class AddStringKeyStoreCommand extends BaseKeyStoreCommand { private final OptionSpec stdinOption; private final OptionSpec forceOption; @@ -55,40 +54,32 @@ InputStream getStdin() { } @Override - protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - try (KeystoreAndPassword keyAndPass = KeyStoreWrapper.readOrCreate(terminal, env.configFile(), options.has(forceOption))) { - if (null == keyAndPass) { + protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception { + String setting = arguments.value(options); + if (setting == null) { + throw new UserException(ExitCodes.USAGE, "The setting name can not be null"); + } + if (keyStore.getSettingNames().contains(setting) && options.has(forceOption) == false) { + if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { + terminal.println("Exiting without modifying keystore."); return; } - KeyStoreWrapper keystore = keyAndPass.getKeystore(); - - String setting = arguments.value(options); - if (setting == null) { - throw new UserException(ExitCodes.USAGE, "The setting name can not be null"); - } - if (keystore.getSettingNames().contains(setting) && options.has(forceOption) == false) { - if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { - terminal.println("Exiting without modifying keystore."); - return; - } - } + } - final char[] value; - if (options.has(stdinOption)) { - BufferedReader stdinReader = new BufferedReader(new InputStreamReader(getStdin(), StandardCharsets.UTF_8)); - value = stdinReader.readLine().toCharArray(); - } else { - value = terminal.readSecret("Enter value for " + setting + ": "); - } + final char[] value; + if (options.has(stdinOption)) { + BufferedReader stdinReader = new BufferedReader(new InputStreamReader(getStdin(), StandardCharsets.UTF_8)); + value = stdinReader.readLine().toCharArray(); + } else { + value = terminal.readSecret("Enter value for " + setting + ": "); + } - try { - keystore.setString(setting, value); - } catch (IllegalArgumentException e) { - throw new UserException(ExitCodes.DATA_ERROR, "String value must contain only ASCII"); - } - keystore.save(env.configFile(), keyAndPass.getPassword()); - } catch (SecurityException e) { - throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the password was correct.", e); + try { + keyStore.setString(setting, value); + } catch (IllegalArgumentException e) { + throw new UserException(ExitCodes.DATA_ERROR, e.getMessage()); } + keyStore.save(env.configFile(), keyStorePassword.getChars()); + } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java new file mode 100644 index 0000000000000..2aead7dafaaa2 --- /dev/null +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java @@ -0,0 +1,107 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.settings; + +import joptsimple.OptionSet; +import org.elasticsearch.cli.EnvironmentAwareCommand; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +import java.nio.file.Path; +import java.util.Arrays; + +public abstract class BaseKeyStoreCommand extends EnvironmentAwareCommand { + + KeyStoreWrapper keyStore; + SecureString keyStorePassword; + boolean keyStoreMustExist = false; + + public BaseKeyStoreCommand(String description) { + super(description); + } + + @Override + protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { + try { + final Path configFile = env.configFile(); + keyStore = KeyStoreWrapper.load(configFile); + if (keyStore == null) { + if (keyStoreMustExist) { + throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found at [" + + KeyStoreWrapper.keystorePath(env.configFile()) + "]. Use 'create' command to create one."); + } else { + if (terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { + terminal.println("Exiting without creating keystore."); + return; + } + } + // allow the user to decide if an auto-created keystore should be password protected + keyStorePassword = + terminal.promptYesNo("Do you want to protect the keystore with a password?", false) == false ? + new SecureString(new char[0]) : readPassword(terminal, true); + keyStore = KeyStoreWrapper.create(); + keyStore.save(configFile, keyStorePassword.getChars()); + } else { + keyStorePassword = keyStore.hasPassword() ? readPassword(terminal, false) : new SecureString(new char[0]); + keyStore.decrypt(keyStorePassword.getChars()); + } + executeCommand(terminal, options, env); + } catch (SecurityException e) { + throw new UserException(ExitCodes.DATA_ERROR, e.getMessage()); + } finally { + clearPassword(); + } + } + + private void clearPassword() { + if (keyStorePassword != null) { + keyStorePassword.close(); + } + } + + /** + * Reads the keystore password from the {@link Terminal}, prompting for verification where applicable and returns it as a + * {@link SecureString}. + * + * @param terminal the terminal to use for user inputs + * @param withVerification whether the user should be prompted for password verification + * @return a SecureString with the password the user entered + * @throws UserException If the user is prompted for verification and enters a different password + */ + static SecureString readPassword(Terminal terminal, boolean withVerification) throws UserException { + final char[] passwordArray; + if (withVerification) { + passwordArray = terminal.readSecret("Enter new password for the elasticsearch keystore (empty for no password): "); + char[] passwordVerification = terminal.readSecret("Enter same password again: "); + if (Arrays.equals(passwordArray, passwordVerification) == false) { + throw new UserException(ExitCodes.DATA_ERROR, "Passwords are not equal, exiting."); + } + Arrays.fill(passwordVerification, '\u0000'); + } else { + passwordArray = terminal.readSecret("Enter password for the elasticsearch keystore : "); + } + final SecureString password = new SecureString(passwordArray); + return password; + } + + protected abstract void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception; +} diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java index f1daf47fd25cf..f4755ce06ebf6 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java @@ -20,39 +20,33 @@ package org.elasticsearch.common.settings; import joptsimple.OptionSet; -import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; -import java.util.Arrays; - /** * A sub-command for the keystore cli which changes the password. */ -class ChangeKeyStorePasswordCommand extends EnvironmentAwareCommand { +class ChangeKeyStorePasswordCommand extends BaseKeyStoreCommand { ChangeKeyStorePasswordCommand() { super("Changes the password of a keystore"); + keyStoreMustExist = true; } @Override - protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - char[] newPassword = null; - try (KeystoreAndPassword keyAndPass = KeyStoreWrapper.readOrCreate(terminal, env.configFile(), false)) { - if (null == keyAndPass) { - return; - } - KeyStoreWrapper keystore = keyAndPass.getKeystore(); - newPassword = KeyStoreWrapper.readPassword(terminal, true); - keystore.save(env.configFile(), newPassword); + protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception { + SecureString newPassword = null; + try { + newPassword = readPassword(terminal, true); + keyStore.save(env.configFile(), newPassword.getChars()); terminal.println("Elasticsearch keystore password changed successfully." + env.configFile()); } catch (SecurityException e) { - throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the password was correct.", e); + throw new UserException(ExitCodes.DATA_ERROR, e.getMessage()); } finally { if (null != newPassword) { - Arrays.fill(newPassword, '\u0000'); + newPassword.close(); } } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index 729cc18433411..e64141f820276 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -45,7 +45,7 @@ class CreateKeyStoreCommand extends EnvironmentAwareCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - char[] password = null; + SecureString password = null; try { Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); if (Files.exists(keystoreFile)) { @@ -55,14 +55,15 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } } KeyStoreWrapper keystore = KeyStoreWrapper.create(); - password = options.has(passwordOption) ? KeyStoreWrapper.readPassword(terminal, true) : new char[0]; - keystore.save(env.configFile(), password); + password = options.has(passwordOption) ? + BaseKeyStoreCommand.readPassword(terminal, true) : new SecureString(new char[0]); + keystore.save(env.configFile(), password.getChars()); terminal.println("Created elasticsearch keystore in " + env.configFile()); } catch (SecurityException e) { - throw new UserException(ExitCodes.IO_ERROR, "Error creating the elasticsearch keystore.", e); + throw new UserException(ExitCodes.IO_ERROR, "Error creating the elasticsearch keystore."); } finally { if (null != password) { - Arrays.fill(password, '\u0000'); + password.close(); } } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java index 6ec23f38b56f6..dcd072ab2d96e 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java @@ -21,45 +21,29 @@ import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import joptsimple.OptionSet; -import org.elasticsearch.cli.EnvironmentAwareCommand; -import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.Terminal; -import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; /** * A subcommand for the keystore cli to list all settings in the keystore. */ -class ListKeyStoreCommand extends EnvironmentAwareCommand { +class ListKeyStoreCommand extends BaseKeyStoreCommand { ListKeyStoreCommand() { super("List entries in the keystore"); + keyStoreMustExist = true; } @Override - protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); - if (keystore == null) { - throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); - } - char[] password; - password = keystore.hasPassword() ? KeyStoreWrapper.readPassword(terminal, false) : new char[0]; - try { - keystore.decrypt(password); - List sortedEntries = new ArrayList<>(keystore.getSettingNames()); - Collections.sort(sortedEntries); - for (String entry : sortedEntries) { - terminal.println(entry); - } - } catch (SecurityException e) { - throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the password was correct.", e); - } finally { - Arrays.fill(password, '\u0000'); + protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception { + List sortedEntries = new ArrayList<>(keyStore.getSettingNames()); + Collections.sort(sortedEntries); + for (String entry : sortedEntries) { + terminal.println(entry); } } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java index 604d8aabdf47b..3fdb9191752b0 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java @@ -19,12 +19,10 @@ package org.elasticsearch.common.settings; -import java.util.Arrays; import java.util.List; import joptsimple.OptionSet; import joptsimple.OptionSpec; -import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.UserException; @@ -33,43 +31,28 @@ /** * A subcommand for the keystore cli to remove a setting. */ -class RemoveSettingKeyStoreCommand extends EnvironmentAwareCommand { +class RemoveSettingKeyStoreCommand extends BaseKeyStoreCommand { private final OptionSpec arguments; RemoveSettingKeyStoreCommand() { super("Remove a setting from the keystore"); + keyStoreMustExist = true; arguments = parser.nonOptions("setting names"); } @Override - protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { + protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception { List settings = arguments.values(options); if (settings.isEmpty()) { throw new UserException(ExitCodes.USAGE, "Must supply at least one setting to remove"); } - - KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); - if (keystore == null) { - throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); - } - char[] password = null; - try { - password = keystore.hasPassword() ? KeyStoreWrapper.readPassword(terminal, false) : new char[0]; - keystore.decrypt(password); - for (String setting : arguments.values(options)) { - if (keystore.getSettingNames().contains(setting) == false) { - throw new UserException(ExitCodes.CONFIG, "Setting [" + setting + "] does not exist in the keystore."); - } - keystore.remove(setting); - } - keystore.save(env.configFile(), password); - } catch (SecurityException e) { - throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the password was correct.", e); - } finally { - if (null != password) { - Arrays.fill(password, '\u0000'); + for (String setting : arguments.values(options)) { + if (keyStore.getSettingNames().contains(setting) == false) { + throw new UserException(ExitCodes.CONFIG, "Setting [" + setting + "] does not exist in the keystore."); } + keyStore.remove(setting); } + keyStore.save(env.configFile(), keyStorePassword.getChars()); } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java index 3421b414fdb2b..78a7ee8413da6 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java @@ -20,41 +20,22 @@ package org.elasticsearch.common.settings; import joptsimple.OptionSet; -import org.elasticsearch.cli.EnvironmentAwareCommand; -import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.Terminal; -import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; -import java.util.Arrays; - /** * A sub-command for the keystore CLI that enables upgrading the keystore format. */ -public class UpgradeKeyStoreCommand extends EnvironmentAwareCommand { +public class UpgradeKeyStoreCommand extends BaseKeyStoreCommand { UpgradeKeyStoreCommand() { super("Upgrade the keystore format"); + keyStoreMustExist = true; } @Override - protected void execute(final Terminal terminal, final OptionSet options, final Environment env) throws Exception { - final KeyStoreWrapper wrapper = KeyStoreWrapper.load(env.configFile()); - if (wrapper == null) { - throw new UserException( - ExitCodes.CONFIG, - "keystore does not exist at [" + KeyStoreWrapper.keystorePath(env.configFile()) + "]"); - } - char[] password; - password = wrapper.hasPassword() ? KeyStoreWrapper.readPassword(terminal, false) : new char[0]; - try { - wrapper.decrypt(new char[0]); - KeyStoreWrapper.upgrade(wrapper, env.configFile(), new char[0]); - } catch (SecurityException e) { - throw new UserException(ExitCodes.DATA_ERROR, "Failed to access the keystore. Please make sure the password was correct.", e); - } finally { - Arrays.fill(password, '\u0000'); - } + protected void executeCommand(final Terminal terminal, final OptionSet options, final Environment env) throws Exception { + KeyStoreWrapper.upgrade(keyStore, env.configFile(), keyStorePassword.getChars()); } } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java index 2de4d014c675a..c4ef4723bd050 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java @@ -64,14 +64,24 @@ public void testMissingPromptCreate() throws Exception { terminal.addSecretInput(password); terminal.addSecretInput(password); terminal.addTextInput("y"); + terminal.addTextInput("y"); execute("foo", file1.toString()); assertSecureFile("foo", file1, password); } + public void testMissingPromptCreateWithoutPassword() throws Exception { + Path file1 = createRandomFile(); + terminal.addTextInput("y"); + terminal.addTextInput("n"); + execute("foo", file1.toString()); + assertSecureFile("foo", file1, ""); + } + public void testMissingPromptCreateNotMatchingPasswpord() throws Exception { String password = "keystorepassword"; Path file1 = createRandomFile(); terminal.addTextInput("y"); + terminal.addTextInput("y"); terminal.addSecretInput(password); terminal.addSecretInput("adifferentpassphase"); UserException e = expectThrows(UserException.class, () -> execute("foo", file1.toString())); @@ -79,15 +89,6 @@ public void testMissingPromptCreateNotMatchingPasswpord() throws Exception { assertThat(e.getMessage(), containsString("Passwords are not equal, exiting")); } - public void testMissingForceCreate() throws Exception { - Path file1 = createRandomFile(); - String password = "keystorepassword"; - terminal.addSecretInput(password); - terminal.addSecretInput(password); - execute("-f", "foo", file1.toString()); - assertSecureFile("foo", file1, password); - } - public void testMissingNoCreate() throws Exception { terminal.addSecretInput(randomFrom("", "keystorepassword")); terminal.addTextInput("n"); // explicit no @@ -207,7 +208,7 @@ public void testIncorrectPassword() throws Exception { terminal.addSecretInput("thewrongkeystorepassword"); UserException e = expectThrows(UserException.class, () -> execute("foo", file.toString())); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Please make sure the password was correct")); + assertThat(e.getMessage(), containsString("Provided keystore password was incorrect")); } public void testAddToUnprotectedKeystore() throws Exception { diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java index 6772662c8302f..699f1aee9e622 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java @@ -55,13 +55,14 @@ public void testInvalidPassphrease() throws Exception { terminal.addSecretInput("thewrongpassword"); UserException e = expectThrows(UserException.class, () -> execute("foo2")); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Please make sure the password was correct")); + assertThat(e.getMessage(), containsString("Provided keystore password was incorrect")); } public void testMissingPromptCreate() throws Exception { String password = "keystorepassword"; terminal.addTextInput("y"); + terminal.addTextInput("y"); terminal.addSecretInput(password); terminal.addSecretInput(password); terminal.addSecretInput("bar"); @@ -69,9 +70,18 @@ public void testMissingPromptCreate() throws Exception { assertSecureString("foo", "bar", password); } + public void testMissingPromptCreateWithoutPassword() throws Exception { + terminal.addTextInput("y"); + terminal.addTextInput("n"); + terminal.addSecretInput("bar"); + execute("foo"); + assertSecureString("foo", "bar", ""); + } + public void testMissingPromptCreateNotMatchingPasswords() throws Exception { String password = "keystorepassword"; terminal.addTextInput("y"); + terminal.addTextInput("y"); terminal.addSecretInput(password); terminal.addSecretInput("anotherpassword"); terminal.addSecretInput("bar"); @@ -80,15 +90,6 @@ public void testMissingPromptCreateNotMatchingPasswords() throws Exception { assertThat(e.getMessage(), containsString("Passwords are not equal, exiting")); } - public void testMissingForceCreate() throws Exception { - String password = "keystorepassword"; - terminal.addSecretInput(password); - terminal.addSecretInput(password); - terminal.addSecretInput("bar"); - execute("-f", "foo"); - assertSecureString("foo", "bar", password); - } - public void testMissingNoCreate() throws Exception { terminal.addTextInput("n"); // explicit no execute("foo"); diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java index 46fcfc830ca3d..ca0b5fa363351 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java @@ -90,6 +90,6 @@ public void testChangeKeyStorePasswordWrongExistingPassword() throws Exception { // We'll only be prompted once (for the old password) UserException e = expectThrows(UserException.class, this::execute); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Please make sure the password was correct")); + assertThat(e.getMessage(), containsString("Provided keystore password was incorrect")); } } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java index f68a731edf8f7..158bfbab8d87c 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java @@ -81,7 +81,7 @@ public void testFileSettingExhaustiveBytes() throws Exception { KeyStoreWrapper keystore = KeyStoreWrapper.create(); byte[] bytes = new byte[256]; for (int i = 0; i < 256; ++i) { - bytes[i] = (byte)i; + bytes[i] = (byte) i; } keystore.setFile("foo", bytes); keystore.save(env.configFile(), new char[0]); @@ -110,7 +110,7 @@ public void testDecryptKeyStoreWithWrongPassword() throws Exception { 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")); + assertThat(exception.getMessage(), containsString("Provided keystore password was incorrect")); } public void testCannotReadStringFromClosedKeystore() throws Exception { @@ -364,7 +364,7 @@ public void testBackcompatV2() throws Exception { byte[] base64Bytes = Base64.getEncoder().encode(fileBytes); char[] chars = new char[base64Bytes.length]; for (int i = 0; i < chars.length; ++i) { - chars[i] = (char)base64Bytes[i]; // PBE only stores the lower 8 bits, so this narrowing is ok + chars[i] = (char) base64Bytes[i]; // PBE only stores the lower 8 bits, so this narrowing is ok } secretKey = secretFactory.generateSecret(new PBEKeySpec(chars)); keystore.setEntry("file_setting", new KeyStore.SecretKeyEntry(secretKey), protectionParameter); diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java index b865f620b02ad..f79fd751465ec 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java @@ -76,7 +76,7 @@ public void testListWithIncorrectPassword() throws Exception { terminal.addSecretInput("thewrongkeystorepassword"); UserException e = expectThrows(UserException.class, this::execute); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Please make sure the password was correct")); + assertThat(e.getMessage(), containsString("Provided keystore password was incorrect")); } public void testListWithUnprotectedKeystore() throws Exception { diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java index 2f09bd700c188..b4cc08c846513 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java @@ -93,7 +93,7 @@ public void testRemoveWithIncorrectPassword() throws Exception { terminal.addSecretInput("thewrongpassword"); UserException e = expectThrows(UserException.class, () -> execute("foo")); assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Please make sure the password was correct")); + assertThat(e.getMessage(), containsString("Provided keystore password was incorrect")); } public void testRemoveFromUnprotectedKeystore() throws Exception { diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommandTests.java index 7f670ae77a27b..9744d73569c60 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommandTests.java @@ -69,7 +69,7 @@ public void testKeystoreUpgrade() throws Exception { public void testKeystoreDoesNotExist() { final UserException e = expectThrows(UserException.class, this::execute); - assertThat(e, hasToString(containsString("keystore does not exist at [" + KeyStoreWrapper.keystorePath(env.configFile()) + "]"))); + assertThat(e, hasToString(containsString("keystore not found at [" + KeyStoreWrapper.keystorePath(env.configFile()) + "]"))); } } 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 3372d836452ad..bf3a7d6c08557 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -31,6 +31,7 @@ import org.elasticsearch.cli.UserException; import org.elasticsearch.common.Randomness; +import javax.crypto.AEADBadTagException; import javax.crypto.Cipher; import javax.crypto.CipherInputStream; import javax.crypto.CipherOutputStream; @@ -69,11 +70,9 @@ import java.util.Set; import java.util.regex.Pattern; -import org.elasticsearch.cli.Terminal; - /** * A disk based container for sensitive settings in Elasticsearch. - *

+ * * Loading a keystore has 2 phases. First, call {@link #load(Path)}. Then call * {@link #decrypt(char[])} with the keystore password, or an empty char array if * {@link #hasPassword()} is {@code false}. Loading and decrypting should happen @@ -81,9 +80,7 @@ */ public class KeyStoreWrapper implements SecureSettings { - /** - * An identifier for the type of data that may be stored in a keystore entry. - */ + /** An identifier for the type of data that may be stored in a keystore entry. */ private enum EntryType { STRING, FILE @@ -96,64 +93,44 @@ private enum EntryType { public static final Setting SEED_SETTING = SecureSetting.secureString("keystore.seed", null); - /** - * Characters that may be used in the bootstrap seed setting added to all keystores. - */ + /** Characters that may be used in the bootstrap seed setting added to all keystores. */ private static final char[] SEED_CHARS = ("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789" + "~!@#$%^&*-_=+?").toCharArray(); - /** - * The name of the keystore file to read and write. - */ + /** The name of the keystore file to read and write. */ private static final String KEYSTORE_FILENAME = "elasticsearch.keystore"; - /** - * The version of the metadata written before the keystore data. - */ + /** The version of the metadata written before the keystore data. */ static final int FORMAT_VERSION = 4; - /** - * The oldest metadata format version that can be read. - */ + /** The oldest metadata format version that can be read. */ private static final int MIN_FORMAT_VERSION = 1; - /** - * The algorithm used to derive the cipher key from a password. - */ + /** The algorithm used to derive the cipher key from a password. */ private static final String KDF_ALGO = "PBKDF2WithHmacSHA512"; - /** - * The number of iterations to derive the cipher key. - */ + /** The number of iterations to derive the cipher key. */ private static final int KDF_ITERS = 10000; /** * The number of bits for the cipher key. - *

+ * * Note: The Oracle JDK 8 ships with a limited JCE policy that restricts key length for AES to 128 bits. * This can be increased to 256 bits once minimum java 9 is the minimum java version. * See http://www.oracle.com/technetwork/java/javase/terms/readme/jdk9-readme-3852447.html#jce - */ + * */ private static final int CIPHER_KEY_BITS = 128; - /** - * The number of bits for the GCM tag. - */ + /** The number of bits for the GCM tag. */ private static final int GCM_TAG_BITS = 128; - /** - * The cipher used to encrypt the keystore data. - */ + /** The cipher used to encrypt the keystore data. */ private static final String CIPHER_ALGO = "AES"; - /** - * The mode used with the cipher algorithm. - */ + /** The mode used with the cipher algorithm. */ private static final String CIPHER_MODE = "GCM"; - /** - * The padding used with the cipher algorithm. - */ + /** The padding used with the cipher algorithm. */ private static final String CIPHER_PADDING = "NoPadding"; // format version changelog: @@ -162,24 +139,16 @@ private enum EntryType { // 3: FIPS compliant algos, ES 6.3 // 4: remove distinction between string/files, ES 6.8/7.1 - /** - * The metadata format version used to read the current keystore wrapper. - */ + /** The metadata format version used to read the current keystore wrapper. */ private final int formatVersion; - /** - * True iff the keystore has a password needed to read. - */ + /** True iff the keystore has a password needed to read. */ private final boolean hasPassword; - /** - * The raw bytes of the encrypted keystore. - */ + /** The raw bytes of the encrypted keystore. */ private final byte[] dataBytes; - /** - * The decrypted secret data. See {@link #decrypt(char[])}. - */ + /** The decrypted secret data. See {@link #decrypt(char[])}. */ private final SetOnce> entries = new SetOnce<>(); private volatile boolean closed; @@ -196,16 +165,12 @@ public int getFormatVersion() { return formatVersion; } - /** - * Returns a path representing the ES keystore in the given config dir. - */ + /** Returns a path representing the ES keystore in the given config dir. */ public static Path keystorePath(Path configDir) { return configDir.resolve(KEYSTORE_FILENAME); } - /** - * Constructs a new keystore with the given password. - */ + /** Constructs a new keystore with the given password. */ public static KeyStoreWrapper create() { KeyStoreWrapper wrapper = new KeyStoreWrapper(FORMAT_VERSION, false, null); wrapper.entries.set(new HashMap<>()); @@ -213,9 +178,7 @@ public static KeyStoreWrapper create() { return wrapper; } - /** - * Add the bootstrap seed setting, which may be used as a unique, secure, random value by the node - */ + /** Add the bootstrap seed setting, which may be used as a unique, secure, random value by the node */ public static void addBootstrapSeed(KeyStoreWrapper wrapper) { assert wrapper.getSettingNames().contains(SEED_SETTING.getKey()) == false; SecureRandom random = Randomness.createSecure(); @@ -225,12 +188,12 @@ public static void addBootstrapSeed(KeyStoreWrapper wrapper) { characters[i] = SEED_CHARS[random.nextInt(SEED_CHARS.length)]; } wrapper.setString(SEED_SETTING.getKey(), characters); - Arrays.fill(characters, (char) 0); + Arrays.fill(characters, (char)0); } /** * Loads information about the Elasticsearch keystore from the provided config directory. - *

+ * * {@link #decrypt(char[])} must be called before reading or writing any entries. * Returns {@code null} if no keystore exists. */ @@ -300,9 +263,7 @@ public static KeyStoreWrapper load(Path configDir) throws IOException { } } - /** - * Upgrades the format of the keystore, if necessary. - */ + /** Upgrades the format of the keystore, if necessary. */ public static void upgrade(KeyStoreWrapper wrapper, Path configDir, char[] password) throws Exception { if (wrapper.getFormatVersion() == FORMAT_VERSION && wrapper.getSettingNames().contains(SEED_SETTING.getKey())) { return; @@ -319,9 +280,7 @@ public boolean isLoaded() { return entries.get() != null; } - /** - * Return true iff calling {@link #decrypt(char[])} requires a non-empty password. - */ + /** Return true iff calling {@link #decrypt(char[])} requires a non-empty password. */ public boolean hasPassword() { return hasPassword; } @@ -341,7 +300,7 @@ private Cipher createCipher(int opmode, char[] password, byte[] salt, byte[] iv) /** * Decrypts the underlying keystore data. - *

+ * * This may only be called once. */ public void decrypt(char[] password) throws GeneralSecurityException, IOException { @@ -398,13 +357,14 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio throw new SecurityException("Keystore has been corrupted or tampered with"); } } catch (IOException e) { + if (e.getCause() instanceof AEADBadTagException) { + throw new SecurityException("Provided keystore password was incorrect", e); + } throw new SecurityException("Keystore has been corrupted or tampered with", e); } } - /** - * Encrypt the keystore entries and return the encrypted data. - */ + /** Encrypt the keystore entries and return the encrypted data. */ private byte[] encrypt(char[] password, byte[] salt, byte[] iv) throws GeneralSecurityException, IOException { assert isLoaded(); @@ -479,16 +439,16 @@ private void decryptLegacyEntries() throws GeneralSecurityException, IOException if (settingType == EntryType.STRING) { ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(CharBuffer.wrap(chars)); bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); - Arrays.fill(byteBuffer.array(), (byte) 0); + Arrays.fill(byteBuffer.array(), (byte)0); } else { assert settingType == EntryType.FILE; // The PBE keyspec gives us chars, we convert to bytes byte[] tmpBytes = new byte[chars.length]; for (int i = 0; i < tmpBytes.length; ++i) { - tmpBytes[i] = (byte) chars[i]; // PBE only stores the lower 8 bits, so this narrowing is ok + tmpBytes[i] = (byte)chars[i]; // PBE only stores the lower 8 bits, so this narrowing is ok } bytes = Base64.getDecoder().decode(tmpBytes); - Arrays.fill(tmpBytes, (byte) 0); + Arrays.fill(tmpBytes, (byte)0); } Arrays.fill(chars, '\0'); @@ -496,9 +456,7 @@ private void decryptLegacyEntries() throws GeneralSecurityException, IOException } } - /** - * Write the keystore to the given config directory. - */ + /** Write the keystore to the given config directory. */ public synchronized void save(Path configDir, char[] password) throws Exception { ensureOpen(); @@ -507,7 +465,7 @@ public synchronized void save(Path configDir, char[] password) throws Exception String tmpFile = KEYSTORE_FILENAME + ".tmp"; try (IndexOutput output = directory.createOutput(tmpFile, IOContext.DEFAULT)) { CodecUtil.writeHeader(output, KEYSTORE_FILENAME, FORMAT_VERSION); - output.writeByte(password.length == 0 ? (byte) 0 : (byte) 1); + output.writeByte(password.length == 0 ? (byte)0 : (byte)1); // new cipher params SecureRandom random = Randomness.createSecure(); @@ -551,62 +509,6 @@ public synchronized void save(Path configDir, char[] password) throws Exception } } - /** - * Reads the keystore password from the {@link Terminal}, prompting for verification where applicable and returns it as a - * {@code char[]}. The caller is responsible to clear the char array after use. - * - * @param terminal the terminal to use for user inputs - * @param withVerification whether the user should be prompted for password verification - * @return a char array with the password the user entered - * @throws UserException If the user is prompted for verification and enters a different password - */ - static char[] readPassword(Terminal terminal, boolean withVerification) throws UserException { - final char[] password; - if (withVerification) { - password = terminal.readSecret("Enter new password for the elasticsearch keystore (empty for no password): "); - char[] passwordVerification = terminal.readSecret("Enter same password again: "); - if (Arrays.equals(password, passwordVerification) == false) { - throw new UserException(ExitCodes.DATA_ERROR, "Passwords are not equal, exiting."); - } - Arrays.fill(passwordVerification, '\u0000'); - } else { - password = terminal.readSecret("Enter password for the elasticsearch keystore : "); - } - return password; - } - - /** - * Reads the keystore from disk or attempts to create it if it doesn't already exist. If the keystore is password protected - * it also reads the password from user input and decrypts the keystore. Returns both in a POJO. The caller is responsible to clear - * the char array with the password after use, calling {@link KeystoreAndPassword#close()} - * - * @param terminal the terminal to use for user inputs - * @param configFile the Path from where to attempt and read the existing keystore - * @param forceCreate if set, the keystore is created without prompting the user - * @return a POJO with the {@link KeyStoreWrapper} and its password, null if the user elected to not create a keystore - */ - static KeystoreAndPassword readOrCreate(Terminal terminal, Path configFile, boolean forceCreate) throws Exception { - KeyStoreWrapper keystore = load(configFile); - final char[] password; - if (keystore == null) { - if (forceCreate == false && - terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { - terminal.println("Exiting without creating keystore."); - return null; - } else { - password = readPassword(terminal, true); - keystore = KeyStoreWrapper.create(); - keystore.save(configFile, password); - terminal.println("Created elasticsearch keystore in " + configFile); - return new KeystoreAndPassword(keystore, password); - } - } else { - password = keystore.hasPassword ? readPassword(terminal, false) : new char[0]; - keystore.decrypt(password); - return new KeystoreAndPassword(keystore, password); - } - } - /** * It is possible to retrieve the setting names even if the keystore is closed. * This allows {@link SecureSetting} to correctly determine that a entry exists even though it cannot be read. Thus attempting to @@ -659,7 +561,7 @@ synchronized void setString(String setting, char[] value) { byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); byte[] oldEntry = entries.get().put(setting, bytes); if (oldEntry != null) { - Arrays.fill(oldEntry, (byte) 0); + Arrays.fill(oldEntry, (byte)0); } } @@ -672,7 +574,7 @@ synchronized void setFile(String setting, byte[] bytes) { byte[] oldEntry = entries.get().put(setting, Arrays.copyOf(bytes, bytes.length)); if (oldEntry != null) { - Arrays.fill(oldEntry, (byte) 0); + Arrays.fill(oldEntry, (byte)0); } } @@ -683,7 +585,7 @@ void remove(String setting) { ensureOpen(); byte[] oldEntry = entries.get().remove(setting); if (oldEntry != null) { - Arrays.fill(oldEntry, (byte) 0); + Arrays.fill(oldEntry, (byte)0); } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassword.java b/server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassword.java deleted file mode 100644 index b0c181fe908cb..0000000000000 --- a/server/src/main/java/org/elasticsearch/common/settings/KeystoreAndPassword.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.common.settings; - -import java.io.Closeable; -import java.io.IOException; -import java.util.Arrays; - -/** - * POJO containing the keystore and its password - */ -final class KeystoreAndPassword implements Closeable { - private final KeyStoreWrapper keystore; - private final char[] password; - - KeystoreAndPassword(KeyStoreWrapper keystore, char[] password) { - this.keystore = keystore; - this.password = password; - } - - public KeyStoreWrapper getKeystore() { - return keystore; - } - - public char[] getPassword() { - return password; - } - - @Override - public void close() throws IOException { - if (null != password) { - Arrays.fill(password, '\u0000'); - } - } -} From ca550745aa8c107389eed6e5392d0467807a04f4 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 9 Jul 2019 23:36:03 +0300 Subject: [PATCH 12/15] address feedback --- .../settings/AddFileKeyStoreCommand.java | 5 +-- .../settings/AddStringKeyStoreCommand.java | 5 +-- .../common/settings/BaseKeyStoreCommand.java | 36 +++++++++++-------- .../ChangeKeyStorePasswordCommand.java | 6 ++-- .../settings/CreateKeyStoreCommand.java | 2 +- .../common/settings/ListKeyStoreCommand.java | 4 +-- .../RemoveSettingKeyStoreCommand.java | 6 ++-- .../settings/UpgradeKeyStoreCommand.java | 5 ++- .../settings/AddFileKeyStoreCommandTests.java | 27 ++------------ .../AddStringKeyStoreCommandTests.java | 24 ------------- 10 files changed, 41 insertions(+), 79 deletions(-) diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java index 5619d0fbd438c..db65250e571d2 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java @@ -42,7 +42,7 @@ class AddFileKeyStoreCommand extends BaseKeyStoreCommand { private final OptionSpec arguments; AddFileKeyStoreCommand() { - super("Add a file setting to the keystore"); + super("Add a file setting to the keystore", false); this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"), "Overwrite existing setting without prompting"); // jopt simple has issue with multiple non options, so we just get one set of them here // and convert to File when necessary @@ -58,6 +58,7 @@ protected void executeCommand(Terminal terminal, OptionSet options, Environment throw new UserException(ExitCodes.USAGE, "Missing setting name"); } String setting = argumentValues.get(0); + final KeyStoreWrapper keyStore = getKeyStore(); if (keyStore.getSettingNames().contains(setting) && options.has(forceOption) == false) { if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { terminal.println("Exiting without modifying keystore."); @@ -77,7 +78,7 @@ protected void executeCommand(Terminal terminal, OptionSet options, Environment String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath"); } keyStore.setFile(setting, Files.readAllBytes(file)); - keyStore.save(env.configFile(), keyStorePassword.getChars()); + keyStore.save(env.configFile(), getKeyStorePassword().getChars()); } @SuppressForbidden(reason = "file arg for cli") diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java index 06204bb46e959..45284fd8ee2c2 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java @@ -42,7 +42,7 @@ class AddStringKeyStoreCommand extends BaseKeyStoreCommand { private final OptionSpec arguments; AddStringKeyStoreCommand() { - super("Add a string setting to the keystore"); + super("Add a string setting to the keystore", false); this.stdinOption = parser.acceptsAll(Arrays.asList("x", "stdin"), "Read setting value from stdin"); this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"), "Overwrite existing setting without prompting"); this.arguments = parser.nonOptions("setting name"); @@ -59,6 +59,7 @@ protected void executeCommand(Terminal terminal, OptionSet options, Environment if (setting == null) { throw new UserException(ExitCodes.USAGE, "The setting name can not be null"); } + final KeyStoreWrapper keyStore = getKeyStore(); if (keyStore.getSettingNames().contains(setting) && options.has(forceOption) == false) { if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { terminal.println("Exiting without modifying keystore."); @@ -79,7 +80,7 @@ protected void executeCommand(Terminal terminal, OptionSet options, Environment } catch (IllegalArgumentException e) { throw new UserException(ExitCodes.DATA_ERROR, e.getMessage()); } - keyStore.save(env.configFile(), keyStorePassword.getChars()); + keyStore.save(env.configFile(), getKeyStorePassword().getChars()); } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java index 2aead7dafaaa2..624e88717ad4b 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java @@ -31,16 +31,17 @@ public abstract class BaseKeyStoreCommand extends EnvironmentAwareCommand { - KeyStoreWrapper keyStore; - SecureString keyStorePassword; - boolean keyStoreMustExist = false; + private KeyStoreWrapper keyStore; + private SecureString keyStorePassword; + private final boolean keyStoreMustExist; - public BaseKeyStoreCommand(String description) { + public BaseKeyStoreCommand(String description, boolean keyStoreMustExist) { super(description); + this.keyStoreMustExist = keyStoreMustExist; } @Override - protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { + protected final void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { try { final Path configFile = env.configFile(); keyStore = KeyStoreWrapper.load(configFile); @@ -54,10 +55,7 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th return; } } - // allow the user to decide if an auto-created keystore should be password protected - keyStorePassword = - terminal.promptYesNo("Do you want to protect the keystore with a password?", false) == false ? - new SecureString(new char[0]) : readPassword(terminal, true); + keyStorePassword = new SecureString(new char[0]); keyStore = KeyStoreWrapper.create(); keyStore.save(configFile, keyStorePassword.getChars()); } else { @@ -68,14 +66,18 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } catch (SecurityException e) { throw new UserException(ExitCodes.DATA_ERROR, e.getMessage()); } finally { - clearPassword(); + if (keyStorePassword != null) { + keyStorePassword.close(); + } } } - private void clearPassword() { - if (keyStorePassword != null) { - keyStorePassword.close(); - } + protected KeyStoreWrapper getKeyStore() { + return keyStore; + } + + protected SecureString getKeyStorePassword() { + return keyStorePassword; } /** @@ -103,5 +105,11 @@ static SecureString readPassword(Terminal terminal, boolean withVerification) th return password; } + /** + * This is called after the keystore password has been read from the stdin and the keystore is decrypted and + * loaded. The keystore and keystore passwords are available to classes extending {@link BaseKeyStoreCommand} + * using {@link BaseKeyStoreCommand#getKeyStore()} and {@link BaseKeyStoreCommand#getKeyStorePassword()} + * respectively. + */ protected abstract void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception; } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java index f4755ce06ebf6..484ac3a783014 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java @@ -31,8 +31,7 @@ class ChangeKeyStorePasswordCommand extends BaseKeyStoreCommand { ChangeKeyStorePasswordCommand() { - super("Changes the password of a keystore"); - keyStoreMustExist = true; + super("Changes the password of a keystore", true); } @Override @@ -40,8 +39,9 @@ protected void executeCommand(Terminal terminal, OptionSet options, Environment SecureString newPassword = null; try { newPassword = readPassword(terminal, true); + final KeyStoreWrapper keyStore = getKeyStore(); keyStore.save(env.configFile(), newPassword.getChars()); - terminal.println("Elasticsearch keystore password changed successfully." + env.configFile()); + terminal.println("Elasticsearch keystore password changed successfully."); } catch (SecurityException e) { throw new UserException(ExitCodes.DATA_ERROR, e.getMessage()); } finally { diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index e64141f820276..873e12eb4c918 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -58,7 +58,7 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th password = options.has(passwordOption) ? BaseKeyStoreCommand.readPassword(terminal, true) : new SecureString(new char[0]); keystore.save(env.configFile(), password.getChars()); - terminal.println("Created elasticsearch keystore in " + env.configFile()); + terminal.println("Created elasticsearch keystore in " + KeyStoreWrapper.keystorePath(env.configFile())); } catch (SecurityException e) { throw new UserException(ExitCodes.IO_ERROR, "Error creating the elasticsearch keystore."); } finally { diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java index dcd072ab2d96e..edd8a68cc6f9f 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java @@ -34,12 +34,12 @@ class ListKeyStoreCommand extends BaseKeyStoreCommand { ListKeyStoreCommand() { - super("List entries in the keystore"); - keyStoreMustExist = true; + super("List entries in the keystore", true); } @Override protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception { + final KeyStoreWrapper keyStore = getKeyStore(); List sortedEntries = new ArrayList<>(keyStore.getSettingNames()); Collections.sort(sortedEntries); for (String entry : sortedEntries) { diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java index 3fdb9191752b0..6e839d4f331ba 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java @@ -36,8 +36,7 @@ class RemoveSettingKeyStoreCommand extends BaseKeyStoreCommand { private final OptionSpec arguments; RemoveSettingKeyStoreCommand() { - super("Remove a setting from the keystore"); - keyStoreMustExist = true; + super("Remove a setting from the keystore", true); arguments = parser.nonOptions("setting names"); } @@ -47,12 +46,13 @@ protected void executeCommand(Terminal terminal, OptionSet options, Environment if (settings.isEmpty()) { throw new UserException(ExitCodes.USAGE, "Must supply at least one setting to remove"); } + final KeyStoreWrapper keyStore = getKeyStore(); for (String setting : arguments.values(options)) { if (keyStore.getSettingNames().contains(setting) == false) { throw new UserException(ExitCodes.CONFIG, "Setting [" + setting + "] does not exist in the keystore."); } keyStore.remove(setting); } - keyStore.save(env.configFile(), keyStorePassword.getChars()); + keyStore.save(env.configFile(), getKeyStorePassword().getChars()); } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java index 78a7ee8413da6..640a76432d3b4 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java @@ -29,13 +29,12 @@ public class UpgradeKeyStoreCommand extends BaseKeyStoreCommand { UpgradeKeyStoreCommand() { - super("Upgrade the keystore format"); - keyStoreMustExist = true; + super("Upgrade the keystore format", true); } @Override protected void executeCommand(final Terminal terminal, final OptionSet options, final Environment env) throws Exception { - KeyStoreWrapper.upgrade(keyStore, env.configFile(), keyStorePassword.getChars()); + KeyStoreWrapper.upgrade(getKeyStore(), env.configFile(), getKeyStorePassword().getChars()); } } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java index c4ef4723bd050..7252ea0e99ca5 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java @@ -58,37 +58,14 @@ private void addFile(KeyStoreWrapper keystore, String setting, Path file, String keystore.save(env.configFile(), password.toCharArray()); } - public void testMissingPromptCreate() throws Exception { - String password = "keystorepassword"; + public void testMissingCreateWithEmptyPassword() throws Exception { + String password = ""; Path file1 = createRandomFile(); - terminal.addSecretInput(password); - terminal.addSecretInput(password); - terminal.addTextInput("y"); terminal.addTextInput("y"); execute("foo", file1.toString()); assertSecureFile("foo", file1, password); } - public void testMissingPromptCreateWithoutPassword() throws Exception { - Path file1 = createRandomFile(); - terminal.addTextInput("y"); - terminal.addTextInput("n"); - execute("foo", file1.toString()); - assertSecureFile("foo", file1, ""); - } - - public void testMissingPromptCreateNotMatchingPasswpord() throws Exception { - String password = "keystorepassword"; - Path file1 = createRandomFile(); - terminal.addTextInput("y"); - terminal.addTextInput("y"); - terminal.addSecretInput(password); - terminal.addSecretInput("adifferentpassphase"); - UserException e = expectThrows(UserException.class, () -> execute("foo", file1.toString())); - assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Passwords are not equal, exiting")); - } - public void testMissingNoCreate() throws Exception { terminal.addSecretInput(randomFrom("", "keystorepassword")); terminal.addTextInput("n"); // explicit no diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java index 699f1aee9e622..7978940412d10 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java @@ -59,37 +59,13 @@ public void testInvalidPassphrease() throws Exception { } - public void testMissingPromptCreate() throws Exception { - String password = "keystorepassword"; - terminal.addTextInput("y"); - terminal.addTextInput("y"); - terminal.addSecretInput(password); - terminal.addSecretInput(password); - terminal.addSecretInput("bar"); - execute("foo"); - assertSecureString("foo", "bar", password); - } - public void testMissingPromptCreateWithoutPassword() throws Exception { terminal.addTextInput("y"); - terminal.addTextInput("n"); terminal.addSecretInput("bar"); execute("foo"); assertSecureString("foo", "bar", ""); } - public void testMissingPromptCreateNotMatchingPasswords() throws Exception { - String password = "keystorepassword"; - terminal.addTextInput("y"); - terminal.addTextInput("y"); - terminal.addSecretInput(password); - terminal.addSecretInput("anotherpassword"); - terminal.addSecretInput("bar"); - UserException e = expectThrows(UserException.class, () -> execute("foo")); - assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); - assertThat(e.getMessage(), containsString("Passwords are not equal, exiting")); - } - public void testMissingNoCreate() throws Exception { terminal.addTextInput("n"); // explicit no execute("foo"); From 4085b178028355f5742ed79862f2ede4fbc38825 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 23 Jul 2019 10:53:28 +0300 Subject: [PATCH 13/15] Take advantage of SecureString being closeable --- .../common/settings/ChangeKeyStorePasswordCommand.java | 8 +------- .../common/settings/CreateKeyStoreCommand.java | 10 ++-------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java index 484ac3a783014..526201ede8f66 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java @@ -36,18 +36,12 @@ class ChangeKeyStorePasswordCommand extends BaseKeyStoreCommand { @Override protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception { - SecureString newPassword = null; - try { - newPassword = readPassword(terminal, true); + try (SecureString newPassword = readPassword(terminal, true)) { final KeyStoreWrapper keyStore = getKeyStore(); keyStore.save(env.configFile(), newPassword.getChars()); terminal.println("Elasticsearch keystore password changed successfully."); } catch (SecurityException e) { throw new UserException(ExitCodes.DATA_ERROR, e.getMessage()); - } finally { - if (null != newPassword) { - newPassword.close(); - } } } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index 873e12eb4c918..379af6f3a47f3 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -45,8 +45,8 @@ class CreateKeyStoreCommand extends EnvironmentAwareCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - SecureString password = null; - try { + try (SecureString password = options.has(passwordOption) ? + BaseKeyStoreCommand.readPassword(terminal, true) : new SecureString(new char[0])) { Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); if (Files.exists(keystoreFile)) { if (terminal.promptYesNo("An elasticsearch keystore already exists. Overwrite?", false) == false) { @@ -55,16 +55,10 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } } KeyStoreWrapper keystore = KeyStoreWrapper.create(); - password = options.has(passwordOption) ? - BaseKeyStoreCommand.readPassword(terminal, true) : new SecureString(new char[0]); keystore.save(env.configFile(), password.getChars()); terminal.println("Created elasticsearch keystore in " + KeyStoreWrapper.keystorePath(env.configFile())); } catch (SecurityException e) { throw new UserException(ExitCodes.IO_ERROR, "Error creating the elasticsearch keystore."); - } finally { - if (null != password) { - password.close(); - } } } } From e44bf27f7a06fc57b38b00646a7142b8f68a043e Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 23 Jul 2019 17:16:46 +0300 Subject: [PATCH 14/15] Explicitly create the elasticsearch.keystore The docker entry script was depending on the -f flag of the add subcommand to create the keystore. This was an undocumented behavior of the -f flag that should originally only "force" the overwrite of the entry in the keystore and not the creation of the keystore itself. This behavior was changed as part of earlier commit in this PR --- distribution/docker/docker-test-entrypoint.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/distribution/docker/docker-test-entrypoint.sh b/distribution/docker/docker-test-entrypoint.sh index a1e5dd0ffda2f..eb0c852ed8c4c 100755 --- a/distribution/docker/docker-test-entrypoint.sh +++ b/distribution/docker/docker-test-entrypoint.sh @@ -1,6 +1,7 @@ #!/bin/bash cd /usr/share/elasticsearch/bin/ -./elasticsearch-users useradd x_pack_rest_user -p x-pack-test-password -r superuser || true +./elasticsearch-users useradd x_pack_rest_user -p x-pack-test-password -r superuser || true +./elasticserach-keystore create echo "testnode" > /tmp/password cat /tmp/password | ./elasticsearch-keystore add -x -f -v 'xpack.security.transport.ssl.keystore.secure_password' cat /tmp/password | ./elasticsearch-keystore add -x -f -v 'xpack.security.http.ssl.keystore.secure_password' From f7ba9ac0ff208223a6d3c1eb2c0ea47c354286b3 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 23 Jul 2019 17:51:32 +0300 Subject: [PATCH 15/15] typo --- distribution/docker/docker-test-entrypoint.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/docker/docker-test-entrypoint.sh b/distribution/docker/docker-test-entrypoint.sh index eb0c852ed8c4c..68160cffd1cee 100755 --- a/distribution/docker/docker-test-entrypoint.sh +++ b/distribution/docker/docker-test-entrypoint.sh @@ -1,7 +1,7 @@ #!/bin/bash cd /usr/share/elasticsearch/bin/ ./elasticsearch-users useradd x_pack_rest_user -p x-pack-test-password -r superuser || true -./elasticserach-keystore create +./elasticsearch-keystore create echo "testnode" > /tmp/password cat /tmp/password | ./elasticsearch-keystore add -x -f -v 'xpack.security.transport.ssl.keystore.secure_password' cat /tmp/password | ./elasticsearch-keystore add -x -f -v 'xpack.security.http.ssl.keystore.secure_password'