From 9efa5be60e3e3b3cd050b11404c9c6d3d7f74822 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 28 Jan 2020 05:32:32 -0500 Subject: [PATCH] Password-protected Keystore Feature Branch PR (#51123) (#51510) * Reload secure settings with password (#43197) If a password is not set, we assume an empty string to be compatible with previous behavior. Only allow the reload to be broadcast to other nodes if TLS is enabled for the transport layer. * Add passphrase support to elasticsearch-keystore (#38498) This change adds support for keystore passphrases to all subcommands of the elasticsearch-keystore cli tool and adds a subcommand for changing the passphrase of an existing keystore. The work to read the passphrase in Elasticsearch when loading, which will be addressed in a different PR. Subcommands of elasticsearch-keystore can handle (open and create) passphrase protected keystores When reading a keystore, a user is only prompted for a passphrase only if the keystore is passphrase protected. When creating a keystore, a user is allowed (default behavior) to create one with an empty passphrase Passphrase can be set to be empty when changing/setting it for an existing keystore Relates to: #32691 Supersedes: #37472 * Restore behavior for force parameter (#44847) Turns out that the behavior of `-f` for the add and add-file sub commands where it would also forcibly create the keystore if it didn't exist, was by design - although undocumented. This change restores that behavior auto-creating a keystore that is not password protected if the force flag is used. The force OptionSpec is moved to the BaseKeyStoreCommand as we will presumably want to maintain the same behavior in any other command that takes a force option. * Handle pwd protected keystores in all CLI tools (#45289) This change ensures that `elasticsearch-setup-passwords` and `elasticsearch-saml-metadata` can handle a password protected elasticsearch.keystore. For setup passwords the user would be prompted to add the elasticsearch keystore password upon running the tool. There is no option to pass the password as a parameter as we assume the user is present in order to enter the desired passwords for the built-in users. For saml-metadata, we prompt for the keystore password at all times even though we'd only need to read something from the keystore when there is a signing or encryption configuration. * Modify docs for setup passwords and saml metadata cli (#45797) Adds a sentence in the documentation of `elasticsearch-setup-passwords` and `elasticsearch-saml-metadata` to describe that users would be prompted for the keystore's password when running these CLI tools, when the keystore is password protected. Co-Authored-By: Lisa Cawley * Elasticsearch keystore passphrase for startup scripts (#44775) This commit allows a user to provide a keystore password on Elasticsearch startup, but only prompts when the keystore exists and is encrypted. The entrypoint in Java code is standard input. When the Bootstrap class is checking for secure keystore settings, it checks whether or not the keystore is encrypted. If so, we read one line from standard input and use this as the password. For simplicity's sake, we allow a maximum passphrase length of 128 characters. (This is an arbitrary limit and could be increased or eliminated. It is also enforced in the keystore tools, so that a user can't create a password that's too long to enter at startup.) In order to provide a password on standard input, we have to account for four different ways of starting Elasticsearch: the bash startup script, the Windows batch startup script, systemd startup, and docker startup. We use wrapper scripts to reduce systemd and docker to the bash case: in both cases, a wrapper script can read a passphrase from the filesystem and pass it to the bash script. In order to simplify testing the need for a passphrase, I have added a has-passwd command to the keystore tool. This command can run silently, and exit with status 0 when the keystore has a password. It exits with status 1 if the keystore doesn't exist or exists and is unencrypted. A good deal of the code-change in this commit has to do with refactoring packaging tests to cleanly use the same tests for both the "archive" and the "package" cases. This required not only moving tests around, but also adding some convenience methods for an abstraction layer over distribution-specific commands. * Adjust docs for password protected keystore (#45054) This commit adds relevant parts in the elasticsearch-keystore sub-commands reference docs and in the reload secure settings API doc. * Fix failing Keystore Passphrase test for feature branch (#50154) One problem with the passphrase-from-file tests, as written, is that they would leave a SystemD environment variable set when they failed, and this setting would cause elasticsearch startup to fail for other tests as well. By using a try-finally, I hope that these tests will fail more gracefully. It appears that our Fedora and Ubuntu environments may be configured to store journald information under /var rather than under /run, so that it will persist between boots. Our destructive tests that read from the journal need to account for this in order to avoid trying to limit the output we check in tests. * Run keystore management tests on docker distros (#50610) * Add Docker handling to PackagingTestCase Keystore tests need to be able to run in the Docker case. We can do this by using a DockerShell instead of a plain Shell when Docker is running. * Improve ES startup check for docker Previously we were checking truncated output for the packaged JDK as an indication that Elasticsearch had started. With new preliminary password checks, we might get a false positive from ES keystore commands, so we have to check specifically that the Elasticsearch class from the Bootstrap package is what's running. * Test password-protected keystore with Docker (#50803) This commit adds two tests for the case where we mount a password-protected keystore into a Docker container and provide a password via a Docker environment variable. We also fix a logging bug where we were logging the identifier for an array of strings rather than the contents of that array. * Add documentation for keystore startup prompting (#50821) When a keystore is password-protected, Elasticsearch will prompt at startup. This commit adds documentation for this prompt for the archive, systemd, and Docker cases. Co-authored-by: Lisa Cawley * Warn when unable to upgrade keystore on debian (#51011) For Red Hat RPM upgrades, we warn if we can't upgrade the keystore. This commit brings the same logic to the code for Debian packages. See the posttrans file for gets executed for RPMs. * Restore handling of string input Adds tests that were mistakenly removed. One of these tests proved we were not handling the the stdin (-x) option correctly when no input was added. This commit restores the original approach of reading stdin one char at a time until there is no more (-1, \r, \n) instead of using readline() that might return null * Apply spotless reformatting * Use '--since' flag to get recent journal messages When we get Elasticsearch logs from journald, we want to fetch only log messages from the last run. There are two reasons for this. First, if there are many logs, we might get a string that's too large for our utility methods. Second, when we're looking for a specific message or error, we almost certainly want to look only at messages from the last execution. Previously, we've been trying to do this by clearing out the physical files under the journald process. But there seems to be some contention over these directories: if journald writes a log file in between when our deletion command deletes the file and when it deletes the log directory, the deletion will fail. It seems to me that we might be able to use journald's "--since" flag to retrieve only log messages from the last run, and that this might be less likely to fail due to race conditions in file deletion. Unfortunately, it looks as if the "--since" flag has a granularity of one-second. I've added a two-second sleep to make sure that there's a sufficient gap between the test that will read from journald and the test before it. * Use new journald wrapper pattern * Update version added in secure settings request Co-authored-by: Lisa Cawley Co-authored-by: Ioannis Kakavas --- Vagrantfile | 1 + distribution/docker/docker-test-entrypoint.sh | 2 +- .../src/docker/bin/docker-entrypoint.sh | 16 +- distribution/packages/build.gradle | 4 + .../packages/src/common/scripts/postinst | 7 +- .../packages/src/common/scripts/posttrans | 7 +- .../src/common/systemd/elasticsearch.service | 2 +- .../src/common/systemd/systemd-entrypoint | 10 + distribution/src/bin/elasticsearch | 17 +- distribution/src/bin/elasticsearch-cli.bat | 2 +- distribution/src/bin/elasticsearch.bat | 42 +- docs/reference/cluster.asciidoc | 2 + .../nodes-reload-secure-settings.asciidoc | 52 ++- docs/reference/commands/keystore.asciidoc | 68 ++- .../reference/commands/saml-metadata.asciidoc | 4 + .../commands/setup-passwords.asciidoc | 4 +- docs/reference/setup/install/docker.asciidoc | 19 + docs/reference/setup/install/systemd.asciidoc | 13 + .../setup/install/targz-daemon.asciidoc | 6 +- .../setup/install/targz-start.asciidoc | 4 + .../setup/install/zip-windows-start.asciidoc | 3 + docs/reference/setup/secure-settings.asciidoc | 9 +- .../java/org/elasticsearch/cli/Command.java | 4 +- .../java/org/elasticsearch/cli/Terminal.java | 73 ++- .../org/elasticsearch/cli/UserException.java | 14 +- .../packaging/test/ArchiveTests.java | 52 +-- .../packaging/test/DockerTests.java | 93 +--- .../test/KeystoreManagementTests.java | 427 ++++++++++++++++++ .../packaging/test/PackageTests.java | 3 +- .../packaging/test/PackagingTestCase.java | 57 ++- .../packaging/util/Archives.java | 27 +- .../elasticsearch/packaging/util/Cleanup.java | 1 + .../packaging/util/Distribution.java | 4 + .../elasticsearch/packaging/util/Docker.java | 17 +- .../packaging/util/FileUtils.java | 2 +- .../packaging/util/Platforms.java | 1 + .../elasticsearch/packaging/util/Shell.java | 2 +- .../NodesReloadSecureSettingsRequest.java | 75 ++- ...desReloadSecureSettingsRequestBuilder.java | 6 + ...nsportNodesReloadSecureSettingsAction.java | 49 +- .../elasticsearch/bootstrap/Bootstrap.java | 48 +- .../cli/KeyStoreAwareCommand.java | 86 ++++ .../settings/AddFileKeyStoreCommand.java | 34 +- .../settings/AddStringKeyStoreCommand.java | 35 +- .../common/settings/BaseKeyStoreCommand.java | 91 ++++ .../ChangeKeyStorePasswordCommand.java | 47 ++ .../settings/CreateKeyStoreCommand.java | 43 +- .../settings/HasPasswordKeyStoreCommand.java | 57 +++ .../common/settings/KeyStoreCli.java | 2 + .../common/settings/KeyStoreWrapper.java | 16 +- .../common/settings/ListKeyStoreCommand.java | 19 +- .../RemoveSettingKeyStoreCommand.java | 22 +- .../settings/UpgradeKeyStoreCommand.java | 18 +- .../RestReloadSecureSettingsAction.java | 38 +- .../elasticsearch/transport/Transport.java | 4 + .../transport/TransportService.java | 4 + .../action/admin/ReloadSecureSettingsIT.java | 334 ++++++++++---- .../bootstrap/BootstrapTests.java | 46 ++ .../elasticsearch/cli/MultiCommandTests.java | 54 +++ .../org/elasticsearch/cli/TerminalTests.java | 66 +++ .../settings/AddFileKeyStoreCommandTests.java | 104 +++-- .../AddStringKeyStoreCommandTests.java | 117 +++-- .../ChangeKeyStorePasswordCommandTests.java | 95 ++++ .../settings/CreateKeyStoreCommandTests.java | 27 ++ .../HasPasswordKeyStoreCommandTests.java | 68 +++ .../settings/KeyStoreCommandTestCase.java | 8 +- .../common/settings/KeyStoreWrapperTests.java | 6 +- .../settings/ListKeyStoreCommandTests.java | 28 +- .../RemoveSettingKeyStoreCommandTests.java | 41 +- .../settings/UpgradeKeyStoreCommandTests.java | 2 +- .../RestReloadSecureSettingsActionTests.java | 53 +++ .../elasticsearch/cli/CommandTestCase.java | 16 +- .../org/elasticsearch/cli/MockTerminal.java | 2 +- .../netty4/SecurityNetty4Transport.java | 5 + .../esnative/tool/SetupPasswordTool.java | 13 +- .../authc/saml/SamlMetadataCommand.java | 9 +- .../transport/nio/SecurityNioTransport.java | 5 + .../esnative/tool/SetupPasswordToolTests.java | 136 ++++-- .../authc/saml/SamlMetadataCommandTests.java | 110 +++-- 79 files changed, 2526 insertions(+), 584 deletions(-) create mode 100644 distribution/packages/src/common/systemd/systemd-entrypoint create mode 100644 qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java create mode 100644 server/src/main/java/org/elasticsearch/cli/KeyStoreAwareCommand.java create mode 100644 server/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java create mode 100644 server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java create mode 100644 server/src/main/java/org/elasticsearch/common/settings/HasPasswordKeyStoreCommand.java create mode 100644 server/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java create mode 100644 server/src/test/java/org/elasticsearch/common/settings/HasPasswordKeyStoreCommandTests.java create mode 100644 server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsActionTests.java diff --git a/Vagrantfile b/Vagrantfile index 8e231c368d0bc..6873ba327125e 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -479,6 +479,7 @@ JAVA ensure curl ensure unzip ensure rsync + ensure expect installed bats || { # Bats lives in a git repository.... 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 0366060257b2c..58d19da3df43c 100644 --- a/distribution/docker/src/docker/bin/docker-entrypoint.sh +++ b/distribution/docker/src/docker/bin/docker-entrypoint.sh @@ -57,8 +57,18 @@ 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 ]] || (run_as_other_user_if_needed elasticsearch-keystore create) - if ! (run_as_other_user_if_needed elasticsearch-keystore list | grep -q '^bootstrap.password$'); then - (run_as_other_user_if_needed echo "$ELASTIC_PASSWORD" | elasticsearch-keystore add -x 'bootstrap.password') + if ! (run_as_other_user_if_needed elasticsearch-keystore has-passwd --silent) ; then + # keystore is unencrypted + if ! (run_as_other_user_if_needed elasticsearch-keystore list | grep -q '^bootstrap.password$'); then + (run_as_other_user_if_needed echo "$ELASTIC_PASSWORD" | elasticsearch-keystore add -x 'bootstrap.password') + fi + else + # keystore requires password + if ! (run_as_other_user_if_needed echo "$KEYSTORE_PASSWORD" \ + | elasticsearch-keystore list | grep -q '^bootstrap.password$') ; then + COMMANDS="$(printf "%s\n%s" "$KEYSTORE_PASSWORD" "$ELASTIC_PASSWORD")" + (run_as_other_user_if_needed echo "$COMMANDS" | elasticsearch-keystore add -x 'bootstrap.password') + fi fi fi fi @@ -70,4 +80,4 @@ if [[ "$(id -u)" == "0" ]]; then fi fi -run_as_other_user_if_needed /usr/share/elasticsearch/bin/elasticsearch +run_as_other_user_if_needed /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/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 537dec2904070..766c71d2840f4 100644 --- a/docs/reference/setup/install/docker.asciidoc +++ b/docs/reference/setup/install/docker.asciidoc @@ -360,6 +360,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 30709a3d86698..6d74efb985521 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")); @@ -251,22 +219,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"); @@ -296,7 +248,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); @@ -393,7 +345,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 93c8ba5b9db76..7f9b0c8a9da2c 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 @@ -19,23 +19,38 @@ package org.elasticsearch.packaging.test; +import com.fasterxml.jackson.databind.JsonNode; +import org.apache.http.client.fluent.Request; +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.Before; +import org.junit.BeforeClass; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + import static java.nio.file.attribute.PosixFilePermissions.fromString; import static java.util.Collections.singletonMap; -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; @@ -56,53 +71,21 @@ import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; -import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; import java.nio.file.Paths; import java.util.Arrays; -import java.util.HashMap; import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.stream.Collectors; - -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; - -import com.fasterxml.jackson.databind.JsonNode; 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()); } @@ -143,44 +126,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")); 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..def1bb3dc2438 --- /dev/null +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java @@ -0,0 +1,427 @@ +/* + * 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.Paths; +import java.nio.file.StandardOpenOption; +import java.util.HashMap; +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 = new HashMap<>(); + volumes.put(localKeystoreFile, dockerKeystore); + Map envVars = new HashMap<>(); + envVars.put("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 = new HashMap<>(); + volumes.put(localKeystoreFile, dockerKeystore); + Map envVars = new HashMap<>(); + envVars.put("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 = Paths.get("/usr/tmp/keystore-tmp"); + Path tempDirectory = Files.createTempDirectory(getTempDir(), KeystoreManagementTests.class.getSimpleName()); + Map volumes = new HashMap<>(); + volumes.put(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.write(tempDirectory.resolve("set-pass.sh"), setPasswordScript.getBytes(StandardCharsets.UTF_8)); + + 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 868cb75a49395..d77ee768dc4af 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)); } @@ -162,6 +162,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 7f5502922bb7e..0c07e54663a03 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 @@ -245,7 +245,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 8c32c7edc3272..d43c2219a031c 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 @@ -60,6 +60,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, @@ -175,18 +177,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(); @@ -502,6 +504,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 7d8c39b642232..b8dfe9dc5f5c1 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,20 +21,25 @@ 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; import org.elasticsearch.plugins.ReloadablePlugin; +import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -77,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) { + 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) @@ -106,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(); } } @@ -128,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 6b5896ac8d493..462c2b897b363 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; @@ -52,9 +55,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; @@ -236,21 +242,59 @@ 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{ 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); + } finally { + password.close(); } 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/AddFileKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java index f5b3cb9cf7104..544c58e038866 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java +++ b/server/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,14 @@ /** * 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 +51,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; @@ -90,11 +76,11 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th throw new UserException(ExitCodes.USAGE, "Unrecognized extra arguments [" + String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath"); } - keystore.setFile(setting, Files.readAllBytes(file)); - keystore.save(env.configFile(), new char[0]); + keyStore.setFile(setting, Files.readAllBytes(file)); + keyStore.save(env.configFile(), getKeyStorePassword().getChars()); } - @SuppressForbidden(reason="file arg for cli") + @SuppressForbidden(reason = "file arg for cli") private Path getPath(String file) { return PathUtils.get(file); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java index ba006cd36f372..6fba5acce75ef 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java +++ b/server/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,16 @@ /** * 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 +55,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; @@ -100,10 +86,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/server/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java new file mode 100644 index 0000000000000..493d455e42f6e --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java @@ -0,0 +1,91 @@ +/* + * 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/server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java b/server/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java new file mode 100644 index 0000000000000..526201ede8f66 --- /dev/null +++ b/server/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/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index 3529d7f6810bd..c8833650581ea 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -21,41 +21,44 @@ 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/server/src/main/java/org/elasticsearch/common/settings/HasPasswordKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/HasPasswordKeyStoreCommand.java new file mode 100644 index 0000000000000..d3cc85299710e --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/settings/HasPasswordKeyStoreCommand.java @@ -0,0 +1,57 @@ +/* + * 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/server/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java index 19a453f7e90fd..f08c83432de3f 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java +++ b/server/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/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/common/settings/ListKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java index 8eef02f213189..edd8a68cc6f9f 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java @@ -25,31 +25,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/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java index 9a83375e6e01a..6e839d4f331ba 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java +++ b/server/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/server/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java b/server/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java index 6338f40ea05fa..640a76432d3b4 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java +++ b/server/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java @@ -20,31 +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/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 f89692caa73a7..32499ce19d576 100644 --- a/server/src/main/java/org/elasticsearch/transport/Transport.java +++ b/server/src/main/java/org/elasticsearch/transport/Transport.java @@ -52,6 +52,10 @@ public interface Transport extends LifecycleComponent { void setMessageListener(TransportMessageListener listener); + 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 e72719d98772b..fa84a2aa0ca9d 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportService.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportService.java @@ -310,6 +310,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/bootstrap/BootstrapTests.java b/server/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java index 6b336fdf2b78c..df2056e8c6b18 100644 --- a/server/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java +++ b/server/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,43 @@ 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/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/common/settings/AddFileKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java index 6cfa2c1fdf255..cd64fdc08d3f5 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java @@ -53,110 +53,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/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java index b5e6a31e14894..274dfb39eff94 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java @@ -43,6 +43,7 @@ protected Command newCommand() { protected Environment createEnv(Map settings) throws UserException { return env; } + @Override InputStream getStdin() { return input; @@ -50,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 { @@ -70,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++) { @@ -163,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); @@ -180,10 +220,19 @@ public void testSpecialCharacterInName() throws Exception { terminal.addSecretInput("value"); final String key = randomAlphaOfLength(4) + '@' + randomAlphaOfLength(4); final UserException e = expectThrows(UserException.class, () -> execute(key)); - final String exceptionString= "Setting name [" + key + "] does not match the allowed setting name pattern [[A-Za-z0-9_\\-.]+]"; + final String exceptionString = "Setting name [" + key + "] does not match the allowed setting name pattern [[A-Za-z0-9_\\-.]+]"; assertThat( - e, - hasToString(containsString(exceptionString))); + 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) { diff --git a/server/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java new file mode 100644 index 0000000000000..ca0b5fa363351 --- /dev/null +++ b/server/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/server/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java index aefedf86e7761..4fd21a9b61a7f 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java @@ -25,9 +25,12 @@ import java.util.Map; import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; +import static org.hamcrest.Matchers.containsString; + public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase { @Override @@ -40,13 +43,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/server/src/test/java/org/elasticsearch/common/settings/HasPasswordKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/HasPasswordKeyStoreCommandTests.java new file mode 100644 index 0000000000000..93e6b0cae2095 --- /dev/null +++ b/server/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/server/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java index 7f8c71889e038..1e5527a1e245b 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java +++ b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java @@ -89,16 +89,16 @@ KeyStoreWrapper loadKeystore(String password) throws Exception { return keystore; } - void assertSecureString(String setting, String value) throws Exception { - assertSecureString(loadKeystore(""), setting, value); + void assertSecureString(String setting, String value, String password) throws Exception { + assertSecureString(loadKeystore(password), setting, value); } void assertSecureString(KeyStoreWrapper keystore, String setting, String value) throws Exception { assertEquals(value, keystore.getString(setting).toString()); } - void assertSecureFile(String setting, Path file) throws Exception { - assertSecureFile(loadKeystore(""), setting, file); + void assertSecureFile(String setting, Path file, String password) throws Exception { + assertSecureFile(loadKeystore(password), setting, file); } void assertSecureFile(KeyStoreWrapper keystore, String setting, Path file) throws Exception { diff --git a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java index 5a1e3790a09c5..aa4dc566e69d1 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java @@ -84,7 +84,7 @@ public void testFileSettingExhaustiveBytes() throws Exception { KeyStoreWrapper keystore = KeyStoreWrapper.create(); byte[] bytes = new byte[256]; for (int i = 0; i < 256; ++i) { - bytes[i] = (byte)i; + bytes[i] = (byte) i; } keystore.setFile("foo", bytes); keystore.save(env.configFile(), new char[0]); @@ -113,7 +113,7 @@ public void testDecryptKeyStoreWithWrongPassword() throws Exception { final KeyStoreWrapper loadedkeystore = KeyStoreWrapper.load(env.configFile()); final SecurityException exception = expectThrows(SecurityException.class, () -> loadedkeystore.decrypt(new char[]{'i', 'n', 'v', 'a', 'l', 'i', 'd'})); - assertThat(exception.getMessage(), containsString("Keystore has been corrupted or tampered with")); + assertThat(exception.getMessage(), containsString("Provided keystore password was incorrect")); } public void testCannotReadStringFromClosedKeystore() throws Exception { @@ -388,7 +388,7 @@ public void testBackcompatV2() throws Exception { byte[] base64Bytes = Base64.getEncoder().encode(fileBytes); char[] chars = new char[base64Bytes.length]; for (int i = 0; i < chars.length; ++i) { - chars[i] = (char)base64Bytes[i]; // PBE only stores the lower 8 bits, so this narrowing is ok + chars[i] = (char) base64Bytes[i]; // PBE only stores the lower 8 bits, so this narrowing is ok } secretKey = secretFactory.generateSecret(new PBEKeySpec(chars)); keystore.setEntry("file_setting", new KeyStore.SecretKeyEntry(secretKey), protectionParameter); diff --git a/server/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java index 27c30d3aa8f58..f79fd751465ec 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java @@ -47,20 +47,42 @@ public void testMissing() throws Exception { } public void testEmpty() throws Exception { - createKeystore(""); + String 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/server/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java index 2259dee31a8cb..b4cc08c846513 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java @@ -41,39 +41,66 @@ protected Environment createEnv(Map settings) throws UserExcepti }; } - public void testMissing() throws Exception { + public void testMissing() { + String 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/server/src/test/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommandTests.java index ec9a1432539d4..075aeaae5a033 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommandTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommandTests.java @@ -73,7 +73,7 @@ public void testKeystoreUpgrade() throws Exception { public void testKeystoreDoesNotExist() { final UserException e = expectThrows(UserException.class, this::execute); - assertThat(e, hasToString(containsString("keystore does not exist at [" + KeyStoreWrapper.keystorePath(env.configFile()) + "]"))); + assertThat(e, hasToString(containsString("keystore not found at [" + KeyStoreWrapper.keystorePath(env.configFile()) + "]"))); } } diff --git a/server/src/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 5ac81a0648019..29d7c3a94ac32 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; @@ -125,7 +125,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); @@ -171,7 +171,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); @@ -221,7 +221,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; @@ -248,10 +248,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 4d0e05a5c320b..780a101d4e3b8 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((e, s) -> 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()); - } - }; - } - - }; + 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((e, s) -> 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 734ea0be0d4cd..9f9b743cbb2fe 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; + } }