Skip to content

Commit a64471d

Browse files
committed
Decentralize plugin security
* Add ability for plugins to declare additional permissions with a custom plugin-security.policy file and corresponding AccessController logic. See the plugin author's guide for more information. * Add warning messages to users for extra plugin permissions in bin/plugin. * When bin/plugin is run interactively (stdin is a controlling terminal and -b/--batch not supplied), require user confirmation. * Improve unit test and IDE support for plugins with additional permissions by exposing plugin's metadata as a maven test resource. Closes #14108 Squashed commit of the following: commit cf8ace6 Author: Robert Muir <[email protected]> Date: Wed Oct 14 13:36:05 2015 -0400 fix new unit test from master merge commit 9be3c5a Merge: 2f168b8 7368231 Author: Robert Muir <[email protected]> Date: Wed Oct 14 12:58:31 2015 -0400 Merge branch 'master' into off_my_back commit 2f168b8 Author: Robert Muir <[email protected]> Date: Wed Oct 14 12:56:04 2015 -0400 improve plugin author documentation commit 6e6c2bf Author: Robert Muir <[email protected]> Date: Wed Oct 14 12:52:14 2015 -0400 move security confirmation after 'plugin already installed' check, to prevent user from answering unnecessary questions. commit 08233a2 Author: Robert Muir <[email protected]> Date: Wed Oct 14 05:36:42 2015 -0400 Add documentation and pluginmanager support commit 05dad86 Author: Robert Muir <[email protected]> Date: Wed Oct 14 02:22:24 2015 -0400 Decentralize plugin permissions (modulo docs and pluginmanager work) Conflicts: core/src/main/java/org/elasticsearch/bootstrap/Security.java core/src/main/resources/org/elasticsearch/bootstrap/security.policy
1 parent f776d78 commit a64471d

File tree

27 files changed

+710
-164
lines changed

27 files changed

+710
-164
lines changed

core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.security.Policy;
3030
import java.security.ProtectionDomain;
3131
import java.security.URIParameter;
32+
import java.util.Map;
3233

