honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #12609 from rmuir/sane_permissions 

improve sanity of securitymanager file permissions
This commit is contained in:
Robert Muir 2015-08-03 17:39:30 -04:00
parent c82717789c 2623ce7960
commit 9769477c9d
2 changed files with 80 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
core/src/main/java/org/elasticsearch/bootstrap/Security.java
View File

@ -120,10 +120,11 @@ final class Security {
// read-only dirs // read-only dirs
addPath(policy, environment.binFile(), "read,readlink"); addPath(policy, environment.binFile(), "read,readlink");
addPath(policy, environment.libFile(), "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.tmpFile(), "read,readlink,write,delete");
addPath(policy, environment.configFile(), "read,readlink,write,delete");
addPath(policy, environment.logsFile(), "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()) { for (Path path : environment.dataFiles()) {
addPath(policy, path, "read,readlink,write,delete"); addPath(policy, path, "read,readlink,write,delete");
} }
@ -134,7 +135,8 @@ final class Security {
addPath(policy, path, "read,readlink,write,delete"); addPath(policy, path, "read,readlink,write,delete");
} }
if (environment.pidFile() != null) { 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; return policy;
} }

104
core/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java
View File

@ -20,6 +20,7 @@
package org.elasticsearch.bootstrap; package org.elasticsearch.bootstrap;
import org.apache.lucene.util.Constants; import org.apache.lucene.util.Constants;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment; import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ElasticsearchTestCase; import org.elasticsearch.test.ElasticsearchTestCase;
@ -28,7 +29,9 @@ import java.io.FilePermission;
import java.io.IOException; import java.io.IOException;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.security.PermissionCollection;
import java.security.Permissions; import java.security.Permissions;
import java.util.Set;
public class SecurityTests extends ElasticsearchTestCase { public class SecurityTests extends ElasticsearchTestCase {
@ -53,26 +56,28 @@ public class SecurityTests extends ElasticsearchTestCase {
} }
// the fake es home // the fake es home
assertFalse(permissions.implies(new FilePermission(esHome.toString(), "read"))); assertNoPermissions(esHome, permissions);
// its parent // its parent
assertFalse(permissions.implies(new FilePermission(path.toString(), "read"))); assertNoPermissions(esHome.getParent(), permissions);
// some other sibling // 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 // 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 */ /** test generated permissions for all configured paths */
public void testEnvironmentPaths() throws Exception { public void testEnvironmentPaths() throws Exception {
Path path = createTempDir(); 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(); Settings.Builder settingsBuilder = Settings.builder();
settingsBuilder.put("path.home", path.resolve("home").toString()); settingsBuilder.put("path.home", esHome.resolve("home").toString());
settingsBuilder.put("path.conf", path.resolve("conf").toString()); settingsBuilder.put("path.conf", esHome.resolve("conf").toString());
settingsBuilder.put("path.plugins", path.resolve("plugins").toString()); settingsBuilder.put("path.plugins", esHome.resolve("plugins").toString());
settingsBuilder.putArray("path.data", path.resolve("data1").toString(), path.resolve("data2").toString()); settingsBuilder.putArray("path.data", esHome.resolve("data1").toString(), esHome.resolve("data2").toString());
settingsBuilder.put("path.logs", path.resolve("logs").toString()); settingsBuilder.put("path.logs", esHome.resolve("logs").toString());
settingsBuilder.put("pidfile", path.resolve("test.pid").toString()); settingsBuilder.put("pidfile", esHome.resolve("test.pid").toString());
Settings settings = settingsBuilder.build(); Settings settings = settingsBuilder.build();
Path fakeTmpDir = createTempDir(); Path fakeTmpDir = createTempDir();
@ -87,29 +92,39 @@ public class SecurityTests extends ElasticsearchTestCase {
System.setProperty("java.io.tmpdir", realTmpDir); 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: // check that all directories got permissions:
assertTrue(permissions.implies(new FilePermission(environment.binFile().toString(), "read")));
assertTrue(permissions.implies(new FilePermission(environment.libFile().toString(), "read"))); // bin file: ro
// config file assertExactPermissions(new FilePermission(environment.binFile().toString(), "read,readlink"), permissions);
// TODO: make read-only // lib file: ro
assertTrue(permissions.implies(new FilePermission(environment.configFile().toString(), "read,readlink,write,delete"))); assertExactPermissions(new FilePermission(environment.libFile().toString(), "read,readlink"), permissions);
// plugins: r/w, TODO: can this be minimized? // config file: ro
assertTrue(permissions.implies(new FilePermission(environment.pluginsFile().toString(), "read,readlink,write,delete"))); assertExactPermissions(new FilePermission(environment.configFile().toString(), "read,readlink"), permissions);
// plugins: ro
assertExactPermissions(new FilePermission(environment.pluginsFile().toString(), "read,readlink"), permissions);
// data paths: r/w // data paths: r/w
for (Path dataPath : environment.dataFiles()) { 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()) { 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 // 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 // temp dir: r/w
assertTrue(permissions.implies(new FilePermission(fakeTmpDir.toString(), "read,readlink,write,delete"))); assertExactPermissions(new FilePermission(fakeTmpDir.toString(), "read,readlink,write,delete"), permissions);
// double check we overwrote java.io.tmpdir correctly for the test // PID file: delete only (for the shutdown hook)
assertFalse(permissions.implies(new FilePermission(realTmpDir.toString(), "read"))); assertExactPermissions(new FilePermission(environment.pidFile().toString(), "delete"), permissions);
// PID file: r/w
assertTrue(permissions.implies(new FilePermission(environment.pidFile().toString(), "read,readlink,write,delete")));
} }
public void testEnsureExists() throws IOException { public void testEnsureExists() throws IOException {
@ -225,9 +240,40 @@ public class SecurityTests extends ElasticsearchTestCase {
} }
Permissions permissions = new Permissions(); Permissions permissions = new Permissions();
Security.addPath(permissions, link, "read"); Security.addPath(permissions, link, "read");
assertTrue(permissions.implies(new FilePermission(link.toString(), "read"))); assertExactPermissions(new FilePermission(link.toString(), "read"), permissions);
assertTrue(permissions.implies(new FilePermission(link.resolve("foo").toString(), "read"))); assertExactPermissions(new FilePermission(link.resolve("foo").toString(), "read"), permissions);
assertTrue(permissions.implies(new FilePermission(target.toString(), "read"))); assertExactPermissions(new FilePermission(target.toString(), "read"), permissions);
assertTrue(permissions.implies(new FilePermission(target.resolve("foo").toString(), "read"))); 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<String> 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")));
} }
} }