Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve startup exceptions (especially file permissions etc) #13050

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 25 additions & 15 deletions core/src/main/java/org/elasticsearch/bootstrap/Security.java
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: missing ending paren

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