3334
/** custom policy for union of static and dynamic permissions */
3435
final class ESPolicy extends Policy {
@@ -41,13 +42,15 @@ final class ESPolicy extends Policy {
4142
final Policy template;
4243
final Policy untrusted;
4344
final PermissionCollection dynamic;
45+
final Map<String,PermissionCollection> plugins;
4446

45-
public ESPolicy(PermissionCollection dynamic) throws Exception {
47+
public ESPolicy(PermissionCollection dynamic, Map<String,PermissionCollection> plugins) throws Exception {
4648
URI policyUri = getClass().getResource(POLICY_RESOURCE).toURI();
4749
URI untrustedUri = getClass().getResource(UNTRUSTED_RESOURCE).toURI();
4850
this.template = Policy.getInstance("JavaPolicy", new URIParameter(policyUri));
4951
this.untrusted = Policy.getInstance("JavaPolicy", new URIParameter(untrustedUri));
5052
this.dynamic = dynamic;
53+
this.plugins = plugins;
5154
}
5255

5356
@Override @SuppressForbidden(reason = "fast equals check is desired")
@@ -66,6 +69,11 @@ public boolean implies(ProtectionDomain domain, Permission permission) {
6669
if (BootstrapInfo.UNTRUSTED_CODEBASE.equals(location.getFile())) {
6770
return untrusted.implies(domain, permission);
6871
}
72+
// check for an additional plugin permission
73+
PermissionCollection plugin = plugins.get(location.getFile());
74+
if (plugin != null && plugin.implies(permission)) {
75+
return true;
76+
}
6977
}
7078

7179
// Special handling for broken AWS code which destroys all SSL security

core/src/main/java/org/elasticsearch/bootstrap/Security.java

Lines changed: 28 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.elasticsearch.common.SuppressForbidden;
2323
import org.elasticsearch.env.Environment;
24+
import org.elasticsearch.plugins.PluginInfo;
2425

2526
import java.io.*;
2627
import java.net.URL;
@@ -30,8 +31,11 @@
3031
import java.nio.file.Files;
3132
import java.nio.file.NotDirectoryException;
3233
import java.nio.file.Path;
34+
import java.security.NoSuchAlgorithmException;
35+
import java.security.PermissionCollection;
3336
import java.security.Permissions;
3437
import java.security.Policy;
38+
import java.security.URIParameter;
3539
import java.util.Collections;
3640
import java.util.HashMap;
3741
import java.util.IdentityHashMap;
@@ -65,7 +69,7 @@
6569
* when they are so dangerous that general code should not be granted the
6670
* permission, but there are extenuating circumstances.
6771
* <p>
68-
* Groovy scripts are assigned no permissions. This does not provide adequate
72+
* Scripts (groovy, javascript, python) are assigned minimal permissions. This does not provide adequate
6973
* sandboxing, as these scripts still have access to ES classes, and could
7074
* modify members, etc that would cause bad things to happen later on their
7175
* behalf (no package protections are yet in place, this would need some
@@ -81,7 +85,7 @@
8185
* <h1>Debugging Security</h1>
8286
* A good place to start when there is a problem is to turn on security debugging:
8387
* <pre>
84-
* JAVA_OPTS="-Djava.security.debug=access:failure" bin/elasticsearch
88+
* JAVA_OPTS="-Djava.security.debug=access,failure" bin/elasticsearch
8589
* </pre>
8690
* See <a href="https://docs.oracle.com/javase/7/docs/technotes/guides/security/troubleshooting-security.html">
8791
* Troubleshooting Security</a> for information.
@@ -97,11 +101,9 @@ private Security() {}
97101
static void configure(Environment environment) throws Exception {
98102
// set properties for jar locations
99103
setCodebaseProperties();
100-
// set properties for problematic plugins
101-
setPluginCodebaseProperties(environment);
102104

103-
// enable security policy: union of template and environment-based paths.
104-
Policy.setPolicy(new ESPolicy(createPermissions(environment)));
105+
// enable security policy: union of template and environment-based paths, and possibly plugin permissions
106+
Policy.setPolicy(new ESPolicy(createPermissions(environment), getPluginPermissions(environment)));
105107

106108
// enable security manager
107109
System.setSecurityManager(new SecurityManager() {
@@ -158,69 +160,39 @@ static void setCodebaseProperties() {
158160
}
159161
}
160162

161-
// mapping of plugins to plugin class name. see getPluginClass why we need this.
162-
// plugin codebase property is always implicit (es.security.plugin.foobar)
163-
// note that this is only read once, when policy is parsed.
164-
static final Map<String,String> SPECIAL_PLUGINS;
165-
static {
166-
Map<String,String> m = new HashMap<>();
167-
m.put("cloud-aws", "org.elasticsearch.plugin.cloud.aws.CloudAwsPlugin");
168-
m.put("cloud-gce", "org.elasticsearch.plugin.cloud.gce.CloudGcePlugin");
169-
m.put("lang-expression", "org.elasticsearch.script.expression.ExpressionPlugin");
170-
m.put("lang-groovy", "org.elasticsearch.script.groovy.GroovyPlugin");
171-
m.put("lang-javascript", "org.elasticsearch.plugin.javascript.JavaScriptPlugin");
172-
m.put("lang-python", "org.elasticsearch.plugin.python.PythonPlugin");
173-
SPECIAL_PLUGINS = Collections.unmodifiableMap(m);
174-
}
175-
176-
/**
177-
* Returns policy property for plugin, if it has special permissions.
178-
* otherwise returns null.
179-
*/
180-
static String getPluginProperty(String pluginName) {
181-
if (SPECIAL_PLUGINS.containsKey(pluginName)) {
182-
return "es.security.plugin." + pluginName;
183-
} else {
184-
return null;
185-
}
186-
}
187-
188-
/**
189-
* Returns plugin class name, if it has special permissions.
190-
* otherwise returns null.
191-
*/
192-
// this is only here to support the intellij IDE
193-
// it sucks to duplicate information, but its worth the tradeoff: sanity
194-
// if it gets out of sync, tests will fail.
195-
static String getPluginClass(String pluginName) {
196-
return SPECIAL_PLUGINS.get(pluginName);
197-
}
198-
199163
/**
200164
* Sets properties (codebase URLs) for policy files.
201165
* we look for matching plugins and set URLs to fit
202166
*/
203167
@SuppressForbidden(reason = "proper use of URL")
204-
static void setPluginCodebaseProperties(Environment environment) throws IOException {
168+
static Map<String,PermissionCollection> getPluginPermissions(Environment environment) throws IOException, NoSuchAlgorithmException {
169+
Map<String,PermissionCollection> map = new HashMap<>();
205170
if (Files.exists(environment.pluginsFile())) {
206171
try (DirectoryStream<Path> stream = Files.newDirectoryStream(environment.pluginsFile())) {
207172
for (Path plugin : stream) {
208-
String prop = getPluginProperty(plugin.getFileName().toString());
209-
if (prop != null) {
210-
if (System.getProperty(prop) != null) {
211-
throw new IllegalStateException("property: " + prop + " is unexpectedly set: " + System.getProperty(prop));
173+
Path policyFile = plugin.resolve(PluginInfo.ES_PLUGIN_POLICY);
174+
if (Files.exists(policyFile)) {
175+
// parse the plugin's policy file into a set of permissions
176+
Policy policy = Policy.getInstance("JavaPolicy", new URIParameter(policyFile.toUri()));
177+
PermissionCollection permissions = policy.getPermissions(Security.class.getProtectionDomain());
178+
// this method is supported with the specific implementation we use, but just check for safety.
179+
if (permissions == Policy.UNSUPPORTED_EMPTY_COLLECTION) {
180+
throw new UnsupportedOperationException("JavaPolicy implementation does not support retrieving permissions");
181+
}
182+
// grant the permissions to each jar in the plugin
183+
try (DirectoryStream<Path> jarStream = Files.newDirectoryStream(plugin, "*.jar")) {
184+
for (Path jar : jarStream) {
185+
if (map.put(jar.toUri().toURL().getFile(), permissions) != null) {
186+
// just be paranoid ok?
187+
throw new IllegalStateException("per-plugin permissions already granted for jar file: " + jar);
188+
}
189+
}
212190
}
213-
System.setProperty(prop, plugin.toUri().toURL().toString() + "*");
214191
}
215192
}
216193
}
217194
}
218-
for (String plugin : SPECIAL_PLUGINS.keySet()) {
219-
String prop = getPluginProperty(plugin);
220-
if (System.getProperty(prop) == null) {
221-
System.setProperty(prop, "file:/dev/null"); // no chance to be interpreted as "all"
222-
}
223-
}
195+
return Collections.unmodifiableMap(map);
224196
}
225197

226198
/** returns dynamic Permissions to configured paths */

core/src/main/java/org/elasticsearch/plugins/PluginInfo.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
public class PluginInfo implements Streamable, ToXContent {
3737

3838
public static final String ES_PLUGIN_PROPERTIES = "plugin-descriptor.properties";
39+
public static final String ES_PLUGIN_POLICY = "plugin-security.policy";
3940

4041
static final class Fields {
4142
static final XContentBuilderString NAME = new XContentBuilderString("name");

core/src/main/java/org/elasticsearch/plugins/PluginManager.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public PluginManager(Environment environment, URL url, OutputMode outputMode, Ti
9898
this.timeout = timeout;
9999
}
100100

101-
public void downloadAndExtract(String name, Terminal terminal) throws IOException {
101+
public void downloadAndExtract(String name, Terminal terminal, boolean batch) throws IOException {
102102
if (name == null && url == null) {
103103
throw new IllegalArgumentException("plugin name or url must be supplied with install.");
104104
}
@@ -122,7 +122,7 @@ public void downloadAndExtract(String name, Terminal terminal) throws IOExceptio
122122
}
123123

124124
Path pluginFile = download(pluginHandle, terminal);
125-
extract(pluginHandle, terminal, pluginFile);
125+
extract(pluginHandle, terminal, pluginFile, batch);
126126
}
127127

128128
private Path download(PluginHandle pluginHandle, Terminal terminal) throws IOException {
@@ -205,7 +205,7 @@ private Path download(PluginHandle pluginHandle, Terminal terminal) throws IOExc
205205
return pluginFile;
206206
}
207207

208-
private void extract(PluginHandle pluginHandle, Terminal terminal, Path pluginFile) throws IOException {
208+
private void extract(PluginHandle pluginHandle, Terminal terminal, Path pluginFile, boolean batch) throws IOException {
209209
// unzip plugin to a staging temp dir, named for the plugin
210210
Path tmp = Files.createTempDirectory(environment.tmpFile(), null);
211211
Path root = tmp.resolve(pluginHandle.name);
@@ -230,6 +230,13 @@ private void extract(PluginHandle pluginHandle, Terminal terminal, Path pluginFi
230230
throw new IOException("plugin directory " + extractLocation.toAbsolutePath() + " already exists. To update the plugin, uninstall it first using 'remove " + pluginHandle.name + "' command");
231231
}
232232

233+
// read optional security policy (extra permissions)
234+
// if it exists, confirm or warn the user
235+
Path policy = root.resolve(PluginInfo.ES_PLUGIN_POLICY);
236+
if (Files.exists(policy)) {
237+
PluginSecurity.readPolicy(policy, terminal, environment, batch);
238+
}
239+
233240
// install plugin
234241
FileSystemUtils.copyDirectoryRecursively(root, extractLocation);
235242
terminal.println("Installed %s into %s", pluginHandle.name, extractLocation.toAbsolutePath());
@@ -333,7 +340,7 @@ private static void setPosixFileAttributes(Path path, UserPrincipal owner, Group
333340
fileAttributeView.setPermissions(permissions);
334341
}
335342

336-
private void tryToDeletePath(Terminal terminal, Path ... paths) {
343+
static void tryToDeletePath(Terminal terminal, Path ... paths) {
337344
for (Path path : paths) {
338345
try {
339346
IOUtils.rm(path);

core/src/main/java/org/elasticsearch/plugins/PluginManagerCliParser.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ static class Install extends Command {
180180

181181
private static final CliToolConfig.Cmd CMD = cmd(NAME, Install.class)
182182
.options(option("t", "timeout").required(false).hasArg(false))
183+
.options(option("b", "batch").required(false))
183184
.build();
184185

185186
static Command parse(Terminal terminal, CommandLine cli) {
@@ -210,21 +211,28 @@ static Command parse(Terminal terminal, CommandLine cli) {
210211
if (cli.hasOption("v")) {
211212
outputMode = OutputMode.VERBOSE;
212213
}
214+
215+
boolean batch = System.console() == null;
216+
if (cli.hasOption("b")) {
217+
batch = true;
218+
}
213219

214-
return new Install(terminal, name, outputMode, optionalPluginUrl, timeout);
220+
return new Install(terminal, name, outputMode, optionalPluginUrl, timeout, batch);
215221
}
216222

217223
final String name;
218224
private OutputMode outputMode;
219225
final URL url;
220226
final TimeValue timeout;
227+
final boolean batch;
221228

222-
Install(Terminal terminal, String name, OutputMode outputMode, URL url, TimeValue timeout) {
229+
Install(Terminal terminal, String name, OutputMode outputMode, URL url, TimeValue timeout, boolean batch) {
223230
super(terminal);
224231
this.name = name;
225232
this.outputMode = outputMode;
226233
this.url = url;
227234
this.timeout = timeout;
235+
this.batch = batch;
228236
}
229237

230238
@Override
@@ -235,7 +243,7 @@ public ExitStatus execute(Settings settings, Environment env) throws Exception {
235243
} else {
236244
terminal.println("-> Installing from " + URLDecoder.decode(url.toString(), "UTF-8") + "...");
237245
}
238-
pluginManager.downloadAndExtract(name, terminal);
246+
pluginManager.downloadAndExtract(name, terminal, batch);
239247
return ExitStatus.OK;
240248
}
241249
}

0 commit comments

Comments
 (0)