Merge pull request #10844 from rmuir/sm_cleanup

Security manager cleanups
This commit is contained in:
Robert Muir 2015-04-28 05:12:07 -04:00
commit 10f2e0e2ba
4 changed files with 101 additions and 72 deletions

View File

@ -59,7 +59,6 @@ public class Bootstrap {
private static Bootstrap bootstrap; private static Bootstrap bootstrap;
private void setup(boolean addShutdownHook, Settings settings, Environment environment) throws Exception { private void setup(boolean addShutdownHook, Settings settings, Environment environment) throws Exception {
setupSecurity(settings, environment);
if (settings.getAsBoolean("bootstrap.mlockall", false)) { if (settings.getAsBoolean("bootstrap.mlockall", false)) {
Natives.tryMlockall(); 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.Files;
import java.nio.file.NoSuchFileException; import java.nio.file.NoSuchFileException;
import java.nio.file.Path; import java.nio.file.Path;
import java.util.HashSet;
import java.util.Set;
/** /**
* Initializes securitymanager with necessary permissions. * Initializes securitymanager with necessary permissions.
@ -69,54 +67,41 @@ class Security {
ByteStreams.copy(in, output); 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. // add permissions for all configured paths.
Set<Path> paths = new HashSet<>(); // TODO: improve test infra so we can reduce permissions where read/write
paths.add(environment.homeFile()); // is not really needed...
paths.add(environment.configFile()); addPath(writer, environment.homeFile(), "read,readlink,write,delete");
paths.add(environment.logsFile()); addPath(writer, environment.configFile(), "read,readlink,write,delete");
paths.add(environment.pluginsFile()); addPath(writer, environment.logsFile(), "read,readlink,write,delete");
addPath(writer, environment.pluginsFile(), "read,readlink,write,delete");
for (Path path : environment.dataFiles()) { for (Path path : environment.dataFiles()) {
paths.add(path); addPath(writer, path, "read,readlink,write,delete");
} }
for (Path path : environment.dataWithClusterFiles()) { for (Path path : environment.dataWithClusterFiles()) {
paths.add(path); addPath(writer, path, "read,readlink,write,delete");
}
writer.write("};");
writer.write(System.lineSeparator());
} }
output.write(createPermissions(paths));
} }
return processed; return processed;
} }
// package private for testing static void addPath(Writer writer, Path path, String permissions) throws IOException {
static byte[] createPermissions(Set<Path> paths) throws IOException { // paths may not exist yet
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); Files.createDirectories(path);
// add each path twice: once for itself, again for files underneath it // add each path twice: once for itself, again for files underneath it
addPath(writer, encode(path), "read,readlink,write,delete"); writer.write("permission java.io.FilePermission \"" + encode(path) + "\", \"" + permissions + "\";");
addRecursivePath(writer, encode(path), "read,readlink,write,delete");
}
writer.write("};");
writer.write(System.lineSeparator()); writer.write(System.lineSeparator());
} writer.write("permission java.io.FilePermission \"" + encode(path) + "${/}-\", \"" + permissions + "\";");
return stream.toByteArray();
}
static void addPath(Writer writer, String path, String permissions) throws IOException {
writer.write("permission java.io.FilePermission \"" + 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(System.lineSeparator()); writer.write(System.lineSeparator());
} }
@ -124,10 +109,6 @@ class Security {
// See "Note Regarding File Path Specifications on Windows Systems". // See "Note Regarding File Path Specifications on Windows Systems".
// https://docs.oracle.com/javase/7/docs/technotes/guides/security/PolicyFiles.html // https://docs.oracle.com/javase/7/docs/technotes/guides/security/PolicyFiles.html
static String encode(Path path) { static String encode(Path path) {
return encode(path.toString()); return path.toString().replace("\\", "\\\\");
}
static String encode(String path) {
return path.replace("\\", "\\\\");
} }
} }

View File

@ -37,9 +37,6 @@ grant {
permission java.io.FilePermission "${project.basedir}${/}lib/sigar{/}-", "read"; permission java.io.FilePermission "${project.basedir}${/}lib/sigar{/}-", "read";
// mvn custom ./m2/repository for dependency jars // mvn custom ./m2/repository for dependency jars
permission java.io.FilePermission "${m2.repository}${/}-", "read"; 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 java.nio.file.LinkPermission "symbolic";
permission groovy.security.GroovyCodeSourcePermission "/groovy/script"; permission groovy.security.GroovyCodeSourcePermission "/groovy/script";
@ -86,7 +83,10 @@ grant {
// needed for natives calls // needed for natives calls
permission java.lang.RuntimePermission "loadLibrary.*"; permission java.lang.RuntimePermission "loadLibrary.*";
// needed for testing access rules etc
permission java.lang.RuntimePermission "createSecurityManager"; permission java.lang.RuntimePermission "createSecurityManager";
permission java.security.SecurityPermission "createPolicy.JavaPolicy";
// reflection hacks: // reflection hacks:
// needed for Striped64 (what is this doing), also enables unmap hack // needed for Striped64 (what is this doing), also enables unmap hack

View File

@ -19,30 +19,77 @@
package org.elasticsearch.bootstrap; 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 org.elasticsearch.test.ElasticsearchTestCase;
import java.nio.charset.StandardCharsets; import java.io.ByteArrayInputStream;
import java.io.FilePermission;
import java.nio.file.Path; 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 { public class SecurityTests extends ElasticsearchTestCase {
/** backslash escaping (e.g. windows paths) */ /** test generated permissions */
public void testEncode() { public void testGeneratedPermissions() throws Exception {
assertEquals("c:\\\\foobar", Security.encode("c:\\foobar")); Path path = createTempDir();
// 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 template processing */ /** test generated permissions for all configured paths */
public void testTemplateProcessing() throws Exception { public void testEnvironmentPaths() throws Exception {
Path path = createTempDir(); Path path = createTempDir();
byte results[] = Security.createPermissions(Collections.singleton(path)); ImmutableSettings.Builder settingsBuilder = ImmutableSettings.builder();
String unicode = new String(results, StandardCharsets.UTF_8); settingsBuilder.put("path.home", path.resolve("home").toString());
// try not to make this test too fragile or useless settingsBuilder.put("path.conf", path.resolve("conf").toString());
assertTrue(unicode.contains("grant {")); settingsBuilder.put("path.plugins", path.resolve("plugins").toString());
assertTrue(unicode.contains(Security.encode(path))); settingsBuilder.putArray("path.data", path.resolve("data1").toString(), path.resolve("data2").toString());
assertTrue(unicode.contains("read")); settingsBuilder.put("path.logs", path.resolve("logs").toString());
assertTrue(unicode.contains("write")); 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")));
}
} }