From f1a9b50e9e066bd580cf77c5ee995f1547d26ceb Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Sat, 21 Nov 2015 23:01:37 -0500 Subject: [PATCH] Ban write access to system properties Followup to https://github.com/elastic/elasticsearch/pull/14914 Shield has to request read-write access to all system properties due to silliness in UnboundID sdk (https://github.com/UnboundID/ldapsdk/blob/556a203094a4239e0ad09c3acaa0704f21f5aa23/src/com/unboundid/util/Debug.java#L166) We should followup with a pull request to them, to not use System.getProperties() here which returns a mutable map (hence: read-write to "*"). Furthermore, the hack has to be wrapped in another hack because gradle doesn't add shield's plugin metadata to the classpath. Of course, if we weren't testing with two plugins in the classpath (which is not very realistic) this would be a non-issue. Original commit: elastic/x-pack-elasticsearch@612cacde6a37bc1a86078cc8ae9df58a1c440ef3 --- .../elasticsearch/shield/ShieldPlugin.java | 32 +++++++++++++++++++ .../plugin-metadata/plugin-security.policy | 4 +++ .../org/elasticsearch/watcher/WatcherF.java | 2 ++ .../bench/WatcherScheduleEngineBenchmark.java | 2 ++ 4 files changed, 40 insertions(+) create mode 100644 shield/src/main/plugin-metadata/plugin-security.policy diff --git a/shield/src/main/java/org/elasticsearch/shield/ShieldPlugin.java b/shield/src/main/java/org/elasticsearch/shield/ShieldPlugin.java index db911ea1e27..8d267678831 100644 --- a/shield/src/main/java/org/elasticsearch/shield/ShieldPlugin.java +++ b/shield/src/main/java/org/elasticsearch/shield/ShieldPlugin.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.shield; +import org.elasticsearch.SpecialPermission; import org.elasticsearch.action.ActionModule; import org.elasticsearch.client.Client; import org.elasticsearch.client.support.Headers; @@ -53,6 +54,8 @@ import org.elasticsearch.transport.TransportModule; import java.io.Closeable; import java.nio.file.Path; import java.util.*; +import java.security.AccessController; +import java.security.PrivilegedAction; /** * @@ -71,6 +74,35 @@ public class ShieldPlugin extends Plugin { private final boolean clientMode; private ShieldLicenseState shieldLicenseState; + // TODO: clean up this library to not ask for write access to all system properties! + static { + // invoke this clinit in unbound with permissions to access all system properties + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + sm.checkPermission(new SpecialPermission()); + } + try { + AccessController.doPrivileged(new PrivilegedAction() { + @Override + public Void run() { + try { + Class.forName("com.unboundid.util.Debug"); + } catch (ClassNotFoundException e) { + throw new RuntimeException(e); + } + return null; + } + }); + // TODO: fix gradle to add all shield resources (plugin metadata) to test classpath + // of watcher plugin, which depends on it directly. This prevents these plugins + // from being initialized correctly by the test framework, and means we have to + // have this leniency. + } catch (ExceptionInInitializerError bogus) { + if (bogus.getCause() instanceof SecurityException == false) { + throw bogus; // some other bug + } + } + } public ShieldPlugin(Settings settings) { this.settings = settings; diff --git a/shield/src/main/plugin-metadata/plugin-security.policy b/shield/src/main/plugin-metadata/plugin-security.policy new file mode 100644 index 00000000000..b104d276aae --- /dev/null +++ b/shield/src/main/plugin-metadata/plugin-security.policy @@ -0,0 +1,4 @@ +grant { + // needed because of problems in unbound LDAP library + permission java.util.PropertyPermission "*", "read,write"; +}; diff --git a/watcher/src/test/java/org/elasticsearch/watcher/WatcherF.java b/watcher/src/test/java/org/elasticsearch/watcher/WatcherF.java index 25032ff0b63..e651477eb8a 100644 --- a/watcher/src/test/java/org/elasticsearch/watcher/WatcherF.java +++ b/watcher/src/test/java/org/elasticsearch/watcher/WatcherF.java @@ -7,6 +7,7 @@ package org.elasticsearch.watcher; import org.elasticsearch.Version; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.license.plugin.LicensePlugin; import org.elasticsearch.node.MockNode; import org.elasticsearch.node.Node; @@ -23,6 +24,7 @@ import java.util.concurrent.CountDownLatch; */ public class WatcherF { + @SuppressForbidden(reason = "not really code or a test") public static void main(String[] args) throws Throwable { Settings.Builder settings = Settings.builder(); settings.put("http.cors.enabled", "true"); diff --git a/watcher/src/test/java/org/elasticsearch/watcher/test/bench/WatcherScheduleEngineBenchmark.java b/watcher/src/test/java/org/elasticsearch/watcher/test/bench/WatcherScheduleEngineBenchmark.java index 0c3361a2b93..f4c2c1bd8bb 100644 --- a/watcher/src/test/java/org/elasticsearch/watcher/test/bench/WatcherScheduleEngineBenchmark.java +++ b/watcher/src/test/java/org/elasticsearch/watcher/test/bench/WatcherScheduleEngineBenchmark.java @@ -13,6 +13,7 @@ import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.client.Client; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.metrics.MeanMetric; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; @@ -63,6 +64,7 @@ public class WatcherScheduleEngineBenchmark { .put("http.cors.enabled", true) .build(); + @SuppressForbidden(reason = "not really code or a test") public static void main(String[] args) throws Exception { System.setProperty("es.logger.prefix", "");