diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Security.java b/core/src/main/java/org/elasticsearch/bootstrap/Security.java index ddbe41157b4..ca6fb9f0eb0 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Security.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Security.java @@ -120,10 +120,11 @@ final class Security { // read-only dirs addPath(policy, environment.binFile(), "read,readlink"); addPath(policy, environment.libFile(), "read,readlink"); + addPath(policy, environment.pluginsFile(), "read,readlink"); + addPath(policy, environment.configFile(), "read,readlink"); + // read-write dirs addPath(policy, environment.tmpFile(), "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"); for (Path path : environment.dataFiles()) { addPath(policy, path, "read,readlink,write,delete"); } @@ -134,7 +135,8 @@ final class Security { addPath(policy, path, "read,readlink,write,delete"); } if (environment.pidFile() != null) { - addPath(policy, environment.pidFile().getParent(), "read,readlink,write,delete"); + // we just need permission to remove the file if its elsewhere. + policy.add(new FilePermission(environment.pidFile().toString(), "delete")); } return policy; } diff --git a/core/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java b/core/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java index 3f72caaa539..38780ec0cac 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.bootstrap; import org.apache.lucene.util.Constants; +import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ElasticsearchTestCase; @@ -28,7 +29,9 @@ import java.io.FilePermission; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.security.PermissionCollection; import java.security.Permissions; +import java.util.Set; public class SecurityTests extends ElasticsearchTestCase { @@ -53,26 +56,28 @@ public class SecurityTests extends ElasticsearchTestCase { } // the fake es home - assertFalse(permissions.implies(new FilePermission(esHome.toString(), "read"))); + assertNoPermissions(esHome, permissions); // its parent - assertFalse(permissions.implies(new FilePermission(path.toString(), "read"))); + assertNoPermissions(esHome.getParent(), permissions); // some other sibling - assertFalse(permissions.implies(new FilePermission(path.resolve("other").toString(), "read"))); + assertNoPermissions(esHome.getParent().resolve("other"), permissions); // double check we overwrote java.io.tmpdir correctly for the test - assertFalse(permissions.implies(new FilePermission(realTmpDir.toString(), "read"))); + assertNoPermissions(PathUtils.get(realTmpDir), permissions); } /** test generated permissions for all configured paths */ public void testEnvironmentPaths() throws Exception { Path path = createTempDir(); + // make a fake ES home and ensure we only grant permissions to that. + Path esHome = path.resolve("esHome"); Settings.Builder settingsBuilder = Settings.builder(); - settingsBuilder.put("path.home", path.resolve("home").toString()); - settingsBuilder.put("path.conf", path.resolve("conf").toString()); - settingsBuilder.put("path.plugins", path.resolve("plugins").toString()); - settingsBuilder.putArray("path.data", path.resolve("data1").toString(), path.resolve("data2").toString()); - settingsBuilder.put("path.logs", path.resolve("logs").toString()); - settingsBuilder.put("pidfile", path.resolve("test.pid").toString()); + settingsBuilder.put("path.home", esHome.resolve("home").toString()); + settingsBuilder.put("path.conf", esHome.resolve("conf").toString()); + settingsBuilder.put("path.plugins", esHome.resolve("plugins").toString()); + settingsBuilder.putArray("path.data", esHome.resolve("data1").toString(), esHome.resolve("data2").toString()); + settingsBuilder.put("path.logs", esHome.resolve("logs").toString()); + settingsBuilder.put("pidfile", esHome.resolve("test.pid").toString()); Settings settings = settingsBuilder.build(); Path fakeTmpDir = createTempDir(); @@ -87,29 +92,39 @@ public class SecurityTests extends ElasticsearchTestCase { System.setProperty("java.io.tmpdir", realTmpDir); } + // the fake es home + assertNoPermissions(esHome, permissions); + // its parent + assertNoPermissions(esHome.getParent(), permissions); + // some other sibling + assertNoPermissions(esHome.getParent().resolve("other"), permissions); + // double check we overwrote java.io.tmpdir correctly for the test + assertNoPermissions(PathUtils.get(realTmpDir), permissions); + // check that all directories got permissions: - 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"))); - // plugins: r/w, TODO: can this be minimized? - assertTrue(permissions.implies(new FilePermission(environment.pluginsFile().toString(), "read,readlink,write,delete"))); + + // bin file: ro + assertExactPermissions(new FilePermission(environment.binFile().toString(), "read,readlink"), permissions); + // lib file: ro + assertExactPermissions(new FilePermission(environment.libFile().toString(), "read,readlink"), permissions); + // config file: ro + assertExactPermissions(new FilePermission(environment.configFile().toString(), "read,readlink"), permissions); + // plugins: ro + assertExactPermissions(new FilePermission(environment.pluginsFile().toString(), "read,readlink"), permissions); + // data paths: r/w for (Path dataPath : environment.dataFiles()) { - assertTrue(permissions.implies(new FilePermission(dataPath.toString(), "read,readlink,write,delete"))); + assertExactPermissions(new FilePermission(dataPath.toString(), "read,readlink,write,delete"), permissions); } for (Path dataPath : environment.dataWithClusterFiles()) { - assertTrue(permissions.implies(new FilePermission(dataPath.toString(), "read,readlink,write,delete"))); + assertExactPermissions(new FilePermission(dataPath.toString(), "read,readlink,write,delete"), permissions); } // logs: r/w - assertTrue(permissions.implies(new FilePermission(environment.logsFile().toString(), "read,readlink,write,delete"))); + assertExactPermissions(new FilePermission(environment.logsFile().toString(), "read,readlink,write,delete"), permissions); // temp dir: r/w - assertTrue(permissions.implies(new FilePermission(fakeTmpDir.toString(), "read,readlink,write,delete"))); - // double check we overwrote java.io.tmpdir correctly for the test - assertFalse(permissions.implies(new FilePermission(realTmpDir.toString(), "read"))); - // PID file: r/w - assertTrue(permissions.implies(new FilePermission(environment.pidFile().toString(), "read,readlink,write,delete"))); + assertExactPermissions(new FilePermission(fakeTmpDir.toString(), "read,readlink,write,delete"), permissions); + // PID file: delete only (for the shutdown hook) + assertExactPermissions(new FilePermission(environment.pidFile().toString(), "delete"), permissions); } public void testEnsureExists() throws IOException { @@ -225,9 +240,40 @@ public class SecurityTests extends ElasticsearchTestCase { } Permissions permissions = new Permissions(); Security.addPath(permissions, link, "read"); - assertTrue(permissions.implies(new FilePermission(link.toString(), "read"))); - assertTrue(permissions.implies(new FilePermission(link.resolve("foo").toString(), "read"))); - assertTrue(permissions.implies(new FilePermission(target.toString(), "read"))); - assertTrue(permissions.implies(new FilePermission(target.resolve("foo").toString(), "read"))); + assertExactPermissions(new FilePermission(link.toString(), "read"), permissions); + assertExactPermissions(new FilePermission(link.resolve("foo").toString(), "read"), permissions); + assertExactPermissions(new FilePermission(target.toString(), "read"), permissions); + assertExactPermissions(new FilePermission(target.resolve("foo").toString(), "read"), permissions); + } + + /** + * checks exact file permissions, meaning those and only those for that path. + */ + static void assertExactPermissions(FilePermission expected, PermissionCollection actual) { + String target = expected.getName(); // see javadocs + Set permissionSet = asSet(expected.getActions().split(",")); + boolean read = permissionSet.remove("read"); + boolean readlink = permissionSet.remove("readlink"); + boolean write = permissionSet.remove("write"); + boolean delete = permissionSet.remove("delete"); + boolean execute = permissionSet.remove("execute"); + assertTrue("unrecognized permission: " + permissionSet, permissionSet.isEmpty()); + assertEquals(read, actual.implies(new FilePermission(target, "read"))); + assertEquals(readlink, actual.implies(new FilePermission(target, "readlink"))); + assertEquals(write, actual.implies(new FilePermission(target, "write"))); + assertEquals(delete, actual.implies(new FilePermission(target, "delete"))); + assertEquals(execute, actual.implies(new FilePermission(target, "execute"))); + } + + /** + * checks that this path has no permissions + */ + static void assertNoPermissions(Path path, PermissionCollection actual) { + String target = path.toString(); + assertFalse(actual.implies(new FilePermission(target, "read"))); + assertFalse(actual.implies(new FilePermission(target, "readlink"))); + assertFalse(actual.implies(new FilePermission(target, "write"))); + assertFalse(actual.implies(new FilePermission(target, "delete"))); + assertFalse(actual.implies(new FilePermission(target, "execute"))); } }