From 052cf1446f9caea16454e0db5860e8e694c61119 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Mon, 20 Jul 2015 20:27:36 -0400 Subject: [PATCH] Remove Environment.homeFile() Today we grant read+write+delete access to any files underneath the home. But we have to remove this, if we want to have improved security of files underneath elasticsearch. --- .../org/elasticsearch/bootstrap/Security.java | 6 ++--- .../org/elasticsearch/env/Environment.java | 27 ++++++++++++------- .../java/org/elasticsearch/node/Node.java | 4 +-- .../elasticsearch/plugins/PluginManager.java | 2 +- .../bootstrap/SecurityTests.java | 7 +++-- .../plugins/PluginManagerTests.java | 4 +-- 6 files changed, 29 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Security.java b/core/src/main/java/org/elasticsearch/bootstrap/Security.java index 4d91341296a..13bada820c1 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Security.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Security.java @@ -117,11 +117,11 @@ final class Security { /** returns dynamic Permissions to configured paths */ static Permissions createPermissions(Environment environment) throws IOException { - // TODO: improve test infra so we can reduce permissions where read/write - // is not really needed... Permissions policy = new Permissions(); + // read-only dirs + addPath(policy, environment.binFile(), "read,readlink"); + addPath(policy, environment.libFile(), "read,readlink"); addPath(policy, environment.tmpFile(), "read,readlink,write,delete"); - addPath(policy, environment.homeFile(), "read,readlink,write,delete"); addPath(policy, environment.configFile(), "read,readlink,write,delete"); addPath(policy, environment.logsFile(), "read,readlink,write,delete"); addPath(policy, environment.pluginsFile(), "read,readlink,write,delete"); diff --git a/core/src/main/java/org/elasticsearch/env/Environment.java b/core/src/main/java/org/elasticsearch/env/Environment.java index 445ec7a61de..113caa0003a 100644 --- a/core/src/main/java/org/elasticsearch/env/Environment.java +++ b/core/src/main/java/org/elasticsearch/env/Environment.java @@ -45,8 +45,6 @@ public class Environment { private final Settings settings; - private final Path homeFile; - private final Path[] dataFiles; private final Path[] dataWithClusterFiles; @@ -57,6 +55,12 @@ public class Environment { private final Path pluginsFile; + /** location of bin/, used by plugin manager */ + private final Path binFile; + + /** location of lib/, */ + private final Path libFile; + private final Path logsFile; /** Path to the PID file (can be null if no PID file is configured) **/ @@ -83,6 +87,7 @@ public class Environment { public Environment(Settings settings) { this.settings = settings; + final Path homeFile; if (settings.get("path.home") != null) { homeFile = PathUtils.get(cleanPath(settings.get("path.home"))); } else { @@ -133,6 +138,9 @@ public class Environment { } else { pidFile = null; } + + binFile = homeFile.resolve("bin"); + libFile = homeFile.resolve("lib"); } /** @@ -142,13 +150,6 @@ public class Environment { return this.settings; } - /** - * The home of the installation. - */ - public Path homeFile() { - return homeFile; - } - /** * The data location. */ @@ -236,6 +237,14 @@ public class Environment { return pluginsFile; } + public Path binFile() { + return binFile; + } + + public Path libFile() { + return libFile; + } + public Path logsFile() { return logsFile; } diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java index 0c4f4d4e7b4..de211dfadc4 100644 --- a/core/src/main/java/org/elasticsearch/node/Node.java +++ b/core/src/main/java/org/elasticsearch/node/Node.java @@ -140,8 +140,8 @@ public class Node implements Releasable { if (logger.isDebugEnabled()) { Environment env = tuple.v2(); - logger.debug("using home [{}], config [{}], data [{}], logs [{}], plugins [{}]", - env.homeFile(), env.configFile(), Arrays.toString(env.dataFiles()), env.logsFile(), env.pluginsFile()); + logger.debug("using config [{}], data [{}], logs [{}], plugins [{}]", + env.configFile(), Arrays.toString(env.dataFiles()), env.logsFile(), env.pluginsFile()); } this.pluginsService = new PluginsService(tuple.v1(), tuple.v2()); diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginManager.java b/core/src/main/java/org/elasticsearch/plugins/PluginManager.java index 53eca549034..bfd05f76832 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginManager.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginManager.java @@ -733,7 +733,7 @@ public class PluginManager { } Path binDir(Environment env) { - return env.homeFile().resolve("bin").resolve(name); + return env.binFile().resolve(name); } Path configDir(Environment env) { diff --git a/core/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java b/core/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java index 3c3db267292..3f72caaa539 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java @@ -53,7 +53,7 @@ public class SecurityTests extends ElasticsearchTestCase { } // the fake es home - assertTrue(permissions.implies(new FilePermission(esHome.toString(), "read"))); + assertFalse(permissions.implies(new FilePermission(esHome.toString(), "read"))); // its parent assertFalse(permissions.implies(new FilePermission(path.toString(), "read"))); // some other sibling @@ -88,9 +88,8 @@ public class SecurityTests extends ElasticsearchTestCase { } // check that all directories got permissions: - // homefile: this is needed unless we break out rules for "lib" dir. - // TODO: make read-only - assertTrue(permissions.implies(new FilePermission(environment.homeFile().toString(), "read,readlink,write,delete"))); + assertTrue(permissions.implies(new FilePermission(environment.binFile().toString(), "read"))); + assertTrue(permissions.implies(new FilePermission(environment.libFile().toString(), "read"))); // config file // TODO: make read-only assertTrue(permissions.implies(new FilePermission(environment.configFile().toString(), "read,readlink,write,delete"))); diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginManagerTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginManagerTests.java index 3d771c70ce0..687a7d14dfb 100644 --- a/core/src/test/java/org/elasticsearch/plugins/PluginManagerTests.java +++ b/core/src/test/java/org/elasticsearch/plugins/PluginManagerTests.java @@ -85,7 +85,7 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest { String pluginName = "plugin-test"; Tuple initialSettings = buildInitialSettings(); Environment env = initialSettings.v2(); - Path binDir = env.homeFile().resolve("bin"); + Path binDir = env.binFile(); if (!Files.exists(binDir)) { Files.createDirectories(binDir); } @@ -212,7 +212,7 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest { String pluginName = "plugin-test"; Tuple initialSettings = buildInitialSettings(); Environment env = initialSettings.v2(); - Path binDir = env.homeFile().resolve("bin"); + Path binDir = env.binFile(); if (!Files.exists(binDir)) { Files.createDirectories(binDir); }