From 9219af81063213773b86f2150520d216fb4862f7 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Thu, 17 Sep 2015 10:17:17 -0400 Subject: [PATCH 1/3] Better simulate problematic plugins permissions in unit tests. We don't have a plugin .zip for unit tests, so we can't do it correctly. But we can approximate it better, so that if code is simply missing an AccessController block at least tests will fail. --- core/pom.xml | 3 +- .../bootstrap/BootstrapForTesting.java | 54 +------- .../bootstrap/MockPluginPolicy.java | 123 ++++++++++++++++++ 3 files changed, 132 insertions(+), 48 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/bootstrap/MockPluginPolicy.java diff --git a/core/pom.xml b/core/pom.xml index 58118a2d032..16f3c7fec26 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -251,6 +251,7 @@ org/elasticsearch/test/**/* org/elasticsearch/bootstrap/BootstrapForTesting.class + org/elasticsearch/bootstrap/MockPluginPolicy.class org/elasticsearch/common/cli/CliToolTestCase.class org/elasticsearch/common/cli/CliToolTestCase$*.class @@ -279,7 +280,7 @@ rest-api-spec/**/* org/elasticsearch/test/**/* org/elasticsearch/bootstrap/BootstrapForTesting.class - org/elasticsearch/bootstrap/XTestSecurityManager*.class + org/elasticsearch/bootstrap/MockPluginPolicy.class org/elasticsearch/common/cli/CliToolTestCase.class org/elasticsearch/common/cli/CliToolTestCase$*.class org/elasticsearch/cluster/MockInternalClusterInfoService.class diff --git a/core/src/test/java/org/elasticsearch/bootstrap/BootstrapForTesting.java b/core/src/test/java/org/elasticsearch/bootstrap/BootstrapForTesting.java index 191f9324eb2..41fad1607e3 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/BootstrapForTesting.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/BootstrapForTesting.java @@ -30,13 +30,8 @@ import org.elasticsearch.common.logging.Loggers; import java.io.FilePermission; import java.net.URL; import java.nio.file.Path; -import java.security.CodeSource; -import java.security.Permission; -import java.security.PermissionCollection; import java.security.Permissions; import java.security.Policy; -import java.security.cert.Certificate; -import java.util.Collections; import java.util.Objects; import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean; @@ -119,14 +114,17 @@ public class BootstrapForTesting { perms.add(new FilePermission(coverageDir.resolve("jacoco-it.exec").toString(), "read,write")); } - // if its an insecure plugin, its not easy to simulate here, since we don't have a real plugin install. - // we just do our best so unit testing can work. integration tests for such plugins are essential. + final Policy policy; + // if its an insecure plugin, we use a wrapper policy impl to try + // to simulate what happens with a real distribution String artifact = System.getProperty("tests.artifact"); String insecurePluginProp = Security.INSECURE_PLUGINS.get(artifact); if (insecurePluginProp != null) { - setInsecurePluginPermissions(perms, insecurePluginProp); + policy = new MockPluginPolicy(perms, insecurePluginProp); + } else { + policy = new ESPolicy(perms); } - Policy.setPolicy(new ESPolicy(perms)); + Policy.setPolicy(policy); System.setSecurityManager(new TestSecurityManager()); Security.selfTest(); @@ -144,44 +142,6 @@ public class BootstrapForTesting { } } - /** - * with a real plugin install, we just set a property to plugin/foo*, which matches - * plugin code and all dependencies. when running unit tests, things are disorganized, - * and might even be on different filesystem roots (windows), so we can't even make - * a URL that will match everything. instead, add the extra permissions globally. - */ - // TODO: maybe wrap with a policy so the extra permissions aren't applied to test classes/framework, - // so that stacks are always polluted and tests fail for missing AccessController blocks... - static void setInsecurePluginPermissions(Permissions permissions, String insecurePluginProp) throws Exception { - // the hack begins! - - // parse whole policy file, with and without the substitution, compute the delta, then add globally. - URL bogus = new URL("file:/bogus"); - ESPolicy policy = new ESPolicy(new Permissions()); - PermissionCollection small = policy.template.getPermissions(new CodeSource(bogus, (Certificate[])null)); - System.setProperty(insecurePluginProp, bogus.toString()); - policy = new ESPolicy(new Permissions()); - System.clearProperty(insecurePluginProp); - PermissionCollection big = policy.template.getPermissions(new CodeSource(bogus, (Certificate[])null)); - - PermissionCollection delta = delta(small, big); - for (Permission p : Collections.list(delta.elements())) { - permissions.add(p); - } - } - - // computes delta of small and big, the slow way - static PermissionCollection delta(PermissionCollection small, PermissionCollection big) { - Permissions extra = new Permissions(); - for (Permission p : Collections.list(big.elements())) { - // check big too, to remove UnresolvedPermissions (acts like NaN) - if (big.implies(p) && small.implies(p) == false) { - extra.add(p); - } - } - return extra; - } - // does nothing, just easy way to make sure the class is loaded. public static void ensureInitialized() {} } diff --git a/core/src/test/java/org/elasticsearch/bootstrap/MockPluginPolicy.java b/core/src/test/java/org/elasticsearch/bootstrap/MockPluginPolicy.java new file mode 100644 index 00000000000..9bb32885da7 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/bootstrap/MockPluginPolicy.java @@ -0,0 +1,123 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.bootstrap; + +import com.carrotsearch.randomizedtesting.RandomizedRunner; + +import org.apache.lucene.util.LuceneTestCase; +import org.elasticsearch.common.io.PathUtils; +import org.elasticsearch.common.logging.Loggers; +import org.junit.Assert; + +import java.net.URL; +import java.nio.file.Path; +import java.security.CodeSource; +import java.security.Permission; +import java.security.PermissionCollection; +import java.security.Permissions; +import java.security.Policy; +import java.security.ProtectionDomain; +import java.security.cert.Certificate; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +/** + * Simulates in unit tests per-plugin permissions. + * Unit tests for plugins do not have a proper plugin structure, + * so we don't know which codebases to apply the permission to. + *

