diff --git a/Vagrantfile b/Vagrantfile index c86b0a910c239..653d8850c585d 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -475,6 +475,7 @@ JAVA ensure curl ensure unzip ensure rsync + ensure expect installed bats || { # Bats lives in a git repository.... diff --git a/build.gradle b/build.gradle index 21e3ba48af740..481094fb1a8a4 100644 --- a/build.gradle +++ b/build.gradle @@ -215,8 +215,8 @@ task verifyVersions { * after the backport of the backcompat code is complete. */ -boolean bwc_tests_enabled = true -final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */ +boolean bwc_tests_enabled = false +final String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/43197" if (bwc_tests_enabled == false) { if (bwc_tests_disabled_issue.isEmpty()) { throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false") diff --git a/distribution/docker/docker-test-entrypoint.sh b/distribution/docker/docker-test-entrypoint.sh index a1e5dd0ffda2f..1dca4b6a35e73 100755 --- a/distribution/docker/docker-test-entrypoint.sh +++ b/distribution/docker/docker-test-entrypoint.sh @@ -1,6 +1,6 @@ #!/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 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' diff --git a/distribution/docker/src/docker/bin/docker-entrypoint.sh b/distribution/docker/src/docker/bin/docker-entrypoint.sh index b46dd72de814c..76f23e1589512 100644 --- a/distribution/docker/src/docker/bin/docker-entrypoint.sh +++ b/distribution/docker/src/docker/bin/docker-entrypoint.sh @@ -41,12 +41,22 @@ if [[ -f bin/elasticsearch-users ]]; then # honor the variable if it's present. if [[ -n "$ELASTIC_PASSWORD" ]]; then [[ -f /usr/share/elasticsearch/config/elasticsearch.keystore ]] || (elasticsearch-keystore create) - if ! (elasticsearch-keystore list | grep -q '^bootstrap.password$'); then - (echo "$ELASTIC_PASSWORD" | elasticsearch-keystore add -x 'bootstrap.password') + if ! (elasticsearch-keystore has-passwd --silent) ; then + # keystore is unencrypted + if ! (elasticsearch-keystore list | grep -q '^bootstrap.password$'); then + (echo "$ELASTIC_PASSWORD" | elasticsearch-keystore add -x 'bootstrap.password') + fi + else + # keystore requires password + if ! (echo "$KEYSTORE_PASSWORD" \ + | elasticsearch-keystore list | grep -q '^bootstrap.password$') ; then + COMMANDS="$(printf "%s\n%s" "$KEYSTORE_PASSWORD" "$ELASTIC_PASSWORD")" + (echo "$COMMANDS" | elasticsearch-keystore add -x 'bootstrap.password') + fi fi fi fi # Signal forwarding and child reaping is handled by `tini`, which is the # actual entrypoint of the container -exec /usr/share/elasticsearch/bin/elasticsearch +exec /usr/share/elasticsearch/bin/elasticsearch <<<"$KEYSTORE_PASSWORD" diff --git a/distribution/packages/build.gradle b/distribution/packages/build.gradle index b7c1a75f5a5ae..f4ad06d19b65f 100644 --- a/distribution/packages/build.gradle +++ b/distribution/packages/build.gradle @@ -231,6 +231,10 @@ Closure commonPackageConfig(String type, boolean oss, boolean jdk) { from "${packagingFiles}/systemd/sysctl/elasticsearch.conf" fileMode 0644 } + into('/usr/share/elasticsearch/bin') { + from "${packagingFiles}/systemd/systemd-entrypoint" + fileMode 0755 + } // ========= sysV init ========= configurationFile '/etc/init.d/elasticsearch' diff --git a/distribution/packages/src/common/scripts/postinst b/distribution/packages/src/common/scripts/postinst index 0c86904ba40e2..d76b9ec763524 100644 --- a/distribution/packages/src/common/scripts/postinst +++ b/distribution/packages/src/common/scripts/postinst @@ -108,7 +108,12 @@ if [ "$PACKAGE" = "deb" ]; then chmod 660 "${ES_PATH_CONF}"/elasticsearch.keystore md5sum "${ES_PATH_CONF}"/elasticsearch.keystore > "${ES_PATH_CONF}"/.elasticsearch.keystore.initial_md5sum else - /usr/share/elasticsearch/bin/elasticsearch-keystore upgrade + if /usr/share/elasticsearch/bin/elasticsearch-keystore has-passwd --silent ; then + echo "### Warning: unable to upgrade encrypted keystore" 1>&2 + echo " Please run elasticsearch-keystore upgrade and enter password" 1>&2 + else + /usr/share/elasticsearch/bin/elasticsearch-keystore upgrade + fi fi fi diff --git a/distribution/packages/src/common/scripts/posttrans b/distribution/packages/src/common/scripts/posttrans index ab989cf5676fd..7b072ee260209 100644 --- a/distribution/packages/src/common/scripts/posttrans +++ b/distribution/packages/src/common/scripts/posttrans @@ -11,7 +11,12 @@ if [ ! -f "${ES_PATH_CONF}"/elasticsearch.keystore ]; then chmod 660 "${ES_PATH_CONF}"/elasticsearch.keystore md5sum "${ES_PATH_CONF}"/elasticsearch.keystore > "${ES_PATH_CONF}"/.elasticsearch.keystore.initial_md5sum else - /usr/share/elasticsearch/bin/elasticsearch-keystore upgrade + if /usr/share/elasticsearch/bin/elasticsearch-keystore has-passwd --silent ; then + echo "### Warning: unable to upgrade encrypted keystore" 1>&2 + echo " Please run elasticsearch-keystore upgrade and enter password" 1>&2 + else + /usr/share/elasticsearch/bin/elasticsearch-keystore upgrade + fi fi ${scripts.footer} diff --git a/distribution/packages/src/common/systemd/elasticsearch.service b/distribution/packages/src/common/systemd/elasticsearch.service index ed32b0708adff..acdc77ca99408 100644 --- a/distribution/packages/src/common/systemd/elasticsearch.service +++ b/distribution/packages/src/common/systemd/elasticsearch.service @@ -19,7 +19,7 @@ WorkingDirectory=/usr/share/elasticsearch User=elasticsearch Group=elasticsearch -ExecStart=/usr/share/elasticsearch/bin/elasticsearch -p ${PID_DIR}/elasticsearch.pid --quiet +ExecStart=/usr/share/elasticsearch/bin/systemd-entrypoint -p ${PID_DIR}/elasticsearch.pid --quiet # StandardOutput is configured to redirect to journalctl since # some error messages may be logged in standard output before diff --git a/distribution/packages/src/common/systemd/systemd-entrypoint b/distribution/packages/src/common/systemd/systemd-entrypoint new file mode 100644 index 0000000000000..e3c3f1eab00a1 --- /dev/null +++ b/distribution/packages/src/common/systemd/systemd-entrypoint @@ -0,0 +1,10 @@ +#!/bin/sh + +# This wrapper script allows SystemD to feed a file containing a passphrase into +# the main Elasticsearch startup script + +if [ -n "$ES_KEYSTORE_PASSPHRASE_FILE" ] ; then + exec /usr/share/elasticsearch/bin/elasticsearch "$@" < "$ES_KEYSTORE_PASSPHRASE_FILE" +else + exec /usr/share/elasticsearch/bin/elasticsearch "$@" +fi diff --git a/distribution/src/bin/elasticsearch b/distribution/src/bin/elasticsearch index 53329cc6bad41..8d460a7a7bbfc 100755 --- a/distribution/src/bin/elasticsearch +++ b/distribution/src/bin/elasticsearch @@ -20,6 +20,19 @@ if [ -z "$ES_TMPDIR" ]; then ES_TMPDIR=`"$JAVA" -cp "$ES_CLASSPATH" org.elasticsearch.tools.launchers.TempDirectory` fi +# get keystore password before setting java options to avoid +# conflicting GC configurations for the keystore tools +unset KEYSTORE_PASSWORD +KEYSTORE_PASSWORD= +if ! echo $* | grep -E -q '(^-h |-h$| -h |--help$|--help |^-V |-V$| -V |--version$|--version )' \ + && "`dirname "$0"`"/elasticsearch-keystore has-passwd --silent +then + if ! read -s -r -p "Elasticsearch keystore password: " KEYSTORE_PASSWORD ; then + echo "Failed to read keystore password on console" 1>&2 + exit 1 + fi +fi + ES_JVM_OPTIONS="$ES_PATH_CONF"/jvm.options ES_JAVA_OPTS=`export ES_TMPDIR; "$JAVA" -cp "$ES_CLASSPATH" org.elasticsearch.tools.launchers.JvmOptionsParser "$ES_JVM_OPTIONS"` @@ -35,7 +48,7 @@ if ! echo $* | grep -E '(^-d |-d$| -d |--daemonize$|--daemonize )' > /dev/null; -Des.bundled_jdk="$ES_BUNDLED_JDK" \ -cp "$ES_CLASSPATH" \ org.elasticsearch.bootstrap.Elasticsearch \ - "$@" + "$@" <<<"$KEYSTORE_PASSWORD" else exec \ "$JAVA" \ @@ -48,7 +61,7 @@ else -cp "$ES_CLASSPATH" \ org.elasticsearch.bootstrap.Elasticsearch \ "$@" \ - <&- & + <<<"$KEYSTORE_PASSWORD" & retval=$? pid=$! [ $retval -eq 0 ] || exit $retval diff --git a/distribution/src/bin/elasticsearch-cli.bat b/distribution/src/bin/elasticsearch-cli.bat index 80b488c66e98c..866e8efc6689b 100644 --- a/distribution/src/bin/elasticsearch-cli.bat +++ b/distribution/src/bin/elasticsearch-cli.bat @@ -25,5 +25,5 @@ set ES_JAVA_OPTS=-Xms4m -Xmx64m -XX:+UseSerialGC %ES_JAVA_OPTS% -cp "%ES_CLASSPATH%" ^ "%ES_MAIN_CLASS%" ^ %* - + exit /b %ERRORLEVEL% diff --git a/distribution/src/bin/elasticsearch.bat b/distribution/src/bin/elasticsearch.bat index 9460554f81f41..48a34fdd332db 100644 --- a/distribution/src/bin/elasticsearch.bat +++ b/distribution/src/bin/elasticsearch.bat @@ -4,6 +4,7 @@ setlocal enabledelayedexpansion setlocal enableextensions SET params='%*' +SET checkpassword=Y :loop FOR /F "usebackq tokens=1* delims= " %%A IN (!params!) DO ( @@ -18,6 +19,20 @@ FOR /F "usebackq tokens=1* delims= " %%A IN (!params!) DO ( SET silent=Y ) + IF "!current!" == "-h" ( + SET checkpassword=N + ) + IF "!current!" == "--help" ( + SET checkpassword=N + ) + + IF "!current!" == "-V" ( + SET checkpassword=N + ) + IF "!current!" == "--version" ( + SET checkpassword=N + ) + IF "!silent!" == "Y" ( SET nopauseonerror=Y ) ELSE ( @@ -41,6 +56,18 @@ IF ERRORLEVEL 1 ( EXIT /B %ERRORLEVEL% ) +SET KEYSTORE_PASSWORD= +IF "%checkpassword%"=="Y" ( + CALL "%~dp0elasticsearch-keystore.bat" has-passwd --silent + IF !ERRORLEVEL! EQU 0 ( + SET /P KEYSTORE_PASSWORD=Elasticsearch keystore password: + IF !ERRORLEVEL! NEQ 0 ( + ECHO Failed to read keystore password on standard input + EXIT /B !ERRORLEVEL! + ) + ) +) + if not defined ES_TMPDIR ( for /f "tokens=* usebackq" %%a in (`CALL %JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.TempDirectory"`) do set ES_TMPDIR=%%a ) @@ -54,7 +81,20 @@ if "%MAYBE_JVM_OPTIONS_PARSER_FAILED%" == "jvm_options_parser_failed" ( exit /b 1 ) -%JAVA% %ES_JAVA_OPTS% -Delasticsearch -Des.path.home="%ES_HOME%" -Des.path.conf="%ES_PATH_CONF%" -Des.distribution.flavor="%ES_DISTRIBUTION_FLAVOR%" -Des.distribution.type="%ES_DISTRIBUTION_TYPE%" -Des.bundled_jdk="%ES_BUNDLED_JDK%" -cp "%ES_CLASSPATH%" "org.elasticsearch.bootstrap.Elasticsearch" !newparams! +rem windows batch pipe will choke on special characters in strings +SET KEYSTORE_PASSWORD=!KEYSTORE_PASSWORD:^^=^^^^! +SET KEYSTORE_PASSWORD=!KEYSTORE_PASSWORD:^&=^^^&! +SET KEYSTORE_PASSWORD=!KEYSTORE_PASSWORD:^|=^^^|! +SET KEYSTORE_PASSWORD=!KEYSTORE_PASSWORD:^<=^^^=^^^>! +SET KEYSTORE_PASSWORD=!KEYSTORE_PASSWORD:^\=^^^\! + +ECHO.!KEYSTORE_PASSWORD!| %JAVA% %ES_JAVA_OPTS% -Delasticsearch ^ + -Des.path.home="%ES_HOME%" -Des.path.conf="%ES_PATH_CONF%" ^ + -Des.distribution.flavor="%ES_DISTRIBUTION_FLAVOR%" ^ + -Des.distribution.type="%ES_DISTRIBUTION_TYPE%" ^ + -Des.bundled_jdk="%ES_BUNDLED_JDK%" ^ + -cp "%ES_CLASSPATH%" "org.elasticsearch.bootstrap.Elasticsearch" !newparams! endlocal endlocal 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 51e0cda8dd096..a36c6fb262550 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,14 +36,16 @@ /** * 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; AddFileKeyStoreCommand() { - super("Add a file setting to the keystore"); - this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"), "Overwrite existing setting without prompting"); + super("Add a file setting to the keystore", false); + this.forceOption = parser.acceptsAll( + Arrays.asList("f", "force"), + "Overwrite existing setting without prompting, creating keystore if necessary" + ); // jopt simple has issue with multiple non options, so we just get one set of them here // and convert to File when necessary // see https://github.com/jopt-simple/jopt-simple/issues/103 @@ -52,27 +53,14 @@ 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; - } - 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 */); - } - + 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) { + 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."); return; @@ -92,8 +80,8 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th "Unrecognized extra arguments [" + String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath" ); } - keystore.setFile(setting, Files.readAllBytes(file)); - keystore.save(env.configFile(), new char[0]); + keyStore.setFile(setting, Files.readAllBytes(file)); + 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 915471188e036..421ea6eaee066 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 @@ -28,7 +28,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,16 +36,18 @@ /** * 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; 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.forceOption = parser.acceptsAll( + Arrays.asList("f", "force"), + "Overwrite existing setting without prompting, creating keystore if necessary" + ); this.arguments = parser.nonOptions("setting name"); } @@ -56,26 +57,13 @@ 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; - } - 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 */); - } - + 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) { + 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."); return; @@ -102,10 +90,11 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th } try { - keystore.setString(setting, value); - } catch (final IllegalArgumentException e) { + keyStore.setString(setting, value); + } catch (IllegalArgumentException e) { throw new UserException(ExitCodes.DATA_ERROR, e.getMessage()); } - keystore.save(env.configFile(), new char[0]); + 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 new file mode 100644 index 0000000000000..0ba7c30c1c503 --- /dev/null +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.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 joptsimple.OptionSet; +import joptsimple.OptionSpec; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.KeyStoreAwareCommand; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +import java.nio.file.Path; + +public abstract class BaseKeyStoreCommand extends KeyStoreAwareCommand { + + private KeyStoreWrapper keyStore; + private SecureString keyStorePassword; + private final boolean keyStoreMustExist; + OptionSpec forceOption; + + public BaseKeyStoreCommand(String description, boolean keyStoreMustExist) { + super(description); + this.keyStoreMustExist = keyStoreMustExist; + } + + @Override + protected final 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 (options.has(forceOption) == false) { + if (terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { + terminal.println("Exiting without creating keystore."); + return; + } + } + keyStorePassword = new SecureString(new char[0]); + 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 { + if (keyStorePassword != null) { + keyStorePassword.close(); + } + } + } + + protected KeyStoreWrapper getKeyStore() { + return keyStore; + } + + protected SecureString getKeyStorePassword() { + return keyStorePassword; + } + + /** + * 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 new file mode 100644 index 0000000000000..526201ede8f66 --- /dev/null +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java @@ -0,0 +1,47 @@ +/* + * 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.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +/** + * A sub-command for the keystore cli which changes the password. + */ +class ChangeKeyStorePasswordCommand extends BaseKeyStoreCommand { + + ChangeKeyStorePasswordCommand() { + super("Changes the password of a keystore", true); + } + + @Override + protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception { + 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()); + } + } +} 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 eeac2c79cff06..8cc3bfcf9ba15 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 @@ -21,40 +21,43 @@ import java.nio.file.Files; import java.nio.file.Path; +import java.util.Arrays; import joptsimple.OptionSet; -import org.elasticsearch.cli.EnvironmentAwareCommand; +import joptsimple.OptionSpec; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.KeyStoreAwareCommand; import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; 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 { +class CreateKeyStoreCommand extends KeyStoreAwareCommand { + + private final OptionSpec passwordOption; CreateKeyStoreCommand() { super("Creates a new elasticsearch 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 { - 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; + try (SecureString password = options.has(passwordOption) ? 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) { + terminal.println("Exiting without creating keystore."); + return; + } } + KeyStoreWrapper keystore = KeyStoreWrapper.create(); + 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."); } - - 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/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/HasPasswordKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/HasPasswordKeyStoreCommand.java new file mode 100644 index 0000000000000..f15b6ba933443 --- /dev/null +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/HasPasswordKeyStoreCommand.java @@ -0,0 +1,58 @@ +/* + * 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.KeyStoreAwareCommand; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +import java.nio.file.Path; + +public class HasPasswordKeyStoreCommand extends KeyStoreAwareCommand { + + static final int NO_PASSWORD_EXIT_CODE = 1; + + HasPasswordKeyStoreCommand() { + super( + "Succeeds if the keystore exists and is password-protected, " + "fails with exit code " + NO_PASSWORD_EXIT_CODE + " otherwise." + ); + } + + @Override + protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { + final Path configFile = env.configFile(); + final KeyStoreWrapper keyStore = KeyStoreWrapper.load(configFile); + + // We handle error printing here so we can respect the "--silent" flag + // We have to throw an exception to get a nonzero exit code + if (keyStore == null) { + terminal.errorPrintln(Terminal.Verbosity.NORMAL, "ERROR: Elasticsearch keystore not found"); + throw new UserException(NO_PASSWORD_EXIT_CODE, null); + } + if (keyStore.hasPassword() == false) { + terminal.errorPrintln(Terminal.Verbosity.NORMAL, "ERROR: Keystore is not password-protected"); + throw new UserException(NO_PASSWORD_EXIT_CODE, null); + } + + terminal.println(Terminal.Verbosity.NORMAL, "Keystore is password-protected"); + } +} 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 19a453f7e90fd..f08c83432de3f 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,6 +35,8 @@ private KeyStoreCli() { subcommands.put("add-file", new AddFileKeyStoreCommand()); subcommands.put("remove", new RemoveSettingKeyStoreCommand()); subcommands.put("upgrade", new UpgradeKeyStoreCommand()); + subcommands.put("passwd", new ChangeKeyStorePasswordCommand()); + subcommands.put("has-passwd", new HasPasswordKeyStoreCommand()); } 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 5fd34472a6718..e1ae480ef33b0 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 @@ -24,31 +24,22 @@ 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"); + super("List entries in the keystore", 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."); - } - - keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); - - List sortedEntries = new ArrayList<>(keystore.getSettingNames()); + 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) { 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 9a83375e6e01a..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 @@ -23,7 +23,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; @@ -32,35 +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"); + super("Remove a setting from the keystore", 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."); - } - - keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); - + final KeyStoreWrapper keyStore = getKeyStore(); for (String setting : arguments.values(options)) { - if (keystore.getSettingNames().contains(setting) == false) { + if (keyStore.getSettingNames().contains(setting) == false) { throw new UserException(ExitCodes.CONFIG, "Setting [" + setting + "] does not exist in the keystore."); } - keystore.remove(setting); + keyStore.remove(setting); } - keystore.save(env.configFile(), new char[0]); + 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 c92d5dc4e3b1e..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 @@ -20,32 +20,21 @@ 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; /** * 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"); + super("Upgrade the keystore format", 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()) + "]" - ); - } - wrapper.decrypt(new char[0]); - KeyStoreWrapper.upgrade(wrapper, env.configFile(), new char[0]); + protected void executeCommand(final Terminal terminal, final OptionSet options, final Environment env) throws Exception { + KeyStoreWrapper.upgrade(getKeyStore(), env.configFile(), getKeyStorePassword().getChars()); } } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java index 6b336fdf2b78c..e4634ad873556 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java @@ -29,17 +29,24 @@ import org.junit.After; import org.junit.Before; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; +import static org.hamcrest.Matchers.equalTo; + public class BootstrapTests extends ESTestCase { Environment env; List fileSystems = new ArrayList<>(); + private static final int MAX_PASSPHRASE_LENGTH = 10; + @After public void closeMockFileSystems() throws IOException { IOUtils.close(fileSystems); @@ -66,4 +73,49 @@ public void testLoadSecureSettings() throws Exception { assertTrue(Files.exists(configPath.resolve("elasticsearch.keystore"))); } } + + public void testReadCharsFromStdin() throws Exception { + assertPassphraseRead("hello", "hello"); + assertPassphraseRead("hello\n", "hello"); + assertPassphraseRead("hello\r\n", "hello"); + + assertPassphraseRead("hellohello", "hellohello"); + assertPassphraseRead("hellohello\n", "hellohello"); + assertPassphraseRead("hellohello\r\n", "hellohello"); + + assertPassphraseRead("hello\nhi\n", "hello"); + assertPassphraseRead("hello\r\nhi\r\n", "hello"); + } + + public void testPassphraseTooLong() throws Exception { + byte[] source = "hellohello!\n".getBytes(StandardCharsets.UTF_8); + try (InputStream stream = new ByteArrayInputStream(source)) { + expectThrows( + RuntimeException.class, + "Password exceeded maximum length of 10", + () -> Bootstrap.readPassphrase(stream, MAX_PASSPHRASE_LENGTH) + ); + } + } + + public void testNoPassPhraseProvided() throws Exception { + byte[] source = "\r\n".getBytes(StandardCharsets.UTF_8); + try (InputStream stream = new ByteArrayInputStream(source)) { + expectThrows( + RuntimeException.class, + "Keystore passphrase required but none provided.", + () -> Bootstrap.readPassphrase(stream, MAX_PASSPHRASE_LENGTH) + ); + } + } + + private void assertPassphraseRead(String source, String expected) { + try (InputStream stream = new ByteArrayInputStream(source.getBytes(StandardCharsets.UTF_8))) { + SecureString result = Bootstrap.readPassphrase(stream, MAX_PASSPHRASE_LENGTH); + assertThat(result, equalTo(expected)); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + } 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 6cfa2c1fdf255..cd64fdc08d3f5 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 @@ -53,110 +53,156 @@ 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 { + public void testMissingCreateWithEmptyPasswordWhenPrompted() throws Exception { + String password = ""; Path file1 = createRandomFile(); terminal.addTextInput("y"); execute("foo", file1.toString()); - assertSecureFile("foo", file1); + assertSecureFile("foo", file1, password); } - public void testMissingForceCreate() throws Exception { + public void testMissingCreateWithEmptyPasswordWithoutPromptIfForced() throws Exception { + String password = ""; Path file1 = createRandomFile(); - terminal.addSecretInput("bar"); execute("-f", "foo", file1.toString()); - assertSecureFile("foo", file1); + assertSecureFile("foo", file1, password); } public void testMissingNoCreate() throws Exception { + terminal.addSecretInput(randomFrom("", "keystorepassword")); terminal.addTextInput("n"); // explicit no execute("foo"); assertNull(KeyStoreWrapper.load(env.configFile())); } public void testOverwritePromptDefault() throws Exception { + String password = "keystorepassword"; Path file = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file, password); + terminal.addSecretInput(password); + terminal.addSecretInput(password); terminal.addTextInput(""); execute("foo", "path/dne"); - assertSecureFile("foo", file); + assertSecureFile("foo", file, password); } public void testOverwritePromptExplicitNo() throws Exception { + String password = "keystorepassword"; Path file = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file, password); + terminal.addSecretInput(password); terminal.addTextInput("n"); // explicit no execute("foo", "path/dne"); - assertSecureFile("foo", file); + assertSecureFile("foo", file, password); } public void testOverwritePromptExplicitYes() throws Exception { + String password = "keystorepassword"; Path file1 = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file1); + 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); + assertSecureFile("foo", file2, password); } public void testOverwriteForceShort() throws Exception { + String password = "keystorepassword"; Path file1 = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file1); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file1, password); Path file2 = createRandomFile(); + terminal.addSecretInput(password); + terminal.addSecretInput(password); execute("-f", "foo", file2.toString()); - assertSecureFile("foo", file2); + assertSecureFile("foo", file2, password); } public void testOverwriteForceLong() throws Exception { + String password = "keystorepassword"; Path file1 = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file1); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file1, password); Path file2 = createRandomFile(); + terminal.addSecretInput(password); execute("--force", "foo", file2.toString()); - assertSecureFile("foo", file2); + assertSecureFile("foo", file2, password); } - public void testForceNonExistent() throws Exception { - createKeystore(""); + public void testForceDoesNotAlreadyExist() throws Exception { + String password = "keystorepassword"; + createKeystore(password); Path file = createRandomFile(); + terminal.addSecretInput(password); execute("--force", "foo", file.toString()); - assertSecureFile("foo", file); + assertSecureFile("foo", file, password); } public void testMissingSettingName() throws Exception { - createKeystore(""); + 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 { - createKeystore(""); + 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 { - createKeystore(""); + 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 { - createKeystore(""); + String password = "keystorepassword"; + createKeystore(password); Path file = createRandomFile(); + 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 password = "keystorepassword"; + createKeystore(password); + Path file = createRandomFile(); + terminal.addSecretInput("thewrongkeystorepassword"); + UserException e = expectThrows(UserException.class, () -> execute("foo", file.toString())); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Provided keystore password was incorrect")); + } + + public void testAddToUnprotectedKeystore() throws Exception { + String password = ""; + Path file = createRandomFile(); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file, password); + terminal.addTextInput(""); + // will not be prompted for a password + execute("foo", "path/dne"); + 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 251714fc3a6bc..4346fbb243714 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 @@ -51,17 +51,27 @@ InputStream getStdin() { }; } - public void testMissingPromptCreate() throws Exception { + public void testInvalidPassphrease() throws Exception { + 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("Provided keystore password was incorrect")); + + } + + public void testMissingPromptCreateWithoutPasswordWhenPrompted() throws Exception { terminal.addTextInput("y"); terminal.addSecretInput("bar"); execute("foo"); - assertSecureString("foo", "bar"); + assertSecureString("foo", "bar", ""); } - public void testMissingForceCreate() throws Exception { + public void testMissingPromptCreateWithoutPasswordWithoutPromptIfForced() throws Exception { terminal.addSecretInput("bar"); execute("-f", "foo"); - assertSecureString("foo", "bar"); + assertSecureString("foo", "bar", ""); } public void testMissingNoCreate() throws Exception { @@ -71,92 +81,118 @@ public void testMissingNoCreate() throws Exception { } public void testOverwritePromptDefault() throws Exception { - createKeystore("", "foo", "bar"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); terminal.addTextInput(""); execute("foo"); - assertSecureString("foo", "bar"); + assertSecureString("foo", "bar", password); } public void testOverwritePromptExplicitNo() throws Exception { - createKeystore("", "foo", "bar"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); terminal.addTextInput("n"); // explicit no execute("foo"); - assertSecureString("foo", "bar"); + assertSecureString("foo", "bar", password); } public void testOverwritePromptExplicitYes() throws Exception { - createKeystore("", "foo", "bar"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); terminal.addTextInput("y"); + terminal.addSecretInput(password); terminal.addSecretInput("newvalue"); execute("foo"); - assertSecureString("foo", "newvalue"); + assertSecureString("foo", "newvalue", password); } public void testOverwriteForceShort() throws Exception { - createKeystore("", "foo", "bar"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); terminal.addSecretInput("newvalue"); execute("-f", "foo"); // force - assertSecureString("foo", "newvalue"); + assertSecureString("foo", "newvalue", password); } public void testOverwriteForceLong() throws Exception { - createKeystore("", "foo", "bar"); + 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"); + assertSecureString("foo", "and yet another secret value", password); } public void testForceNonExistent() throws Exception { - createKeystore(""); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); terminal.addSecretInput("value"); execute("--force", "foo"); // force - assertSecureString("foo", "value"); + assertSecureString("foo", "value", password); } public void testPromptForValue() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + String password = "keystorepassword"; + KeyStoreWrapper.create().save(env.configFile(), password.toCharArray()); + terminal.addSecretInput(password); terminal.addSecretInput("secret value"); execute("foo"); - assertSecureString("foo", "secret value"); + assertSecureString("foo", "secret value", password); } public void testStdinShort() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + 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"); + assertSecureString("foo", "secret value 1", password); } public void testStdinLong() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + 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"); + assertSecureString("foo", "secret value 2", password); } public void testStdinNoInput() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + String password = "keystorepassword"; + KeyStoreWrapper.create().save(env.configFile(), password.toCharArray()); + terminal.addSecretInput(password); setInput(""); execute("-x", "foo"); - assertSecureString("foo", ""); + assertSecureString("foo", "", password); } public void testStdinInputWithLineBreaks() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + String password = "keystorepassword"; + KeyStoreWrapper.create().save(env.configFile(), password.toCharArray()); + terminal.addSecretInput(password); setInput("Typedthisandhitenter\n"); execute("-x", "foo"); - assertSecureString("foo", "Typedthisandhitenter"); + assertSecureString("foo", "Typedthisandhitenter", password); } public void testStdinInputWithCarriageReturn() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + String password = "keystorepassword"; + KeyStoreWrapper.create().save(env.configFile(), password.toCharArray()); + terminal.addSecretInput(password); setInput("Typedthisandhitenter\r"); execute("-x", "foo"); - assertSecureString("foo", "Typedthisandhitenter"); + assertSecureString("foo", "Typedthisandhitenter", password); } public void testAddUtf8String() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + String password = "keystorepassword"; + KeyStoreWrapper.create().save(env.configFile(), password.toCharArray()); + terminal.addSecretInput(password); final int stringSize = randomIntBetween(8, 16); try (CharArrayWriter secretChars = new CharArrayWriter(stringSize)) { for (int i = 0; i < stringSize; i++) { @@ -164,12 +200,15 @@ public void testAddUtf8String() throws Exception { } setInput(secretChars.toString()); execute("-x", "foo"); - assertSecureString("foo", secretChars.toString()); + assertSecureString("foo", secretChars.toString(), password); } } public void testMissingSettingName() throws Exception { - createKeystore(""); + 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); @@ -185,6 +224,15 @@ public void testSpecialCharacterInName() throws Exception { assertThat(e, hasToString(containsString(exceptionString))); } + public void testAddToUnprotectedKeystore() throws Exception { + String password = ""; + createKeystore(password, "foo", "bar"); + terminal.addTextInput(""); + // will not be prompted for a password + execute("foo"); + assertSecureString("foo", "bar", password); + } + void setInput(String inputStr) { input = new ByteArrayInputStream(inputStr.getBytes(StandardCharsets.UTF_8)); } 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 new file mode 100644 index 0000000000000..ca0b5fa363351 --- /dev/null +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.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 ChangeKeyStorePasswordCommandTests extends KeyStoreCommandTestCase { + @Override + protected Command newCommand() { + return new ChangeKeyStorePasswordCommand() { + @Override + protected Environment createEnv(Map settings) throws UserException { + return env; + } + }; + } + + public void testSetKeyStorePassword() throws Exception { + createKeystore(""); + loadKeystore(""); + terminal.addSecretInput("thepassword"); + terminal.addSecretInput("thepassword"); + // Prompted twice for the new password, since we didn't have an existing password + execute(); + loadKeystore("thepassword"); + } + + public void testChangeKeyStorePassword() throws Exception { + 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("thepassword"); + } + + public void testChangeKeyStorePasswordToEmpty() throws Exception { + createKeystore("theoldpassword"); + loadKeystore("theoldpassword"); + terminal.addSecretInput("theoldpassword"); + terminal.addSecretInput(""); + terminal.addSecretInput(""); + // Prompted thrice: Once for the existing and twice for the new password + execute(); + loadKeystore(""); + } + + public void testChangeKeyStorePasswordWrongVerification() throws Exception { + 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("Passwords are not equal, exiting")); + } + + public void testChangeKeyStorePasswordWrongExistingPassword() throws Exception { + 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("Provided keystore password was incorrect")); + } +} 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 aefedf86e7761..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 @@ -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,34 @@ protected Environment createEnv(Map settings) throws UserExcepti }; } + public void testNotMatchingPasswords() throws Exception { + 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("Passwords are not equal, exiting")); + } + + public void testDefaultNotPromptForPassword() throws Exception { + execute(); + Path configDir = env.configFile(); + assertNotNull(KeyStoreWrapper.load(configDir)); + } + public void testPosix() throws Exception { + 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 password = randomFrom("", "keystorepassword"); + terminal.addSecretInput(password); + terminal.addSecretInput(password); env = setupEnv(false, fileSystems); execute(); Path configDir = env.configFile(); @@ -54,6 +78,7 @@ public void testNotPosix() throws Exception { } public void testOverwrite() throws Exception { + String password = randomFrom("", "keystorepassword"); Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); byte[] content = "not a keystore".getBytes(StandardCharsets.UTF_8); Files.write(keystoreFile, content); @@ -67,6 +92,8 @@ public void testOverwrite() throws Exception { assertArrayEquals(content, Files.readAllBytes(keystoreFile)); terminal.addTextInput("y"); + 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/HasPasswordKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/HasPasswordKeyStoreCommandTests.java new file mode 100644 index 0000000000000..93e6b0cae2095 --- /dev/null +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/HasPasswordKeyStoreCommandTests.java @@ -0,0 +1,68 @@ +/* + * 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.UserException; +import org.elasticsearch.env.Environment; + +import java.util.Map; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.emptyString; +import static org.hamcrest.Matchers.nullValue; + +public class HasPasswordKeyStoreCommandTests extends KeyStoreCommandTestCase { + @Override + protected Command newCommand() { + return new HasPasswordKeyStoreCommand() { + @Override + protected Environment createEnv(Map settings) throws UserException { + return env; + } + }; + } + + public void testFailsWithNoKeystore() throws Exception { + UserException e = expectThrows(UserException.class, this::execute); + assertEquals("Unexpected exit code", HasPasswordKeyStoreCommand.NO_PASSWORD_EXIT_CODE, e.exitCode); + assertThat("Exception should have null message", e.getMessage(), is(nullValue())); + } + + public void testFailsWhenKeystoreLacksPassword() throws Exception { + createKeystore(""); + UserException e = expectThrows(UserException.class, this::execute); + assertEquals("Unexpected exit code", HasPasswordKeyStoreCommand.NO_PASSWORD_EXIT_CODE, e.exitCode); + assertThat("Exception should have null message", e.getMessage(), is(nullValue())); + } + + public void testSucceedsWhenKeystoreHasPassword() throws Exception { + createKeystore("password"); + String output = execute(); + assertThat(output, containsString("Keystore is password-protected")); + } + + public void testSilentSucceedsWhenKeystoreHasPassword() throws Exception { + createKeystore("password"); + String output = execute("--silent"); + assertThat(output, is(emptyString())); + } +} diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java index 96c79c692e414..0ac39e466dd04 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java +++ b/distribution/tools/keystore-cli/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/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 baf39557688e6..b58c9ec07dbdb 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 @@ -114,7 +114,7 @@ public void testDecryptKeyStoreWithWrongPassword() throws Exception { 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 { 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 27c30d3aa8f58..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 @@ -47,20 +47,42 @@ public void testMissing() throws Exception { } public void testEmpty() throws Exception { - createKeystore(""); + String password = randomFrom("", "keystorepassword"); + createKeystore(password); + terminal.addSecretInput(password); execute(); assertEquals("keystore.seed\n", terminal.getOutput()); } public void testOne() throws Exception { - createKeystore("", "foo", "bar"); + String password = randomFrom("", "keystorepassword"); + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); execute(); assertEquals("foo\nkeystore.seed\n", terminal.getOutput()); } public void testMultiple() throws Exception { - createKeystore("", "foo", "1", "baz", "2", "bar", "3"); + 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 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("Provided keystore password was incorrect")); + } + + public void testListWithUnprotectedKeystore() throws Exception { + createKeystore("", "foo", "bar"); + execute(); + // Not prompted for a password + assertEquals("foo\nkeystore.seed\n", terminal.getOutput()); + } } 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 2259dee31a8cb..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 @@ -41,39 +41,66 @@ protected Environment createEnv(Map settings) throws UserExcepti }; } - public void testMissing() throws Exception { + public void testMissing() { + 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 { - createKeystore(""); + 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 { - createKeystore(""); + 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 { - createKeystore("", "foo", "bar"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); execute("foo"); - assertFalse(loadKeystore("").getSettingNames().contains("foo")); + assertFalse(loadKeystore(password).getSettingNames().contains("foo")); } public void testMany() throws Exception { - createKeystore("", "foo", "1", "bar", "2", "baz", "3"); + String password = "keystorepassword"; + createKeystore(password, "foo", "1", "bar", "2", "baz", "3"); + terminal.addSecretInput(password); execute("foo", "baz"); - Set settings = loadKeystore("").getSettingNames(); + Set settings = loadKeystore(password).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 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("Provided keystore password was incorrect")); + } + + public void testRemoveFromUnprotectedKeystore() throws Exception { + String password = ""; + createKeystore(password, "foo", "bar"); + // will not be prompted for a password + execute("foo"); + assertFalse(loadKeystore(password).getSettingNames().contains("foo")); + } } 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 2b066ec4a1f21..07cbb5574c520 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 @@ -71,7 +71,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/docs/reference/cluster.asciidoc b/docs/reference/cluster.asciidoc index 6f224809adc3d..1c406f0bc1822 100644 --- a/docs/reference/cluster.asciidoc +++ b/docs/reference/cluster.asciidoc @@ -99,6 +99,8 @@ include::cluster/nodes-hot-threads.asciidoc[] include::cluster/nodes-info.asciidoc[] +include::cluster/nodes-reload-secure-settings.asciidoc[] + include::cluster/nodes-stats.asciidoc[] include::cluster/pending.asciidoc[] diff --git a/docs/reference/cluster/nodes-reload-secure-settings.asciidoc b/docs/reference/cluster/nodes-reload-secure-settings.asciidoc index 1ef75d07e22c8..66133c705cc49 100644 --- a/docs/reference/cluster/nodes-reload-secure-settings.asciidoc +++ b/docs/reference/cluster/nodes-reload-secure-settings.asciidoc @@ -1,13 +1,11 @@ [[cluster-nodes-reload-secure-settings]] -== Nodes Reload Secure Settings +=== Nodes reload secure settings API +++++ +Nodes reload secure settings +++++ -The cluster nodes reload secure settings API is used to re-read the -local node's encrypted keystore. Specifically, it will prompt the keystore -decryption and reading across the cluster. The keystore's plain content is -used to reinitialize all compatible plugins. A compatible plugin can be -reinitialized without restarting the node. The operation is -complete when all compatible plugins have finished reinitializing. Subsequently, -the keystore is closed and any changes to it will not be reflected on the node. + +The cluster nodes reload secure settings API is used to re-load the keystore on each node. [source,console] -------------------------------------------------- @@ -21,9 +19,41 @@ The first command reloads the keystore on each node. The seconds allows to selectively target `nodeId1` and `nodeId2`. The node selection options are detailed <>. -Note: It is an error if secure settings are inconsistent across the cluster -nodes, yet this consistency is not enforced whatsoever. Hence, reloading specific -nodes is not standard. It is only justifiable when retrying failed reload operations. +NOTE: {es} requires consistent secure settings across the cluster nodes, but this consistency is not enforced. +Hence, reloading specific nodes is not standard. It is only justifiable when retrying failed reload operations. + +==== Reload Password Protected Secure Settings + +When the {es} keystore is password protected and not simply obfuscated, the password for the keystore needs +to be provided in the request to reload the secure settings. +Reloading the settings for the whole cluster assumes that all nodes' keystores are protected with the same password +and is only allowed when {ref}/configuring-tls.html#tls-transport[node to node communications are encrypted] + +[source,js] +-------------------------------------------------- +POST _nodes/reload_secure_settings +{ + "reload_secure_settings": "s3cr3t" <1> +} +-------------------------------------------------- +// NOTCONSOLE + +<1> The common password that the {es} keystore is encrypted with in every node of the cluster. + +Alternatively the secure settings can be reloaded on a per node basis, locally accessing the API and passing the +node-specific {es} keystore password. + +[source,js] +-------------------------------------------------- +POST _nodes/_local/reload_secure_settings +{ + "reload_secure_settings": "s3cr3t" <1> +} +-------------------------------------------------- +// NOTCONSOLE + +<1> The password that the {es} keystore is encrypted with on the local node. + [float] [[rest-reload-secure-settings]] diff --git a/docs/reference/commands/keystore.asciidoc b/docs/reference/commands/keystore.asciidoc index 7b2df6ee24c77..085f8acb7adcb 100644 --- a/docs/reference/commands/keystore.asciidoc +++ b/docs/reference/commands/keystore.asciidoc @@ -11,9 +11,9 @@ in the {es} keystore. [source,shell] -------------------------------------------------- bin/elasticsearch-keystore -([add ] [--stdin] | -[add-file ] | [create] | -[list] | [remove ] | [upgrade]) +([add ] [-f] [--stdin] | +[add-file ] | [create] [-p] | +[list] | [passwd] | [remove ] | [upgrade]) [-h, --help] ([-s, --silent] | [-v, --verbose]) -------------------------------------------------- @@ -26,6 +26,9 @@ IMPORTANT: This command should be run as the user that will run {es}. Currently, all secure settings are node-specific settings that must have the same value on every node. Therefore you must run this command on every node. +When the keystore is password-protected, you must supply the password each time +{es} starts. + Modifications to the keystore do not take effect until you restart {es}. Only some settings are designed to be read from the keystore. However, there @@ -38,15 +41,34 @@ keystore, see the setting reference. === Parameters `add `:: Adds settings to the keystore. By default, you are prompted -for the value of the setting. +for the value of the setting. If the keystore is password protected, you are +also prompted to enter the password. If the setting already exists in the +keystore, you must confirm that you want to overwrite the current value. If the +keystore does not exist, you must confirm that you want to create a keystore. To +avoid these two confirmation prompts, use the `-f` parameter. `add-file `:: Adds a file to the keystore. `create`:: Creates the keystore. +`-f`:: When used with the `add` parameter, the command no longer prompts you +before overwriting existing entries in the keystore. Also, if you haven't +created a keystore yet, it creates a keystore that is obfuscated but not +password protected. + `-h, --help`:: Returns all of the command parameters. -`list`:: Lists the settings in the keystore. +`list`:: Lists the settings in the keystore. If the keystore is password +protected, you are prompted to enter the password. + +`-p`:: When used with the `create` parameter, the command prompts you to enter a +keystore password. If you don't specify the `-p` flag or if you enter an empty +password, the keystore is obfuscated but not password protected. + +`passwd`:: Changes or sets the keystore password. If the keystore is password +protected, you are prompted to enter the current password and the new one. You +can optionally use an empty string to remove the password. If the keystore is +not password protected, you can use this command to set a password. `remove `:: Removes a setting from the keystore. @@ -71,11 +93,26 @@ To create the `elasticsearch.keystore`, use the `create` command: [source,sh] ---------------------------------------------------------------- -bin/elasticsearch-keystore create +bin/elasticsearch-keystore create -p +---------------------------------------------------------------- + +You are prompted to enter the keystore password. A password-protected +`elasticsearch.keystore` file is created alongside the `elasticsearch.yml` file. + +[discrete] +[[changing-keystore-password]] +==== Change the password of the keystore + +To change the password of the `elasticsearch.keystore`, use the `passwd` command: + +[source,sh] +---------------------------------------------------------------- +bin/elasticsearch-keystore passwd ---------------------------------------------------------------- -A `elasticsearch.keystore` file is created alongside the `elasticsearch.yml` -file. +If the {es} keystore is password protected, you are prompted to enter the +current password and then enter the new one. If it is not password protected, +you are prompted to set a password. [discrete] [[list-settings]] @@ -88,6 +125,9 @@ To list the settings in the keystore, use the `list` command. bin/elasticsearch-keystore list ---------------------------------------------------------------- +If the {es} keystore is password protected, you are prompted to enter the +password. + [discrete] [[add-string-to-keystore]] ==== Add settings to the keystore @@ -100,8 +140,10 @@ can be added with the `add` command: bin/elasticsearch-keystore add the.setting.name.to.set ---------------------------------------------------------------- -You are prompted to enter the value of the setting. To pass the value -through standard input (stdin), use the `--stdin` flag: +You are prompted to enter the value of the setting. If the {es} keystore is +password protected, you are also prompted to enter the password. + +To pass the setting value through standard input (stdin), use the `--stdin` flag: [source,sh] ---------------------------------------------------------------- @@ -121,6 +163,9 @@ after the setting name. bin/elasticsearch-keystore add-file the.setting.name.to.set /path/example-file.json ---------------------------------------------------------------- +If the {es} keystore is password protected, you are prompted to enter the +password. + [discrete] [[remove-settings]] ==== Remove settings from the keystore @@ -132,6 +177,9 @@ To remove a setting from the keystore, use the `remove` command: bin/elasticsearch-keystore remove the.setting.name.to.remove ---------------------------------------------------------------- +If the {es} keystore is password protected, you are prompted to enter the +password. + [discrete] [[keystore-upgrade]] ==== Upgrade the keystore diff --git a/docs/reference/commands/saml-metadata.asciidoc b/docs/reference/commands/saml-metadata.asciidoc index 5309f83288f89..78db77ea4661b 100644 --- a/docs/reference/commands/saml-metadata.asciidoc +++ b/docs/reference/commands/saml-metadata.asciidoc @@ -40,6 +40,10 @@ ensure its integrity and authenticity before sharing it with the Identity Provid The key used for signing the metadata file need not necessarily be the same as the keys already used in the saml realm configuration for SAML message signing. +If your {es} keystore is password protected, you +are prompted to enter the password when you run the +`elasticsearch-saml-metadata` command. + [float] === Parameters diff --git a/docs/reference/commands/setup-passwords.asciidoc b/docs/reference/commands/setup-passwords.asciidoc index 1c17c5544e7be..db13dc5350201 100644 --- a/docs/reference/commands/setup-passwords.asciidoc +++ b/docs/reference/commands/setup-passwords.asciidoc @@ -22,7 +22,9 @@ bin/elasticsearch-setup-passwords auto|interactive This command is intended for use only during the initial configuration of the {es} {security-features}. It uses the <> -to run user management API requests. After you set a password for the `elastic` +to run user management API requests. If your {es} keystore is password protected, +before you can set the passwords for the built-in users, you must enter the keystore password. +After you set a password for the `elastic` user, the bootstrap password is no longer active and you cannot use this command. Instead, you can change passwords by using the *Management > Users* UI in {kib} or the <>. diff --git a/docs/reference/setup/install/docker.asciidoc b/docs/reference/setup/install/docker.asciidoc index b50b10f18a8ca..28867f04ed795 100644 --- a/docs/reference/setup/install/docker.asciidoc +++ b/docs/reference/setup/install/docker.asciidoc @@ -354,6 +354,25 @@ IMPORTANT: The container **runs {es} as user `elasticsearch` using uid:gid `1000:0`**. Bind mounted host directories and files must be accessible by this user, and the data and log directories must be writable by this user. +[[docker-keystore-bind-mount]] +===== Mounting an {es} keystore + +By default, {es} will auto-generate a keystore file for secure settings. This +file is obfuscated but not encrypted. If you want to encrypt your +<> with a password, you must use the +`elasticsearch-keystore` utility to create a password-protected keystore and +bind-mount it to the container as +`/usr/share/elasticsearch/config/elasticsearch.keystore`. In order to provide +the Docker container with the password at startup, set the Docker environment +value `KEYSTORE_PASSWORD` to the value of your password. For example, a `docker +run` command might have the following options: + +[source, sh] +-------------------------------------------- +-v full_path_to/elasticsearch.keystore:/usr/share/elasticsearch/config/elasticsearch.keystore +-E KEYSTORE_PASSWORD=mypassword +-------------------------------------------- + [[_c_customized_image]] ===== Using custom Docker images In some environments, it might make more sense to prepare a custom image that contains diff --git a/docs/reference/setup/install/systemd.asciidoc b/docs/reference/setup/install/systemd.asciidoc index bf94e95fb63df..274a599e68f09 100644 --- a/docs/reference/setup/install/systemd.asciidoc +++ b/docs/reference/setup/install/systemd.asciidoc @@ -21,6 +21,19 @@ These commands provide no feedback as to whether Elasticsearch was started successfully or not. Instead, this information will be written in the log files located in `/var/log/elasticsearch/`. +If you have password-protected your {es} keystore, you will need to provide +`systemd` with the keystore password using a local file and systemd environment +variables. This local file should be protected while it exists and may be +safely deleted once Elasticsearch is up and running. + +[source,sh] +----------------------------------------------------------------------------------- +echo "keystore_password" > /path/to/my_pwd_file.tmp +chmod 600 /path/to/my_pwd_file.tmp +sudo systemctl set-environment ES_KEYSTORE_PASSPHRASE_FILE=/path/to/my_pwd_file.tmp +sudo systemctl start elasticsearch.service +----------------------------------------------------------------------------------- + By default the Elasticsearch service doesn't log information in the `systemd` journal. To enable `journalctl` logging, the `--quiet` option must be removed from the `ExecStart` command line in the `elasticsearch.service` file. diff --git a/docs/reference/setup/install/targz-daemon.asciidoc b/docs/reference/setup/install/targz-daemon.asciidoc index 1325503687a07..2ccd0519945b4 100644 --- a/docs/reference/setup/install/targz-daemon.asciidoc +++ b/docs/reference/setup/install/targz-daemon.asciidoc @@ -8,13 +8,17 @@ the process ID in a file using the `-p` option: ./bin/elasticsearch -d -p pid -------------------------------------------- +If you have password-protected the {es} keystore, you will be prompted +to enter the keystore's password. See <> for more +details. + Log messages can be found in the `$ES_HOME/logs/` directory. To shut down Elasticsearch, kill the process ID recorded in the `pid` file: [source,sh] -------------------------------------------- -pkill -F pid +pkill -F pid -------------------------------------------- NOTE: The startup scripts provided in the <> and <> diff --git a/docs/reference/setup/install/targz-start.asciidoc b/docs/reference/setup/install/targz-start.asciidoc index 907b2a7317d79..cf90e05d173f6 100644 --- a/docs/reference/setup/install/targz-start.asciidoc +++ b/docs/reference/setup/install/targz-start.asciidoc @@ -7,6 +7,10 @@ Elasticsearch can be started from the command line as follows: ./bin/elasticsearch -------------------------------------------- +If you have password-protected the {es} keystore, you will be prompted +to enter the keystore's password. See <> for more +details. + By default, Elasticsearch runs in the foreground, prints its logs to the standard output (`stdout`), and can be stopped by pressing `Ctrl-C`. diff --git a/docs/reference/setup/install/zip-windows-start.asciidoc b/docs/reference/setup/install/zip-windows-start.asciidoc index 7ecea449d2895..718259e4b77fc 100644 --- a/docs/reference/setup/install/zip-windows-start.asciidoc +++ b/docs/reference/setup/install/zip-windows-start.asciidoc @@ -7,5 +7,8 @@ Elasticsearch can be started from the command line as follows: .\bin\elasticsearch.bat -------------------------------------------- +If you have password-protected the {es} keystore, you will be prompted to +enter the keystore's password. See <> for more details. + By default, Elasticsearch runs in the foreground, prints its logs to `STDOUT`, and can be stopped by pressing `Ctrl-C`. diff --git a/docs/reference/setup/secure-settings.asciidoc b/docs/reference/setup/secure-settings.asciidoc index e565877f22f5e..f35c374735095 100644 --- a/docs/reference/setup/secure-settings.asciidoc +++ b/docs/reference/setup/secure-settings.asciidoc @@ -14,9 +14,6 @@ reference. All the modifications to the keystore take affect only after restarting {es}. -NOTE: The {es} keystore currently only provides obfuscation. In the future, -password protection will be added. - These settings, just like the regular ones in the `elasticsearch.yml` config file, need to be specified on each node in the cluster. Currently, all secure settings are node-specific settings that must have the same value on every node. @@ -37,7 +34,13 @@ using the `bin/elasticsearch-keystore add` command, call: [source,console] ---- POST _nodes/reload_secure_settings +{ + "reload_secure_settings": "s3cr3t" <1> +} ---- +// NOTCONSOLE + +<1> The password that the {es} keystore is encrypted with. This API decrypts and re-reads the entire keystore, on every cluster node, but only the *reloadable* secure settings are applied. Changes to other diff --git a/libs/cli/src/main/java/org/elasticsearch/cli/Command.java b/libs/cli/src/main/java/org/elasticsearch/cli/Command.java index 9ce77604a5014..ec23f62b090a5 100644 --- a/libs/cli/src/main/java/org/elasticsearch/cli/Command.java +++ b/libs/cli/src/main/java/org/elasticsearch/cli/Command.java @@ -97,7 +97,9 @@ public final int main(String[] args, Terminal terminal) throws Exception { if (e.exitCode == ExitCodes.USAGE) { printHelp(terminal, true); } - terminal.errorPrintln(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); + if (e.getMessage() != null) { + terminal.errorPrintln(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); + } return e.exitCode; } return ExitCodes.OK; diff --git a/libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java b/libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java index 74af7e2e3102f..aff1b8a85a01f 100644 --- a/libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java +++ b/libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java @@ -24,7 +24,9 @@ import java.io.IOException; import java.io.InputStreamReader; import java.io.PrintWriter; +import java.io.Reader; import java.nio.charset.Charset; +import java.util.Arrays; import java.util.Locale; /** @@ -78,6 +80,16 @@ public void setVerbosity(Verbosity verbosity) { /** Reads password text from the terminal input. See {@link Console#readPassword()}}. */ public abstract char[] readSecret(String prompt); + /** Read password text form terminal input up to a maximum length. */ + public char[] readSecret(String prompt, int maxLength) { + char[] result = readSecret(prompt); + if (result.length > maxLength) { + Arrays.fill(result, '\0'); + throw new IllegalStateException("Secret exceeded maximum length of " + maxLength); + } + return result; + } + /** Returns a Writer which can be used to write to the terminal directly using standard output. */ public abstract PrintWriter getWriter(); @@ -151,6 +163,45 @@ public final boolean promptYesNo(String prompt, boolean defaultYes) { } } + /** + * Read from the reader until we find a newline. If that newline + * character is immediately preceded by a carriage return, we have + * a Windows-style newline, so we discard the carriage return as well + * as the newline. + */ + public static char[] readLineToCharArray(Reader reader, int maxLength) { + char[] buf = new char[maxLength + 2]; + try { + int len = 0; + int next; + while ((next = reader.read()) != -1) { + char nextChar = (char) next; + if (nextChar == '\n') { + break; + } + if (len < buf.length) { + buf[len] = nextChar; + } + len++; + } + + if (len > 0 && len < buf.length && buf[len-1] == '\r') { + len--; + } + + if (len > maxLength) { + Arrays.fill(buf, '\0'); + throw new RuntimeException("Input exceeded maximum length of " + maxLength); + } + + char[] shortResult = Arrays.copyOf(buf, len); + Arrays.fill(buf, '\0'); + return shortResult; + } catch (IOException e) { + throw new RuntimeException(e); + } + } + public void flush() { this.getWriter().flush(); this.getErrorWriter().flush(); @@ -184,10 +235,13 @@ public char[] readSecret(String prompt) { } } - private static class SystemTerminal extends Terminal { + /** visible for testing */ + static class SystemTerminal extends Terminal { private static final PrintWriter WRITER = newWriter(); + private BufferedReader reader; + SystemTerminal() { super(System.lineSeparator()); } @@ -197,6 +251,14 @@ private static PrintWriter newWriter() { return new PrintWriter(System.out); } + /** visible for testing */ + BufferedReader getReader() { + if (reader == null) { + reader = new BufferedReader(new InputStreamReader(System.in, Charset.defaultCharset())); + } + return reader; + } + @Override public PrintWriter getWriter() { return WRITER; @@ -205,9 +267,8 @@ public PrintWriter getWriter() { @Override public String readText(String text) { getErrorWriter().print(text); // prompts should go to standard error to avoid mixing with list output - BufferedReader reader = new BufferedReader(new InputStreamReader(System.in, Charset.defaultCharset())); try { - final String line = reader.readLine(); + final String line = getReader().readLine(); if (line == null) { throw new IllegalStateException("unable to read from standard input; is standard input open and a tty attached?"); } @@ -221,5 +282,11 @@ public String readText(String text) { public char[] readSecret(String text) { return readText(text).toCharArray(); } + + @Override + public char[] readSecret(String text, int maxLength) { + getErrorWriter().println(text); + return readLineToCharArray(getReader(), maxLength); + } } } diff --git a/libs/cli/src/main/java/org/elasticsearch/cli/UserException.java b/libs/cli/src/main/java/org/elasticsearch/cli/UserException.java index 4749b1b87b7aa..fd6ec7807a5d7 100644 --- a/libs/cli/src/main/java/org/elasticsearch/cli/UserException.java +++ b/libs/cli/src/main/java/org/elasticsearch/cli/UserException.java @@ -19,6 +19,8 @@ package org.elasticsearch.cli; +import org.elasticsearch.common.Nullable; + /** * An exception representing a user fixable problem in {@link Command} usage. */ @@ -27,20 +29,26 @@ public class UserException extends Exception { /** The exist status the cli should use when catching this user error. */ public final int exitCode; - /** Constructs a UserException with an exit status and message to show the user. */ - public UserException(int exitCode, String msg) { + /** + * Constructs a UserException with an exit status and message to show the user. + *

+ * To suppress cli output on error, supply a null message. + */ + public UserException(int exitCode, @Nullable String msg) { super(msg); this.exitCode = exitCode; } /** * Constructs a new user exception with specified exit status, message, and underlying cause. + *

+ * To suppress cli output on error, supply a null message. * * @param exitCode the exit code * @param msg the message * @param cause the underlying cause */ - public UserException(final int exitCode, final String msg, final Throwable cause) { + public UserException(final int exitCode, @Nullable final String msg, final Throwable cause) { super(msg, cause); this.exitCode = exitCode; } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java index 9691636f5ccda..efada9c9a89f4 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java @@ -20,7 +20,6 @@ package org.elasticsearch.packaging.test; import org.apache.http.client.fluent.Request; -import org.elasticsearch.packaging.util.Archives; import org.elasticsearch.packaging.util.FileUtils; import org.elasticsearch.packaging.util.Installation; import org.elasticsearch.packaging.util.Platforms; @@ -33,12 +32,8 @@ import java.nio.file.Paths; import java.util.stream.Stream; -import static org.elasticsearch.packaging.util.Archives.ARCHIVE_OWNER; import static org.elasticsearch.packaging.util.Archives.installArchive; import static org.elasticsearch.packaging.util.Archives.verifyArchiveInstallation; -import static org.elasticsearch.packaging.util.FileMatcher.Fileness.File; -import static org.elasticsearch.packaging.util.FileMatcher.file; -import static org.elasticsearch.packaging.util.FileMatcher.p660; import static org.elasticsearch.packaging.util.FileUtils.append; import static org.elasticsearch.packaging.util.FileUtils.cp; import static org.elasticsearch.packaging.util.FileUtils.getTempDir; @@ -105,33 +100,6 @@ public void test31BadJavaHome() throws Exception { } - public void test40CreateKeystoreManually() throws Exception { - final Installation.Executables bin = installation.executables(); - - Platforms.onLinux(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " 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.keystoreTool + " create"); - sh.chown(installation.config("elasticsearch.keystore")); - }); - - assertThat(installation.config("elasticsearch.keystore"), file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660)); - - Platforms.onLinux(() -> { - final Result r = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " list"); - assertThat(r.stdout, containsString("keystore.seed")); - }); - - Platforms.onWindows(() -> { - final Result r = sh.run(bin.keystoreTool + " list"); - assertThat(r.stdout, containsString("keystore.seed")); - }); - } - public void test50StartAndStop() throws Exception { // cleanup from previous test rm(installation.config("elasticsearch.keystore")); @@ -247,22 +215,6 @@ public void test53JavaHomeWithSpecialCharacters() throws Exception { }); } - public void test60AutoCreateKeystore() throws Exception { - sh.chown(installation.config("elasticsearch.keystore")); - assertThat(installation.config("elasticsearch.keystore"), file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660)); - - final Installation.Executables bin = installation.executables(); - Platforms.onLinux(() -> { - final Result result = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " list"); - assertThat(result.stdout, containsString("keystore.seed")); - }); - - Platforms.onWindows(() -> { - final Result result = sh.run(bin.keystoreTool + " list"); - assertThat(result.stdout, containsString("keystore.seed")); - }); - } - public void test70CustomPathConfAndJvmOptions() throws Exception { final Path tempConf = getTempDir().resolve("esconf-alternate"); @@ -292,7 +244,7 @@ public void test70CustomPathConfAndJvmOptions() throws Exception { assertThat(nodesResponse, containsString("\"heap_init_in_bytes\":536870912")); assertThat(nodesResponse, containsString("\"using_compressed_ordinary_object_pointers\":\"false\"")); - Archives.stopElasticsearch(installation); + stopElasticsearch(); } finally { rm(tempConf); @@ -389,7 +341,7 @@ public void test93ElasticsearchNodeCustomDataPathAndNotEsHomeWorkDir() throws Ex sh.setWorkingDirectory(getTempDir()); startElasticsearch(); - Archives.stopElasticsearch(installation); + stopElasticsearch(); Result result = sh.run("echo y | " + installation.executables().nodeTool + " unsafe-bootstrap"); assertThat(result.stdout, containsString("Master node was successfully bootstrapped")); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index 4d0ecb0c83355..563a707ccc32e 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -21,14 +21,11 @@ import com.fasterxml.jackson.databind.JsonNode; import org.apache.http.client.fluent.Request; -import org.elasticsearch.packaging.util.Distribution; -import org.elasticsearch.packaging.util.Docker.DockerShell; import org.elasticsearch.packaging.util.Installation; import org.elasticsearch.packaging.util.Platforms; import org.elasticsearch.packaging.util.ServerUtils; import org.elasticsearch.packaging.util.Shell.Result; import org.junit.After; -import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -42,21 +39,17 @@ import java.util.stream.Collectors; import static java.nio.file.attribute.PosixFilePermissions.fromString; -import static org.elasticsearch.packaging.util.Docker.assertPermissionsAndOwnership; import static org.elasticsearch.packaging.util.Docker.copyFromContainer; -import static org.elasticsearch.packaging.util.Docker.ensureImageIsLoaded; import static org.elasticsearch.packaging.util.Docker.existsInContainer; import static org.elasticsearch.packaging.util.Docker.getContainerLogs; import static org.elasticsearch.packaging.util.Docker.getImageLabels; import static org.elasticsearch.packaging.util.Docker.getJson; import static org.elasticsearch.packaging.util.Docker.mkDirWithPrivilegeEscalation; -import static org.elasticsearch.packaging.util.Docker.removeContainer; import static org.elasticsearch.packaging.util.Docker.rmDirWithPrivilegeEscalation; import static org.elasticsearch.packaging.util.Docker.runContainer; import static org.elasticsearch.packaging.util.Docker.runContainerExpectingFailure; import static org.elasticsearch.packaging.util.Docker.verifyContainerInstallation; import static org.elasticsearch.packaging.util.Docker.waitForElasticsearch; -import static org.elasticsearch.packaging.util.Docker.waitForPathToExist; import static org.elasticsearch.packaging.util.FileMatcher.p600; import static org.elasticsearch.packaging.util.FileMatcher.p660; import static org.elasticsearch.packaging.util.FileMatcher.p775; @@ -78,25 +71,15 @@ import static org.junit.Assume.assumeTrue; public class DockerTests extends PackagingTestCase { - protected DockerShell sh; private Path tempDir; @BeforeClass public static void filterDistros() { - assumeTrue("only Docker", distribution.packaging == Distribution.Packaging.DOCKER); - - ensureImageIsLoaded(distribution); - } - - @AfterClass - public static void cleanup() { - // runContainer also calls this, so we don't need this method to be annotated as `@After` - removeContainer(); + assumeTrue("only Docker", distribution().isDocker()); } @Before public void setupTest() throws IOException { - sh = new DockerShell(); installation = runContainer(distribution()); tempDir = Files.createTempDirectory(getTempDir(), DockerTests.class.getSimpleName()); } @@ -137,44 +120,10 @@ public void test020PluginsListWithNoPlugins() { assertThat("Expected no plugins to be listed", r.stdout, emptyString()); } - /** - * Check that a keystore can be manually created using the provided CLI tool. - */ - public void test040CreateKeystoreManually() throws InterruptedException { - final Installation.Executables bin = installation.executables(); - - final Path keystorePath = installation.config("elasticsearch.keystore"); - - waitForPathToExist(keystorePath); - - // Move the auto-created one out of the way, or else the CLI prompts asks us to confirm - sh.run("mv " + keystorePath + " " + keystorePath + ".bak"); - - sh.run(bin.keystoreTool + " create"); - - final Result r = sh.run(bin.keystoreTool + " list"); - assertThat(r.stdout, containsString("keystore.seed")); - } - - /** - * Check that the default keystore is automatically created - */ - public void test041AutoCreateKeystore() throws Exception { - final Path keystorePath = installation.config("elasticsearch.keystore"); - - waitForPathToExist(keystorePath); - - assertPermissionsAndOwnership(keystorePath, p660); - - final Installation.Executables bin = installation.executables(); - final Result result = sh.run(bin.keystoreTool + " list"); - assertThat(result.stdout, containsString("keystore.seed")); - } - /** * Check that the JDK's cacerts file is a symlink to the copy provided by the operating system. */ - public void test042JavaUsesTheOsProvidedKeystore() { + public void test040JavaUsesTheOsProvidedKeystore() { final String path = sh.run("realpath jdk/lib/security/cacerts").stdout; assertThat(path, equalTo("/etc/pki/ca-trust/extracted/java/cacerts")); @@ -183,7 +132,7 @@ public void test042JavaUsesTheOsProvidedKeystore() { /** * Checks that there are Amazon trusted certificates in the cacaerts keystore. */ - public void test043AmazonCaCertsAreInTheKeystore() { + public void test041AmazonCaCertsAreInTheKeystore() { final boolean matches = sh.run("jdk/bin/keytool -cacerts -storepass changeit -list | grep trustedCertEntry").stdout.lines() .anyMatch(line -> line.contains("amazonrootca")); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java new file mode 100644 index 0000000000000..53319187f55d8 --- /dev/null +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java @@ -0,0 +1,420 @@ +/* + * 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.packaging.test; + +import org.elasticsearch.packaging.util.Distribution; +import org.elasticsearch.packaging.util.Docker; +import org.elasticsearch.packaging.util.FileUtils; +import org.elasticsearch.packaging.util.Installation; +import org.elasticsearch.packaging.util.Packages; +import org.elasticsearch.packaging.util.Platforms; +import org.elasticsearch.packaging.util.ServerUtils; +import org.elasticsearch.packaging.util.Shell; +import org.junit.Ignore; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.Map; + +import static org.elasticsearch.packaging.util.Archives.ARCHIVE_OWNER; +import static org.elasticsearch.packaging.util.Archives.installArchive; +import static org.elasticsearch.packaging.util.Archives.verifyArchiveInstallation; +import static org.elasticsearch.packaging.util.Docker.assertPermissionsAndOwnership; +import static org.elasticsearch.packaging.util.Docker.runContainer; +import static org.elasticsearch.packaging.util.Docker.runContainerExpectingFailure; +import static org.elasticsearch.packaging.util.Docker.waitForElasticsearch; +import static org.elasticsearch.packaging.util.Docker.waitForPathToExist; +import static org.elasticsearch.packaging.util.FileMatcher.Fileness.File; +import static org.elasticsearch.packaging.util.FileMatcher.file; +import static org.elasticsearch.packaging.util.FileMatcher.p660; +import static org.elasticsearch.packaging.util.FileUtils.getTempDir; +import static org.elasticsearch.packaging.util.FileUtils.rm; +import static org.elasticsearch.packaging.util.Packages.assertInstalled; +import static org.elasticsearch.packaging.util.Packages.assertRemoved; +import static org.elasticsearch.packaging.util.Packages.installPackage; +import static org.elasticsearch.packaging.util.Packages.verifyPackageInstallation; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.junit.Assume.assumeThat; +import static org.junit.Assume.assumeTrue; + +public class KeystoreManagementTests extends PackagingTestCase { + + public static final String ERROR_INCORRECT_PASSWORD = "Provided keystore password was incorrect"; + public static final String ERROR_KEYSTORE_NOT_PASSWORD_PROTECTED = "ERROR: Keystore is not password-protected"; + public static final String ERROR_KEYSTORE_NOT_FOUND = "ERROR: Elasticsearch keystore not found"; + + /** Test initial archive state */ + public void test10InstallArchiveDistribution() throws Exception { + assumeTrue(distribution().isArchive()); + + installation = installArchive(sh, distribution); + verifyArchiveInstallation(installation, distribution()); + + final Installation.Executables bin = installation.executables(); + Shell.Result r = sh.runIgnoreExitCode(bin.keystoreTool.toString() + " has-passwd"); + assertFalse("has-passwd should fail", r.isSuccess()); + assertThat("has-passwd should indicate missing keystore", + r.stderr, containsString(ERROR_KEYSTORE_NOT_FOUND)); + } + + /** Test initial package state */ + public void test11InstallPackageDistribution() throws Exception { + assumeTrue(distribution().isPackage()); + + assertRemoved(distribution); + installation = installPackage(sh, distribution); + assertInstalled(distribution); + verifyPackageInstallation(installation, distribution, sh); + + final Installation.Executables bin = installation.executables(); + Shell.Result r = sh.runIgnoreExitCode(bin.keystoreTool.toString() + " has-passwd"); + assertFalse("has-passwd should fail", r.isSuccess()); + assertThat("has-passwd should indicate unprotected keystore", + r.stderr, containsString(ERROR_KEYSTORE_NOT_PASSWORD_PROTECTED)); + Shell.Result r2 = bin.keystoreTool.run("list"); + assertThat(r2.stdout, containsString("keystore.seed")); + } + + /** Test initial Docker state */ + public void test12InstallDockerDistribution() throws Exception { + assumeTrue(distribution().isDocker()); + + installation = Docker.runContainer(distribution()); + + try { + waitForPathToExist(installation.config("elasticsearch.keystore")); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + + final Installation.Executables bin = installation.executables(); + Shell.Result r = sh.runIgnoreExitCode(bin.keystoreTool.toString() + " has-passwd"); + assertFalse("has-passwd should fail", r.isSuccess()); + assertThat("has-passwd should indicate unprotected keystore", + r.stdout, containsString(ERROR_KEYSTORE_NOT_PASSWORD_PROTECTED)); + Shell.Result r2 = bin.keystoreTool.run("list"); + assertThat(r2.stdout, containsString("keystore.seed")); + } + + public void test20CreateKeystoreManually() throws Exception { + rmKeystoreIfExists(); + createKeystore(); + + final Installation.Executables bin = installation.executables(); + verifyKeystorePermissions(); + + Shell.Result r = bin.keystoreTool.run("list"); + assertThat(r.stdout, containsString("keystore.seed")); + } + + public void test30AutoCreateKeystore() throws Exception { + assumeTrue("Packages and docker are installed with a keystore file", distribution.isArchive()); + rmKeystoreIfExists(); + + startElasticsearch(); + stopElasticsearch(); + + Platforms.onWindows(() -> sh.chown(installation.config("elasticsearch.keystore"))); + + verifyKeystorePermissions(); + + final Installation.Executables bin = installation.executables(); + Shell.Result r = bin.keystoreTool.run("list"); + assertThat(r.stdout, containsString("keystore.seed")); + } + + public void test40KeystorePasswordOnStandardInput() throws Exception { + assumeTrue("packages will use systemd, which doesn't handle stdin", + distribution.isArchive()); + assumeThat(installation, is(notNullValue())); + + String password = "^|<>\\&exit"; // code insertion on Windows if special characters are not escaped + + rmKeystoreIfExists(); + createKeystore(); + setKeystorePassword(password); + + assertPasswordProtectedKeystore(); + + awaitElasticsearchStartup(startElasticsearchStandardInputPassword(password)); + ServerUtils.runElasticsearchTests(); + stopElasticsearch(); + } + + public void test41WrongKeystorePasswordOnStandardInput() { + assumeTrue("packages will use systemd, which doesn't handle stdin", + distribution.isArchive()); + assumeThat(installation, is(notNullValue())); + + assertPasswordProtectedKeystore(); + + Shell.Result result = startElasticsearchStandardInputPassword("wrong"); + assertElasticsearchFailure(result, ERROR_INCORRECT_PASSWORD, null); + } + + @Ignore /* Ignored for feature branch, awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */ + public void test42KeystorePasswordOnTty() throws Exception { + assumeTrue("expect command isn't on Windows", + distribution.platform != Distribution.Platform.WINDOWS); + assumeTrue("packages will use systemd, which doesn't handle stdin", + distribution.isArchive()); + assumeThat(installation, is(notNullValue())); + + String password = "keystorepass"; + + rmKeystoreIfExists(); + createKeystore(); + setKeystorePassword(password); + + assertPasswordProtectedKeystore(); + + awaitElasticsearchStartup(startElasticsearchTtyPassword(password)); + ServerUtils.runElasticsearchTests(); + stopElasticsearch(); + } + + @Ignore /* Ignored for feature branch, awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */ + public void test43WrongKeystorePasswordOnTty() throws Exception { + assumeTrue("expect command isn't on Windows", + distribution.platform != Distribution.Platform.WINDOWS); + assumeTrue("packages will use systemd, which doesn't handle stdin", + distribution.isArchive()); + assumeThat(installation, is(notNullValue())); + + assertPasswordProtectedKeystore(); + + Shell.Result result = startElasticsearchTtyPassword("wrong"); + // error will be on stdout for "expect" + assertThat(result.stdout, containsString(ERROR_INCORRECT_PASSWORD)); + } + + public void test50KeystorePasswordFromFile() throws Exception { + assumeTrue("only for systemd", Platforms.isSystemd() && distribution().isPackage()); + String password = "!@#$%^&*()|\\<>/?"; + Path esKeystorePassphraseFile = installation.config.resolve("eks"); + + rmKeystoreIfExists(); + createKeystore(); + setKeystorePassword(password); + + assertPasswordProtectedKeystore(); + + try { + sh.run("sudo systemctl set-environment ES_KEYSTORE_PASSPHRASE_FILE=" + esKeystorePassphraseFile); + + Files.createFile(esKeystorePassphraseFile); + Files.write(esKeystorePassphraseFile, + (password + System.lineSeparator()).getBytes(StandardCharsets.UTF_8), + StandardOpenOption.WRITE); + + startElasticsearch(); + ServerUtils.runElasticsearchTests(); + stopElasticsearch(); + } finally { + sh.run("sudo systemctl unset-environment ES_KEYSTORE_PASSPHRASE_FILE"); + } + } + + public void test51WrongKeystorePasswordFromFile() throws Exception { + assumeTrue("only for systemd", Platforms.isSystemd() && distribution().isPackage()); + Path esKeystorePassphraseFile = installation.config.resolve("eks"); + + assertPasswordProtectedKeystore(); + + try { + sh.run("sudo systemctl set-environment ES_KEYSTORE_PASSPHRASE_FILE=" + esKeystorePassphraseFile); + + if (Files.exists(esKeystorePassphraseFile)) { + rm(esKeystorePassphraseFile); + } + + Files.createFile(esKeystorePassphraseFile); + Files.write(esKeystorePassphraseFile, + ("wrongpassword" + System.lineSeparator()).getBytes(StandardCharsets.UTF_8), + StandardOpenOption.WRITE); + + Packages.JournaldWrapper journaldWrapper = new Packages.JournaldWrapper(sh); + Shell.Result result = runElasticsearchStartCommand(); + assertElasticsearchFailure(result, ERROR_INCORRECT_PASSWORD, journaldWrapper); + } finally { + sh.run("sudo systemctl unset-environment ES_KEYSTORE_PASSPHRASE_FILE"); + } + } + + /** + * Check that we can mount a password-protected keystore to a docker image + * and provide a password via an environment variable. + */ + public void test60DockerEnvironmentVariablePassword() throws Exception { + assumeTrue(distribution().isDocker()); + String password = "password"; + Path dockerKeystore = installation.config("elasticsearch.keystore"); + + Path localKeystoreFile = getKeystoreFileFromDockerContainer(password, dockerKeystore); + + // restart ES with password and mounted keystore + Map volumes = Map.of(localKeystoreFile, dockerKeystore); + Map envVars = Map.of("KEYSTORE_PASSWORD", password); + runContainer(distribution(), volumes, envVars); + waitForElasticsearch(installation); + ServerUtils.runElasticsearchTests(); + } + + /** + * Check that if we provide the wrong password for a mounted and password-protected + * keystore, Elasticsearch doesn't start. + */ + public void test61DockerEnvironmentVariableBadPassword() throws Exception { + assumeTrue(distribution().isDocker()); + String password = "password"; + Path dockerKeystore = installation.config("elasticsearch.keystore"); + + Path localKeystoreFile = getKeystoreFileFromDockerContainer(password, dockerKeystore); + + // restart ES with password and mounted keystore + Map volumes = Map.of(localKeystoreFile, dockerKeystore); + Map envVars = Map.of("KEYSTORE_PASSWORD", "wrong"); + Shell.Result r = runContainerExpectingFailure(distribution(), volumes, envVars); + assertThat(r.stderr, containsString(ERROR_INCORRECT_PASSWORD)); + } + + /** + * In the Docker context, it's a little bit tricky to get a password-protected + * keystore. All of the utilities we'd want to use are on the Docker image. + * This method mounts a temporary directory to a Docker container, password-protects + * the keystore, and then returns the path of the file that appears in the + * mounted directory (now accessible from the local filesystem). + */ + private Path getKeystoreFileFromDockerContainer(String password, Path dockerKeystore) throws IOException { + // Mount a temporary directory for copying the keystore + Path dockerTemp = Path.of("/usr/tmp/keystore-tmp"); + Path tempDirectory = Files.createTempDirectory(getTempDir(), KeystoreManagementTests.class.getSimpleName()); + Map volumes = Map.of(tempDirectory, dockerTemp); + + // It's very tricky to properly quote a pipeline that you're passing to + // a docker exec command, so we're just going to put a small script in the + // temp folder. + String setPasswordScript = "echo \"" + password + "\n" + password + + "\n\" | " + installation.executables().keystoreTool.toString() + " passwd"; + Files.writeString(tempDirectory.resolve("set-pass.sh"), setPasswordScript); + + runContainer(distribution(), volumes, null); + try { + waitForPathToExist(dockerTemp); + waitForPathToExist(dockerKeystore); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + + // We need a local shell to put the correct permissions on our mounted directory. + Shell localShell = new Shell(); + localShell.run("docker exec --tty " + Docker.getContainerId() + " chown elasticsearch:root " + dockerTemp); + localShell.run("docker exec --tty " + Docker.getContainerId() + " chown elasticsearch:root " + dockerTemp.resolve("set-pass.sh")); + + sh.run("bash " + dockerTemp.resolve("set-pass.sh")); + + // copy keystore to temp file to make it available to docker host + sh.run("cp " + dockerKeystore + " " + dockerTemp); + return tempDirectory.resolve("elasticsearch.keystore"); + } + + private void createKeystore() throws Exception { + Path keystore = installation.config("elasticsearch.keystore"); + final Installation.Executables bin = installation.executables(); + bin.keystoreTool.run("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.chown(keystore); + }); + + if (distribution().isDocker()) { + try { + waitForPathToExist(keystore); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + } + + private void rmKeystoreIfExists() { + Path keystore = installation.config("elasticsearch.keystore"); + if (distribution().isDocker()) { + try { + waitForPathToExist(keystore); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + + // Move the auto-created one out of the way, or else the CLI prompts asks us to confirm + sh.run("rm " + keystore); + } else { + if (Files.exists(keystore)) { + FileUtils.rm(keystore); + } + } + } + + private void setKeystorePassword(String password) throws Exception { + final Installation.Executables bin = installation.executables(); + + // set the password by passing it to stdin twice + Platforms.onLinux(() -> { + bin.keystoreTool.run("passwd", password + "\n" + password + "\n"); + }); + + Platforms.onWindows(() -> { + sh.run("Invoke-Command -ScriptBlock {echo \'" + password + "\'; echo \'" + password + "\'} | " + + bin.keystoreTool + " passwd"); + }); + } + + private void assertPasswordProtectedKeystore() { + Shell.Result r = installation.executables().keystoreTool.run("has-passwd"); + assertThat("keystore should be password protected", r.exitCode, is(0)); + } + + private void verifyKeystorePermissions() { + Path keystore = installation.config("elasticsearch.keystore"); + switch (distribution.packaging) { + case TAR: + case ZIP: + assertThat(keystore, file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660)); + break; + case DEB: + case RPM: + assertThat(keystore, file(File, "root", "elasticsearch", p660)); + break; + case DOCKER: + assertPermissionsAndOwnership(keystore, p660); + break; + default: + throw new IllegalStateException("Unknown Elasticsearch packaging type."); + } + } +} diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java index 50799e338ba40..53869967f1878 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java @@ -105,7 +105,7 @@ private void assertRunsWithJavaHome() throws Exception { Files.write(installation.envFile, originalEnvFile); } - assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"), + assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "elasticsearch*.log.gz"), containsString(systemJavaHome)); } @@ -161,6 +161,7 @@ public void test40StartServer() throws Exception { runElasticsearchTests(); verifyPackageInstallation(installation, distribution(), sh); // check startup script didn't change permissions + stopElasticsearch(); } public void test50Remove() throws Exception { diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index c66bb3cb7f329..bf8ad09ad6a6f 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -34,6 +34,8 @@ import org.elasticsearch.packaging.util.Packages; import org.elasticsearch.packaging.util.Platforms; import org.elasticsearch.packaging.util.Shell; +import org.junit.After; +import org.junit.AfterClass; import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; @@ -44,9 +46,12 @@ import org.junit.runner.RunWith; import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; import static org.elasticsearch.packaging.util.Cleanup.cleanEverything; +import static org.elasticsearch.packaging.util.Docker.ensureImageIsLoaded; +import static org.elasticsearch.packaging.util.Docker.removeContainer; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.junit.Assume.assumeFalse; @@ -115,9 +120,23 @@ public static void cleanup() throws Exception { @BeforeClass public static void createShell() throws Exception { - sh = new Shell(); + if (distribution().isDocker()) { + ensureImageIsLoaded(distribution); + sh = new Docker.DockerShell(); + } else { + sh = new Shell(); + } + } + + @AfterClass + public static void cleanupDocker() { + if (distribution().isDocker()) { + // runContainer also calls this, so we don't need this method to be annotated as `@After` + removeContainer(); + } } + @Before public void setup() throws Exception { assumeFalse(failed); // skip rest of tests once one fails @@ -133,6 +152,24 @@ public void setup() throws Exception { } } + @After + public void teardown() throws Exception { + // move log file so we can avoid false positives when grepping for + // messages in logs during test + if (installation != null && Files.exists(installation.logs)) { + Path logFile = installation.logs.resolve("elasticsearch.log"); + String prefix = this.getClass().getSimpleName() + "." + testNameRule.getMethodName(); + if (Files.exists(logFile)) { + Path newFile = installation.logs.resolve(prefix + ".elasticsearch.log"); + FileUtils.mv(logFile, newFile); + } + for (Path rotatedLogFile : FileUtils.lsGlob(installation.logs, "elasticsearch*.tar.gz")) { + Path newRotatedLogFile = installation.logs.resolve(prefix + "." + rotatedLogFile.getFileName()); + FileUtils.mv(rotatedLogFile, newRotatedLogFile); + } + } + } + /** The {@link Distribution} that should be tested in this case */ protected static Distribution distribution() { return distribution; @@ -205,7 +242,7 @@ public Shell.Result runElasticsearchStartCommand() throws Exception { switch (distribution.packaging) { case TAR: case ZIP: - return Archives.runElasticsearchStartCommand(installation, sh); + return Archives.runElasticsearchStartCommand(installation, sh, ""); case DEB: case RPM: return Packages.runElasticsearchStartCommand(sh); @@ -263,7 +300,18 @@ public void startElasticsearch() throws Exception { awaitElasticsearchStartup(runElasticsearchStartCommand()); } - public void assertElasticsearchFailure(Shell.Result result, String expectedMessage) { + public Shell.Result startElasticsearchStandardInputPassword(String password) { + assertTrue("Only archives support passwords on standard input", distribution().isArchive()); + return Archives.runElasticsearchStartCommand(installation, sh, password); + } + + public Shell.Result startElasticsearchTtyPassword(String password) throws Exception { + assertTrue("Only archives support passwords on TTY", distribution().isArchive()); + return Archives.startElasticsearchWithTty(installation, sh, password); + } + + + public void assertElasticsearchFailure(Shell.Result result, String expectedMessage, Packages.JournaldWrapper journaldWrapper) { if (Files.exists(installation.logs.resolve("elasticsearch.log"))) { @@ -277,7 +325,7 @@ public void assertElasticsearchFailure(Shell.Result result, String expectedMessa // For systemd, retrieve the error from journalctl assertThat(result.stderr, containsString("Job for elasticsearch.service failed")); - Shell.Result error = sh.run("journalctl --boot --unit elasticsearch.service"); + Shell.Result error = journaldWrapper.getLogs(); assertThat(error.stdout, containsString(expectedMessage)); } else if (Platforms.WINDOWS == true) { @@ -297,4 +345,5 @@ public void assertElasticsearchFailure(Shell.Result result, String expectedMessa assertThat(result.stderr, containsString(expectedMessage)); } } + } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java index 83ff3fec8fa26..47d6b5bef57f3 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java @@ -244,7 +244,28 @@ private static void verifyDefaultInstallation(Installation es, Distribution dist ).forEach(configFile -> assertThat(es.config(configFile), file(File, owner, owner, p660))); } - public static Shell.Result runElasticsearchStartCommand(Installation installation, Shell sh) { + public static Shell.Result startElasticsearch(Installation installation, Shell sh) { + return runElasticsearchStartCommand(installation, sh, ""); + } + + public static Shell.Result startElasticsearchWithTty(Installation installation, Shell sh, String keystorePassword) throws Exception { + final Path pidFile = installation.home.resolve("elasticsearch.pid"); + final Installation.Executables bin = installation.executables(); + + // requires the "expect" utility to be installed + String script = "expect -c \"$(cat< ELASTICSEARCH_FILES_LINUX = Arrays.asList( "/usr/share/elasticsearch", + "/etc/elasticsearch/elasticsearch.keystore", "/etc/elasticsearch", "/var/lib/elasticsearch", "/var/log/elasticsearch", diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Distribution.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Distribution.java index 13b2f31c7e4fd..72bd79ff7b466 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Distribution.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Distribution.java @@ -64,6 +64,10 @@ public boolean isPackage() { return packaging == Packaging.RPM || packaging == Packaging.DEB; } + public boolean isDocker() { + return packaging == Packaging.DOCKER; + } + public enum Packaging { TAR(".tar.gz", Platforms.LINUX || Platforms.DARWIN), diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index 6040225a05194..f754d0289a6ab 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -61,6 +61,8 @@ public class Docker { private static final Shell sh = new Shell(); private static final DockerShell dockerShell = new DockerShell(); + public static final int STARTUP_SLEEP_INTERVAL_MILLISECONDS = 1000; + public static final int STARTUP_ATTEMPTS_MAX = 10; /** * Tracks the currently running Docker image. An earlier implementation used a fixed container name, @@ -186,18 +188,18 @@ public static void waitForElasticsearchToStart() { do { try { // Give the container a chance to crash out - Thread.sleep(1000); + Thread.sleep(STARTUP_SLEEP_INTERVAL_MILLISECONDS); - psOutput = dockerShell.run("ps -w ax").stdout; + psOutput = dockerShell.run("ps -ww ax").stdout; - if (psOutput.contains("/usr/share/elasticsearch/jdk/bin/java")) { + if (psOutput.contains("org.elasticsearch.bootstrap.Elasticsearch")) { isElasticsearchRunning = true; break; } } catch (Exception e) { logger.warn("Caught exception while waiting for ES to start", e); } - } while (attempt++ < 5); + } while (attempt++ < STARTUP_ATTEMPTS_MAX); if (isElasticsearchRunning == false) { final Shell.Result dockerLogs = getContainerLogs(); @@ -513,6 +515,13 @@ private static void withLogging(CheckedRunnable r) thro } } + /** + * @return The ID of the container that this class will be operating on. + */ + public static String getContainerId() { + return containerId; + } + public static JsonNode getJson(String path) throws Exception { final String pluginsResponse = makeRequest(Request.Get("http://localhost:9200/" + path)); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/FileUtils.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/FileUtils.java index eb57e66239eec..8d1dff077c3a5 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/FileUtils.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/FileUtils.java @@ -153,7 +153,7 @@ public static void append(Path file, String text) { public static String slurp(Path file) { try { - return String.join("\n", Files.readAllLines(file, StandardCharsets.UTF_8)); + return String.join(System.lineSeparator(), Files.readAllLines(file, StandardCharsets.UTF_8)); } catch (IOException e) { throw new RuntimeException(e); } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Platforms.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Platforms.java index b0778bf460ee6..fa324690bf6cc 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Platforms.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Platforms.java @@ -28,6 +28,7 @@ public class Platforms { public static final boolean LINUX = OS_NAME.startsWith("Linux"); public static final boolean WINDOWS = OS_NAME.startsWith("Windows"); public static final boolean DARWIN = OS_NAME.startsWith("Mac OS X"); + public static final PlatformAction NO_ACTION = () -> {}; public static String getOsRelease() { if (LINUX) { diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Shell.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Shell.java index 95141aae17359..d8fd73f6d1ccf 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Shell.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Shell.java @@ -171,7 +171,7 @@ private Result runScriptIgnoreExitCode(String[] command) { readFileIfExists(stdErr) ); throw new IllegalStateException( - "Timed out running shell command: " + command + "\n" + + "Timed out running shell command: " + Arrays.toString(command) + "\n" + "Result:\n" + result ); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/NodesReloadSecureSettingsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/NodesReloadSecureSettingsRequest.java index 1e5e2b07cde7e..7796fba531ce4 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/NodesReloadSecureSettingsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/NodesReloadSecureSettingsRequest.java @@ -19,30 +19,97 @@ package org.elasticsearch.action.admin.cluster.node.reload; +import org.elasticsearch.Version; import org.elasticsearch.action.support.nodes.BaseNodesRequest; import org.elasticsearch.common.io.stream.StreamInput; import java.io.IOException; +import org.elasticsearch.common.CharArrays; +import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.settings.SecureString; + +import java.util.Arrays; + /** - * Request for a reload secure settings action. + * Request for a reload secure settings action */ public class NodesReloadSecureSettingsRequest extends BaseNodesRequest { + /** + * The password is used to re-read and decrypt the contents + * of the node's keystore (backing the implementation of + * {@code SecureSettings}). + */ + @Nullable + private SecureString secureSettingsPassword; + public NodesReloadSecureSettingsRequest() { super((String[]) null); } public NodesReloadSecureSettingsRequest(StreamInput in) throws IOException { super(in); + if (in.getVersion().onOrAfter(Version.V_7_7_0)) { + final BytesReference bytesRef = in.readOptionalBytesReference(); + if (bytesRef != null) { + byte[] bytes = BytesReference.toBytes(bytesRef); + try { + this.secureSettingsPassword = new SecureString(CharArrays.utf8BytesToChars(bytes)); + } finally { + Arrays.fill(bytes, (byte) 0); + } + } else { + this.secureSettingsPassword = null; + } + } } /** - * Reload secure settings only on certain nodes, based on the nodes IDs specified. If none are passed, secure settings will be reloaded - * on all the nodes. + * Reload secure settings only on certain nodes, based on the nodes ids + * specified. If none are passed, secure settings will be reloaded on all the + * nodes. */ - public NodesReloadSecureSettingsRequest(final String... nodesIds) { + public NodesReloadSecureSettingsRequest(String... nodesIds) { super(nodesIds); } + @Nullable + public SecureString getSecureSettingsPassword() { + return secureSettingsPassword; + } + + public void setSecureStorePassword(SecureString secureStorePassword) { + this.secureSettingsPassword = secureStorePassword; + } + + public void closePassword() { + if (this.secureSettingsPassword != null) { + this.secureSettingsPassword.close(); + } + } + + boolean hasPassword() { + return this.secureSettingsPassword != null && this.secureSettingsPassword.length() > 0; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + if (out.getVersion().onOrAfter(Version.V_7_4_0)) { + if (this.secureSettingsPassword == null) { + out.writeOptionalBytesReference(null); + } else { + final byte[] passwordBytes = CharArrays.toUtf8Bytes(this.secureSettingsPassword.getChars()); + try { + out.writeOptionalBytesReference(new BytesArray(passwordBytes)); + } finally { + Arrays.fill(passwordBytes, (byte) 0); + } + } + } + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/NodesReloadSecureSettingsRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/NodesReloadSecureSettingsRequestBuilder.java index c8250455e6ba3..c3c0401efdf17 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/NodesReloadSecureSettingsRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/NodesReloadSecureSettingsRequestBuilder.java @@ -21,6 +21,7 @@ import org.elasticsearch.action.support.nodes.NodesOperationRequestBuilder; import org.elasticsearch.client.ElasticsearchClient; +import org.elasticsearch.common.settings.SecureString; /** * Builder for the reload secure settings nodes request @@ -32,4 +33,9 @@ public NodesReloadSecureSettingsRequestBuilder(ElasticsearchClient client, Nodes super(client, action, new NodesReloadSecureSettingsRequest()); } + public NodesReloadSecureSettingsRequestBuilder setSecureStorePassword(SecureString secureStorePassword) { + request.setSecureStorePassword(secureStorePassword); + return this; + } + } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java index 117b31d9fad79..21c4e4a4336c4 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java @@ -21,16 +21,20 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ExceptionsHelper; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.FailedNodeException; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.nodes.BaseNodeRequest; import org.elasticsearch.action.support.nodes.TransportNodesAction; +import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.KeyStoreWrapper; +import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.plugins.PluginsService; @@ -78,15 +82,39 @@ protected NodesReloadSecureSettingsResponse.NodeResponse newNodeResponse(StreamI return new NodesReloadSecureSettingsResponse.NodeResponse(in); } + @Override + protected void doExecute(Task task, NodesReloadSecureSettingsRequest request, + ActionListener listener) { + if (request.hasPassword() && isNodeLocal(request) == false && isNodeTransportTLSEnabled() == false) { + request.closePassword(); + listener.onFailure( + new ElasticsearchException("Secure settings cannot be updated cluster wide when TLS for the transport layer" + + " is not enabled. Enable TLS or use the API with a `_local` filter on each node.")); + } else { + super.doExecute(task, request, ActionListener.wrap(response -> { + request.closePassword(); + listener.onResponse(response); + }, e -> { + request.closePassword(); + listener.onFailure(e); + })); + } + } + @Override protected NodesReloadSecureSettingsResponse.NodeResponse nodeOperation(NodeRequest nodeReloadRequest, Task task) { + final NodesReloadSecureSettingsRequest request = nodeReloadRequest.request; + // We default to using an empty string as the keystore password so that we mimic pre 7.3 API behavior + final SecureString secureSettingsPassword = request.hasPassword() ? request.getSecureSettingsPassword() : + new SecureString(new char[0]); try (KeyStoreWrapper keystore = KeyStoreWrapper.load(environment.configFile())) { // reread keystore from config file if (keystore == null) { return new NodesReloadSecureSettingsResponse.NodeResponse(clusterService.localNode(), new IllegalStateException("Keystore is missing")); } - keystore.decrypt(new char[0]); + // decrypt the keystore using the password from the request + keystore.decrypt(secureSettingsPassword.getChars()); // add the keystore to the original node settings object final Settings settingsWithKeystore = Settings.builder() .put(environment.settings(), false) @@ -107,6 +135,8 @@ protected NodesReloadSecureSettingsResponse.NodeResponse nodeOperation(NodeReque return new NodesReloadSecureSettingsResponse.NodeResponse(clusterService.localNode(), null); } catch (final Exception e) { return new NodesReloadSecureSettingsResponse.NodeResponse(clusterService.localNode(), e); + } finally { + secureSettingsPassword.close(); } } @@ -129,4 +159,20 @@ public void writeTo(StreamOutput out) throws IOException { request.writeTo(out); } } + + /** + * Returns true if the node is configured for TLS on the transport layer + */ + private boolean isNodeTransportTLSEnabled() { + return transportService.isTransportSecure(); + } + + private boolean isNodeLocal(NodesReloadSecureSettingsRequest request) { + if (null == request.concreteNodes()) { + resolveRequest(request, clusterService.state()); + assert request.concreteNodes() != null; + } + final DiscoveryNode[] nodes = request.concreteNodes(); + return nodes.length == 1 && nodes[0].getId().equals(clusterService.localNode().getId()); + } } diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index f59f082596d7b..10951e26a9636 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -26,6 +26,9 @@ import org.apache.logging.log4j.core.appender.ConsoleAppender; import org.apache.logging.log4j.core.config.Configurator; import org.apache.lucene.util.Constants; +import org.elasticsearch.cli.KeyStoreAwareCommand; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.core.internal.io.IOUtils; import org.apache.lucene.util.StringHelper; import org.elasticsearch.ElasticsearchException; @@ -51,9 +54,12 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; import java.io.PrintStream; import java.io.UnsupportedEncodingException; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.security.NoSuchAlgorithmException; import java.util.Collections; @@ -234,14 +240,25 @@ static SecureSettings loadSecureSettings(Environment initialEnv) throws Bootstra throw new BootstrapException(e); } + SecureString password; try { + if (keystore != null && keystore.hasPassword()) { + password = readPassphrase(System.in, KeyStoreAwareCommand.MAX_PASSPHRASE_LENGTH); + } else { + password = new SecureString(new char[0]); + } + } catch (IOException e) { + throw new BootstrapException(e); + } + + try (password) { if (keystore == null) { final KeyStoreWrapper keyStoreWrapper = KeyStoreWrapper.create(); keyStoreWrapper.save(initialEnv.configFile(), new char[0]); return keyStoreWrapper; } else { - keystore.decrypt(new char[0] /* TODO: read password from stdin */); - KeyStoreWrapper.upgrade(keystore, initialEnv.configFile(), new char[0]); + keystore.decrypt(password.getChars()); + KeyStoreWrapper.upgrade(keystore, initialEnv.configFile(), password.getChars()); } } catch (Exception e) { throw new BootstrapException(e); @@ -249,6 +266,31 @@ static SecureSettings loadSecureSettings(Environment initialEnv) throws Bootstra return keystore; } + // visible for tests + /** + * Read from an InputStream up to the first carriage return or newline, + * returning no more than maxLength characters. + */ + static SecureString readPassphrase(InputStream stream, int maxLength) throws IOException { + SecureString passphrase; + + try(InputStreamReader reader = new InputStreamReader(stream, StandardCharsets.UTF_8)) { + passphrase = new SecureString(Terminal.readLineToCharArray(reader, maxLength)); + } catch (RuntimeException e) { + if (e.getMessage().startsWith("Input exceeded maximum length")) { + throw new IllegalStateException("Password exceeded maximum length of " + maxLength, e); + } + throw e; + } + + if (passphrase.length() == 0) { + passphrase.close(); + throw new IllegalStateException("Keystore passphrase required but none provided."); + } + + return passphrase; + } + private static Environment createEnvironment( final Path pidFile, final SecureSettings secureSettings, diff --git a/server/src/main/java/org/elasticsearch/cli/KeyStoreAwareCommand.java b/server/src/main/java/org/elasticsearch/cli/KeyStoreAwareCommand.java new file mode 100644 index 0000000000000..dd26366f9b071 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/cli/KeyStoreAwareCommand.java @@ -0,0 +1,86 @@ +/* + * 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.cli; + +import joptsimple.OptionSet; +import org.elasticsearch.common.settings.KeyStoreWrapper; +import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.env.Environment; + +import javax.crypto.AEADBadTagException; +import java.io.IOException; +import java.security.GeneralSecurityException; +import java.util.Arrays; + +/** + * An {@link org.elasticsearch.cli.EnvironmentAwareCommand} that needs to access the elasticsearch keystore, possibly + * decrypting it if it is password protected. + */ +public abstract class KeyStoreAwareCommand extends EnvironmentAwareCommand { + public KeyStoreAwareCommand(String description) { + super(description); + } + + /** Arbitrarily chosen maximum passphrase length */ + public static final int MAX_PASSPHRASE_LENGTH = 128; + + /** + * 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 + */ + protected 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): ", + MAX_PASSPHRASE_LENGTH); + char[] passwordVerification = terminal.readSecret("Enter same password again: ", + MAX_PASSPHRASE_LENGTH); + 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 : "); + } + return new SecureString(passwordArray); + } + + /** + * Decrypt the {@code keyStore}, prompting the user to enter the password in the {@link Terminal} if it is password protected + */ + protected static void decryptKeyStore(KeyStoreWrapper keyStore, Terminal terminal) + throws UserException, GeneralSecurityException, IOException { + try (SecureString keystorePassword = keyStore.hasPassword() ? + readPassword(terminal, false) : new SecureString(new char[0])) { + keyStore.decrypt(keystorePassword.getChars()); + } catch (SecurityException e) { + if (e.getCause() instanceof AEADBadTagException) { + throw new UserException(ExitCodes.DATA_ERROR, "Wrong password for elasticsearch.keystore"); + } + } + } + + protected abstract void execute(Terminal terminal, OptionSet options, Environment env) 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 db37892265507..d3080df034ca3 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -34,6 +34,7 @@ import org.elasticsearch.common.Randomness; import org.elasticsearch.common.hash.MessageDigests; +import javax.crypto.AEADBadTagException; import javax.crypto.Cipher; import javax.crypto.CipherInputStream; import javax.crypto.CipherOutputStream; @@ -378,6 +379,9 @@ 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); } } @@ -580,7 +584,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); @@ -593,7 +599,9 @@ synchronized void setString(String setting, char[] value) { } } - /** Set a file setting. */ + /** + * Set a file setting. + */ synchronized void setFile(String setting, byte[] bytes) { ensureOpen(); validateSettingName(setting); @@ -604,7 +612,9 @@ synchronized void setFile(String setting, byte[] bytes) { } } - /** 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); diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsAction.java index cb21c7e30dac0..e5f85c569ee6b 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsAction.java @@ -19,10 +19,14 @@ package org.elasticsearch.rest.action.admin.cluster; +import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsRequest; import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsRequestBuilder; import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsResponse; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.BytesRestResponse; @@ -39,6 +43,14 @@ public final class RestReloadSecureSettingsAction extends BaseRestHandler { + static final ObjectParser PARSER = + new ObjectParser<>("reload_secure_settings", NodesReloadSecureSettingsRequest::new); + + static { + PARSER.declareString((request, value) -> request.setSecureStorePassword(new SecureString(value.toCharArray())), + new ParseField("secure_settings_password")); + } + public RestReloadSecureSettingsAction(RestController controller) { controller.registerHandler(POST, "/_nodes/reload_secure_settings", this); controller.registerHandler(POST, "/_nodes/{nodeId}/reload_secure_settings", this); @@ -53,22 +65,28 @@ public String getName() { public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { final String[] nodesIds = Strings.splitStringByCommaToArray(request.param("nodeId")); final NodesReloadSecureSettingsRequestBuilder nodesRequestBuilder = client.admin() - .cluster() - .prepareReloadSecureSettings() - .setTimeout(request.param("timeout")) - .setNodesIds(nodesIds); + .cluster() + .prepareReloadSecureSettings() + .setTimeout(request.param("timeout")) + .setNodesIds(nodesIds); + request.withContentOrSourceParamParserOrNull(parser -> { + if (parser != null) { + final NodesReloadSecureSettingsRequest nodesRequest = nodesRequestBuilder.request(); + nodesRequestBuilder.setSecureStorePassword(nodesRequest.getSecureSettingsPassword()); + } + }); + return channel -> nodesRequestBuilder .execute(new RestBuilderListener(channel) { @Override public RestResponse buildResponse(NodesReloadSecureSettingsResponse response, XContentBuilder builder) - throws Exception { + throws Exception { builder.startObject(); - { - RestActions.buildNodesHeader(builder, channel.request(), response); - builder.field("cluster_name", response.getClusterName().value()); - response.toXContent(builder, channel.request()); - } + RestActions.buildNodesHeader(builder, channel.request(), response); + builder.field("cluster_name", response.getClusterName().value()); + response.toXContent(builder, channel.request()); builder.endObject(); + nodesRequestBuilder.request().closePassword(); return new BytesRestResponse(RestStatus.OK, builder); } }); diff --git a/server/src/main/java/org/elasticsearch/transport/Transport.java b/server/src/main/java/org/elasticsearch/transport/Transport.java index 1a2ff9eafe7ea..f7c51e8b63e7c 100644 --- a/server/src/main/java/org/elasticsearch/transport/Transport.java +++ b/server/src/main/java/org/elasticsearch/transport/Transport.java @@ -54,6 +54,10 @@ public interface Transport extends LifecycleComponent { void setLocalNode(DiscoveryNode localNode); + default boolean isSecure() { + return false; + } + /** * The address the transport is bound on. */ diff --git a/server/src/main/java/org/elasticsearch/transport/TransportService.java b/server/src/main/java/org/elasticsearch/transport/TransportService.java index 72590d1b3369e..9b6827ed65151 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportService.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportService.java @@ -301,6 +301,10 @@ public TransportStats stats() { return transport.getStats(); } + public boolean isTransportSecure() { + return transport.isSecure(); + } + public BoundTransportAddress boundAddress() { return transport.boundAddress(); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java index 3f9e258ffec1c..fbd3fe0432e6c 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java @@ -19,10 +19,13 @@ package org.elasticsearch.action.admin; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsResponse; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.KeyStoreWrapper; import org.elasticsearch.common.settings.SecureSettings; +import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.plugins.Plugin; @@ -42,50 +45,53 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicReference; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.containsString; +@ESIntegTestCase.ClusterScope(minNumDataNodes = 2) public class ReloadSecureSettingsIT extends ESIntegTestCase { public void testMissingKeystoreFile() throws Exception { final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) - .stream().findFirst().get(); + .stream().findFirst().get(); final Environment environment = internalCluster().getInstance(Environment.class); final AtomicReference reloadSettingsError = new AtomicReference<>(); // keystore file should be missing for this test case Files.deleteIfExists(KeyStoreWrapper.keystorePath(environment.configFile())); final int initialReloadCount = mockReloadablePlugin.getReloadCount(); final CountDownLatch latch = new CountDownLatch(1); - client().admin().cluster().prepareReloadSecureSettings().execute( - new ActionListener() { - @Override - public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { - try { - assertThat(nodesReloadResponse, notNullValue()); - final Map nodesMap = nodesReloadResponse.getNodesMap(); - assertThat(nodesMap.size(), equalTo(cluster().size())); - for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { - assertThat(nodeResponse.reloadException(), notNullValue()); - assertThat(nodeResponse.reloadException(), instanceOf(IllegalStateException.class)); - assertThat(nodeResponse.reloadException().getMessage(), containsString("Keystore is missing")); - } - } catch (final AssertionError e) { - reloadSettingsError.set(e); - } finally { - latch.countDown(); + final SecureString emptyPassword = randomBoolean() ? new SecureString(new char[0]) : null; + client().admin().cluster().prepareReloadSecureSettings().setSecureStorePassword(emptyPassword) + .setNodesIds(Strings.EMPTY_ARRAY).execute( + new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + try { + assertThat(nodesReloadResponse, notNullValue()); + final Map nodesMap = nodesReloadResponse.getNodesMap(); + assertThat(nodesMap.size(), equalTo(cluster().size())); + for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { + assertThat(nodeResponse.reloadException(), notNullValue()); + assertThat(nodeResponse.reloadException(), instanceOf(IllegalStateException.class)); + assertThat(nodeResponse.reloadException().getMessage(), containsString("Keystore is missing")); } - } - - @Override - public void onFailure(Exception e) { - reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + } catch (final AssertionError e) { + reloadSettingsError.set(e); + } finally { latch.countDown(); } - }); + } + + @Override + public void onFailure(Exception e) { + reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + latch.countDown(); + } + }); latch.await(); if (reloadSettingsError.get() != null) { throw reloadSettingsError.get(); @@ -97,7 +103,7 @@ public void onFailure(Exception e) { public void testInvalidKeystoreFile() throws Exception { final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) - .stream().findFirst().get(); + .stream().findFirst().get(); final Environment environment = internalCluster().getInstance(Environment.class); final AtomicReference reloadSettingsError = new AtomicReference<>(); final int initialReloadCount = mockReloadablePlugin.getReloadCount(); @@ -109,35 +115,163 @@ public void testInvalidKeystoreFile() throws Exception { Files.copy(keystore, KeyStoreWrapper.keystorePath(environment.configFile()), StandardCopyOption.REPLACE_EXISTING); } final CountDownLatch latch = new CountDownLatch(1); - client().admin().cluster().prepareReloadSecureSettings().execute( - new ActionListener() { - @Override - public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { - try { - assertThat(nodesReloadResponse, notNullValue()); - final Map nodesMap = nodesReloadResponse.getNodesMap(); - assertThat(nodesMap.size(), equalTo(cluster().size())); - for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { - assertThat(nodeResponse.reloadException(), notNullValue()); - } - } catch (final AssertionError e) { - reloadSettingsError.set(e); - } finally { - latch.countDown(); + final SecureString emptyPassword = randomBoolean() ? new SecureString(new char[0]) : null; + client().admin().cluster().prepareReloadSecureSettings().setSecureStorePassword(emptyPassword) + .setNodesIds(Strings.EMPTY_ARRAY).execute( + new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + try { + assertThat(nodesReloadResponse, notNullValue()); + final Map nodesMap = nodesReloadResponse.getNodesMap(); + assertThat(nodesMap.size(), equalTo(cluster().size())); + for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { + assertThat(nodeResponse.reloadException(), notNullValue()); } + } catch (final AssertionError e) { + reloadSettingsError.set(e); + } finally { + latch.countDown(); + } + } + + @Override + public void onFailure(Exception e) { + reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + latch.countDown(); + } + }); + latch.await(); + if (reloadSettingsError.get() != null) { + throw reloadSettingsError.get(); + } + // in the invalid keystore format case no reload should be triggered + assertThat(mockReloadablePlugin.getReloadCount(), equalTo(initialReloadCount)); + } + + public void testReloadAllNodesWithPasswordWithoutTLSFails() throws Exception { + final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); + final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) + .stream().findFirst().get(); + final Environment environment = internalCluster().getInstance(Environment.class); + final AtomicReference reloadSettingsError = new AtomicReference<>(); + final int initialReloadCount = mockReloadablePlugin.getReloadCount(); + final char[] password = randomAlphaOfLength(12).toCharArray(); + writeEmptyKeystore(environment, password); + final CountDownLatch latch = new CountDownLatch(1); + client().admin() + .cluster() + .prepareReloadSecureSettings() + // No filter should try to hit all nodes + .setNodesIds(Strings.EMPTY_ARRAY) + .setSecureStorePassword(new SecureString(password)) + .execute(new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + reloadSettingsError.set(new AssertionError("Nodes request succeeded when it should have failed", null)); + latch.countDown(); + } + + @Override + public void onFailure(Exception e) { + assertThat(e, instanceOf(ElasticsearchException.class)); + assertThat(e.getMessage(), + containsString("Secure settings cannot be updated cluster wide when TLS for the transport layer is not enabled")); + latch.countDown(); + } + }); + latch.await(); + if (reloadSettingsError.get() != null) { + throw reloadSettingsError.get(); + } + //no reload should be triggered + assertThat(mockReloadablePlugin.getReloadCount(), equalTo(initialReloadCount)); + } + + public void testReloadLocalNodeWithPasswordWithoutTLSSucceeds() throws Exception { + final Environment environment = internalCluster().getInstance(Environment.class); + final AtomicReference reloadSettingsError = new AtomicReference<>(); + final char[] password = randomAlphaOfLength(12).toCharArray(); + writeEmptyKeystore(environment, password); + final CountDownLatch latch = new CountDownLatch(1); + client().admin() + .cluster() + .prepareReloadSecureSettings() + .setNodesIds("_local") + .setSecureStorePassword(new SecureString(password)) + .execute(new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + try { + assertThat(nodesReloadResponse, notNullValue()); + final Map nodesMap = nodesReloadResponse.getNodesMap(); + assertThat(nodesMap.size(), equalTo(1)); + assertThat(nodesReloadResponse.getNodes().size(), equalTo(1)); + final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse = nodesReloadResponse.getNodes().get(0); + assertThat(nodeResponse.reloadException(), nullValue()); + } catch (final AssertionError e) { + reloadSettingsError.set(e); + } finally { + latch.countDown(); } + } + + @Override + public void onFailure(Exception e) { + reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + latch.countDown(); + } + }); + latch.await(); + if (reloadSettingsError.get() != null) { + throw reloadSettingsError.get(); + } + } - @Override - public void onFailure(Exception e) { - reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + public void testWrongKeystorePassword() throws Exception { + final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); + final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) + .stream().findFirst().get(); + final Environment environment = internalCluster().getInstance(Environment.class); + final AtomicReference reloadSettingsError = new AtomicReference<>(); + final int initialReloadCount = mockReloadablePlugin.getReloadCount(); + // "some" keystore should be present in this case + writeEmptyKeystore(environment, new char[0]); + final CountDownLatch latch = new CountDownLatch(1); + client().admin() + .cluster() + .prepareReloadSecureSettings() + .setNodesIds("_local") + .setSecureStorePassword(new SecureString(new char[]{'W', 'r', 'o', 'n', 'g'})) + .execute(new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + try { + assertThat(nodesReloadResponse, notNullValue()); + final Map nodesMap = nodesReloadResponse.getNodesMap(); + assertThat(nodesMap.size(), equalTo(1)); + for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { + assertThat(nodeResponse.reloadException(), notNullValue()); + assertThat(nodeResponse.reloadException(), instanceOf(SecurityException.class)); + } + } catch (final AssertionError e) { + reloadSettingsError.set(e); + } finally { latch.countDown(); } - }); + } + + @Override + public void onFailure(Exception e) { + reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + latch.countDown(); + } + }); latch.await(); if (reloadSettingsError.get() != null) { throw reloadSettingsError.get(); } - // in the invalid keystore format case no reload should be triggered + // in the wrong password case no reload should be triggered assertThat(mockReloadablePlugin.getReloadCount(), equalTo(initialReloadCount)); } @@ -145,12 +279,12 @@ public void testMisbehavingPlugin() throws Exception { final Environment environment = internalCluster().getInstance(Environment.class); final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) - .stream().findFirst().get(); + .stream().findFirst().get(); // make plugins throw on reload for (final String nodeName : internalCluster().getNodeNames()) { internalCluster().getInstance(PluginsService.class, nodeName) - .filterPlugins(MisbehavingReloadablePlugin.class) - .stream().findFirst().get().setShouldThrow(true); + .filterPlugins(MisbehavingReloadablePlugin.class) + .stream().findFirst().get().setShouldThrow(true); } final AtomicReference reloadSettingsError = new AtomicReference<>(); final int initialReloadCount = mockReloadablePlugin.getReloadCount(); @@ -158,34 +292,36 @@ public void testMisbehavingPlugin() throws Exception { final SecureSettings secureSettings = writeEmptyKeystore(environment, new char[0]); // read seed setting value from the test case (not from the node) final String seedValue = KeyStoreWrapper.SEED_SETTING - .get(Settings.builder().put(environment.settings()).setSecureSettings(secureSettings).build()) - .toString(); + .get(Settings.builder().put(environment.settings()).setSecureSettings(secureSettings).build()) + .toString(); final CountDownLatch latch = new CountDownLatch(1); - client().admin().cluster().prepareReloadSecureSettings().execute( - new ActionListener() { - @Override - public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { - try { - assertThat(nodesReloadResponse, notNullValue()); - final Map nodesMap = nodesReloadResponse.getNodesMap(); - assertThat(nodesMap.size(), equalTo(cluster().size())); - for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { - assertThat(nodeResponse.reloadException(), notNullValue()); - assertThat(nodeResponse.reloadException().getMessage(), containsString("If shouldThrow I throw")); - } - } catch (final AssertionError e) { - reloadSettingsError.set(e); - } finally { - latch.countDown(); + final SecureString emptyPassword = randomBoolean() ? new SecureString(new char[0]) : null; + client().admin().cluster().prepareReloadSecureSettings().setSecureStorePassword(emptyPassword) + .setNodesIds(Strings.EMPTY_ARRAY).execute( + new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + try { + assertThat(nodesReloadResponse, notNullValue()); + final Map nodesMap = nodesReloadResponse.getNodesMap(); + assertThat(nodesMap.size(), equalTo(cluster().size())); + for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { + assertThat(nodeResponse.reloadException(), notNullValue()); + assertThat(nodeResponse.reloadException().getMessage(), containsString("If shouldThrow I throw")); } - } - - @Override - public void onFailure(Exception e) { - reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + } catch (final AssertionError e) { + reloadSettingsError.set(e); + } finally { latch.countDown(); } - }); + } + + @Override + public void onFailure(Exception e) { + reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + latch.countDown(); + } + }); latch.await(); if (reloadSettingsError.get() != null) { throw reloadSettingsError.get(); @@ -200,7 +336,7 @@ public void onFailure(Exception e) { public void testReloadWhileKeystoreChanged() throws Exception { final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) - .stream().findFirst().get(); + .stream().findFirst().get(); final Environment environment = internalCluster().getInstance(Environment.class); final int initialReloadCount = mockReloadablePlugin.getReloadCount(); for (int i = 0; i < randomIntBetween(4, 8); i++) { @@ -208,8 +344,8 @@ public void testReloadWhileKeystoreChanged() throws Exception { final SecureSettings secureSettings = writeEmptyKeystore(environment, new char[0]); // read seed setting value from the test case (not from the node) final String seedValue = KeyStoreWrapper.SEED_SETTING - .get(Settings.builder().put(environment.settings()).setSecureSettings(secureSettings).build()) - .toString(); + .get(Settings.builder().put(environment.settings()).setSecureSettings(secureSettings).build()) + .toString(); // reload call successfulReloadCall(); assertThat(mockReloadablePlugin.getSeedValue(), equalTo(seedValue)); @@ -228,30 +364,32 @@ protected Collection> nodePlugins() { private void successfulReloadCall() throws InterruptedException { final AtomicReference reloadSettingsError = new AtomicReference<>(); final CountDownLatch latch = new CountDownLatch(1); - client().admin().cluster().prepareReloadSecureSettings().execute( - new ActionListener() { - @Override - public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { - try { - assertThat(nodesReloadResponse, notNullValue()); - final Map nodesMap = nodesReloadResponse.getNodesMap(); - assertThat(nodesMap.size(), equalTo(cluster().size())); - for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { - assertThat(nodeResponse.reloadException(), nullValue()); - } - } catch (final AssertionError e) { - reloadSettingsError.set(e); - } finally { - latch.countDown(); + final SecureString emptyPassword = randomBoolean() ? new SecureString(new char[0]) : null; + client().admin().cluster().prepareReloadSecureSettings().setSecureStorePassword(emptyPassword) + .setNodesIds(Strings.EMPTY_ARRAY).execute( + new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + try { + assertThat(nodesReloadResponse, notNullValue()); + final Map nodesMap = nodesReloadResponse.getNodesMap(); + assertThat(nodesMap.size(), equalTo(cluster().size())); + for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { + assertThat(nodeResponse.reloadException(), nullValue()); } - } - - @Override - public void onFailure(Exception e) { - reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + } catch (final AssertionError e) { + reloadSettingsError.set(e); + } finally { latch.countDown(); } - }); + } + + @Override + public void onFailure(Exception e) { + reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + latch.countDown(); + } + }); latch.await(); if (reloadSettingsError.get() != null) { throw reloadSettingsError.get(); diff --git a/server/src/test/java/org/elasticsearch/cli/MultiCommandTests.java b/server/src/test/java/org/elasticsearch/cli/MultiCommandTests.java index 38c0edaee801e..736b19aaef067 100644 --- a/server/src/test/java/org/elasticsearch/cli/MultiCommandTests.java +++ b/server/src/test/java/org/elasticsearch/cli/MultiCommandTests.java @@ -29,6 +29,9 @@ import java.util.concurrent.atomic.AtomicBoolean; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.emptyString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; public class MultiCommandTests extends CommandTestCase { @@ -200,4 +203,55 @@ public void testCloseWhenSubCommandCloseThrowsException() throws Exception { assertTrue("SubCommand2 was not closed when close method is invoked", subCommand2.closeCalled.get()); } + // Tests for multicommand error logging + + static class ErrorHandlingMultiCommand extends MultiCommand { + ErrorHandlingMultiCommand() { + super("error catching", () -> {}); + } + + @Override + protected boolean addShutdownHook() { + return false; + } + } + + static class ErrorThrowingSubCommand extends Command { + ErrorThrowingSubCommand() { + super("error throwing", () -> {}); + } + @Override + protected void execute(Terminal terminal, OptionSet options) throws Exception { + throw new UserException(1, "Dummy error"); + } + + @Override + protected boolean addShutdownHook() { + return false; + } + } + + public void testErrorDisplayedWithDefault() throws Exception { + MockTerminal terminal = new MockTerminal(); + MultiCommand mc = new ErrorHandlingMultiCommand(); + mc.subcommands.put("throw", new ErrorThrowingSubCommand()); + mc.main(new String[]{"throw", "--silent"}, terminal); + assertThat(terminal.getOutput(), is(emptyString())); + assertThat(terminal.getErrorOutput(), equalTo("ERROR: Dummy error\n")); + } + + public void testNullErrorMessageSuppressesErrorOutput() throws Exception { + MockTerminal terminal = new MockTerminal(); + MultiCommand mc = new ErrorHandlingMultiCommand(); + mc.subcommands.put("throw", new ErrorThrowingSubCommand() { + @Override + protected void execute(Terminal terminal, OptionSet options) throws Exception { + throw new UserException(1, null); + } + }); + mc.main(new String[]{"throw", "--silent"}, terminal); + assertThat(terminal.getOutput(), is(emptyString())); + assertThat(terminal.getErrorOutput(), is(emptyString())); + } + } diff --git a/server/src/test/java/org/elasticsearch/cli/TerminalTests.java b/server/src/test/java/org/elasticsearch/cli/TerminalTests.java index 99bbe9d618441..85b8ec5bf2684 100644 --- a/server/src/test/java/org/elasticsearch/cli/TerminalTests.java +++ b/server/src/test/java/org/elasticsearch/cli/TerminalTests.java @@ -21,7 +21,14 @@ import org.elasticsearch.test.ESTestCase; +import java.io.BufferedReader; +import java.io.StringReader; + +import static org.elasticsearch.cli.Terminal.readLineToCharArray; +import static org.hamcrest.Matchers.equalTo; + public class TerminalTests extends ESTestCase { + public void testVerbosity() throws Exception { MockTerminal terminal = new MockTerminal(); terminal.setVerbosity(Terminal.Verbosity.SILENT); @@ -95,6 +102,22 @@ public void testPromptYesNoCase() throws Exception { assertFalse(terminal.promptYesNo("Answer?", true)); } + public void testMaxSecretLength() throws Exception { + MockTerminal terminal = new MockTerminal(); + String secret = "A very long secret, too long in fact for our purposes."; + terminal.addSecretInput(secret); + + expectThrows(IllegalStateException.class, "Secret exceeded maximum length of ", + () -> terminal.readSecret("Secret? ", secret.length() - 1)); + } + + public void testTerminalReusesBufferedReaders() throws Exception { + Terminal.SystemTerminal terminal = new Terminal.SystemTerminal(); + BufferedReader reader1 = terminal.getReader(); + BufferedReader reader2 = terminal.getReader(); + assertSame("System terminal should not create multiple buffered readers", reader1, reader2); + } + private void assertPrinted(MockTerminal logTerminal, Terminal.Verbosity verbosity, String text) throws Exception { logTerminal.println(verbosity, text); String output = logTerminal.getOutput(); @@ -121,4 +144,47 @@ private void assertErrorNotPrinted(MockTerminal logTerminal, Terminal.Verbosity assertTrue(output, output.isEmpty()); } + public void testSystemTerminalReadsSingleLines() throws Exception { + assertRead("\n", ""); + assertRead("\r\n", ""); + + assertRead("hello\n", "hello"); + assertRead("hello\r\n", "hello"); + + assertRead("hellohello\n", "hellohello"); + assertRead("hellohello\r\n", "hellohello"); + } + + public void testSystemTerminalReadsMultipleLines() throws Exception { + assertReadLines("hello\nhello\n", "hello", "hello"); + assertReadLines("hello\r\nhello\r\n", "hello", "hello"); + + assertReadLines("one\ntwo\n\nthree", "one", "two", "", "three"); + assertReadLines("one\r\ntwo\r\n\r\nthree", "one", "two", "", "three"); + } + + public void testSystemTerminalLineExceedsMaxCharacters() throws Exception { + try (StringReader reader = new StringReader("hellohellohello!\n")) { + expectThrows(RuntimeException.class, "Input exceeded maximum length of 10", + () -> readLineToCharArray(reader, 10)); + } + } + + private void assertRead(String source, String expected) { + try (StringReader reader = new StringReader(source)) { + char[] result = readLineToCharArray(reader, 10); + assertThat(result, equalTo(expected.toCharArray())); + } + } + + private void assertReadLines(String source, String... expected) { + try (StringReader reader = new StringReader(source)) { + char[] result; + for (String exp : expected) { + result = readLineToCharArray(reader, 10); + assertThat(result, equalTo(exp.toCharArray())); + } + } + } + } diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsActionTests.java new file mode 100644 index 0000000000000..7dfd294e8ae34 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsActionTests.java @@ -0,0 +1,53 @@ +/* + * 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.rest.action.admin.cluster; + +import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsRequest; +import org.elasticsearch.common.xcontent.DeprecationHandler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.test.ESTestCase; + +import static org.hamcrest.Matchers.nullValue; + +public class RestReloadSecureSettingsActionTests extends ESTestCase { + + public void testParserWithPassword() throws Exception { + final String request = "{" + + "\"secure_settings_password\": \"secure_settings_password_string\"" + + "}"; + try (XContentParser parser = XContentType.JSON.xContent() + .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, request)) { + NodesReloadSecureSettingsRequest reloadSecureSettingsRequest = RestReloadSecureSettingsAction.PARSER.parse(parser, null); + assertEquals("secure_settings_password_string", reloadSecureSettingsRequest.getSecureSettingsPassword().toString()); + } + } + + public void testParserWithoutPassword() throws Exception { + final String request = "{" + + "}"; + try (XContentParser parser = XContentType.JSON.xContent() + .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, request)) { + NodesReloadSecureSettingsRequest reloadSecureSettingsRequest = RestReloadSecureSettingsAction.PARSER.parse(parser, null); + assertThat(reloadSecureSettingsRequest.getSecureSettingsPassword(), nullValue()); + } + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java index e9c6a2eec9c31..e8a518dffd721 100644 --- a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java @@ -30,9 +30,6 @@ public abstract class CommandTestCase extends ESTestCase { /** The terminal that execute uses. */ protected final MockTerminal terminal = new MockTerminal(); - /** The last command that was executed. */ - protected Command command; - @Before public void resetTerminal() { terminal.reset(); @@ -43,13 +40,20 @@ public void resetTerminal() { protected abstract Command newCommand(); /** - * Runs the command with the given args. + * Runs a command with the given args. * * Output can be found in {@link #terminal}. - * The command created can be found in {@link #command}. */ public String execute(String... args) throws Exception { - command = newCommand(); + return execute(newCommand(), args); + } + + /** + * Runs the specified command with the given args. + *

+ * Output can be found in {@link #terminal}. + */ + public String execute(Command command, String... args) throws Exception { command.mainWithoutErrorHandling(args, terminal); return terminal.getOutput(); } diff --git a/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java index cff5c1b49fbc7..4959e6436f487 100644 --- a/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java +++ b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java @@ -85,7 +85,7 @@ public void addTextInput(String input) { textInput.add(input); } - /** Adds an an input that will be return from {@link #readText(String)}. Values are read in FIFO order. */ + /** Adds an an input that will be return from {@link #readSecret(String)}. Values are read in FIFO order. */ public void addSecretInput(String input) { secretInput.add(input); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/netty4/SecurityNetty4Transport.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/netty4/SecurityNetty4Transport.java index 6e2b9c1a7efdd..624b90125b0db 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/netty4/SecurityNetty4Transport.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/transport/netty4/SecurityNetty4Transport.java @@ -131,6 +131,11 @@ protected ServerChannelInitializer getSslChannelInitializer(final String name, f return new SslChannelInitializer(name, sslConfiguration); } + @Override + public boolean isSecure() { + return this.sslEnabled; + } + private class SecurityClientChannelInitializer extends ClientChannelInitializer { private final boolean hostnameVerificationEnabled; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java index 9f853134d00b7..c2e72fb1faa3f 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java @@ -9,8 +9,8 @@ import joptsimple.OptionSet; import joptsimple.OptionSpec; import org.elasticsearch.ExceptionsHelper; -import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.KeyStoreAwareCommand; import org.elasticsearch.cli.LoggingAwareMultiCommand; import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.Terminal.Verbosity; @@ -123,7 +123,7 @@ class AutoSetup extends SetupCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { terminal.println(Verbosity.VERBOSE, "Running with configuration path: " + env.configFile()); - setupOptions(options, env); + setupOptions(terminal, options, env); checkElasticKeystorePasswordValid(terminal, env); checkClusterHealth(terminal); @@ -169,7 +169,7 @@ class InteractiveSetup extends SetupCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { terminal.println(Verbosity.VERBOSE, "Running with configuration path: " + env.configFile()); - setupOptions(options, env); + setupOptions(terminal, options, env); checkElasticKeystorePasswordValid(terminal, env); checkClusterHealth(terminal); @@ -219,7 +219,7 @@ private void changedPasswordCallback(Terminal terminal, String user, SecureStrin * An abstract class that provides functionality common to both the auto and * interactive setup modes. */ - private abstract class SetupCommand extends EnvironmentAwareCommand { + private abstract class SetupCommand extends KeyStoreAwareCommand { boolean shouldPrompt; @@ -246,10 +246,9 @@ public void close() { } } - void setupOptions(OptionSet options, Environment env) throws Exception { + void setupOptions(Terminal terminal, OptionSet options, Environment env) throws Exception { keyStoreWrapper = keyStoreFunction.apply(env); - // TODO: We currently do not support keystore passwords - keyStoreWrapper.decrypt(new char[0]); + decryptKeyStore(keyStoreWrapper, terminal); Settings.Builder settingsBuilder = Settings.builder(); settingsBuilder.put(env.settings(), true); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java index 68be01a2e3fb9..3a2b87afe1fa6 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java @@ -32,8 +32,8 @@ import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.KeyStoreAwareCommand; import org.elasticsearch.cli.SuppressForbidden; import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.UserException; @@ -68,7 +68,7 @@ /** * CLI tool to generate SAML Metadata for a Service Provider (realm) */ -public class SamlMetadataCommand extends EnvironmentAwareCommand { +public class SamlMetadataCommand extends KeyStoreAwareCommand { static final String METADATA_SCHEMA = "saml-schema-metadata-2.0.xsd"; @@ -415,13 +415,12 @@ private SortedSet sorted(Set strings) { /** * @TODO REALM-SETTINGS[TIM] This can be redone a lot now the realm settings are keyed by type */ - private RealmConfig findRealm(Terminal terminal, OptionSet options, Environment env) throws UserException, IOException, Exception { + private RealmConfig findRealm(Terminal terminal, OptionSet options, Environment env) throws Exception { keyStoreWrapper = keyStoreFunction.apply(env); final Settings settings; if (keyStoreWrapper != null) { - // TODO: We currently do not support keystore passwords - keyStoreWrapper.decrypt(new char[0]); + decryptKeyStore(keyStoreWrapper, terminal); final Settings.Builder settingsBuilder = Settings.builder(); settingsBuilder.put(env.settings(), true); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioTransport.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioTransport.java index d546b88a8ce9c..3b7600c55b535 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioTransport.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioTransport.java @@ -126,6 +126,11 @@ protected Function clientChannelFactoryFunctio }; } + @Override + public boolean isSecure() { + return this.sslEnabled; + } + private class SecurityTcpChannelFactory extends TcpChannelFactory { private final String profileName; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java index e80c4636e9766..d9bec2ba3701d 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java @@ -31,7 +31,6 @@ import org.elasticsearch.xpack.core.security.user.ElasticUser; import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm; import org.elasticsearch.xpack.security.authc.esnative.tool.HttpResponse.HttpResponseBuilder; -import org.hamcrest.CoreMatchers; import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Rule; @@ -40,6 +39,7 @@ import org.mockito.InOrder; import org.mockito.Mockito; +import javax.crypto.AEADBadTagException; import javax.net.ssl.SSLException; import java.io.IOException; import java.net.HttpURLConnection; @@ -55,9 +55,11 @@ import java.util.Map; import java.util.Set; +import static org.hamcrest.CoreMatchers.containsString; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -68,8 +70,11 @@ public class SetupPasswordToolTests extends CommandTestCase { private final String pathHomeParameter = "-Epath.home=" + createTempDir(); private SecureString bootstrapPassword; private CommandLineHttpClient httpClient; - private KeyStoreWrapper keyStore; private List usersInSetOrder; + private KeyStoreWrapper passwordProtectedKeystore; + private KeyStoreWrapper keyStore; + private KeyStoreWrapper usedKeyStore; + @Rule public ExpectedException thrown = ExpectedException.none(); @@ -79,19 +84,15 @@ public void setSecretsAndKeyStore() throws Exception { boolean useFallback = randomBoolean(); bootstrapPassword = useFallback ? new SecureString("0xCAFEBABE".toCharArray()) : new SecureString("bootstrap-password".toCharArray()); - this.keyStore = mock(KeyStoreWrapper.class); - this.httpClient = mock(CommandLineHttpClient.class); - - when(keyStore.isLoaded()).thenReturn(true); - if (useFallback) { - when(keyStore.getSettingNames()).thenReturn(new HashSet<>(Arrays.asList(ReservedRealm.BOOTSTRAP_ELASTIC_PASSWORD.getKey(), - KeyStoreWrapper.SEED_SETTING.getKey()))); - when(keyStore.getString(ReservedRealm.BOOTSTRAP_ELASTIC_PASSWORD.getKey())).thenReturn(bootstrapPassword); - } else { - when(keyStore.getSettingNames()).thenReturn(Collections.singleton(KeyStoreWrapper.SEED_SETTING.getKey())); - when(keyStore.getString(KeyStoreWrapper.SEED_SETTING.getKey())).thenReturn(bootstrapPassword); + keyStore = mockKeystore(false, useFallback); + // create a password protected keystore eitherway, so that it can be used for SetupPasswordToolTests#testWrongKeystorePassword + passwordProtectedKeystore = mockKeystore(true, useFallback); + usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); + if (usedKeyStore.hasPassword()) { + terminal.addSecretInput("keystore-password"); } + this.httpClient = mock(CommandLineHttpClient.class); when(httpClient.getDefaultURL()).thenReturn("http://localhost:9200"); HttpResponse httpResponse = new HttpResponse(HttpURLConnection.HTTP_OK, new HashMap()); @@ -122,35 +123,29 @@ public void setSecretsAndKeyStore() throws Exception { } } + private KeyStoreWrapper mockKeystore(boolean isPasswordProtected, boolean useFallback) throws Exception { + KeyStoreWrapper keyStore = mock(KeyStoreWrapper.class); + when(keyStore.isLoaded()).thenReturn(true); + if (useFallback) { + when(keyStore.getSettingNames()).thenReturn(new HashSet<>(Arrays.asList(ReservedRealm.BOOTSTRAP_ELASTIC_PASSWORD.getKey(), + KeyStoreWrapper.SEED_SETTING.getKey()))); + when(keyStore.getString(ReservedRealm.BOOTSTRAP_ELASTIC_PASSWORD.getKey())).thenReturn(bootstrapPassword); + } else { + when(keyStore.getSettingNames()).thenReturn(Collections.singleton(KeyStoreWrapper.SEED_SETTING.getKey())); + when(keyStore.getString(KeyStoreWrapper.SEED_SETTING.getKey())).thenReturn(bootstrapPassword); + } + if (isPasswordProtected) { + when(keyStore.hasPassword()).thenReturn(true); + doNothing().when(keyStore).decrypt("keystore-password".toCharArray()); + doThrow(new SecurityException("Provided keystore password was incorrect", new AEADBadTagException())) + .when(keyStore).decrypt("wrong-password".toCharArray()); + } + return keyStore; + } + @Override protected Command newCommand() { - return new SetupPasswordTool(env -> httpClient, env -> keyStore) { - - @Override - protected AutoSetup newAutoSetup() { - return new AutoSetup() { - @Override - protected Environment createEnv(Map settings) throws UserException { - Settings.Builder builder = Settings.builder(); - settings.forEach((k, v) -> builder.put(k, v)); - return TestEnvironment.newEnvironment(builder.build()); - } - }; - } - - @Override - protected InteractiveSetup newInteractiveSetup() { - return new InteractiveSetup() { - @Override - protected Environment createEnv(Map settings) throws UserException { - Settings.Builder builder = Settings.builder(); - settings.forEach((k, v) -> builder.put(k, v)); - return TestEnvironment.newEnvironment(builder.build()); - } - }; - } - - }; + return getSetupPasswordCommandWithKeyStore(usedKeyStore); } public void testAutoSetup() throws Exception { @@ -161,8 +156,12 @@ public void testAutoSetup() throws Exception { terminal.addTextInput("Y"); execute("auto", pathHomeParameter); } - - verify(keyStore).decrypt(new char[0]); + if (usedKeyStore.hasPassword()) { + // SecureString is already closed (zero-filled) and keystore-password is 17 char long + verify(usedKeyStore).decrypt(new char[17]); + } else { + verify(usedKeyStore).decrypt(new char[0]); + } InOrder inOrder = Mockito.inOrder(httpClient); @@ -397,7 +396,7 @@ public void testInteractiveSetup() throws Exception { ArgumentCaptor> passwordCaptor = ArgumentCaptor.forClass((Class) CheckedSupplier.class); inOrder.verify(httpClient).execute(eq("PUT"), eq(urlWithRoute), eq(ElasticUser.NAME), eq(bootstrapPassword), passwordCaptor.capture(), any(CheckedFunction.class)); - assertThat(passwordCaptor.getValue().get(), CoreMatchers.containsString(user + "-password")); + assertThat(passwordCaptor.getValue().get(), containsString(user + "-password")); } } @@ -405,6 +404,9 @@ public void testInteractivePasswordsFatFingers() throws Exception { URL url = new URL(httpClient.getDefaultURL()); terminal.reset(); + if (usedKeyStore.hasPassword()) { + terminal.addSecretInput("keystore-password"); + } terminal.addTextInput("Y"); for (String user : SetupPasswordTool.USERS) { // fail in strength and match @@ -435,10 +437,25 @@ public void testInteractivePasswordsFatFingers() throws Exception { ArgumentCaptor> passwordCaptor = ArgumentCaptor.forClass((Class) CheckedSupplier.class); inOrder.verify(httpClient).execute(eq("PUT"), eq(urlWithRoute), eq(ElasticUser.NAME), eq(bootstrapPassword), passwordCaptor.capture(), any(CheckedFunction.class)); - assertThat(passwordCaptor.getValue().get(), CoreMatchers.containsString(user + "-password")); + assertThat(passwordCaptor.getValue().get(), containsString(user + "-password")); } } + public void testWrongKeystorePassword() throws Exception { + Command commandWithPasswordProtectedKeystore = getSetupPasswordCommandWithKeyStore(passwordProtectedKeystore); + terminal.reset(); + terminal.addSecretInput("wrong-password"); + final UserException e = expectThrows(UserException.class, () -> { + if (randomBoolean()) { + execute(commandWithPasswordProtectedKeystore, "auto", pathHomeParameter, "-b", "true"); + } else { + terminal.addTextInput("Y"); + execute(commandWithPasswordProtectedKeystore, "auto", pathHomeParameter); + } + }); + assertThat(e.getMessage(), containsString("Wrong password for elasticsearch.keystore")); + } + private URL authenticateUrl(URL url) throws MalformedURLException, URISyntaxException { return new URL(url, (url.toURI().getPath() + "/_security/_authenticate").replaceAll("/+", "/") + "?pretty"); } @@ -462,4 +479,35 @@ private HttpResponse createHttpResponse(final int httpStatus, final String respo builder.withResponseBody(responseJson); return builder.build(); } + + private Command getSetupPasswordCommandWithKeyStore(KeyStoreWrapper keyStore) { + return new SetupPasswordTool(env -> httpClient, (e) -> keyStore) { + + @Override + protected AutoSetup newAutoSetup() { + return new AutoSetup() { + @Override + protected Environment createEnv(Map settings) throws UserException { + Settings.Builder builder = Settings.builder(); + settings.forEach((k, v) -> builder.put(k, v)); + return TestEnvironment.newEnvironment(builder.build()); + } + }; + } + + @Override + protected InteractiveSetup newInteractiveSetup() { + return new InteractiveSetup() { + @Override + protected Environment createEnv(Map settings) throws UserException { + Settings.Builder builder = Settings.builder(); + settings.forEach((k, v) -> builder.put(k, v)); + return TestEnvironment.newEnvironment(builder.build()); + } + }; + } + + }; + + } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommandTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommandTests.java index fc0eb25e3a49d..637a69ebf9549 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommandTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommandTests.java @@ -18,6 +18,7 @@ import org.elasticsearch.xpack.core.security.authc.RealmSettings; import org.elasticsearch.xpack.core.ssl.CertParsingUtils; import org.elasticsearch.xpack.core.ssl.PemUtils; +import org.hamcrest.CoreMatchers; import org.junit.Before; import org.opensaml.saml.common.xml.SAMLConstants; import org.opensaml.saml.saml2.metadata.EntityDescriptor; @@ -33,6 +34,7 @@ import org.opensaml.xmlsec.signature.X509Data; import org.opensaml.xmlsec.signature.support.SignatureValidator; +import javax.crypto.AEADBadTagException; import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Path; @@ -54,25 +56,35 @@ import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.startsWith; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class SamlMetadataCommandTests extends SamlTestCase { private KeyStoreWrapper keyStore; + private KeyStoreWrapper passwordProtectedKeystore; @Before public void setup() throws Exception { SamlUtils.initialize(logger); this.keyStore = mock(KeyStoreWrapper.class); when(keyStore.isLoaded()).thenReturn(true); + this.passwordProtectedKeystore = mock(KeyStoreWrapper.class); + when(passwordProtectedKeystore.isLoaded()).thenReturn(true); + when(passwordProtectedKeystore.hasPassword()).thenReturn(true); + doNothing().when(passwordProtectedKeystore).decrypt("keystore-password".toCharArray()); + doThrow(new SecurityException("Provided keystore password was incorrect", new AEADBadTagException())) + .when(passwordProtectedKeystore).decrypt("wrong-password".toCharArray()); } public void testDefaultOptions() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[0]); final boolean useSigningCredentials = randomBoolean(); @@ -93,6 +105,9 @@ public void testDefaultOptions() throws Exception { final MockTerminal terminal = new MockTerminal(); + if (usedKeyStore.hasPassword()) { + terminal.addSecretInput("keystore-password"); + } // What is the friendly name for "principal" attribute "urn:oid:0.9.2342.19200300.100.1.1" [default: principal] terminal.addTextInput(""); @@ -147,6 +162,7 @@ public void testDefaultOptions() throws Exception { } public void testFailIfMultipleRealmsExist() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Settings settings = Settings.builder() .put("path.home", createTempDir()) .put(RealmSettings.PREFIX + "saml.saml_a.type", "saml") @@ -158,11 +174,10 @@ public void testFailIfMultipleRealmsExist() throws Exception { .build(); final Environment env = TestEnvironment.newEnvironment(settings); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[0]); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final UserException userException = expectThrows(UserException.class, () -> command.buildEntityDescriptor(terminal, options, env)); assertThat(userException.getMessage(), containsString("multiple SAML realms")); assertThat(terminal.getErrorOutput(), containsString("saml_a")); @@ -171,6 +186,7 @@ public void testFailIfMultipleRealmsExist() throws Exception { } public void testSpecifyRealmNameAsParameter() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Settings settings = Settings.builder() .put("path.home", createTempDir()) .put(RealmSettings.PREFIX + "saml.saml_a.type", "saml") @@ -182,12 +198,12 @@ public void testSpecifyRealmNameAsParameter() throws Exception { .build(); final Environment env = TestEnvironment.newEnvironment(settings); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> keyStore); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[] { "-realm", "saml_b" }); - final MockTerminal terminal = new MockTerminal(); + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); assertThat(descriptor, notNullValue()); @@ -202,6 +218,7 @@ public void testSpecifyRealmNameAsParameter() throws Exception { } public void testHandleAttributes() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Settings settings = Settings.builder() .put("path.home", createTempDir()) .put(RealmSettings.PREFIX + "saml.saml1.type", "saml") @@ -212,14 +229,13 @@ public void testHandleAttributes() throws Exception { .build(); final Environment env = TestEnvironment.newEnvironment(settings); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[] { "-attribute", "urn:oid:0.9.2342.19200300.100.1.3", "-attribute", "groups" }); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); // What is the friendly name for command line attribute "urn:oid:0.9.2342.19200300.100.1.3" [default: none] terminal.addTextInput("mail"); // What is the standard (urn) name for attribute "groups" (required) @@ -256,6 +272,7 @@ public void testHandleAttributes() throws Exception { } public void testHandleAttributesInBatchMode() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Settings settings = Settings.builder() .put("path.home", createTempDir()) .put(RealmSettings.PREFIX + "saml.saml1.type", "saml") @@ -265,13 +282,13 @@ public void testHandleAttributesInBatchMode() throws Exception { .build(); final Environment env = TestEnvironment.newEnvironment(settings); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[] { "-attribute", "urn:oid:0.9.2342.19200300.100.1.3", "-batch" }); - final MockTerminal terminal = new MockTerminal(); + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); assertThat(descriptor, notNullValue()); @@ -294,10 +311,11 @@ public void testHandleAttributesInBatchMode() throws Exception { public void testSigningMetadataWithPfx() throws Exception { assumeFalse("Can't run in a FIPS JVM, PKCS12 keystores are not usable", inFipsJvm()); + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); final Path p12Path = getDataPath("saml.p12"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[]{ "-signing-bundle", p12Path.toString() }); @@ -319,8 +337,7 @@ public void testSigningMetadataWithPfx() throws Exception { final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); // What is the friendly name for "principal" attribute "urn:oid:0.9.2342.19200300.100.1.1" [default: principal] terminal.addTextInput(""); terminal.addSecretInput(""); @@ -354,10 +371,11 @@ public void testSigningMetadataWithPfx() throws Exception { public void testSigningMetadataWithPasswordProtectedPfx() throws Exception { assumeFalse("Can't run in a FIPS JVM, PKCS12 keystores are not usable", inFipsJvm()); + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); final Path p12Path = getDataPath("saml_with_password.p12"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[]{ "-signing-bundle", p12Path.toString(), "-signing-key-password", "saml" @@ -379,8 +397,7 @@ public void testSigningMetadataWithPasswordProtectedPfx() throws Exception { final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); command.possiblySignDescriptor(terminal, options, descriptor, env); assertThat(descriptor, notNullValue()); @@ -390,10 +407,11 @@ public void testSigningMetadataWithPasswordProtectedPfx() throws Exception { } public void testErrorSigningMetadataWithWrongPassword() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); final Path signingKeyPath = getDataPath("saml_with_password.key"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> keyStore); final OptionSet options = command.getParser().parse(new String[]{ "-signing-cert", certPath.toString(), "-signing-key", signingKeyPath.toString(), @@ -417,8 +435,7 @@ public void testErrorSigningMetadataWithWrongPassword() throws Exception { final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); final UserException userException = expectThrows(UserException.class, () -> command.possiblySignDescriptor(terminal, options, descriptor, env)); @@ -427,11 +444,12 @@ public void testErrorSigningMetadataWithWrongPassword() throws Exception { } public void testSigningMetadataWithPem() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); //Use this keypair for signing the metadata also final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> keyStore); final OptionSet options = command.getParser().parse(new String[]{ "-signing-cert", certPath.toString(), "-signing-key", keyPath.toString() @@ -453,8 +471,7 @@ public void testSigningMetadataWithPem() throws Exception { final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); command.possiblySignDescriptor(terminal, options, descriptor, env); assertThat(descriptor, notNullValue()); @@ -464,13 +481,14 @@ public void testSigningMetadataWithPem() throws Exception { } public void testSigningMetadataWithPasswordProtectedPem() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); //Use same keypair for signing the metadata final Path signingKeyPath = getDataPath("saml_with_password.key"); final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> keyStore); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[]{ "-signing-cert", certPath.toString(), "-signing-key", signingKeyPath.toString(), @@ -494,8 +512,7 @@ public void testSigningMetadataWithPasswordProtectedPem() throws Exception { final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); command.possiblySignDescriptor(terminal, options, descriptor, env); assertThat(descriptor, notNullValue()); @@ -505,13 +522,14 @@ public void testSigningMetadataWithPasswordProtectedPem() throws Exception { } public void testSigningMetadataWithPasswordProtectedPemInTerminal() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); //Use same keypair for signing the metadata final Path signingKeyPath = getDataPath("saml_with_password.key"); final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[]{ "-signing-cert", certPath.toString(), "-signing-key", signingKeyPath.toString() @@ -534,8 +552,7 @@ public void testSigningMetadataWithPasswordProtectedPemInTerminal() throws Excep final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); terminal.addSecretInput("saml"); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); @@ -547,6 +564,7 @@ public void testSigningMetadataWithPasswordProtectedPemInTerminal() throws Excep } public void testDefaultOptionsWithSigningAndMultipleEncryptionKeys() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Path dir = createTempDir(); final Path ksEncryptionFile = dir.resolve("saml-encryption.p12"); @@ -578,7 +596,7 @@ public void testDefaultOptionsWithSigningAndMultipleEncryptionKeys() throws Exce secureSettings.setString(RealmSettings.PREFIX + "saml.my_saml.encryption.keystore.secure_password", "ks-password"); secureSettings.setString(RealmSettings.PREFIX + "saml.my_saml.encryption.keystore.secure_key_password", "key-password"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> keyStore); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[0]); final boolean useSigningCredentials = randomBoolean(); @@ -603,8 +621,7 @@ public void testDefaultOptionsWithSigningAndMultipleEncryptionKeys() throws Exce final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); // What is the friendly name for "principal" attribute // "urn:oid:0.9.2342.19200300.100.1.1" [default: principal] terminal.addTextInput(""); @@ -679,6 +696,27 @@ public void testDefaultOptionsWithSigningAndMultipleEncryptionKeys() throws Exce } } + public void testWrongKeystorePassword() { + final Path certPath = getDataPath("saml.crt"); + final Path keyPath = getDataPath("saml.key"); + + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> passwordProtectedKeystore); + final OptionSet options = command.getParser().parse(new String[]{ + "-signing-cert", certPath.toString(), + "-signing-key", keyPath.toString() + }); + final Settings settings = Settings.builder().put("path.home", createTempDir()).build(); + final Environment env = TestEnvironment.newEnvironment(settings); + + final MockTerminal terminal = new MockTerminal(); + terminal.addSecretInput("wrong-password"); + + UserException e = expectThrows(UserException.class, () -> { + command.buildEntityDescriptor(terminal, options, env); + }); + assertThat(e.getMessage(), CoreMatchers.containsString("Wrong password for elasticsearch.keystore")); + } + private String getAliasName(final Tuple certKeyPair) { // Keys are pre-generated with the same name, so add the serial no to the alias so that keystore entries won't be overwritten return certKeyPair.v1().getSubjectX500Principal().getName().toLowerCase(Locale.US) + "-"+ @@ -700,4 +738,12 @@ private boolean validateSignature(Signature signature) { return false; } } + + private MockTerminal getTerminalPossiblyWithPassword(KeyStoreWrapper keyStore) { + final MockTerminal terminal = new MockTerminal(); + if (keyStore.hasPassword()) { + terminal.addSecretInput("keystore-password"); + } + return terminal; + } }