Skip to content

Commit

Permalink
Improve startup exceptions (especially file permissions etc)
Browse files Browse the repository at this point in the history
Squashed commit of the following:

commit 0014d31c1c478977cc8c50eb635eab0fb91449e4
Author: Robert Muir <rmuir@apache.org>
Date:   Fri Aug 21 18:20:35 2015 -0400

    Add missing paren to javadocs

commit bb46142
Author: Robert Muir <rmuir@apache.org>
Date:   Fri Aug 21 18:08:45 2015 -0400

    Improve startup exceptions (especially file permissions etc)
  • Loading branch information
rmuir committed Aug 21, 2015
1 parent 907f648 commit d96af93
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 23 deletions.
40 changes: 25 additions & 15 deletions core/src/main/java/org/elasticsearch/bootstrap/Security.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,25 +118,25 @@ static void setCodebaseProperties() {
static Permissions createPermissions(Environment environment) throws IOException {
Permissions policy = new Permissions();
// read-only dirs
addPath(policy, environment.binFile(), "read,readlink");
addPath(policy, environment.libFile(), "read,readlink");
addPath(policy, environment.pluginsFile(), "read,readlink");
addPath(policy, environment.configFile(), "read,readlink");
addPath(policy, environment.scriptsFile(), "read,readlink");
addPath(policy, "path.home", environment.binFile(), "read,readlink");
addPath(policy, "path.home", environment.libFile(), "read,readlink");
addPath(policy, "path.plugins", environment.pluginsFile(), "read,readlink");
addPath(policy, "path.conf", environment.configFile(), "read,readlink");
addPath(policy, "path.scripts", environment.scriptsFile(), "read,readlink");
// read-write dirs
addPath(policy, environment.tmpFile(), "read,readlink,write,delete");
addPath(policy, environment.logsFile(), "read,readlink,write,delete");
addPath(policy, "java.io.tmpdir", environment.tmpFile(), "read,readlink,write,delete");
addPath(policy, "path.logs", environment.logsFile(), "read,readlink,write,delete");
if (environment.sharedDataFile() != null) {
addPath(policy, environment.sharedDataFile(), "read,readlink,write,delete");
addPath(policy, "path.shared_data", environment.sharedDataFile(), "read,readlink,write,delete");
}
for (Path path : environment.dataFiles()) {
addPath(policy, path, "read,readlink,write,delete");
addPath(policy, "path.data", path, "read,readlink,write,delete");
}
for (Path path : environment.dataWithClusterFiles()) {
addPath(policy, path, "read,readlink,write,delete");
addPath(policy, "path.data", path, "read,readlink,write,delete");
}
for (Path path : environment.repoFiles()) {
addPath(policy, path, "read,readlink,write,delete");
addPath(policy, "path.repo", path, "read,readlink,write,delete");
}
if (environment.pidFile() != null) {
// we just need permission to remove the file if its elsewhere.
Expand All @@ -145,10 +145,20 @@ static Permissions createPermissions(Environment environment) throws IOException
return policy;
}

/** 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
ensureDirectoryExists(path);
/**
* Add access to path (and all files underneath it)
* @param policy current policy to add permissions to
* @param configurationName the configuration name associated with the path (for error messages only)
* @param path the path itself
* @param permissions set of filepermissions to grant to the path
*/
static void addPath(Permissions policy, String configurationName, Path path, String permissions) {
// paths may not exist yet, this also checks accessibility
try {
ensureDirectoryExists(path);
} catch (IOException e) {
throw new IllegalStateException("Unable to access '" + configurationName + "' (" + path + ")", e);
}

// add each path twice: once for itself, again for files underneath it
policy.add(new FilePermission(path.toString(), permissions));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,7 @@ public void printStackTrace(PrintStream s) {
cause = getFirstGuiceCause((CreationException)cause);
}

String message = cause.getMessage();
if (message == null) {
message = "Unknown Error";
}
String message = cause.toString();
s.println(message);

if (cause != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public PluginsService(Settings settings, Environment environment) {
List<Bundle> bundles = getPluginBundles(environment);
tupleBuilder.addAll(loadBundles(bundles));
} catch (IOException ex) {
throw new IllegalStateException(ex);
throw new IllegalStateException("Unable to initialize plugins", ex);
}

plugins = tupleBuilder.build();
Expand Down Expand Up @@ -279,9 +279,10 @@ static class Bundle {
}

static List<Bundle> getPluginBundles(Environment environment) throws IOException {
ESLogger logger = Loggers.getLogger(Bootstrap.class);
ESLogger logger = Loggers.getLogger(PluginsService.class);

Path pluginsDirectory = environment.pluginsFile();
// TODO: remove this leniency, but tests bogusly rely on it
if (!isAccessibleDirectory(pluginsDirectory, logger)) {
return Collections.emptyList();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public class BootstrapForTesting {
}
}
// java.io.tmpdir
Security.addPath(perms, javaTmpDir, "read,readlink,write,delete");
Security.addPath(perms, "java.io.tmpdir", javaTmpDir, "read,readlink,write,delete");
// custom test config file
if (Strings.hasLength(System.getProperty("tests.config"))) {
perms.add(new FilePermission(System.getProperty("tests.config"), "read,readlink"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ public void testSymlinkPermissions() throws IOException {
assumeNoException("test cannot create symbolic links with security manager enabled", e);
}
Permissions permissions = new Permissions();
Security.addPath(permissions, link, "read");
Security.addPath(permissions, "testing", link, "read");
assertExactPermissions(new FilePermission(link.toString(), "read"), permissions);
assertExactPermissions(new FilePermission(link.resolve("foo").toString(), "read"), permissions);
assertExactPermissions(new FilePermission(target.toString(), "read"), permissions);
Expand Down

0 comments on commit d96af93

Please sign in to comment.