+ * As an approximation, we just exclude es/test/framework classes, + * because they will be present in stacks and fail tests for the + * simple case where an AccessController block is missing. + */ +final class MockPluginPolicy extends Policy { + final ESPolicy standardPolicy; + final PermissionCollection extraPermissions; + final Set extraSources; + + /** + * Create a new MockPluginPolicy with dynamic {@code permissions} and + * adding the extra plugin permissions from {@code insecurePluginProp} to + * all code except test classes. + */ + MockPluginPolicy(Permissions permissions, String insecurePluginProp) throws Exception { + // the hack begins! + + // parse whole policy file, with and without the substitution, compute the delta + standardPolicy = new ESPolicy(permissions); + + URL bogus = new URL("file:/bogus"); // its "any old codebase" this time: generic permissions + PermissionCollection smallPermissions = standardPolicy.template.getPermissions(new CodeSource(bogus, (Certificate[])null)); + Set small = new HashSet<>(Collections.list(smallPermissions.elements())); + + // set the URL for the property substitution, this time it will also have special permissions + System.setProperty(insecurePluginProp, bogus.toString()); + ESPolicy biggerPolicy = new ESPolicy(permissions); + System.clearProperty(insecurePluginProp); + PermissionCollection bigPermissions = biggerPolicy.template.getPermissions(new CodeSource(bogus, (Certificate[])null)); + Set big = new HashSet<>(Collections.list(bigPermissions.elements())); + + // compute delta to remove all the generic permissions + // we want equals() vs implies() for this check, in case we need + // to pass along any UnresolvedPermission to the plugin + big.removeAll(small); + + // build collection of the special permissions for easy checking + extraPermissions = new Permissions(); + for (Permission p : big) { + extraPermissions.add(p); + } + + // every element in classpath except test-classes/ + extraSources = new HashSet(); + for (URL location : JarHell.parseClassPath()) { + Path path = PathUtils.get(location.toURI()); + String baseName = path.getFileName().toString(); + if (baseName.contains("test-classes") == false) { + extraSources.add(new CodeSource(location, (Certificate[])null)); + } + } + // exclude some obvious places + // es core + extraSources.remove(Bootstrap.class.getProtectionDomain().getCodeSource()); + // es test framework + extraSources.remove(getClass().getProtectionDomain().getCodeSource()); + // lucene test framework + extraSources.remove(LuceneTestCase.class.getProtectionDomain().getCodeSource()); + // test runner + extraSources.remove(RandomizedRunner.class.getProtectionDomain().getCodeSource()); + // junit library + extraSources.remove(Assert.class.getProtectionDomain().getCodeSource()); + + Loggers.getLogger(getClass()).debug("Apply permissions [{}] to codebases [{}]", extraPermissions, extraSources); + } + + @Override + public boolean implies(ProtectionDomain domain, Permission permission) { + if (standardPolicy.implies(domain, permission)) { + return true; + } else if (extraSources.contains(domain.getCodeSource())) { + return extraPermissions.implies(permission); + } else { + return false; + } + } +} From d7a07d7a2757d14519e32b4fd6159f471e8beeff Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Thu, 17 Sep 2015 11:03:50 -0400 Subject: [PATCH 2/3] Fix missing AccessControllerBlock in GCE code This fix imported from #13612 --- .../cloud/gce/GceComputeServiceImpl.java | 14 +++++++++++--- .../discovery/gce/GceUnicastHostsProvider.java | 3 +-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/plugins/cloud-gce/src/main/java/org/elasticsearch/cloud/gce/GceComputeServiceImpl.java b/plugins/cloud-gce/src/main/java/org/elasticsearch/cloud/gce/GceComputeServiceImpl.java index f89f6bc19f2..1a8a7dd7895 100644 --- a/plugins/cloud-gce/src/main/java/org/elasticsearch/cloud/gce/GceComputeServiceImpl.java +++ b/plugins/cloud-gce/src/main/java/org/elasticsearch/cloud/gce/GceComputeServiceImpl.java @@ -37,6 +37,7 @@ import org.elasticsearch.common.unit.TimeValue; import java.io.IOException; import java.security.AccessController; import java.security.GeneralSecurityException; +import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.util.*; @@ -59,13 +60,20 @@ public class GceComputeServiceImpl extends AbstractLifecycleComponent instances = zones.stream().map((zoneId) -> { try { - Compute.Instances.List list = client().instances().list(project, zoneId); - InstanceList instanceList = list.execute(); + // hack around code messiness in GCE code + // TODO: get this fixed + InstanceList instanceList = AccessController.doPrivileged(new PrivilegedExceptionAction() { + @Override + public InstanceList run() throws Exception { + Compute.Instances.List list = client().instances().list(project, zoneId); + return list.execute(); + } + }); if (instanceList.isEmpty()) { return Collections.EMPTY_LIST; } return instanceList.getItems(); - } catch (IOException e) { + } catch (PrivilegedActionException e) { logger.warn("Problem fetching instance list for zone {}", zoneId); logger.debug("Full exception:", e); return Collections.EMPTY_LIST; diff --git a/plugins/cloud-gce/src/main/java/org/elasticsearch/discovery/gce/GceUnicastHostsProvider.java b/plugins/cloud-gce/src/main/java/org/elasticsearch/discovery/gce/GceUnicastHostsProvider.java index 5415f583607..79cd11666ad 100644 --- a/plugins/cloud-gce/src/main/java/org/elasticsearch/discovery/gce/GceUnicastHostsProvider.java +++ b/plugins/cloud-gce/src/main/java/org/elasticsearch/discovery/gce/GceUnicastHostsProvider.java @@ -242,8 +242,7 @@ public class GceUnicastHostsProvider extends AbstractComponent implements Unicas } } catch (Throwable e) { - logger.warn("Exception caught during discovery {} : {}", e.getClass().getName(), e.getMessage()); - logger.trace("Exception caught during discovery", e); + logger.warn("Exception caught during discovery: {}", e, e.getMessage()); } logger.debug("{} node(s) added", cachedDiscoNodes.size()); From 0fdc16927efa89a98285fd3b9ba51d4f1a7cb1f2 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Thu, 17 Sep 2015 11:51:00 -0400 Subject: [PATCH 3/3] Elaborate more on how this works --- .../java/org/elasticsearch/bootstrap/MockPluginPolicy.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/test/java/org/elasticsearch/bootstrap/MockPluginPolicy.java b/core/src/test/java/org/elasticsearch/bootstrap/MockPluginPolicy.java index 9bb32885da7..104aad59291 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/MockPluginPolicy.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/MockPluginPolicy.java @@ -46,7 +46,9 @@ import java.util.Set; *

* As an approximation, we just exclude es/test/framework classes, * because they will be present in stacks and fail tests for the - * simple case where an AccessController block is missing. + * simple case where an AccessController block is missing, because + * java security checks every codebase in the stacktrace, and we + * are sure to pollute it. */ final class MockPluginPolicy extends Policy { final ESPolicy standardPolicy;