Change approach to pass temp directory to Environment

Author: David Roberts

core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java

@@ -35,6 +35,7 @@
 import org.elasticsearch.common.PidFile;
 import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.inject.CreationException;
+import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.logging.ESLoggerFactory;
 import org.elasticsearch.common.logging.LogConfigurator;
 import org.elasticsearch.common.logging.Loggers;
@@ -43,6 +44,7 @@
 import org.elasticsearch.common.settings.SecureSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.BoundTransportAddress;
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.monitor.jvm.JvmInfo;
 import org.elasticsearch.monitor.os.OsProbe;
@@ -56,10 +58,14 @@
 import java.io.PrintStream;
 import java.io.UnsupportedEncodingException;
 import java.net.URISyntaxException;
+import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.PosixFilePermissions;
 import java.security.NoSuchAlgorithmException;
 import java.util.List;
 import java.util.Collections;
+import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 
 /**
@@ -239,12 +245,13 @@ static SecureSettings loadSecureSettings(Environment initialEnv) throws Bootstra
         return keystore;
     }
 
+    @SuppressForbidden(reason = "gets java.io.tmpdir")
     private static Environment createEnvironment(
             final boolean foreground,
             final Path pidFile,
             final SecureSettings secureSettings,
             final Settings initialSettings,
-            final Path configPath) {
+            final Path configPath) throws IOException {
         Terminal terminal = foreground ? Terminal.DEFAULT : null;
         Settings.Builder builder = Settings.builder();
         if (pidFile != null) {
@@ -254,7 +261,22 @@ private static Environment createEnvironment(
         if (secureSettings != null) {
             builder.setSecureSettings(secureSettings);
         }
-        return InternalSettingsPreparer.prepareEnvironment(builder.build(), terminal, Collections.emptyMap(), configPath);
+        return InternalSettingsPreparer.prepareEnvironment(builder.build(), terminal, Collections.emptyMap(), configPath,
+            makeSecureTmpPath(PathUtils.get(System.getProperty("java.io.tmpdir"))));
+    }
+
+    static Path makeSecureTmpPath(Path baseDir) throws IOException {
+        try {
+            // On POSIX file systems everyone can see the contents of the /tmp directory, so create a
+            // sub-directory under it that only the user running Elasticsearch can list the contents of.
+            Set<PosixFilePermission> attrs = Sets.newHashSet(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE,
+                PosixFilePermission.OWNER_EXECUTE);
+            return Files.createTempDirectory(baseDir, "elasticsearch", PosixFilePermissions.asFileAttribute(attrs));
+        } catch (UnsupportedOperationException e) {
+            // Assume this isn't a POSIX file system.  On Windows each user's %TEMP% directory is visible only to
+            // them and administrators, so the exact permissions of this sub-directory are less important.
+            return Files.createTempDirectory(baseDir, "elasticsearch");
+        }
     }
 
     private void start() throws NodeValidationException {
@@ -285,8 +307,9 @@ static void init(
         INSTANCE = new Bootstrap();
 
         final SecureSettings keystore = loadSecureSettings(initialEnv);
-        final Environment environment = createEnvironment(foreground, pidFile, keystore, initialEnv.settings(), initialEnv.configFile());
+        final Environment environment;
         try {
+            environment = createEnvironment(foreground, pidFile, keystore, initialEnv.settings(), initialEnv.configFile());
             LogConfigurator.configure(environment);
         } catch (IOException e) {
             throw new BootstrapException(e);

core/src/main/java/org/elasticsearch/bootstrap/Security.java

@@ -268,6 +268,7 @@ static void addClasspathPermissions(Permissions policy) throws IOException {
     /**
      * Adds access to all configurable paths.
      */
+    @SuppressForbidden(reason = "gets java.io.tmpdir")
     static void addFilePermissions(Permissions policy, Environment environment) throws IOException {
         // read-only dirs
         addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.binFile(), "read,readlink");
@@ -276,7 +277,8 @@ static void addFilePermissions(Permissions policy, Environment environment) thro
         addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.pluginsFile(), "read,readlink");
         addDirectoryPath(policy, "path.conf'", environment.configFile(), "read,readlink");
         // read-write dirs
