Add Intellij support for plugins with extra permissions.

graduate this from a hack for insecure plugins to something we can
live with for per-module/plugin permissions, it now works reasonably
in unit tests and with Intellij and Eclipse IDEs.

remove security warnings: we will deal with these issues in a secure
way, if we cannot, then the plugin shouldn't be in our core codebase.
This commit is contained in:
Robert Muir 2015-09-17 23:51:51 -04:00
parent 5e5e10c91b
commit 3504fefb6b
5 changed files with 60 additions and 36 deletions

View File

@ -54,6 +54,6 @@ public final class BootstrapInfo {
* that require additional privileges as a workaround.
*/
public static Set<String> getInsecurePluginList() {
return Collections.unmodifiableSet(Security.INSECURE_PLUGINS.keySet());
return Collections.unmodifiableSet(Security.SPECIAL_PLUGINS.keySet());
}
}

View File

@ -20,8 +20,6 @@
package org.elasticsearch.bootstrap;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.logging.ESLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.env.Environment;
import java.io.*;
@ -159,15 +157,39 @@ final class Security {
}
}
// mapping of insecure plugins to codebase properties
// mapping of plugins to plugin class name. see getPluginClass why we need this.
// plugin codebase property is always implicit (es.security.plugin.foobar)
// note that this is only read once, when policy is parsed.
static final Map<String,String> INSECURE_PLUGINS;
static final Map<String,String> SPECIAL_PLUGINS;
static {
Map<String,String> m = new HashMap<>();
m.put("repository-s3", "es.security.insecure.plugin.repository-s3");
m.put("discovery-ec2", "es.security.insecure.plugin.discovery-ec2");
m.put("cloud-gce", "es.security.insecure.plugin.cloud-gce" );
INSECURE_PLUGINS = Collections.unmodifiableMap(m);
m.put("repository-s3", "org.elasticsearch.plugin.repository.s3.S3RepositoryPlugin");
m.put("discovery-ec2", "org.elasticsearch.plugin.discovery.ec2.Ec2DiscoveryPlugin");
m.put("cloud-gce", "org.elasticsearch.plugin.cloud.gce.CloudGcePlugin");
SPECIAL_PLUGINS = Collections.unmodifiableMap(m);
}
/**
* Returns policy property for plugin, if it has special permissions.
* otherwise returns null.
*/
static String getPluginProperty(String pluginName) {
if (SPECIAL_PLUGINS.containsKey(pluginName)) {
return "es.security.plugin." + pluginName;
} else {
return null;
}
}
/**
* Returns plugin class name, if it has special permissions.
* otherwise returns null.
*/
// this is only here to support the intellij IDE
// it sucks to duplicate information, but its worth the tradeoff: sanity
// if it gets out of sync, tests will fail.
static String getPluginClass(String pluginName) {
return SPECIAL_PLUGINS.get(pluginName);
}
/**
@ -179,20 +201,18 @@ final class Security {
if (Files.exists(environment.pluginsFile())) {
try (DirectoryStream<Path> stream = Files.newDirectoryStream(environment.pluginsFile())) {
for (Path plugin : stream) {
String prop = INSECURE_PLUGINS.get(plugin.getFileName().toString());
String prop = getPluginProperty(plugin.getFileName().toString());
if (prop != null) {
if (System.getProperty(prop) != null) {
throw new IllegalStateException("property: " + prop + " is unexpectedly set: " + System.getProperty(prop));
}
System.setProperty(prop, plugin.toUri().toURL().toString() + "*");
ESLogger logger = Loggers.getLogger(Security.class);
logger.warn("Adding permissions for insecure plugin [{}]", plugin.getFileName());
logger.warn("There are unresolved issues with third-party code that may reduce the security of the system");
}
}
}
}
for (String prop : INSECURE_PLUGINS.values()) {
for (String plugin : SPECIAL_PLUGINS.keySet()) {
String prop = getPluginProperty(plugin);
if (System.getProperty(prop) == null) {
System.setProperty(prop, "file:/dev/null"); // no chance to be interpreted as "all"
}

View File

@ -37,22 +37,22 @@ grant codeBase "${es.security.jar.lucene.core}" {
permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
};
//// Insecure plugin permissions:
//// These are dangerous permissions due to problems in third-party library code
//// We try our best to contain and protect the issues, but ultimately warn the user
//// when installing these plugins that it can introduce a security risk
//// Special plugin permissions:
//// These are dangerous permissions only needed by special plugins that we don't
//// want to grant in general. Some may be due to problems in third-party library code,
//// others may just be more obscure integrations.
grant codeBase "${es.security.insecure.plugin.repository-s3}" {
grant codeBase "${es.security.plugin.repository-s3}" {
// needed because of problems in aws-sdk
permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
};
grant codeBase "${es.security.insecure.plugin.discovery-ec2}" {
grant codeBase "${es.security.plugin.discovery-ec2}" {
// needed because of problems in aws-sdk
permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
};
grant codeBase "${es.security.insecure.plugin.cloud-gce}" {
grant codeBase "${es.security.plugin.cloud-gce}" {
// needed because of problems in cloud-gce
permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
};

View File

@ -32,6 +32,7 @@ import java.net.URL;
import java.nio.file.Path;
import java.security.Permissions;
import java.security.Policy;
import java.util.Map;
import java.util.Objects;
import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean;
@ -115,16 +116,24 @@ public class BootstrapForTesting {
}
final Policy policy;
// if its an insecure plugin, we use a wrapper policy impl to try
// if its a plugin with special permissions, we use a wrapper policy impl to try
// to simulate what happens with a real distribution
String artifact = System.getProperty("tests.artifact");
// in case we are running from the IDE:
if (artifact == null || System.getProperty("tests.maven") == null) {
artifact = PathUtils.get(System.getProperty("user.dir")).toAbsolutePath().getFileName().toString();
if (artifact == null && System.getProperty("tests.maven") == null) {
// look for plugin classname as a resource to determine what project we are.
// while gross, this will work with any IDE.
for (Map.Entry<String,String> kv : Security.SPECIAL_PLUGINS.entrySet()) {
String resource = kv.getValue().replace('.', '/') + ".class";
if (BootstrapForTesting.class.getClassLoader().getResource(resource) != null) {
artifact = kv.getKey();
break;
}
}
}
String insecurePluginProp = Security.INSECURE_PLUGINS.get(artifact);
if (insecurePluginProp != null) {
policy = new MockPluginPolicy(perms, insecurePluginProp);
String pluginProp = Security.getPluginProperty(artifact);
if (pluginProp != null) {
policy = new MockPluginPolicy(perms, pluginProp);
} else {
policy = new ESPolicy(perms);
}
@ -132,14 +141,10 @@ public class BootstrapForTesting {
System.setSecurityManager(new TestSecurityManager());
Security.selfTest();
if (insecurePluginProp != null) {
if (pluginProp != null) {
// initialize the plugin class, in case it has one-time hacks (unit tests often won't do this)
String clazz = System.getProperty("tests.plugin.classname");
if (clazz != null) {
Class.forName(clazz);
} else if (System.getProperty("tests.maven") != null) {
throw new IllegalStateException("plugin classname is needed for insecure plugin unit tests: something wrong with build");
}
String clazz = Security.getPluginClass(artifact);
Class.forName(clazz);
}
} catch (Exception e) {
throw new RuntimeException("unable to install test security manager", e);

View File

@ -664,9 +664,8 @@
<tests.cluster>${tests.cluster}</tests.cluster>
<tests.iters>${tests.iters}</tests.iters>
<tests.project>${project.groupId}:${project.artifactId}</tests.project>
<!-- these two are needed for insecure plugins, remove if possible! -->
<!-- needed to know which project we are in for plugin permissions -->
<tests.artifact>${project.artifactId}</tests.artifact>
<tests.plugin.classname>${elasticsearch.plugin.classname}</tests.plugin.classname>
<tests.maxfailures>${tests.maxfailures}</tests.maxfailures>
<tests.failfast>${tests.failfast}</tests.failfast>
<tests.class>${tests.class}</tests.class>