From 221caa1c5e62b2cc78c8b9a823882469f517a634 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 22 Nov 2016 10:26:36 -0500 Subject: [PATCH] Refactor handling for bad default permissions This commit refactors the handling of bad default permissions that come from the system security policy. Relates #21735 --- .../org/elasticsearch/bootstrap/ESPolicy.java | 59 +++++++++++++++++-- .../bootstrap/ESPolicyUnitTests.java | 16 ++++- 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java b/core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java index ddc25c88535..795d4d899fb 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java @@ -21,10 +21,10 @@ package org.elasticsearch.bootstrap; import org.elasticsearch.common.SuppressForbidden; -import java.net.SocketPermission; -import java.net.URL; import java.io.FilePermission; import java.io.IOException; +import java.net.SocketPermission; +import java.net.URL; import java.security.CodeSource; import java.security.Permission; import java.security.PermissionCollection; @@ -32,6 +32,7 @@ import java.security.Permissions; import java.security.Policy; import java.security.ProtectionDomain; import java.util.Map; +import java.util.function.Predicate; /** custom policy for union of static and dynamic permissions */ final class ESPolicy extends Policy { @@ -133,18 +134,66 @@ final class ESPolicy extends Policy { // TODO: remove this hack when insecure defaults are removed from java + /** + * Wraps a bad default permission, applying a pre-implies to any permissions before checking if the wrapped bad default permission + * implies a permission. + */ + private static class BadDefaultPermission extends Permission { + + private final Permission badDefaultPermission; + private final Predicate preImplies; + + /** + * Construct an instance with a pre-implies check to apply to desired permissions. + * + * @param badDefaultPermission the bad default permission to wrap + * @param preImplies a test that is applied to a desired permission before checking if the bad default permission that + * this instance wraps implies the desired permission + */ + public BadDefaultPermission(final Permission badDefaultPermission, final Predicate preImplies) { + super(badDefaultPermission.getName()); + this.badDefaultPermission = badDefaultPermission; + this.preImplies = preImplies; + } + + @Override + public final boolean implies(Permission permission) { + return preImplies.test(permission) && badDefaultPermission.implies(permission); + } + + @Override + public final boolean equals(Object obj) { + return badDefaultPermission.equals(obj); + } + + @Override + public int hashCode() { + return badDefaultPermission.hashCode(); + } + + @Override + public String getActions() { + return badDefaultPermission.getActions(); + } + + } + // default policy file states: // "It is strongly recommended that you either remove this permission // from this policy file or further restrict it to code sources // that you specify, because Thread.stop() is potentially unsafe." // not even sure this method still works... - static final Permission BAD_DEFAULT_NUMBER_ONE = new RuntimePermission("stopThread"); + private static final Permission BAD_DEFAULT_NUMBER_ONE = new BadDefaultPermission(new RuntimePermission("stopThread"), p -> true); // default policy file states: // "allows anyone to listen on dynamic ports" // specified exactly because that is what we want, and fastest since it won't imply any // expensive checks for the implicit "resolve" - static final Permission BAD_DEFAULT_NUMBER_TWO = new SocketPermission("localhost:0", "listen"); + private static final Permission BAD_DEFAULT_NUMBER_TWO = + new BadDefaultPermission( + new SocketPermission("localhost:0", "listen"), + // we apply this pre-implies test because some SocketPermission#implies calls do expensive reverse-DNS resolves + p -> p instanceof SocketPermission && p.getActions().contains("listen")); /** * Wraps the Java system policy, filtering out bad default permissions that @@ -159,7 +208,7 @@ final class ESPolicy extends Policy { @Override public boolean implies(ProtectionDomain domain, Permission permission) { - if (BAD_DEFAULT_NUMBER_ONE.equals(permission) || BAD_DEFAULT_NUMBER_TWO.equals(permission)) { + if (BAD_DEFAULT_NUMBER_ONE.implies(permission) || BAD_DEFAULT_NUMBER_TWO.implies(permission)) { return false; } return delegate.implies(domain, permission); diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/ESPolicyUnitTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/ESPolicyUnitTests.java index 255dfb3ec5c..ed71934c72a 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/ESPolicyUnitTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/ESPolicyUnitTests.java @@ -22,6 +22,7 @@ package org.elasticsearch.bootstrap; import org.elasticsearch.test.ESTestCase; import java.io.FilePermission; +import java.net.SocketPermission; import java.security.AllPermission; import java.security.CodeSource; import java.security.Permission; @@ -36,7 +37,7 @@ import java.util.Collections; * we don't allow messing with the policy */ public class ESPolicyUnitTests extends ESTestCase { - /** + /** * Test policy with null codesource. *

* This can happen when restricting privileges with doPrivileged, @@ -55,7 +56,7 @@ public class ESPolicyUnitTests extends ESTestCase { assertFalse(policy.implies(new ProtectionDomain(null, noPermissions), new FilePermission("foo", "read"))); } - /** + /** * test with null location *

* its unclear when/if this happens, see https://bugs.openjdk.java.net/browse/JDK-8129972 @@ -67,4 +68,15 @@ public class ESPolicyUnitTests extends ESTestCase { assertFalse(policy.implies(new ProtectionDomain(new CodeSource(null, (Certificate[]) null), noPermissions), new FilePermission("foo", "read"))); } + + public void testListen() { + assumeTrue("test cannot run with security manager", System.getSecurityManager() == null); + final PermissionCollection noPermissions = new Permissions(); + final ESPolicy policy = new ESPolicy(noPermissions, Collections.emptyMap(), true); + assertFalse( + policy.implies( + new ProtectionDomain(ESPolicyUnitTests.class.getProtectionDomain().getCodeSource(), noPermissions), + new SocketPermission("localhost:" + randomFrom(0, randomIntBetween(49152, 65535)), "listen"))); + } + }