Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Password-protected Keystore Feature Branch PR #51123

Merged
merged 30 commits into from
Jan 28, 2020
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
b0be180
Password Protected Keystore (Feature Branch) (cleaned up) (#49268)
williamrandolph Dec 11, 2019
fab3d56
Merging master to feature branch
williamrandolph Dec 12, 2019
d64ab04
Merge branch 'master' into feature-pwd-protected-keystore-2
williamrandolph Dec 13, 2019
35b4f52
Fix failing Keystore Passphrase test for feature branch (#50154)
williamrandolph Dec 13, 2019
6695e53
Merge master into feature branch
williamrandolph Jan 2, 2020
1284ecc
Run keystore management tests on docker distros (#50610)
williamrandolph Jan 8, 2020
1c656f2
Merging master into feature branch
williamrandolph Jan 8, 2020
3d7cf1f
Test password-protected keystore with Docker (#50803)
williamrandolph Jan 10, 2020
93ca7e6
Merging changes from master
williamrandolph Jan 13, 2020
663fc0b
Add documentation for keystore startup prompting (#50821)
williamrandolph Jan 14, 2020
91cb2c9
Merge branch 'master' into feature-pwd-protected-keystore-2
williamrandolph Jan 15, 2020
6238d85
Warn when unable to upgrade keystore on debian (#51011)
williamrandolph Jan 15, 2020
75bdbdd
Merge master into feature branch
williamrandolph Jan 16, 2020
0b2d5d1
Merge branch 'feature-pwd-protected-keystore-2' of github.com:elastic…
williamrandolph Jan 16, 2020
81458e7
Increase Docker startup wait time for tests
williamrandolph Jan 16, 2020
3775061
Restore handling of string input
jkakavas Jan 22, 2020
bf27d31
address feedback
jkakavas Jan 22, 2020
ba7434f
Apply spotless reformatting
williamrandolph Jan 22, 2020
bd9465d
Merging changes from master
williamrandolph Jan 22, 2020
e6156ef
Reformat after merge from master
williamrandolph Jan 22, 2020
f88837b
Back out merge commit errors
williamrandolph Jan 22, 2020
f61d756
Respond to PR comments
williamrandolph Jan 22, 2020
9508733
Stop journald before trying to delete its files
williamrandolph Jan 22, 2020
936ba91
Adjust clearing the journalctl logs
williamrandolph Jan 23, 2020
8ede626
Merging changes from master
williamrandolph Jan 23, 2020
8389ea0
Use '--since' flag to get recent journal messages
williamrandolph Jan 23, 2020
2f82284
Use --since when reading journal
williamrandolph Jan 23, 2020
6757785
Merging changes from master
williamrandolph Jan 24, 2020
f93ab59
Use new journald wrapper pattern
williamrandolph Jan 24, 2020
a8103a6
Update version added in secure settings request
williamrandolph Jan 27, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Vagrantfile
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ JAVA
ensure curl
ensure unzip
ensure rsync
ensure expect

installed bats || {
# Bats lives in a git repository....
Expand Down
4 changes: 2 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ task verifyVersions {
* after the backport of the backcompat code is complete.
*/

boolean bwc_tests_enabled = true
final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */
boolean bwc_tests_enabled = false
final String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/43197"
if (bwc_tests_enabled == false) {
if (bwc_tests_disabled_issue.isEmpty()) {
throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false")
Expand Down
2 changes: 1 addition & 1 deletion distribution/docker/docker-test-entrypoint.sh
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
16 changes: 13 additions & 3 deletions distribution/docker/src/docker/bin/docker-entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
4 changes: 4 additions & 0 deletions distribution/packages/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
7 changes: 6 additions & 1 deletion distribution/packages/src/common/scripts/postinst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny nit / observation - is it worth putting the command in quotes?

Suggested change
echo " Please run elasticsearch-keystore upgrade and enter password" 1>&2
echo " Please run 'elasticsearch-keystore upgrade' and enter password" 1>&2

else
/usr/share/elasticsearch/bin/elasticsearch-keystore upgrade
fi
fi
fi

Expand Down
7 changes: 6 additions & 1 deletion distribution/packages/src/common/scripts/posttrans
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions distribution/packages/src/common/systemd/systemd-entrypoint
Original file line number Diff line number Diff line change
@@ -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
17 changes: 15 additions & 2 deletions distribution/src/bin/elasticsearch
Original file line number Diff line number Diff line change
Expand Up @@ -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 )' \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a bit fragile to me. What about something like:

CHECK_KEYSTORE=false
for option in "$@"; do
  case "$option" in
    -h|--help|-V|--version)
      CHECK_KEYSTORE=true
      ;;
  esac
done
if [[ $CHECK_KEYSTORE == true ]] \
    && "`dirname "$0"`"/elasticsearch-keystore has-passwd --silent
  # etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied a pattern checking for daemonization below. I'd like to put this off for a follow-up PR so it can be discussed with some other team members in isolation. That being said I don't have any objection to doing a loop over options and it seems to work just fine in my quick local tests.

&& "`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"`

Expand All @@ -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" \
Expand All @@ -48,7 +61,7 @@ else
-cp "$ES_CLASSPATH" \
org.elasticsearch.bootstrap.Elasticsearch \
"$@" \
<&- &
<<<"$KEYSTORE_PASSWORD" &
retval=$?
pid=$!
[ $retval -eq 0 ] || exit $retval
Expand Down
2 changes: 1 addition & 1 deletion distribution/src/bin/elasticsearch-cli.bat
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ set ES_JAVA_OPTS=-Xms4m -Xmx64m -XX:+UseSerialGC %ES_JAVA_OPTS%
-cp "%ES_CLASSPATH%" ^
"%ES_MAIN_CLASS%" ^
%*

exit /b %ERRORLEVEL%
42 changes: 41 additions & 1 deletion distribution/src/bin/elasticsearch.bat
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ setlocal enabledelayedexpansion
setlocal enableextensions

SET params='%*'
SET checkpassword=Y

:loop
FOR /F "usebackq tokens=1* delims= " %%A IN (!params!) DO (
Expand All @@ -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 (
Expand All @@ -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
)
Expand All @@ -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:^>=^^^>!
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,42 +36,29 @@
/**
* A subcommand for the keystore cli which adds a file setting.
*/
class AddFileKeyStoreCommand extends EnvironmentAwareCommand {
class AddFileKeyStoreCommand extends BaseKeyStoreCommand {

private final OptionSpec<Void> forceOption;
private final OptionSpec<String> 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
this.arguments = parser.nonOptions("setting [filepath]");
}

@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<String> 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;
Expand All @@ -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);
}
Expand Down
Loading