Security manager cleanups

1. initialize SM after things like mlockall. Their tests currently
   don't run with securitymanager enabled, and its simpler to just
   run mlockall etc first.
2. remove redundant test permissions (junit4.childvm.cwd/temp). This
   is alreay added as java.io.tmpdir.
3. improve tests to load the generated policy with some various
   settings and assert things about the permissions on configured
   directories.
4. refactor logic to make it easier to fine-grain the permissions later.
   for example we currently allow write access to conf/. In the future
   I think we can improve testing so we are able to make improvements here.
This commit is contained in:
Robert Muir 2015-04-27 20:29:57 -04:00
parent d164526d27
commit f599c237bd
4 changed files with 101 additions and 72 deletions

View File

@ -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);
}
/**

View File

@ -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<Path> 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<Path> 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("\\", "\\\\");
}
}

View File

@ -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

View File

@ -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")));
}
}