-        addDirectoryPath(policy, "java.io.tmpdir", environment.tmpFile(), "read,readlink,write,delete");
+        addDirectoryPath(policy, "java.io.tmpdir", PathUtils.get(System.getProperty("java.io.tmpdir")),
+                "read,readlink,write,delete");
         addDirectoryPath(policy, Environment.PATH_LOGS_SETTING.getKey(), environment.logsFile(), "read,readlink,write,delete");
         if (environment.sharedDataFile() != null) {
             addDirectoryPath(policy, Environment.PATH_SHARED_DATA_SETTING.getKey(), environment.sharedDataFile(),

core/src/main/java/org/elasticsearch/cli/EnvironmentAwareCommand.java

@@ -23,6 +23,7 @@
 import joptsimple.OptionSpec;
 import joptsimple.util.KeyValuePair;
 import org.elasticsearch.common.SuppressForbidden;
+import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.node.InternalSettingsPreparer;
@@ -70,12 +71,14 @@ protected void execute(Terminal terminal, OptionSet options) throws Exception {
     }
 
     /** Create an {@link Environment} for the command to use. Overrideable for tests. */
+    @SuppressForbidden(reason = "gets java.io.tmpdir")
     protected Environment createEnv(final Terminal terminal, final Map<String, String> settings) throws UserException {
         final String esPathConf = System.getProperty("es.path.conf");
         if (esPathConf == null) {
             throw new UserException(ExitCodes.CONFIG, "the system property [es.path.conf] must be set");
         }
-        return InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, terminal, settings, getConfigPath(esPathConf));
+        return InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, terminal, settings, getConfigPath(esPathConf),
+            PathUtils.get(System.getProperty("java.io.tmpdir")));
     }
 
     @SuppressForbidden(reason = "need path to construct environment")

core/src/main/java/org/elasticsearch/env/Environment.java

