From 86fc8ceac71fbcfe95d6675eeb6b2ffc2cb71d41 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Sat, 2 May 2015 14:42:06 -0400 Subject: [PATCH] simplify security rules --- .../org/elasticsearch/bootstrap/Security.java | 139 ++++++++---------- .../bootstrap/SecurityTests.java | 36 ++--- 2 files changed, 79 insertions(+), 96 deletions(-) diff --git a/src/main/java/org/elasticsearch/bootstrap/Security.java b/src/main/java/org/elasticsearch/bootstrap/Security.java index 536ccd47908..a9eedb7816a 100644 --- a/src/main/java/org/elasticsearch/bootstrap/Security.java +++ b/src/main/java/org/elasticsearch/bootstrap/Security.java @@ -19,18 +19,19 @@ package org.elasticsearch.bootstrap; -import com.google.common.io.ByteStreams; - import org.apache.lucene.util.StringHelper; -import org.elasticsearch.common.logging.ESLogger; -import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.env.Environment; import java.io.*; -import java.nio.charset.StandardCharsets; +import java.net.URI; import java.nio.file.Files; -import java.nio.file.NoSuchFileException; import java.nio.file.Path; +import java.security.Permission; +import java.security.PermissionCollection; +import java.security.Permissions; +import java.security.Policy; +import java.security.ProtectionDomain; +import java.security.URIParameter; /** * Initializes securitymanager with necessary permissions. @@ -47,84 +48,74 @@ class Security { * Initializes securitymanager for the environment * Can only happen once! */ - static void configure(Environment environment) throws IOException { - ESLogger log = Loggers.getLogger(Security.class); - // init lucene random seed. it will use /dev/urandom where available. + static void configure(Environment environment) throws Exception { + // init lucene random seed. it will use /dev/urandom where available: StringHelper.randomId(); - InputStream config = Security.class.getResourceAsStream(POLICY_RESOURCE); - if (config == null) { - throw new NoSuchFileException(POLICY_RESOURCE); - } - Path newConfig = processTemplate(config, environment); - System.setProperty("java.security.policy", newConfig.toString()); + + // enable security policy: union of template and environment-based paths. + URI template = Security.class.getResource(POLICY_RESOURCE).toURI(); + Policy.setPolicy(new ESPolicy(template, createPermissions(environment))); + + // enable security manager System.setSecurityManager(new SecurityManager()); - try { - // don't hide securityexception here, it means java.io.tmpdir is not accessible! - Files.delete(newConfig); - } catch (SecurityException broken) { - log.error("unable to properly access temporary files, run with -Djava.security.debug=policy for more information"); - throw broken; - } catch (IOException ignore) { - // e.g. virus scanner on windows - } + + // do some basic tests + selfTest(); } - - // package-private for testing - static Path processTemplate(InputStream template, Environment environment) throws IOException { - Path processed = Files.createTempFile(null, null); - try (OutputStream output = new BufferedOutputStream(Files.newOutputStream(processed))) { - // copy the template as-is. - try (InputStream in = new BufferedInputStream(template)) { - ByteStreams.copy(in, output); - } - // 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"); - - // generate explicit perms for actual temp dir: - // (in case there is java.io.tmpdir sheistiness on windows) - addPath(writer, processed.getParent(), "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()); - } + /** 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(); + 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"); + for (Path path : environment.dataFiles()) { + addPath(policy, path, "read,readlink,write,delete"); } - return processed; + for (Path path : environment.dataWithClusterFiles()) { + addPath(policy, path, "read,readlink,write,delete"); + } + + return policy; } - static void addPath(Writer writer, Path path, String permissions) throws IOException { + /** Add access to path (and all files underneath it */ + static void addPath(Permissions policy, 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()); - writer.write("permission java.io.FilePermission \"" + encode(path) + "${/}-\", \"" + permissions + "\";"); - writer.write(System.lineSeparator()); + policy.add(new FilePermission(path.toString(), permissions)); + policy.add(new FilePermission(path.toString() + path.getFileSystem().getSeparator() + "-", permissions)); } - - // Any backslashes in paths must be escaped, because it is the escape character when parsing. - // 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 path.toString().replace("\\", "\\\\"); + + /** Simple checks that everything is ok */ + static void selfTest() { + // check we can manipulate temporary files + try { + Files.delete(Files.createTempFile(null, null)); + } catch (IOException ignored) { + // potentially virus scanner + } catch (SecurityException problem) { + throw new SecurityException("Security misconfiguration: cannot access java.io.tmpdir", problem); + } + } + + /** custom policy for union of static and dynamic permissions */ + static class ESPolicy extends Policy { + final Policy template; + final PermissionCollection dynamic; + + ESPolicy(URI template, PermissionCollection dynamic) throws Exception { + this.template = Policy.getInstance("JavaPolicy", new URIParameter(template)); + this.dynamic = dynamic; + } + + @Override + public boolean implies(ProtectionDomain domain, Permission permission) { + return template.implies(domain, permission) || dynamic.implies(permission); + } } } diff --git a/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java b/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java index 4c2ddcd47eb..edbcafdddbd 100644 --- a/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java +++ b/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java @@ -24,12 +24,9 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ElasticsearchTestCase; -import java.io.ByteArrayInputStream; import java.io.FilePermission; import java.nio.file.Path; -import java.security.Policy; -import java.security.ProtectionDomain; -import java.security.URIParameter; +import java.security.Permissions; public class SecurityTests extends ElasticsearchTestCase { @@ -42,17 +39,15 @@ public class SecurityTests extends ElasticsearchTestCase { 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); + Environment environment = new Environment(settings); + Permissions permissions = Security.createPermissions(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"))); + assertTrue(permissions.implies(new FilePermission(esHome.toString(), "read"))); // its parent - assertFalse(policy.implies(domain, new FilePermission(path.toString(), "read"))); + assertFalse(permissions.implies(new FilePermission(path.toString(), "read"))); // some other sibling - assertFalse(policy.implies(domain, new FilePermission(path.resolve("other").toString(), "read"))); + assertFalse(permissions.implies(new FilePermission(path.resolve("other").toString(), "read"))); } /** test generated permissions for all configured paths */ @@ -67,29 +62,26 @@ public class SecurityTests extends ElasticsearchTestCase { 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())); + Environment environment = new Environment(settings); + Permissions permissions = Security.createPermissions(environment); // 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"))); + assertTrue(permissions.implies(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"))); + assertTrue(permissions.implies(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"))); + assertTrue(permissions.implies(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"))); + assertTrue(permissions.implies(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"))); + assertTrue(permissions.implies(new FilePermission(dataPath.toString(), "read,readlink,write,delete"))); } // logs: r/w - assertTrue(policy.implies(domain, new FilePermission(environment.logsFile().toString(), "read,readlink,write,delete"))); + assertTrue(permissions.implies(new FilePermission(environment.logsFile().toString(), "read,readlink,write,delete"))); } }