Remove security manager from tests (#127087)

Now that entitlements are always used, there is no need to run tests
with security manager (a future enhancement will run tests with
entitlements). This commit removes setting up security manager from
tests.
This commit is contained in:
Ryan Ernst 2025-04-22 09:08:09 -07:00 committed by GitHub
parent 093ebd356b
commit b5e92db171
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
25 changed files with 4 additions and 319 deletions

View file

@ -38,7 +38,6 @@ import static org.hamcrest.Matchers.not;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
@ESTestCase.WithoutSecurityManager
public class APMJvmOptionsTests extends ESTestCase {
private Path installDir;

View file

@ -15,7 +15,6 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.node.NodeRoleSettings;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.ESTestCase.WithoutSecurityManager;
import java.io.IOException;
import java.util.ArrayList;
@ -41,7 +40,6 @@ import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@WithoutSecurityManager
@SuppressFileSystems("*")
public class JvmErgonomicsTests extends ESTestCase {

View file

@ -15,7 +15,6 @@ import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.core.IOUtils;
import org.elasticsearch.core.Strings;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.ESTestCase.WithoutSecurityManager;
import org.junit.AfterClass;
import org.junit.BeforeClass;
@ -43,7 +42,6 @@ import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.not;
@WithoutSecurityManager
@LuceneTestCase.SuppressFileSystems("*")
public class JvmOptionsParserTests extends ESTestCase {

View file

@ -11,7 +11,6 @@ package org.elasticsearch.server.cli;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.ESTestCase.WithoutSecurityManager;
import org.hamcrest.Matcher;
import java.util.Collections;
@ -21,7 +20,6 @@ import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
// TODO: rework these tests to mock jvm option finder so they can run with security manager, no forking needed
@WithoutSecurityManager
public class MachineDependentHeapTests extends ESTestCase {
public void testDefaultHeapSize() throws Exception {

View file

@ -10,7 +10,6 @@
package org.elasticsearch.cli;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.ESTestCase.WithoutSecurityManager;
import java.io.IOException;
import java.io.OutputStream;
@ -23,7 +22,6 @@ import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
@WithoutSecurityManager
public class TerminalTests extends ESTestCase {
public void testSystemTerminalIfRedirected() {

View file

@ -23,7 +23,6 @@ import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasEntry;
@ESTestCase.WithoutSecurityManager
public class InstrumentationServiceImplTests extends ESTestCase {
final InstrumentationService instrumentationService = new InstrumentationServiceImpl();

View file

@ -39,7 +39,6 @@ import static org.hamcrest.Matchers.equalTo;
* This tests {@link InstrumenterImpl} can instrument various method signatures
* (e.g. overloaded methods, overloaded targets, multiple instrumentation, etc.)
*/
@ESTestCase.WithoutSecurityManager
public class InstrumenterTests extends ESTestCase {
private static final Logger logger = LogManager.getLogger(InstrumenterTests.class);

View file

@ -13,7 +13,6 @@ import org.elasticsearch.test.ESTestCase;
import static org.elasticsearch.entitlement.bridge.UtilTests.MockSensitiveClass.mockSensitiveMethod;
@ESTestCase.WithoutSecurityManager
public class UtilTests extends ESTestCase {
public void testCallerClass() {

View file

@ -27,7 +27,6 @@ import static org.hamcrest.Matchers.both;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.startsWith;
@ESTestCase.WithoutSecurityManager
public class EntitlementInitializationTests extends ESTestCase {
private static PathLookup TEST_PATH_LOOKUP;

View file

@ -39,7 +39,6 @@ import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEnt
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
@ESTestCase.WithoutSecurityManager
public class FileAccessTreeTests extends ESTestCase {
static Path root;

View file

@ -46,7 +46,6 @@ import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.sameInstance;
@ESTestCase.WithoutSecurityManager
public class PolicyManagerTests extends ESTestCase {
/**

View file

@ -33,7 +33,6 @@ import java.util.Set;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
@ESTestCase.WithoutSecurityManager
public class PolicyParserTests extends ESTestCase {
public static String TEST_ABSOLUTE_PATH_TO_FILE;

View file

@ -34,7 +34,6 @@ import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
@ESTestCase.WithoutSecurityManager
public class PolicyUtilsTests extends ESTestCase {
public void testCreatePluginPolicyWithPatch() {

View file

@ -20,7 +20,6 @@ import java.util.List;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
@ESTestCase.WithoutSecurityManager
public class ProcessLimitsTests extends ESTestCase {
private static final NativeAccess nativeAccess = NativeAccess.instance();

View file

@ -16,7 +16,6 @@ import static org.apache.lucene.tests.util.LuceneTestCase.assumeTrue;
import static org.junit.Assert.fail;
/** Simple tests system call filter is working. */
@ESTestCase.WithoutSecurityManager
public class SystemCallFilterTests extends ESTestCase {
/** command to try to run in tests */

View file

@ -12,7 +12,6 @@ package org.elasticsearch.nativeaccess;
import org.apache.lucene.util.Constants;
import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.ESTestCase.WithoutSecurityManager;
import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
import org.elasticsearch.test.jar.JarUtils;
import org.junit.BeforeClass;
@ -28,7 +27,6 @@ import static org.elasticsearch.nativeaccess.PosixNativeAccess.ENABLE_JDK_VECTOR
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
@WithoutSecurityManager
public class VectorSystemPropertyTests extends ESTestCase {
static Path jarPath;

View file

@ -14,7 +14,6 @@ import org.elasticsearch.common.ReferenceDocs;
import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.node.NodeValidationException;
import org.elasticsearch.test.AbstractBootstrapCheckTestCase;
import org.elasticsearch.test.ESTestCase.WithoutSecurityManager;
import org.hamcrest.Matcher;
import org.junit.After;
import org.junit.Before;
@ -31,7 +30,6 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
@WithoutSecurityManager
public class EvilBootstrapChecksTests extends AbstractBootstrapCheckTestCase {
private String esEnforceBootstrapChecks = System.getProperty(ES_ENFORCE_BOOTSTRAP_CHECKS);

View file

@ -34,7 +34,6 @@ import static org.elasticsearch.entitlement.runtime.policy.PolicyManager.ALL_UNN
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@ESTestCase.WithoutSecurityManager
public class ScopeResolverTests extends ESTestCase {
/**
* A test agent package name for use in tests.

View file

@ -39,7 +39,6 @@ import static org.hamcrest.Matchers.is;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@ESTestCase.WithoutSecurityManager
public class MasterNodeFileWatchingServiceTests extends ESTestCase {
static final DiscoveryNode localNode = DiscoveryNodeUtils.create("local-node");

View file

@ -39,7 +39,6 @@ import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
@ESTestCase.WithoutSecurityManager
@LuceneTestCase.SuppressFileSystems(value = "ExtrasFS")
public class PluginsLoaderTests extends ESTestCase {

View file

@ -40,7 +40,6 @@ import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
@ESTestCase.WithoutSecurityManager
public class UberModuleClassLoaderTests extends ESTestCase {
private static final String MODULE_NAME = "synthetic";

View file

@ -22,7 +22,6 @@ import java.util.List;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.instanceOf;
@ESTestCase.WithoutSecurityManager
public class ReloadAwarePluginTests extends ESTestCase {
public void testNodeInitializesReloadingPlugin() throws Exception {

View file

@ -9,56 +9,23 @@
package org.elasticsearch.bootstrap;
import com.carrotsearch.randomizedtesting.RandomizedRunner;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.tests.util.LuceneTestCase;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.network.IfConfig;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.core.Booleans;
import org.elasticsearch.core.PathUtils;
import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.jdk.JarHell;
import org.elasticsearch.jdk.RuntimeVersionFeature;
import org.elasticsearch.plugins.PluginDescriptor;
import org.elasticsearch.secure_sm.SecureSM;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.PrivilegedOperations;
import org.elasticsearch.test.mockito.SecureMockMaker;
import org.junit.Assert;
import java.io.Closeable;
import java.io.InputStream;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.net.SocketPermission;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.AccessController;
import java.security.Permission;
import java.security.Permissions;
import java.security.Policy;
import java.security.PrivilegedAction;
import java.security.ProtectionDomain;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Properties;
import java.util.Set;
import java.util.stream.Collectors;
import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean;
import static org.elasticsearch.bootstrap.ESPolicy.POLICY_RESOURCE;
import static org.elasticsearch.bootstrap.FilePermissionUtils.addDirectoryPath;
/**
* Initializes natives and installs test security manager
@ -79,9 +46,10 @@ public class BootstrapForTesting {
Path javaTmpDir = PathUtils.get(
Objects.requireNonNull(System.getProperty("java.io.tmpdir"), "please set ${java.io.tmpdir} in pom.xml")
);
try {
Security.ensureDirectoryExists(javaTmpDir);
} catch (Exception e) {
Files.createDirectories(javaTmpDir);
} catch (IOException e) {
throw new RuntimeException("unable to create test temp directory", e);
}
@ -118,96 +86,6 @@ public class BootstrapForTesting {
// Log ifconfig output before SecurityManager is installed
IfConfig.logIfNecessary();
// install security manager if available and requested
if (RuntimeVersionFeature.isSecurityManagerAvailable() && systemPropertyAsBoolean("tests.security.manager", true)) {
try {
// initialize paths the same exact way as bootstrap
Permissions perms = new Permissions();
Security.addClasspathPermissions(perms);
// java.io.tmpdir
FilePermissionUtils.addDirectoryPath(perms, "java.io.tmpdir", javaTmpDir, "read,readlink,write,delete", false);
// custom test config file
if (Strings.hasLength(System.getProperty("tests.config"))) {
FilePermissionUtils.addSingleFilePath(perms, PathUtils.get(System.getProperty("tests.config")), "read,readlink");
}
// jacoco coverage output file
final boolean testsCoverage = Booleans.parseBoolean(System.getProperty("tests.coverage", "false"));
if (testsCoverage) {
Path coverageDir = PathUtils.get(System.getProperty("tests.coverage.dir"));
FilePermissionUtils.addSingleFilePath(perms, coverageDir.resolve("jacoco.exec"), "read,write");
// in case we get fancy and use the -integration goals later:
FilePermissionUtils.addSingleFilePath(perms, coverageDir.resolve("jacoco-it.exec"), "read,write");
}
// intellij hack: intellij test runner wants setIO and will
// screw up all test logging without it!
if (System.getProperty("tests.gradle") == null) {
perms.add(new RuntimePermission("setIO"));
}
// add bind permissions for testing
// ephemeral ports (note, on java 7 before update 51, this is a different permission)
// this should really be the only one allowed for tests, otherwise they have race conditions
perms.add(new SocketPermission("localhost:0", "listen,resolve"));
// ... but tests are messy. like file permissions, just let them live in a fantasy for now.
// TODO: cut over all tests to bind to ephemeral ports
perms.add(new SocketPermission("localhost:1024-", "listen,resolve"));
// read test-framework permissions
Map<String, URL> codebases = getCodebases();
final Policy testFramework = PolicyUtil.readPolicy(Elasticsearch.class.getResource("test-framework.policy"), codebases);
final Policy runnerPolicy;
if (System.getProperty("tests.gradle") != null) {
runnerPolicy = PolicyUtil.readPolicy(Elasticsearch.class.getResource("gradle.policy"), codebases);
} else if (codebases.containsKey("junit-rt.jar")) {
runnerPolicy = PolicyUtil.readPolicy(Elasticsearch.class.getResource("intellij.policy"), codebases);
} else {
runnerPolicy = PolicyUtil.readPolicy(Elasticsearch.class.getResource("eclipse.policy"), codebases);
}
// this mimicks the recursive data path permission added in Security.java
Permissions fastPathPermissions = new Permissions();
addDirectoryPath(fastPathPermissions, "java.io.tmpdir-fastpath", javaTmpDir, "read,readlink,write,delete", true);
final Policy esPolicy = new ESPolicy(
PolicyUtil.readPolicy(ESPolicy.class.getResource(POLICY_RESOURCE), codebases),
perms,
getPluginPermissions(),
true,
Security.toFilePermissions(fastPathPermissions),
Map.of()
);
Policy.setPolicy(new Policy() {
@Override
public boolean implies(ProtectionDomain domain, Permission permission) {
// implements union
return esPolicy.implies(domain, permission)
|| testFramework.implies(domain, permission)
|| runnerPolicy.implies(domain, permission);
}
});
Security.prepopulateSecurityCaller();
Security.setSecurityManager(SecureSM.createTestSecureSM());
Security.selfTest();
// guarantee plugin classes are initialized first, in case they have one-time hacks.
// this just makes unit testing more realistic
for (URL url : Collections.list(
BootstrapForTesting.class.getClassLoader().getResources(PluginDescriptor.INTERNAL_DESCRIPTOR_FILENAME)
)) {
Properties properties = new Properties();
try (InputStream stream = FileSystemUtils.openFileURLStream(url)) {
properties.load(stream);
}
String clazz = properties.getProperty("classname");
if (clazz != null) {
Class.forName(clazz);
}
}
} catch (Exception e) {
throw new RuntimeException("unable to install test security manager", e);
}
}
}
static Map<String, URL> getCodebases() {
@ -244,133 +122,6 @@ public class BootstrapForTesting {
}
}
/**
* we don't know which codesources belong to which plugin, so just remove the permission from key codebases
* like core, test-framework, etc. this way tests fail if accesscontroller blocks are missing.
*/
@SuppressForbidden(reason = "accesses fully qualified URLs to configure security")
static Map<URL, Policy> getPluginPermissions() throws Exception {
List<URL> pluginPolicies = Collections.list(
BootstrapForTesting.class.getClassLoader().getResources(PluginDescriptor.ES_PLUGIN_POLICY)
);
if (pluginPolicies.isEmpty()) {
return Collections.emptyMap();
}
// compute classpath minus obvious places, all other jars will get the permission.
Set<URL> codebases = new HashSet<>(parseClassPathWithSymlinks());
Set<URL> excluded = new HashSet<>(
Arrays.asList(
// es core
Elasticsearch.class.getProtectionDomain().getCodeSource().getLocation(),
// es test framework
BootstrapForTesting.class.getProtectionDomain().getCodeSource().getLocation(),
// lucene test framework
LuceneTestCase.class.getProtectionDomain().getCodeSource().getLocation(),
// randomized runner
RandomizedRunner.class.getProtectionDomain().getCodeSource().getLocation(),
// junit library
Assert.class.getProtectionDomain().getCodeSource().getLocation()
)
);
codebases.removeAll(excluded);
final Map<String, URL> codebasesMap = PolicyUtil.getCodebaseJarMap(codebases);
// parse each policy file, with codebase substitution from the classpath
final List<Policy> policies = new ArrayList<>(pluginPolicies.size());
for (URL policyFile : pluginPolicies) {
Map<String, URL> policyCodebases = codebasesMap;
// if the codebases file is inside a jar, then we don't need to load it since the jar will
// have already been read from the classpath
if (policyFile.toString().contains(".jar!") == false) {
Path policyPath = PathUtils.get(policyFile.toURI());
Path codebasesPath = policyPath.getParent().resolve("plugin-security.codebases");
if (Files.exists(codebasesPath)) {
// load codebase to class map used for tests
policyCodebases = new HashMap<>(codebasesMap);
Map<String, String> codebasesProps = parsePropertiesFile(codebasesPath);
for (var entry : codebasesProps.entrySet()) {
addClassCodebase(policyCodebases, entry.getKey(), entry.getValue());
}
}
}
policies.add(PolicyUtil.readPolicy(policyFile, policyCodebases));
}
// consult each policy file for those codebases
Map<URL, Policy> map = new HashMap<>();
for (URL url : codebases) {
map.put(url, new Policy() {
@Override
public boolean implies(ProtectionDomain domain, Permission permission) {
// implements union
for (Policy p : policies) {
if (p.implies(domain, permission)) {
return true;
}
}
return false;
}
});
}
return Collections.unmodifiableMap(map);
}
static Map<String, String> parsePropertiesFile(Path propertiesFile) throws Exception {
Properties props = new Properties();
try (InputStream is = Files.newInputStream(propertiesFile)) {
props.load(is);
}
return props.entrySet().stream().collect(Collectors.toMap(e -> e.getKey().toString(), e -> e.getValue().toString()));
}
/**
* return parsed classpath, but with symlinks resolved to destination files for matching
* this is for matching the toRealPath() in the code where we have a proper plugin structure
*/
@SuppressForbidden(reason = "does evil stuff with paths and urls because devs and jenkins do evil stuff with paths and urls")
static Set<URL> parseClassPathWithSymlinks() throws Exception {
Set<URL> raw = JarHell.parseClassPath();
Set<URL> cooked = Sets.newHashSetWithExpectedSize(raw.size());
for (URL url : raw) {
Path path = PathUtils.get(url.toURI());
if (Files.exists(path)) {
boolean added = cooked.add(path.toRealPath().toUri().toURL());
if (added == false) {
throw new IllegalStateException("Duplicate in classpath after resolving symlinks: " + url);
}
}
}
return raw;
}
// does nothing, just easy way to make sure the class is loaded.
public static void ensureInitialized() {}
/**
* Temporarily dsiables security manager for a test.
*
* <p> This method is only callable by {@link org.elasticsearch.test.ESTestCase}.
*
* @return A closeable object which restores the test security manager
*/
@SuppressWarnings("removal")
public static Closeable disableTestSecurityManager() {
var caller = Thread.currentThread().getStackTrace()[2];
if (ESTestCase.class.getName().equals(caller.getClassName()) == false) {
throw new SecurityException("Cannot disable test SecurityManager directly. Use @NoSecurityManager to disable on a test suite");
}
final var sm = System.getSecurityManager();
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
Security.setSecurityManager(null);
return null;
});
return () -> AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
Security.setSecurityManager(sm);
return null;
});
}
}

View file

@ -25,7 +25,6 @@ import static org.hamcrest.Matchers.emptyString;
/**
* A base test case for cli tools.
*/
@ESTestCase.WithoutSecurityManager
public abstract class CommandTestCase extends ESTestCase {
/** The terminal that execute uses */

View file

@ -119,7 +119,6 @@ import org.elasticsearch.index.analysis.TokenFilterFactory;
import org.elasticsearch.index.analysis.TokenizerFactory;
import org.elasticsearch.indices.IndicesModule;
import org.elasticsearch.indices.analysis.AnalysisModule;
import org.elasticsearch.jdk.RuntimeVersionFeature;
import org.elasticsearch.logging.internal.spi.LoggerFactory;
import org.elasticsearch.plugins.AnalysisPlugin;
import org.elasticsearch.plugins.Plugin;
@ -157,14 +156,8 @@ import org.junit.Rule;
import org.junit.internal.AssumptionViolatedException;
import org.junit.rules.RuleChain;
import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
import java.lang.annotation.ElementType;
import java.lang.annotation.Inherited;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.invoke.MethodHandles;
import java.math.BigInteger;
import java.net.InetAddress;
@ -493,36 +486,6 @@ public abstract class ESTestCase extends LuceneTestCase {
/** called after a test is finished, but only if successful */
protected void afterIfSuccessful() throws Exception {}
/**
* Marks a test suite or a test method that should run without security manager enabled.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target({ ElementType.TYPE })
@Inherited
public @interface WithoutSecurityManager {
}
private static Closeable securityManagerRestorer;
// disable security manager if test is annotated to run without it
@BeforeClass
public static void maybeStashClassSecurityManager() {
if (RuntimeVersionFeature.isSecurityManagerAvailable()) {
if (getTestClass().isAnnotationPresent(WithoutSecurityManager.class)) {
securityManagerRestorer = BootstrapForTesting.disableTestSecurityManager();
}
}
}
@AfterClass
public static void maybeRestoreClassSecurityManager() throws IOException {
if (securityManagerRestorer != null) {
securityManagerRestorer.close();
securityManagerRestorer = null;
}
}
// setup mock filesystems for this test run. we change PathUtils
// so that all accesses are plumbed thru any mock wrappers