From 250c9f278a9dd63da49893effcae2fb9e4651095 Mon Sep 17 00:00:00 2001
From: William Brafford <william.brafford@elastic.co>
Date: Mon, 24 Jun 2019 11:17:37 -0400
Subject: [PATCH 1/8] Read keystore password from stdin on startup

This is initial and exporatory work for incorporating keystore passwords
in the Elasticsearch startup process. The "user interface" that prompts
for and reads a password is in the bash startup script. The bash script
uses existing command-line utilities to test whether the keystore exists
and a password is needed; if so, the password is given to the Java
startup process using a "here string" and a new command-line lag that
alerts Java to data coming in over standard input.

Known issues: Java code that reads from standard input is not
thread-safe, initial keystore verification is unacceptably slow.

Relates #32691, #38498
---
 distribution/src/bin/elasticsearch            | 19 +++++++++-
 .../bootstrap/EvilElasticsearchCliTests.java  |  4 +--
 .../elasticsearch/bootstrap/Bootstrap.java    | 36 ++++++++++++++-----
 .../bootstrap/BootstrapException.java         |  2 +-
 .../bootstrap/Elasticsearch.java              | 10 ++++--
 .../bootstrap/ElasticsearchCliTests.java      | 22 ++++++------
 .../bootstrap/ESElasticsearchCliTestCase.java |  6 ++--
 7 files changed, 70 insertions(+), 29 deletions(-)

diff --git a/distribution/src/bin/elasticsearch b/distribution/src/bin/elasticsearch
index b7ed2b648b76f..d1b32af677531 100755
--- a/distribution/src/bin/elasticsearch
+++ b/distribution/src/bin/elasticsearch
@@ -24,6 +24,22 @@ ES_JVM_OPTIONS="$ES_PATH_CONF"/jvm.options
 JVM_OPTIONS=`"$JAVA" -cp "$ES_CLASSPATH" org.elasticsearch.tools.launchers.JvmOptionsParser "$ES_JVM_OPTIONS"`
 ES_JAVA_OPTS="${JVM_OPTIONS//\$\{ES_TMPDIR\}/$ES_TMPDIR}"
 
+KEYSTORE_PASSWORD=
+# If keystore exists and is encrypted, prompt for a password
+if [ -f "$ES_PATH_CONF"/elasticsearch.keystore ]
+    && ! echo $* | grep -E '(^-h |-h$| -h |--help$|--help )' >/dev/null 2>&1 \
+    && ! echo $* | grep -E '(^-V |-V$| -V |--version$|--version )' >/dev/null 2>&1 \
+    && ! "`dirname "$0"`"/elasticsearch-keystore list <<<"" >/dev/null 2>&1 ; then
+  echo -n "Elasticsearch keystore password: "
+  read -s KEYSTORE_PASSWORD
+  echo
+
+  if ! "`dirname "$0"`"/elasticsearch-keystore list <<<$KEYSTORE_PASSWORD >/dev/null 2>&1 ; then
+    echo "Incorrect password for encrypted keystore" >&2
+    exit 1
+  fi
+fi
+
 # manual parsing to find out, if process should be detached
 if ! echo $* | grep -E '(^-d |-d$| -d |--daemonize$|--daemonize )' > /dev/null; then
   exec \
@@ -36,7 +52,8 @@ 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:+-i}"\
+    <<<$KEYSTORE_PASSWORD
 else
   exec \
     "$JAVA" \
diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java
index f0a64f6599c79..70d47a8bb5199 100644
--- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java
+++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java
@@ -38,7 +38,7 @@ public void testPathHome() throws Exception {
                 ExitCodes.OK,
                 true,
                 output -> {},
-                (foreground, pidFile, quiet, esSettings) -> {
+                (foreground, pidFile, quiet, stdin, esSettings) -> {
                     Settings settings = esSettings.settings();
                     assertThat(settings.size(), equalTo(2));
                     assertEquals(value, settings.get("path.home"));
@@ -51,7 +51,7 @@ public void testPathHome() throws Exception {
                 ExitCodes.OK,
                 true,
                 output -> {},
-                (foreground, pidFile, quiet, esSettings) -> {
+                (foreground, pidFile, quiet, stdin, esSettings) -> {
                     Settings settings = esSettings.settings();
                     assertThat(settings.size(), equalTo(2));
                     assertEquals(commandLineValue, settings.get("path.home"));
diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
index a4a2696fdad73..d4eaa46323c92 100644
--- a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
+++ b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
@@ -49,9 +49,12 @@
 import org.elasticsearch.node.Node;
 import org.elasticsearch.node.NodeValidationException;
 
+import java.io.BufferedReader;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.io.InputStreamReader;
 import java.io.PrintStream;
+import java.io.Reader;
 import java.io.UnsupportedEncodingException;
 import java.net.URISyntaxException;
 import java.nio.file.Path;
@@ -226,7 +229,7 @@ protected void validateNodeBeforeAcceptingRequests(
         };
     }
 
-    static SecureSettings loadSecureSettings(Environment initialEnv) throws BootstrapException {
+    static SecureSettings loadSecureSettings(Environment initialEnv, boolean stdin) throws BootstrapException {
         final KeyStoreWrapper keystore;
         try {
             keystore = KeyStoreWrapper.load(initialEnv.configFile());
@@ -234,14 +237,30 @@ static SecureSettings loadSecureSettings(Environment initialEnv) throws Bootstra
             throw new BootstrapException(e);
         }
 
+        char[] password;
+        if (stdin) {
+            try (
+                BufferedReader br = new BufferedReader(
+                     new InputStreamReader(System.in)
+                )
+            ) {
+                password = br.readLine().toCharArray();
+            } catch (IOException e) {
+                throw new BootstrapException(e);
+            }
+        } else {
+            // TODO[wrb]: is there a case w/o stdin?
+            password = new char[0];
+        }
+
         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);
+                KeyStoreWrapper.upgrade(keystore, initialEnv.configFile(), password);
             }
         } catch (Exception e) {
             throw new BootstrapException(e);
@@ -290,17 +309,18 @@ static void stop() throws IOException {
      * This method is invoked by {@link Elasticsearch#main(String[])} to startup elasticsearch.
      */
     static void init(
-            final boolean foreground,
-            final Path pidFile,
-            final boolean quiet,
-            final Environment initialEnv) throws BootstrapException, NodeValidationException, UserException {
+        final boolean foreground,
+        final Path pidFile,
+        final boolean quiet,
+        final boolean stdin,
+        final Environment initialEnv) throws BootstrapException, NodeValidationException, UserException {
         // force the class initializer for BootstrapInfo to run before
         // the security manager is installed
         BootstrapInfo.init();
 
         INSTANCE = new Bootstrap();
 
-        final SecureSettings keystore = loadSecureSettings(initialEnv);
+        final SecureSettings keystore = loadSecureSettings(initialEnv, stdin);
         final Environment environment = createEnvironment(pidFile, keystore, initialEnv.settings(), initialEnv.configFile());
 
         LogConfigurator.setNodeName(Node.NODE_NAME_SETTING.get(environment.settings()));
diff --git a/server/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java b/server/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java
index 635afaf959967..a10bd27a6958b 100644
--- a/server/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java
+++ b/server/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java
@@ -26,7 +26,7 @@
  * during bootstrap should explicitly declare the checked exceptions that they can throw, rather
  * than declaring the top-level checked exception {@link Exception}. This exception exists to wrap
  * these checked exceptions so that
- * {@link Bootstrap#init(boolean, Path, boolean, org.elasticsearch.env.Environment)}
+ * {@link Bootstrap#init(boolean, Path, boolean, boolean, org.elasticsearch.env.Environment)}
  * does not have to declare all of these checked exceptions.
  */
 class BootstrapException extends Exception {
diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java
index 48cf00164558f..657efcdf95b68 100644
--- a/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java
+++ b/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java
@@ -49,6 +49,7 @@ class Elasticsearch extends EnvironmentAwareCommand {
     private final OptionSpecBuilder daemonizeOption;
     private final OptionSpec<Path> pidfileOption;
     private final OptionSpecBuilder quietOption;
+    private final OptionSpecBuilder stdinOption;
 
     // visible for testing
     Elasticsearch() {
@@ -67,6 +68,8 @@ class Elasticsearch extends EnvironmentAwareCommand {
             "Turns off standard output/error streams logging in console")
             .availableUnless(versionOption)
             .availableUnless(daemonizeOption);
+        stdinOption = parser.acceptsAll(Arrays.asList("i", "stdin"),
+            "Listens for a keystore password on stdin");
     }
 
     /**
@@ -138,6 +141,7 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th
         final boolean daemonize = options.has(daemonizeOption);
         final Path pidFile = pidfileOption.value(options);
         final boolean quiet = options.has(quietOption);
+        final boolean stdin = options.has(stdinOption);
 
         // a misconfigured java.io.tmpdir can cause hard-to-diagnose problems later, so reject it immediately
         try {
@@ -147,16 +151,16 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th
         }
 
         try {
-            init(daemonize, pidFile, quiet, env);
+            init(daemonize, pidFile, quiet, stdin, env);
         } catch (NodeValidationException e) {
             throw new UserException(ExitCodes.CONFIG, e.getMessage());
         }
     }
 
-    void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment initialEnv)
+    void init(final boolean daemonize, final Path pidFile, final boolean quiet, final boolean stdin, Environment initialEnv)
         throws NodeValidationException, UserException {
         try {
-            Bootstrap.init(!daemonize, pidFile, quiet, initialEnv);
+            Bootstrap.init(!daemonize, pidFile, quiet, stdin, initialEnv);
         } catch (BootstrapException | RuntimeException e) {
             // format exceptions to the console in a special way
             // to avoid 2MB stacktraces from guice, etc.
diff --git a/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java b/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java
index f59584bb90cc5..aa3418d28ee97 100644
--- a/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java
+++ b/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java
@@ -76,7 +76,7 @@ private void runTestThatVersionIsReturned(String... args) throws Exception {
     }
 
     private void runTestVersion(int expectedStatus, Consumer<String> outputConsumer, String... args) throws Exception {
-        runTest(expectedStatus, false, outputConsumer, (foreground, pidFile, quiet, esSettings) -> {}, args);
+        runTest(expectedStatus, false, outputConsumer, (foreground, pidFile, quiet, stdin, esSettings) -> {}, args);
     }
 
     public void testPositionalArgs() throws Exception {
@@ -84,19 +84,19 @@ public void testPositionalArgs() throws Exception {
             ExitCodes.USAGE,
             false,
             output -> assertThat(output, containsString("Positional arguments not allowed, found [foo]")),
-            (foreground, pidFile, quiet, esSettings) -> {},
+            (foreground, pidFile, quiet, stdin, esSettings) -> {},
             "foo");
         runTest(
             ExitCodes.USAGE,
             false,
             output -> assertThat(output, containsString("Positional arguments not allowed, found [foo, bar]")),
-            (foreground, pidFile, quiet, esSettings) -> {},
+            (foreground, pidFile, quiet, stdin, esSettings) -> {},
             "foo", "bar");
         runTest(
             ExitCodes.USAGE,
             false,
             output -> assertThat(output, containsString("Positional arguments not allowed, found [foo]")),
-            (foreground, pidFile, quiet, esSettings) -> {},
+            (foreground, pidFile, quiet, stdin, esSettings) -> {},
             "-E", "foo=bar", "foo", "-E", "baz=qux");
     }
 
@@ -116,7 +116,7 @@ private void runPidFileTest(final int expectedStatus, final boolean expectedInit
                 expectedStatus,
                 expectedInit,
                 outputConsumer,
-                (foreground, pidFile, quiet, esSettings) -> assertThat(pidFile.toString(), equalTo(expectedPidFile.toString())),
+                (foreground, pidFile, quiet, stdin, esSettings) -> assertThat(pidFile.toString(), equalTo(expectedPidFile.toString())),
                 args);
     }
 
@@ -131,7 +131,7 @@ private void runDaemonizeTest(final boolean expectedDaemonize, final String... a
                 ExitCodes.OK,
                 true,
                 output -> {},
-                (foreground, pidFile, quiet, esSettings) -> assertThat(foreground, equalTo(!expectedDaemonize)),
+                (foreground, pidFile, quiet, stdin, esSettings) -> assertThat(foreground, equalTo(!expectedDaemonize)),
                 args);
     }
 
@@ -146,7 +146,7 @@ private void runQuietTest(final boolean expectedQuiet, final String... args) thr
                 ExitCodes.OK,
                 true,
                 output -> {},
-                (foreground, pidFile, quiet, esSettings) -> assertThat(quiet, equalTo(expectedQuiet)),
+                (foreground, pidFile, quiet, stdin, esSettings) -> assertThat(quiet, equalTo(expectedQuiet)),
                 args);
     }
 
@@ -155,7 +155,7 @@ public void testElasticsearchSettings() throws Exception {
                 ExitCodes.OK,
                 true,
                 output -> {},
-                (foreground, pidFile, quiet, env) -> {
+                (foreground, pidFile, quiet, stdin, env) -> {
                     Settings settings = env.settings();
                     assertEquals("bar", settings.get("foo"));
                     assertEquals("qux", settings.get("baz"));
@@ -168,7 +168,7 @@ public void testElasticsearchSettingCanNotBeEmpty() throws Exception {
                 ExitCodes.USAGE,
                 false,
                 output -> assertThat(output, containsString("setting [foo] must not be empty")),
-                (foreground, pidFile, quiet, esSettings) -> {},
+                (foreground, pidFile, quiet, stdin, esSettings) -> {},
                 "-E", "foo=");
     }
 
@@ -177,7 +177,7 @@ public void testElasticsearchSettingCanNotBeDuplicated() throws Exception {
                 ExitCodes.USAGE,
                 false,
                 output -> assertThat(output, containsString("setting [foo] already set, saw [bar] and [baz]")),
-                (foreground, pidFile, quiet, initialEnv) -> {},
+                (foreground, pidFile, quiet, stdin, initialEnv) -> {},
                 "-E", "foo=bar", "-E", "foo=baz");
     }
 
@@ -186,7 +186,7 @@ public void testUnknownOption() throws Exception {
                 ExitCodes.USAGE,
                 false,
                 output -> assertThat(output, containsString("network.host is not a recognized option")),
-                (foreground, pidFile, quiet, esSettings) -> {},
+                (foreground, pidFile, quiet, stdin, esSettings) -> {},
                 "--network.host");
     }
 
diff --git a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java
index 62b2b422f7803..1763cb9baf484 100644
--- a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java
+++ b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java
@@ -35,7 +35,7 @@
 abstract class ESElasticsearchCliTestCase extends ESTestCase {
 
     interface InitConsumer {
-        void accept(boolean foreground, Path pidFile, boolean quiet, Environment initialEnv);
+        void accept(boolean foreground, Path pidFile, boolean quiet, boolean stdin, Environment initialEnv);
     }
 
     void runTest(
@@ -57,9 +57,9 @@ protected Environment createEnv(final Map<String, String> settings) throws UserE
                     return new Environment(realSettings, home.resolve("config"));
                 }
                 @Override
-                void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment initialEnv) {
+                void init(final boolean daemonize, final Path pidFile, final boolean quiet, boolean stdin, Environment initialEnv) {
                     init.set(true);
-                    initConsumer.accept(!daemonize, pidFile, quiet, initialEnv);
+                    initConsumer.accept(!daemonize, pidFile, quiet, stdin, initialEnv);
                 }
 
                 @Override

From a167d5c22b4bbb03799bf60013211d6f326a375e Mon Sep 17 00:00:00 2001
From: William Brafford <william.brafford@elastic.co>
Date: Mon, 24 Jun 2019 14:52:56 -0400
Subject: [PATCH 2/8] Add thread safety and fix typos

We now use the "Terminal" class, which encapsulates thread-safe access
to stdin.

Additionally, we now test for a particular "encrypted" byte flag in the
keystore file rather than running the slow keystore utility, and we
don't test the password before passing it to Elasticsearch, which avoids
a second call to the keystore utility.
---
 distribution/src/bin/elasticsearch                 | 12 ++++--------
 .../org/elasticsearch/bootstrap/Bootstrap.java     | 14 +++++---------
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/distribution/src/bin/elasticsearch b/distribution/src/bin/elasticsearch
index d1b32af677531..1b4c747d455d5 100755
--- a/distribution/src/bin/elasticsearch
+++ b/distribution/src/bin/elasticsearch
@@ -26,18 +26,14 @@ ES_JAVA_OPTS="${JVM_OPTIONS//\$\{ES_TMPDIR\}/$ES_TMPDIR}"
 
 KEYSTORE_PASSWORD=
 # If keystore exists and is encrypted, prompt for a password
-if [ -f "$ES_PATH_CONF"/elasticsearch.keystore ]
+if [ -f "$ES_PATH_CONF"/elasticsearch.keystore ] \
     && ! echo $* | grep -E '(^-h |-h$| -h |--help$|--help )' >/dev/null 2>&1 \
     && ! echo $* | grep -E '(^-V |-V$| -V |--version$|--version )' >/dev/null 2>&1 \
-    && ! "`dirname "$0"`"/elasticsearch-keystore list <<<"" >/dev/null 2>&1 ; then
+    && hexdump -b -s 31 -n1 "$ES_PATH_CONF"/elasticsearch.keystore | head -1 | cut -d' ' -f2 | grep '001' >/dev/null 2>&1
+then
   echo -n "Elasticsearch keystore password: "
   read -s KEYSTORE_PASSWORD
   echo
-
-  if ! "`dirname "$0"`"/elasticsearch-keystore list <<<$KEYSTORE_PASSWORD >/dev/null 2>&1 ; then
-    echo "Incorrect password for encrypted keystore" >&2
-    exit 1
-  fi
 fi
 
 # manual parsing to find out, if process should be detached
@@ -52,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:+-i}"\
+    "$@" ${KEYSTORE_PASSWORD:+-i} \
     <<<$KEYSTORE_PASSWORD
 else
   exec \
diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
index d4eaa46323c92..fa516e24467b3 100644
--- a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
+++ b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
@@ -26,6 +26,7 @@
 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.Terminal;
 import org.elasticsearch.core.internal.io.IOUtils;
 import org.apache.lucene.util.StringHelper;
 import org.elasticsearch.ElasticsearchException;
@@ -239,15 +240,7 @@ static SecureSettings loadSecureSettings(Environment initialEnv, boolean stdin)
 
         char[] password;
         if (stdin) {
-            try (
-                BufferedReader br = new BufferedReader(
-                     new InputStreamReader(System.in)
-                )
-            ) {
-                password = br.readLine().toCharArray();
-            } catch (IOException e) {
-                throw new BootstrapException(e);
-            }
+            password = Terminal.DEFAULT.readSecret("Elasticsearch password? ");
         } else {
             // TODO[wrb]: is there a case w/o stdin?
             password = new char[0];
@@ -263,6 +256,9 @@ static SecureSettings loadSecureSettings(Environment initialEnv, boolean stdin)
                 KeyStoreWrapper.upgrade(keystore, initialEnv.configFile(), password);
             }
         } catch (Exception e) {
+            if (password.length != 0) {
+                throw new BootstrapException(new SecurityException("Incorrect keystore password"));
+            }
             throw new BootstrapException(e);
         }
         return keystore;

From e390afe7455877b5b76a99929e767bf80dfd580f Mon Sep 17 00:00:00 2001
From: William Brafford <william.brafford@elastic.co>
Date: Thu, 27 Jun 2019 14:59:40 -0400
Subject: [PATCH 3/8] Remove command line flag for standard input

In a discussion meeting, it was pointed out to me that the keystore
wapper object's hasPassword() method makes the standard input flag that
I added redundant, so I'm backing out that fairly intrusive change to
the Java code.
---
 distribution/src/bin/elasticsearch            |  3 +--
 .../bootstrap/EvilElasticsearchCliTests.java  |  4 ++--
 .../elasticsearch/bootstrap/Bootstrap.java    |  7 +++---
 .../bootstrap/BootstrapException.java         |  2 +-
 .../bootstrap/Elasticsearch.java              | 10 +++------
 .../bootstrap/ElasticsearchCliTests.java      | 22 +++++++++----------
 .../bootstrap/ESElasticsearchCliTestCase.java |  6 ++---
 7 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/distribution/src/bin/elasticsearch b/distribution/src/bin/elasticsearch
index 1b4c747d455d5..10a03b951bb7b 100755
--- a/distribution/src/bin/elasticsearch
+++ b/distribution/src/bin/elasticsearch
@@ -48,8 +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:+-i} \
-    <<<$KEYSTORE_PASSWORD
+    "$@" <<<$KEYSTORE_PASSWORD
 else
   exec \
     "$JAVA" \
diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java
index 70d47a8bb5199..f0a64f6599c79 100644
--- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java
+++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java
@@ -38,7 +38,7 @@ public void testPathHome() throws Exception {
                 ExitCodes.OK,
                 true,
                 output -> {},
-                (foreground, pidFile, quiet, stdin, esSettings) -> {
+                (foreground, pidFile, quiet, esSettings) -> {
                     Settings settings = esSettings.settings();
                     assertThat(settings.size(), equalTo(2));
                     assertEquals(value, settings.get("path.home"));
@@ -51,7 +51,7 @@ public void testPathHome() throws Exception {
                 ExitCodes.OK,
                 true,
                 output -> {},
-                (foreground, pidFile, quiet, stdin, esSettings) -> {
+                (foreground, pidFile, quiet, esSettings) -> {
                     Settings settings = esSettings.settings();
                     assertThat(settings.size(), equalTo(2));
                     assertEquals(commandLineValue, settings.get("path.home"));
diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
index fa516e24467b3..fad92818cb3ff 100644
--- a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
+++ b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
@@ -230,7 +230,7 @@ protected void validateNodeBeforeAcceptingRequests(
         };
     }
 
-    static SecureSettings loadSecureSettings(Environment initialEnv, boolean stdin) throws BootstrapException {
+    static SecureSettings loadSecureSettings(Environment initialEnv) throws BootstrapException {
         final KeyStoreWrapper keystore;
         try {
             keystore = KeyStoreWrapper.load(initialEnv.configFile());
@@ -239,7 +239,7 @@ static SecureSettings loadSecureSettings(Environment initialEnv, boolean stdin)
         }
 
         char[] password;
-        if (stdin) {
+        if (keystore != null && keystore.hasPassword()) {
             password = Terminal.DEFAULT.readSecret("Elasticsearch password? ");
         } else {
             // TODO[wrb]: is there a case w/o stdin?
@@ -308,7 +308,6 @@ static void init(
         final boolean foreground,
         final Path pidFile,
         final boolean quiet,
-        final boolean stdin,
         final Environment initialEnv) throws BootstrapException, NodeValidationException, UserException {
         // force the class initializer for BootstrapInfo to run before
         // the security manager is installed
@@ -316,7 +315,7 @@ static void init(
 
         INSTANCE = new Bootstrap();
 
-        final SecureSettings keystore = loadSecureSettings(initialEnv, stdin);
+        final SecureSettings keystore = loadSecureSettings(initialEnv);
         final Environment environment = createEnvironment(pidFile, keystore, initialEnv.settings(), initialEnv.configFile());
 
         LogConfigurator.setNodeName(Node.NODE_NAME_SETTING.get(environment.settings()));
diff --git a/server/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java b/server/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java
index a10bd27a6958b..635afaf959967 100644
--- a/server/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java
+++ b/server/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java
@@ -26,7 +26,7 @@
  * during bootstrap should explicitly declare the checked exceptions that they can throw, rather
  * than declaring the top-level checked exception {@link Exception}. This exception exists to wrap
  * these checked exceptions so that
- * {@link Bootstrap#init(boolean, Path, boolean, boolean, org.elasticsearch.env.Environment)}
+ * {@link Bootstrap#init(boolean, Path, boolean, org.elasticsearch.env.Environment)}
  * does not have to declare all of these checked exceptions.
  */
 class BootstrapException extends Exception {
diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java
index 657efcdf95b68..48cf00164558f 100644
--- a/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java
+++ b/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java
@@ -49,7 +49,6 @@ class Elasticsearch extends EnvironmentAwareCommand {
     private final OptionSpecBuilder daemonizeOption;
     private final OptionSpec<Path> pidfileOption;
     private final OptionSpecBuilder quietOption;
-    private final OptionSpecBuilder stdinOption;
 
     // visible for testing
     Elasticsearch() {
@@ -68,8 +67,6 @@ class Elasticsearch extends EnvironmentAwareCommand {
             "Turns off standard output/error streams logging in console")
             .availableUnless(versionOption)
             .availableUnless(daemonizeOption);
-        stdinOption = parser.acceptsAll(Arrays.asList("i", "stdin"),
-            "Listens for a keystore password on stdin");
     }
 
     /**
@@ -141,7 +138,6 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th
         final boolean daemonize = options.has(daemonizeOption);
         final Path pidFile = pidfileOption.value(options);
         final boolean quiet = options.has(quietOption);
-        final boolean stdin = options.has(stdinOption);
 
         // a misconfigured java.io.tmpdir can cause hard-to-diagnose problems later, so reject it immediately
         try {
@@ -151,16 +147,16 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th
         }
 
         try {
-            init(daemonize, pidFile, quiet, stdin, env);
+            init(daemonize, pidFile, quiet, env);
         } catch (NodeValidationException e) {
             throw new UserException(ExitCodes.CONFIG, e.getMessage());
         }
     }
 
-    void init(final boolean daemonize, final Path pidFile, final boolean quiet, final boolean stdin, Environment initialEnv)
+    void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment initialEnv)
         throws NodeValidationException, UserException {
         try {
-            Bootstrap.init(!daemonize, pidFile, quiet, stdin, initialEnv);
+            Bootstrap.init(!daemonize, pidFile, quiet, initialEnv);
         } catch (BootstrapException | RuntimeException e) {
             // format exceptions to the console in a special way
             // to avoid 2MB stacktraces from guice, etc.
diff --git a/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java b/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java
index aa3418d28ee97..f59584bb90cc5 100644
--- a/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java
+++ b/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java
@@ -76,7 +76,7 @@ private void runTestThatVersionIsReturned(String... args) throws Exception {
     }
 
     private void runTestVersion(int expectedStatus, Consumer<String> outputConsumer, String... args) throws Exception {
-        runTest(expectedStatus, false, outputConsumer, (foreground, pidFile, quiet, stdin, esSettings) -> {}, args);
+        runTest(expectedStatus, false, outputConsumer, (foreground, pidFile, quiet, esSettings) -> {}, args);
     }
 
     public void testPositionalArgs() throws Exception {
@@ -84,19 +84,19 @@ public void testPositionalArgs() throws Exception {
             ExitCodes.USAGE,
             false,
             output -> assertThat(output, containsString("Positional arguments not allowed, found [foo]")),
-            (foreground, pidFile, quiet, stdin, esSettings) -> {},
+            (foreground, pidFile, quiet, esSettings) -> {},
             "foo");
         runTest(
             ExitCodes.USAGE,
             false,
             output -> assertThat(output, containsString("Positional arguments not allowed, found [foo, bar]")),
-            (foreground, pidFile, quiet, stdin, esSettings) -> {},
+            (foreground, pidFile, quiet, esSettings) -> {},
             "foo", "bar");
         runTest(
             ExitCodes.USAGE,
             false,
             output -> assertThat(output, containsString("Positional arguments not allowed, found [foo]")),
-            (foreground, pidFile, quiet, stdin, esSettings) -> {},
+            (foreground, pidFile, quiet, esSettings) -> {},
             "-E", "foo=bar", "foo", "-E", "baz=qux");
     }
 
@@ -116,7 +116,7 @@ private void runPidFileTest(final int expectedStatus, final boolean expectedInit
                 expectedStatus,
                 expectedInit,
                 outputConsumer,
-                (foreground, pidFile, quiet, stdin, esSettings) -> assertThat(pidFile.toString(), equalTo(expectedPidFile.toString())),
+                (foreground, pidFile, quiet, esSettings) -> assertThat(pidFile.toString(), equalTo(expectedPidFile.toString())),
                 args);
     }
 
@@ -131,7 +131,7 @@ private void runDaemonizeTest(final boolean expectedDaemonize, final String... a
                 ExitCodes.OK,
                 true,
                 output -> {},
-                (foreground, pidFile, quiet, stdin, esSettings) -> assertThat(foreground, equalTo(!expectedDaemonize)),
+                (foreground, pidFile, quiet, esSettings) -> assertThat(foreground, equalTo(!expectedDaemonize)),
                 args);
     }
 
@@ -146,7 +146,7 @@ private void runQuietTest(final boolean expectedQuiet, final String... args) thr
                 ExitCodes.OK,
                 true,
                 output -> {},
-                (foreground, pidFile, quiet, stdin, esSettings) -> assertThat(quiet, equalTo(expectedQuiet)),
+                (foreground, pidFile, quiet, esSettings) -> assertThat(quiet, equalTo(expectedQuiet)),
                 args);
     }
 
@@ -155,7 +155,7 @@ public void testElasticsearchSettings() throws Exception {
                 ExitCodes.OK,
                 true,
                 output -> {},
-                (foreground, pidFile, quiet, stdin, env) -> {
+                (foreground, pidFile, quiet, env) -> {
                     Settings settings = env.settings();
                     assertEquals("bar", settings.get("foo"));
                     assertEquals("qux", settings.get("baz"));
@@ -168,7 +168,7 @@ public void testElasticsearchSettingCanNotBeEmpty() throws Exception {
                 ExitCodes.USAGE,
                 false,
                 output -> assertThat(output, containsString("setting [foo] must not be empty")),
-                (foreground, pidFile, quiet, stdin, esSettings) -> {},
+                (foreground, pidFile, quiet, esSettings) -> {},
                 "-E", "foo=");
     }
 
@@ -177,7 +177,7 @@ public void testElasticsearchSettingCanNotBeDuplicated() throws Exception {
                 ExitCodes.USAGE,
                 false,
                 output -> assertThat(output, containsString("setting [foo] already set, saw [bar] and [baz]")),
-                (foreground, pidFile, quiet, stdin, initialEnv) -> {},
+                (foreground, pidFile, quiet, initialEnv) -> {},
                 "-E", "foo=bar", "-E", "foo=baz");
     }
 
@@ -186,7 +186,7 @@ public void testUnknownOption() throws Exception {
                 ExitCodes.USAGE,
                 false,
                 output -> assertThat(output, containsString("network.host is not a recognized option")),
-                (foreground, pidFile, quiet, stdin, esSettings) -> {},
+                (foreground, pidFile, quiet, esSettings) -> {},
                 "--network.host");
     }
 
diff --git a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java
index 1763cb9baf484..62b2b422f7803 100644
--- a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java
+++ b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java
@@ -35,7 +35,7 @@
 abstract class ESElasticsearchCliTestCase extends ESTestCase {
 
     interface InitConsumer {
-        void accept(boolean foreground, Path pidFile, boolean quiet, boolean stdin, Environment initialEnv);
+        void accept(boolean foreground, Path pidFile, boolean quiet, Environment initialEnv);
     }
 
     void runTest(
@@ -57,9 +57,9 @@ protected Environment createEnv(final Map<String, String> settings) throws UserE
                     return new Environment(realSettings, home.resolve("config"));
                 }
                 @Override
-                void init(final boolean daemonize, final Path pidFile, final boolean quiet, boolean stdin, Environment initialEnv) {
+                void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment initialEnv) {
                     init.set(true);
-                    initConsumer.accept(!daemonize, pidFile, quiet, stdin, initialEnv);
+                    initConsumer.accept(!daemonize, pidFile, quiet, initialEnv);
                 }
 
                 @Override

From 8c99702cd74971e3e4591716b10b568f4dd254d0 Mon Sep 17 00:00:00 2001
From: William Brafford <william.brafford@elastic.co>
Date: Mon, 1 Jul 2019 16:31:31 -0400
Subject: [PATCH 4/8] Let bin/elasticsearch read passwd from FIFO/file

---
 distribution/src/bin/elasticsearch               | 16 +++++++++++++---
 .../org/elasticsearch/bootstrap/Bootstrap.java   | 11 ++++-------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/distribution/src/bin/elasticsearch b/distribution/src/bin/elasticsearch
index 10a03b951bb7b..d9bd5bc287a04 100755
--- a/distribution/src/bin/elasticsearch
+++ b/distribution/src/bin/elasticsearch
@@ -24,6 +24,7 @@ ES_JVM_OPTIONS="$ES_PATH_CONF"/jvm.options
 JVM_OPTIONS=`"$JAVA" -cp "$ES_CLASSPATH" org.elasticsearch.tools.launchers.JvmOptionsParser "$ES_JVM_OPTIONS"`
 ES_JAVA_OPTS="${JVM_OPTIONS//\$\{ES_TMPDIR\}/$ES_TMPDIR}"
 
+unset KEYSTORE_PASSWORD
 KEYSTORE_PASSWORD=
 # If keystore exists and is encrypted, prompt for a password
 if [ -f "$ES_PATH_CONF"/elasticsearch.keystore ] \
@@ -31,9 +32,18 @@ if [ -f "$ES_PATH_CONF"/elasticsearch.keystore ] \
     && ! echo $* | grep -E '(^-V |-V$| -V |--version$|--version )' >/dev/null 2>&1 \
     && hexdump -b -s 31 -n1 "$ES_PATH_CONF"/elasticsearch.keystore | head -1 | cut -d' ' -f2 | grep '001' >/dev/null 2>&1
 then
-  echo -n "Elasticsearch keystore password: "
-  read -s KEYSTORE_PASSWORD
-  echo
+  if [ -e "$ES_KEYSTORE_PASSPHRASE_FILE" ] ; then
+    # read from a file or a FIFO
+    if [ -p "$ES_KEYSTORE_PASSPHRASE_FILE" ] ; then
+      echo "Reading keystore passphrase from FIFO (blocking operation)"
+    fi
+  read -s KEYSTORE_PASSWORD <"$ES_KEYSTORE_PASSPHRASE_FILE"
+  else
+    # read from standard input
+    echo -n "Elasticsearch keystore password: "
+    read -s KEYSTORE_PASSWORD
+    echo
+  fi
 fi
 
 # manual parsing to find out, if process should be detached
diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
index fad92818cb3ff..a7a20629a2741 100644
--- a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
+++ b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
@@ -50,12 +50,9 @@
 import org.elasticsearch.node.Node;
 import org.elasticsearch.node.NodeValidationException;
 
-import java.io.BufferedReader;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.InputStreamReader;
 import java.io.PrintStream;
-import java.io.Reader;
 import java.io.UnsupportedEncodingException;
 import java.net.URISyntaxException;
 import java.nio.file.Path;
@@ -305,10 +302,10 @@ static void stop() throws IOException {
      * This method is invoked by {@link Elasticsearch#main(String[])} to startup elasticsearch.
      */
     static void init(
-        final boolean foreground,
-        final Path pidFile,
-        final boolean quiet,
-        final Environment initialEnv) throws BootstrapException, NodeValidationException, UserException {
+            final boolean foreground,
+            final Path pidFile,
+            final boolean quiet,
+            final Environment initialEnv) throws BootstrapException, NodeValidationException, UserException {
         // force the class initializer for BootstrapInfo to run before
         // the security manager is installed
         BootstrapInfo.init();

From ef66eafb1299874ef8762ebeb9b3980a016a467d Mon Sep 17 00:00:00 2001
From: William Brafford <william.brafford@elastic.co>
Date: Wed, 3 Jul 2019 13:11:16 -0400
Subject: [PATCH 5/8] Improve bin/elasticsearch portability

It's convenient to use the bin/elasticsearch-keystore list command to
test whether or not the keystore is encrypted. When no password is
supplied to an encrypted keystore, the command has a non-zero exit
status. However, many JVMs on OSX with default networking configuration
will take at least five seconds to run an elasticsearch-keystore
command. This is because we have to initialize log4j 2, and that
initialization code will call InetAddress.getLocalHost().getHostname(),
which is the bit of code with the bug. Since all our OSX distributions
come in tarballs, I've parameterized bin/elasticsearch so that we check
the encrypted flag byte with "dd". It might be nice to do this just for
Darwin distributions. For RPMs and Debian distros, we use the keystore
tool for the check.

I've also added a little bit more output to the bin/elasticsearch
command, and improved handling of the case where the script fails to
read any lines from standard input.
---
 distribution/build.gradle          |  8 ++++++++
 distribution/src/bin/elasticsearch | 14 +++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/distribution/build.gradle b/distribution/build.gradle
index 9606604036101..41b634301c0db 100644
--- a/distribution/build.gradle
+++ b/distribution/build.gradle
@@ -549,6 +549,14 @@ subprojects {
       'license.text': [
         'deb': licenseText,
       ],
+
+      'keystore.passwd.check': [
+        /* Avoid unfortunate 5-second delay for elasticsearch-cli
+           scripts on certain OSX/JVM combos by reading the
+           "encrypted" flag byte in the keystore directly */
+        'tar': '[ "$(echo -n "\000")" = "$(dd if="$ES_PATH_CONF"/elasticsearch.keystore ibs=1 skip=31 count=1 2>/dev/null)" ]',
+        'def': '"`dirname "$0"`"/elasticsearch-keystore list <&-'
+      ],
     ]
     Map<String, String> result = [:]
     expansions = expansions.each { key, value ->
diff --git a/distribution/src/bin/elasticsearch b/distribution/src/bin/elasticsearch
index d9bd5bc287a04..b4a9b5f52c9e0 100755
--- a/distribution/src/bin/elasticsearch
+++ b/distribution/src/bin/elasticsearch
@@ -30,18 +30,22 @@ KEYSTORE_PASSWORD=
 if [ -f "$ES_PATH_CONF"/elasticsearch.keystore ] \
     && ! echo $* | grep -E '(^-h |-h$| -h |--help$|--help )' >/dev/null 2>&1 \
     && ! echo $* | grep -E '(^-V |-V$| -V |--version$|--version )' >/dev/null 2>&1 \
-    && hexdump -b -s 31 -n1 "$ES_PATH_CONF"/elasticsearch.keystore | head -1 | cut -d' ' -f2 | grep '001' >/dev/null 2>&1
+    && ! ${keystore.passwd.check} >/dev/null 2>&1
 then
   if [ -e "$ES_KEYSTORE_PASSPHRASE_FILE" ] ; then
     # read from a file or a FIFO
     if [ -p "$ES_KEYSTORE_PASSPHRASE_FILE" ] ; then
-      echo "Reading keystore passphrase from FIFO (blocking operation)"
+      echo "Reading keystore passphrase from FIFO $ES_KEYSTORE_PASSPHRASE_FILE (blocking operation)" 1>&2
+    else
+      echo "Reading keystore passphrase from file $ES_KEYSTORE_PASSPHRASE_FILE" 1>&2
     fi
-  read -s KEYSTORE_PASSWORD <"$ES_KEYSTORE_PASSPHRASE_FILE"
+  read -s KEYSTORE_PASSWORD -u "$ES_KEYSTORE_PASSPHRASE_FILE"
   else
     # read from standard input
-    echo -n "Elasticsearch keystore password: "
-    read -s KEYSTORE_PASSWORD
+    if ! read -s -p "Elasticsearch keystore password: " KEYSTORE_PASSWORD ; then
+      echo "Failed to read keystore password on standard input" 1>&2
+      exit 1
+    fi
     echo
   fi
 fi

From 6a2ecc546a7ed2e92b90e58b06d9a3d5dd8dd2f2 Mon Sep 17 00:00:00 2001
From: William Brafford <william.brafford@elastic.co>
Date: Wed, 10 Jul 2019 17:00:12 -0400
Subject: [PATCH 6/8] Avoid multiple buffered readers of standard input

Java's classic buffered readers are efficient because they read more
than is strictly needed for an operation and store it in a buffer. The
implication of this is that you shouldn't use multiple buffered readers
on the same input stream. Unfortunately, our CLI tools were doing this
in a few places. Tests didn't catch this because our mock Terminal class
doesn't reveal the problem.

First, the SystemTerminal's readText method was creating a new
BufferedReader per invocation, which meant that it cannot reliably read
two lines in a row. There is a new TerminalTest that covers this case.
The code fix was to lazily initialize a new BufferedReader the first
time we try to read from standard input.

Second, the AddStringKeyStoreCommand class created its own
BufferedReader when the -x/--stdin option was supplied. I'm not sure
what this flag really accomplishes, other than perhaps not printing a
prompt. Without the flag, the input still comes in over stdin. At any
rate, I've replaced the BufferedReader with an invocation of the
Terminal and added a test in AddStringKeystoreCommandTests.
---
 .../settings/AddStringKeyStoreCommand.java     |  3 +--
 .../AddStringKeyStoreCommandTests.java         | 18 ++++++++++++++++--
 .../java/org/elasticsearch/cli/Terminal.java   | 12 ++++++++++--
 .../org/elasticsearch/cli/TerminalTests.java   | 16 ++++++++++++++++
 .../org/elasticsearch/cli/CommandTestCase.java |  6 +++++-
 5 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java
index 06204bb46e959..dfe1572b6ecff 100644
--- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java
+++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java
@@ -68,8 +68,7 @@ protected void executeCommand(Terminal terminal, OptionSet options, Environment
 
         final char[] value;
         if (options.has(stdinOption)) {
-            BufferedReader stdinReader = new BufferedReader(new InputStreamReader(getStdin(), StandardCharsets.UTF_8));
-            value = stdinReader.readLine().toCharArray();
+            value = terminal.readSecret("");
         } else {
             value = terminal.readSecret("Enter value for " + setting + ": ");
         }
diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java
index 699f1aee9e622..65d97b64b2a5e 100644
--- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java
+++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java
@@ -26,6 +26,7 @@
 
 import org.elasticsearch.cli.Command;
 import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.cli.UserException;
 import org.elasticsearch.env.Environment;
 
@@ -164,7 +165,7 @@ public void testStdinShort() throws Exception {
         String password = "keystorepassword";
         KeyStoreWrapper.create().save(env.configFile(), password.toCharArray());
         terminal.addSecretInput(password);
-        setInput("secret value 1");
+        terminal.addSecretInput("secret value 1");
         execute("-x", "foo");
         assertSecureString("foo", "secret value 1", password);
     }
@@ -173,11 +174,24 @@ public void testStdinLong() throws Exception {
         String password = "keystorepassword";
         KeyStoreWrapper.create().save(env.configFile(), password.toCharArray());
         terminal.addSecretInput(password);
-        setInput("secret value 2");
+        terminal.addSecretInput("secret value 2");
         execute("--stdin", "foo");
         assertSecureString("foo", "secret value 2", password);
     }
 
+    public void testStdinSystemTerminal() throws Exception {
+        InputStream sysin = System.in; // save for later
+
+        String password = "keystorepassword";
+        KeyStoreWrapper.create().save(env.configFile(), password.toCharArray());
+
+        String input = password + "\nbar\n";
+        System.setIn(new ByteArrayInputStream(input.getBytes()));
+        execute(Terminal.DEFAULT, "--stdin", "foo");
+
+        System.setIn(sysin);    // cleanup
+    }
+
     public void testMissingSettingName() throws Exception {
         String password = "keystorepassword";
         createKeystore(password);
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 a0ebff5d67041..a54d60cf875d6 100644
--- a/libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java
+++ b/libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java
@@ -149,6 +149,8 @@ private static class SystemTerminal extends Terminal {
 
         private static final PrintWriter WRITER = newWriter();
 
+        private BufferedReader reader;
+
         SystemTerminal() {
             super(System.lineSeparator());
         }
@@ -158,6 +160,13 @@ private static PrintWriter newWriter() {
             return new PrintWriter(System.out);
         }
 
+        private BufferedReader getReader() {
+            if (reader == null) {
+                reader = new BufferedReader(new InputStreamReader(System.in));
+            }
+            return reader;
+        }
+
         @Override
         public PrintWriter getWriter() {
             return WRITER;
@@ -166,9 +175,8 @@ public PrintWriter getWriter() {
         @Override
         public String readText(String text) {
             getWriter().print(text);
-            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?");
                 }
diff --git a/server/src/test/java/org/elasticsearch/cli/TerminalTests.java b/server/src/test/java/org/elasticsearch/cli/TerminalTests.java
index 3b409c2add636..b8faa279a28ea 100644
--- a/server/src/test/java/org/elasticsearch/cli/TerminalTests.java
+++ b/server/src/test/java/org/elasticsearch/cli/TerminalTests.java
@@ -21,6 +21,9 @@
 
 import org.elasticsearch.test.ESTestCase;
 
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+
 public class TerminalTests extends ESTestCase {
     public void testVerbosity() throws Exception {
         MockTerminal terminal = new MockTerminal();
@@ -75,6 +78,19 @@ public void testPromptYesNoCase() throws Exception {
         assertFalse(terminal.promptYesNo("Answer?", true));
     }
 
+    public void testReadTwiceFromSystemTerminal() {
+        InputStream sysIn = System.in;      // save for cleanup
+
+        InputStream in = new ByteArrayInputStream("foo\nbar\n".getBytes());
+        System.setIn(in);
+        Terminal terminal = Terminal.DEFAULT;
+        assertEquals(terminal.readText("say foo"), "foo");
+        assertEquals(terminal.readText("say bar"), "bar");
+
+        // cleanup
+        System.setIn(sysIn);
+    }
+
     private void assertPrinted(MockTerminal logTerminal, Terminal.Verbosity verbosity, String text) throws Exception {
         logTerminal.println(verbosity, text);
         String output = logTerminal.getOutput();
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..741dd737a5e40 100644
--- a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java
+++ b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java
@@ -49,8 +49,12 @@ public void resetTerminal() {
      * The command created can be found in {@link #command}.
      */
     public String execute(String... args) throws Exception {
+        execute(this.terminal, args);
+        return terminal.getOutput();
+    }
+
+    public void execute(Terminal terminal, String... args) throws Exception {
         command = newCommand();
         command.mainWithoutErrorHandling(args, terminal);
-        return terminal.getOutput();
     }
 }

From 85c9160dfbfd47bc588498052bd1fc9c218f36bc Mon Sep 17 00:00:00 2001
From: William Brafford <william.brafford@elastic.co>
Date: Thu, 11 Jul 2019 14:26:44 -0400
Subject: [PATCH 7/8] Avoid forbidden and restricted APIs

I've reworked the unit tests in accordance with gradlew check. Instead
of setting System.in, I've created a test class specifically for the
SystemTerminal. Unlike the MockTerminal we already have, it doesn't
verify output, but it does exercise some code paths that the
MockTerminal skips. I had to add a new protected method to the Terminal
class, which is less than ideal. Perhaps these tests should be
integration tests rather than unit tests.
---
 distribution/src/bin/elasticsearch            |  2 +-
 .../AddStringKeyStoreCommandTests.java        | 12 ++---
 .../java/org/elasticsearch/cli/Terminal.java  | 17 ++++++-
 .../org/elasticsearch/cli/TerminalTests.java  | 13 ++---
 .../elasticsearch/cli/TestSystemTerminal.java | 51 +++++++++++++++++++
 5 files changed, 77 insertions(+), 18 deletions(-)
 create mode 100644 test/framework/src/main/java/org/elasticsearch/cli/TestSystemTerminal.java

diff --git a/distribution/src/bin/elasticsearch b/distribution/src/bin/elasticsearch
index b4a9b5f52c9e0..016c8b6a8b0db 100755
--- a/distribution/src/bin/elasticsearch
+++ b/distribution/src/bin/elasticsearch
@@ -39,7 +39,7 @@ then
     else
       echo "Reading keystore passphrase from file $ES_KEYSTORE_PASSPHRASE_FILE" 1>&2
     fi
-  read -s KEYSTORE_PASSWORD -u "$ES_KEYSTORE_PASSPHRASE_FILE"
+  read -s KEYSTORE_PASSWORD <"$ES_KEYSTORE_PASSPHRASE_FILE"
   else
     # read from standard input
     if ! read -s -p "Elasticsearch keystore password: " KEYSTORE_PASSWORD ; then
diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java
index 65d97b64b2a5e..2060db92af504 100644
--- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java
+++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java
@@ -21,12 +21,14 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
+import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.Map;
 
 import org.elasticsearch.cli.Command;
 import org.elasticsearch.cli.ExitCodes;
 import org.elasticsearch.cli.Terminal;
+import org.elasticsearch.cli.TestSystemTerminal;
 import org.elasticsearch.cli.UserException;
 import org.elasticsearch.env.Environment;
 
@@ -180,16 +182,12 @@ public void testStdinLong() throws Exception {
     }
 
     public void testStdinSystemTerminal() throws Exception {
-        InputStream sysin = System.in; // save for later
-
         String password = "keystorepassword";
         KeyStoreWrapper.create().save(env.configFile(), password.toCharArray());
-
         String input = password + "\nbar\n";
-        System.setIn(new ByteArrayInputStream(input.getBytes()));
-        execute(Terminal.DEFAULT, "--stdin", "foo");
-
-        System.setIn(sysin);    // cleanup
+        InputStream in = new ByteArrayInputStream(input.getBytes(Charset.defaultCharset()));
+        Terminal term = new TestSystemTerminal(in);
+        execute(term, "--stdin", "foo");
     }
 
     public void testMissingSettingName() throws Exception {
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 a54d60cf875d6..6259d1459a07e 100644
--- a/libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java
+++ b/libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java
@@ -22,6 +22,7 @@
 import java.io.BufferedReader;
 import java.io.Console;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.PrintWriter;
 import java.nio.charset.Charset;
@@ -59,6 +60,11 @@ protected Terminal(String lineSeparator) {
         this.lineSeparator = lineSeparator;
     }
 
+    /** Visible for testing in subclasses: set an input other than standard input */
+    protected void setInput(InputStream inputStream) throws IOException {
+        throw new AssertionError("unsupported operation");
+    }
+
     /** Sets the verbosity of the terminal. */
     public void setVerbosity(Verbosity verbosity) {
         this.verbosity = verbosity;
@@ -162,11 +168,20 @@ private static PrintWriter newWriter() {
 
         private BufferedReader getReader() {
             if (reader == null) {
-                reader = new BufferedReader(new InputStreamReader(System.in));
+                reader = new BufferedReader(new InputStreamReader(System.in, Charset.defaultCharset()));
             }
             return reader;
         }
 
+        /** Visible for testing the system terminal with custom input */
+        @Override
+        protected void setInput(InputStream inputStream) throws IOException {
+            if (reader != null) {
+                reader.close();
+            }
+            reader = new BufferedReader(new InputStreamReader(inputStream, Charset.defaultCharset()));
+        }
+
         @Override
         public PrintWriter getWriter() {
             return WRITER;
diff --git a/server/src/test/java/org/elasticsearch/cli/TerminalTests.java b/server/src/test/java/org/elasticsearch/cli/TerminalTests.java
index b8faa279a28ea..486c2c550fd64 100644
--- a/server/src/test/java/org/elasticsearch/cli/TerminalTests.java
+++ b/server/src/test/java/org/elasticsearch/cli/TerminalTests.java
@@ -23,6 +23,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
+import java.nio.charset.Charset;
 
 public class TerminalTests extends ESTestCase {
     public void testVerbosity() throws Exception {
@@ -78,17 +79,11 @@ public void testPromptYesNoCase() throws Exception {
         assertFalse(terminal.promptYesNo("Answer?", true));
     }
 
-    public void testReadTwiceFromSystemTerminal() {
-        InputStream sysIn = System.in;      // save for cleanup
-
-        InputStream in = new ByteArrayInputStream("foo\nbar\n".getBytes());
-        System.setIn(in);
-        Terminal terminal = Terminal.DEFAULT;
+    public void testReadTwiceFromSystemTerminal() throws Exception {
+        InputStream in = new ByteArrayInputStream("foo\nbar\n".getBytes(Charset.defaultCharset()));
+        Terminal terminal = new TestSystemTerminal(in);
         assertEquals(terminal.readText("say foo"), "foo");
         assertEquals(terminal.readText("say bar"), "bar");
-
-        // cleanup
-        System.setIn(sysIn);
     }
 
     private void assertPrinted(MockTerminal logTerminal, Terminal.Verbosity verbosity, String text) throws Exception {
diff --git a/test/framework/src/main/java/org/elasticsearch/cli/TestSystemTerminal.java b/test/framework/src/main/java/org/elasticsearch/cli/TestSystemTerminal.java
new file mode 100644
index 0000000000000..1b2142571dab0
--- /dev/null
+++ b/test/framework/src/main/java/org/elasticsearch/cli/TestSystemTerminal.java
@@ -0,0 +1,51 @@
+/*
+ * 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 java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+
+public class TestSystemTerminal extends Terminal {
+    Terminal delegate = Terminal.DEFAULT;
+
+    public TestSystemTerminal(InputStream inputStream) throws IOException {
+        super(System.lineSeparator());
+        this.delegate.setInput(inputStream);
+        if (!"SystemTerminal".equals(this.delegate.getClass().getSimpleName())) {
+            throw new AssertionError("delegate must be a SystemTerminal");
+        }
+    }
+
+    @Override
+    public String readText(String prompt) {
+        return this.delegate.readText(prompt);
+    }
+
+    @Override
+    public char[] readSecret(String prompt) {
+        return this.delegate.readSecret(prompt);
+    }
+
+    @Override
+    public PrintWriter getWriter() {
+        return this.delegate.getWriter();
+    }
+}

From 3d11327a96b1bdb05abca5b02c85d25ac623913a Mon Sep 17 00:00:00 2001
From: William Brafford <william.brafford@elastic.co>
Date: Thu, 11 Jul 2019 18:01:11 -0400
Subject: [PATCH 8/8] Add keystore passphrase support to docker images

This work makes it possible to mount a password-protected keystore in a
docker image and provide that keystore's password as a docker
environment variable. There is a little bit of trickery around dirty
output from the keystore-list command, and the control flow might have
gotten a little too tangled and nested, but I hope it works for a first
cut.
---
 .../docker/src/docker/bin/docker-entrypoint.sh | 18 +++++++++++++++---
 distribution/src/bin/elasticsearch             |  4 ++--
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/distribution/docker/src/docker/bin/docker-entrypoint.sh b/distribution/docker/src/docker/bin/docker-entrypoint.sh
index b9aab95bc2e96..f24b69263990f 100644
--- a/distribution/docker/src/docker/bin/docker-entrypoint.sh
+++ b/distribution/docker/src/docker/bin/docker-entrypoint.sh
@@ -84,8 +84,20 @@ 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 list >/dev/null 2>&1) ; 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
+      # ugh - when keystore is password protected, prompt gets mixed up in list output
+      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
@@ -97,4 +109,4 @@ if [[ "$(id -u)" == "0" ]]; then
   fi
 fi
 
-run_as_other_user_if_needed /usr/share/elasticsearch/bin/elasticsearch "${es_opts[@]}"
+run_as_other_user_if_needed /usr/share/elasticsearch/bin/elasticsearch "${es_opts[@]}" <<<"$KEYSTORE_PASSWORD"
diff --git a/distribution/src/bin/elasticsearch b/distribution/src/bin/elasticsearch
index 016c8b6a8b0db..f6534c9aa3f59 100755
--- a/distribution/src/bin/elasticsearch
+++ b/distribution/src/bin/elasticsearch
@@ -62,7 +62,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
+    "$@" <<<"$KEYSTORE_PASSWORD"
 else
   exec \
     "$JAVA" \
@@ -75,7 +75,7 @@ else
     -cp "$ES_CLASSPATH" \
     org.elasticsearch.bootstrap.Elasticsearch \
     "$@" \
-    <&- &
+    <<<"$KEYSTORE_PASSWORD" &
   retval=$?
   pid=$!
   [ $retval -eq 0 ] || exit $retval