diff --git a/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index 19bc81b9972..d351a9d4ea2 100644 --- a/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -59,7 +59,6 @@ public class Bootstrap { private static Bootstrap bootstrap; private void setup(boolean addShutdownHook, Settings settings, Environment environment) throws Exception { - setupSecurity(settings, environment); if (settings.getAsBoolean("bootstrap.mlockall", false)) { Natives.tryMlockall(); } @@ -90,6 +89,8 @@ public class Bootstrap { } }); } + // install SM after natives, JNA can require strange permissions + setupSecurity(settings, environment); } /** diff --git a/src/main/java/org/elasticsearch/bootstrap/Security.java b/src/main/java/org/elasticsearch/bootstrap/Security.java index afa8362771e..67ac531f0e7 100644 --- a/src/main/java/org/elasticsearch/bootstrap/Security.java +++ b/src/main/java/org/elasticsearch/bootstrap/Security.java @@ -29,8 +29,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; -import java.util.HashSet; -import java.util.Set; /** * Initializes securitymanager with necessary permissions. @@ -68,55 +66,42 @@ class Security { try (InputStream in = new BufferedInputStream(template)) { ByteStreams.copy(in, output); } - - // add permissions for all configured paths. - Set paths = new HashSet<>(); - paths.add(environment.homeFile()); - paths.add(environment.configFile()); - paths.add(environment.logsFile()); - paths.add(environment.pluginsFile()); - for (Path path : environment.dataFiles()) { - paths.add(path); + + // all policy files are UTF-8: + // https://docs.oracle.com/javase/7/docs/technotes/guides/security/PolicyFiles.html + try (Writer writer = new OutputStreamWriter(output, StandardCharsets.UTF_8)) { + writer.write(System.lineSeparator()); + writer.write("grant {"); + writer.write(System.lineSeparator()); + + // add permissions for all configured paths. + // TODO: improve test infra so we can reduce permissions where read/write + // is not really needed... + addPath(writer, environment.homeFile(), "read,readlink,write,delete"); + addPath(writer, environment.configFile(), "read,readlink,write,delete"); + addPath(writer, environment.logsFile(), "read,readlink,write,delete"); + addPath(writer, environment.pluginsFile(), "read,readlink,write,delete"); + for (Path path : environment.dataFiles()) { + addPath(writer, path, "read,readlink,write,delete"); + } + for (Path path : environment.dataWithClusterFiles()) { + addPath(writer, path, "read,readlink,write,delete"); + } + + writer.write("};"); + writer.write(System.lineSeparator()); } - for (Path path : environment.dataWithClusterFiles()) { - paths.add(path); - } - output.write(createPermissions(paths)); } return processed; } - // package private for testing - static byte[] createPermissions(Set paths) throws IOException { - ByteArrayOutputStream stream = new ByteArrayOutputStream(); - - // all policy files are UTF-8: - // https://docs.oracle.com/javase/7/docs/technotes/guides/security/PolicyFiles.html - try (Writer writer = new OutputStreamWriter(stream, StandardCharsets.UTF_8)) { - writer.write(System.lineSeparator()); - writer.write("grant {"); - writer.write(System.lineSeparator()); - for (Path path : paths) { - // data paths actually may not exist yet. - Files.createDirectories(path); - // add each path twice: once for itself, again for files underneath it - addPath(writer, encode(path), "read,readlink,write,delete"); - addRecursivePath(writer, encode(path), "read,readlink,write,delete"); - } - writer.write("};"); - writer.write(System.lineSeparator()); - } - - return stream.toByteArray(); - } - - static void addPath(Writer writer, String path, String permissions) throws IOException { - writer.write("permission java.io.FilePermission \"" + path + "\", \"" + permissions + "\";"); + static void addPath(Writer writer, Path path, String permissions) throws IOException { + // paths may not exist yet + Files.createDirectories(path); + // add each path twice: once for itself, again for files underneath it + writer.write("permission java.io.FilePermission \"" + encode(path) + "\", \"" + permissions + "\";"); writer.write(System.lineSeparator()); - } - - static void addRecursivePath(Writer writer, String path, String permissions) throws IOException { - writer.write("permission java.io.FilePermission \"" + path + "${/}-\", \"" + permissions + "\";"); + writer.write("permission java.io.FilePermission \"" + encode(path) + "${/}-\", \"" + permissions + "\";"); writer.write(System.lineSeparator()); } @@ -124,10 +109,6 @@ class Security { // See "Note Regarding File Path Specifications on Windows Systems". // https://docs.oracle.com/javase/7/docs/technotes/guides/security/PolicyFiles.html static String encode(Path path) { - return encode(path.toString()); - } - - static String encode(String path) { - return path.replace("\\", "\\\\"); + return path.toString().replace("\\", "\\\\"); } } diff --git a/src/main/resources/org/elasticsearch/bootstrap/security.policy b/src/main/resources/org/elasticsearch/bootstrap/security.policy index 44b89c47c58..92056a3ab81 100644 --- a/src/main/resources/org/elasticsearch/bootstrap/security.policy +++ b/src/main/resources/org/elasticsearch/bootstrap/security.policy @@ -37,9 +37,6 @@ grant { permission java.io.FilePermission "${project.basedir}${/}lib/sigar{/}-", "read"; // mvn custom ./m2/repository for dependency jars permission java.io.FilePermission "${m2.repository}${/}-", "read"; - // per-jvm directory - permission java.io.FilePermission "${junit4.childvm.cwd}${/}temp", "read,write"; - permission java.io.FilePermission "${junit4.childvm.cwd}${/}temp${/}-", "read,write,delete"; permission java.nio.file.LinkPermission "symbolic"; permission groovy.security.GroovyCodeSourcePermission "/groovy/script"; @@ -86,7 +83,10 @@ grant { // needed for natives calls permission java.lang.RuntimePermission "loadLibrary.*"; + + // needed for testing access rules etc permission java.lang.RuntimePermission "createSecurityManager"; + permission java.security.SecurityPermission "createPolicy.JavaPolicy"; // reflection hacks: // needed for Striped64 (what is this doing), also enables unmap hack diff --git a/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java b/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java index d3a27f56b1f..4c2ddcd47eb 100644 --- a/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java +++ b/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java @@ -19,30 +19,77 @@ package org.elasticsearch.bootstrap; +import org.elasticsearch.common.settings.ImmutableSettings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; import org.elasticsearch.test.ElasticsearchTestCase; -import java.nio.charset.StandardCharsets; +import java.io.ByteArrayInputStream; +import java.io.FilePermission; import java.nio.file.Path; -import java.util.Collections; +import java.security.Policy; +import java.security.ProtectionDomain; +import java.security.URIParameter; public class SecurityTests extends ElasticsearchTestCase { - /** backslash escaping (e.g. windows paths) */ - public void testEncode() { - assertEquals("c:\\\\foobar", Security.encode("c:\\foobar")); - } - - /** test template processing */ - public void testTemplateProcessing() throws Exception { + /** test generated permissions */ + public void testGeneratedPermissions() throws Exception { Path path = createTempDir(); - - byte results[] = Security.createPermissions(Collections.singleton(path)); - String unicode = new String(results, StandardCharsets.UTF_8); - // try not to make this test too fragile or useless - assertTrue(unicode.contains("grant {")); - assertTrue(unicode.contains(Security.encode(path))); - assertTrue(unicode.contains("read")); - assertTrue(unicode.contains("write")); + // make a fake ES home and ensure we only grant permissions to that. + Path esHome = path.resolve("esHome"); + ImmutableSettings.Builder settingsBuilder = ImmutableSettings.builder(); + settingsBuilder.put("path.home", esHome.toString()); + Settings settings = settingsBuilder.build(); + + Environment environment = new Environment(settings); + Path policyFile = Security.processTemplate(new ByteArrayInputStream(new byte[0]), environment); + + ProtectionDomain domain = getClass().getProtectionDomain(); + Policy policy = Policy.getInstance("JavaPolicy", new URIParameter(policyFile.toUri())); + // the fake es home + assertTrue(policy.implies(domain, new FilePermission(esHome.toString(), "read"))); + // its parent + assertFalse(policy.implies(domain, new FilePermission(path.toString(), "read"))); + // some other sibling + assertFalse(policy.implies(domain, new FilePermission(path.resolve("other").toString(), "read"))); + } + + /** test generated permissions for all configured paths */ + public void testEnvironmentPaths() throws Exception { + Path path = createTempDir(); + + ImmutableSettings.Builder settingsBuilder = ImmutableSettings.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()); + Settings settings = settingsBuilder.build(); + + Environment environment = new Environment(settings); + Path policyFile = Security.processTemplate(new ByteArrayInputStream(new byte[0]), environment); + + ProtectionDomain domain = getClass().getProtectionDomain(); + Policy policy = Policy.getInstance("JavaPolicy", new URIParameter(policyFile.toUri())); + + // check that all directories got permissions: + // homefile: this is needed unless we break out rules for "lib" dir. + // TODO: make read-only + assertTrue(policy.implies(domain, new FilePermission(environment.homeFile().toString(), "read,readlink,write,delete"))); + // config file + // TODO: make read-only + assertTrue(policy.implies(domain, new FilePermission(environment.configFile().toString(), "read,readlink,write,delete"))); + // plugins: r/w, TODO: can this be minimized? + assertTrue(policy.implies(domain, new FilePermission(environment.pluginsFile().toString(), "read,readlink,write,delete"))); + // data paths: r/w + for (Path dataPath : environment.dataFiles()) { + assertTrue(policy.implies(domain, new FilePermission(dataPath.toString(), "read,readlink,write,delete"))); + } + for (Path dataPath : environment.dataWithClusterFiles()) { + assertTrue(policy.implies(domain, new FilePermission(dataPath.toString(), "read,readlink,write,delete"))); + } + // logs: r/w + assertTrue(policy.implies(domain, new FilePermission(environment.logsFile().toString(), "read,readlink,write,delete"))); } - }