@@ -82,14 +82,20 @@ public class Environment {
     /** Path to the PID file (can be null if no PID file is configured) **/
     private final Path pidFile;
 
-    /** Path to the temporary file directory used by the JDK */
-    private final Path tmpFile = PathUtils.get(System.getProperty("java.io.tmpdir"));
+    /** Path to the temporary file directory */
+    private final Path tmpFile;
 
+    /** Convenience method for tests - do not use in production code */
     public Environment(Settings settings) {
         this(settings, null);
     }
 
-    public Environment(final Settings settings, final Path configPath) {
+    /** Convenience method for tests - do not use in production code */
+    public Environment(Settings settings, Path configPath) {
+        this(settings, configPath, PathUtils.get(System.getProperty("java.io.tmpdir")));
+    }
+
+    public Environment(final Settings settings, final Path configPath, final Path tmpPath) {
         final Path homeFile;
         if (PATH_HOME_SETTING.exists(settings)) {
             homeFile = PathUtils.get(PATH_HOME_SETTING.get(settings)).normalize();
@@ -103,6 +109,8 @@ public Environment(final Settings settings, final Path configPath) {
             configFile = homeFile.resolve("config");
         }
 
+        tmpFile = Objects.requireNonNull(tmpPath);
+
         pluginsFile = homeFile.resolve("plugins");
 
         List<String> dataPaths = PATH_DATA_SETTING.get(settings);

core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java

@@ -31,7 +31,9 @@
 import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.collect.Tuple;
+import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsException;
 import org.elasticsearch.env.Environment;
@@ -62,8 +64,10 @@ public static Settings prepareSettings(Settings input) {
      * @param terminal the Terminal to use for input/output
      * @return the {@link Settings} and {@link Environment} as a {@link Tuple}
      */
+    @SuppressForbidden(reason = "gets java.io.tmpdir")
     public static Environment prepareEnvironment(Settings input, Terminal terminal) {
-        return prepareEnvironment(input, terminal, Collections.emptyMap(), null);
+        return prepareEnvironment(input, terminal, Collections.emptyMap(), null,
+                PathUtils.get(System.getProperty("java.io.tmpdir")));
     }
 
     /**
@@ -76,13 +80,15 @@ public static Environment prepareEnvironment(Settings input, Terminal terminal)
      * @param terminal   the Terminal to use for input/output
      * @param properties map of properties key/value pairs (usually from the command-line)
      * @param configPath path to config directory; (use null to indicate the default)
+     * @param tmpPath    path to use for temporary files
      * @return the {@link Settings} and {@link Environment} as a {@link Tuple}
      */
-    public static Environment prepareEnvironment(Settings input, Terminal terminal, Map<String, String> properties, Path configPath) {
+    public static Environment prepareEnvironment(Settings input, Terminal terminal, Map<String, String> properties, Path configPath,
+                                                 Path tmpPath) {
         // just create enough settings to build the environment, to get the config dir
         Settings.Builder output = Settings.builder();
         initializeSettings(output, input, properties);
-        Environment environment = new Environment(output.build(), configPath);
+        Environment environment = new Environment(output.build(), configPath, tmpPath);
 
         if (Files.exists(environment.configFile().resolve("elasticsearch.yaml"))) {
             throw new SettingsException("elasticsearch.yaml was deprecated in 5.5.0 and must be renamed to elasticsearch.yml");
@@ -106,11 +112,11 @@ public static Environment prepareEnvironment(Settings input, Terminal terminal,
         initializeSettings(output, input, properties);
         finalizeSettings(output, terminal);
 
-        environment = new Environment(output.build(), configPath);
+        environment = new Environment(output.build(), configPath, tmpPath);
 
         // we put back the path.logs so we can use it in the logging configuration file
         output.put(Environment.PATH_LOGS_SETTING.getKey(), environment.logsFile().toAbsolutePath().normalize().toString());
-        return new Environment(output.build(), configPath);
+        return new Environment(output.build(), configPath, tmpPath);
     }
 
     /**

core/src/main/java/org/elasticsearch/node/Node.java

@@ -305,7 +305,7 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
 
             // create the environment based on the finalized (processed) view of the settings
             // this is just to makes sure that people get the same settings, no matter where they ask them from
-            this.environment = new Environment(this.settings, environment.configFile());
+            this.environment = new Environment(this.settings, environment.configFile(), environment.tmpFile());
             Environment.assertEquivalent(environment, this.environment);
 
             final List<ExecutorBuilder<?>> executorBuilders = pluginsService.getExecutorBuilders(settings);

core/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java

@@ -21,6 +21,7 @@
 
 import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.cluster.ClusterName;
+import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.settings.MockSecureSettings;
 import org.elasticsearch.common.settings.SecureSetting;
 import org.elasticsearch.common.settings.SecureString;
@@ -181,7 +182,8 @@ public void testSecureSettings() {
 
     public void testDefaultPropertiesDoNothing() throws Exception {
         Map<String, String> props = Collections.singletonMap("default.setting", "foo");
-        Environment env = InternalSettingsPreparer.prepareEnvironment(baseEnvSettings, null, props, null);
+        Environment env = InternalSettingsPreparer.prepareEnvironment(baseEnvSettings, null, props, null,
+            PathUtils.get(System.getProperty("java.io.tmpdir")));
         assertEquals("foo", env.settings().get("default.setting"));
         assertNull(env.settings().get("setting"));
     }

qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilBootstrapTests.java

@@ -0,0 +1,66 @@
+/*
+ * 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.bootstrap;
+
+import org.elasticsearch.test.ESTestCase;
+import org.junit.BeforeClass;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.nio.file.attribute.PosixFilePermission;
+import java.util.Set;
+
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.not;
+
+/**
+ * This has to be an evil test because it checks file permissions, and security manager does not allow that.
+ */
+public class EvilBootstrapTests extends ESTestCase {
+
+    private static boolean isPosix;
+
+    @BeforeClass
+    public static void checkPosix() throws IOException {
+        isPosix = Files.getFileAttributeView(createTempFile(), PosixFileAttributeView.class) != null;
+    }
+
+    public void testTmpDirPerms() throws IOException {
+        // %TEMP% directories are restricted by default on Windows, so in this case it doesn't
+        // really matter what permissions the directory we create beneath java.io.tmpdir have.
+        // And if the user has changed java.io.tmpdir then they should take responsibility for
+        // ensuring the chosen location is as secure as they require.
+        assumeTrue("Temp directory permissions not set on Windows", isPosix);
+
+        final Path tmpPath = Bootstrap.makeSecureTmpPath(createTempDir());
+
+        Set<PosixFilePermission> perms = Files.getPosixFilePermissions(tmpPath);
+        assertThat(perms,
+            containsInAnyOrder(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE));
+        assertThat(perms, not(contains(PosixFilePermission.GROUP_READ)));
+        assertThat(perms, not(contains(PosixFilePermission.GROUP_WRITE)));
+        assertThat(perms, not(contains(PosixFilePermission.GROUP_EXECUTE)));
+        assertThat(perms, not(contains(PosixFilePermission.OTHERS_READ)));
+        assertThat(perms, not(contains(PosixFilePermission.OTHERS_WRITE)));
+        assertThat(perms, not(contains(PosixFilePermission.OTHERS_EXECUTE)));
+    }
+}

test/framework/src/main/java/org/elasticsearch/node/MockNode.java

@@ -25,6 +25,7 @@
 import org.elasticsearch.cluster.MockInternalClusterInfoService;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.BoundTransportAddress;
@@ -66,7 +67,8 @@ public MockNode(Settings settings, Collection<Class<? extends Plugin>> classpath
     }
 
     public MockNode(Settings settings, Collection<Class<? extends Plugin>> classpathPlugins, Path configPath) {
-        this(InternalSettingsPreparer.prepareEnvironment(settings, null, Collections.emptyMap(), configPath), classpathPlugins);
+        this(InternalSettingsPreparer.prepareEnvironment(settings, null, Collections.emptyMap(), configPath,
+            PathUtils.get(System.getProperty("java.io.tmpdir"))), classpathPlugins);
     }
 
     public MockNode(Environment environment, Collection<Class<? extends Plugin>> classpathPlugins) {