Do not try to enable SecurityManager on JDK 24 (#117999) (#119975)

rjernst · ldematte · elasticsearchmachine · web-flow · commit 940ad90304a8 · 2025-01-11T07:09:15.000+11:00
* Do not try to enable SecurityManager on JDK 24 (#117999) * cleanup * [CI] Auto commit changes from spotless * more * [CI] Auto commit changes from spotless --------- Co-authored-by: Lorenzo Dematté <lorenzo.dematte@elastic.co> Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java
@@ -19,6 +19,7 @@
 import org.elasticsearch.gradle.test.GradleTestPolicySetupPlugin;
 import org.elasticsearch.gradle.test.SystemPropertyCommandLineArgumentProvider;
 import org.gradle.api.Action;
+import org.gradle.api.JavaVersion;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
 import org.gradle.api.Task;
@@ -109,7 +110,6 @@ public void execute(Task t) {
             test.jvmArgs(
                 "-Xmx" + System.getProperty("tests.heap.size", "512m"),
                 "-Xms" + System.getProperty("tests.heap.size", "512m"),
-                "-Djava.security.manager=allow",
                 "-Dtests.testfeatures.enabled=true",
                 "--add-opens=java.base/java.util=ALL-UNNAMED",
                 // TODO: only open these for mockito when it is modularized
@@ -124,6 +124,13 @@ public void execute(Task t) {
             );
 
             test.getJvmArgumentProviders().add(new SimpleCommandLineArgumentProvider("-XX:HeapDumpPath=" + heapdumpDir));
+            test.getJvmArgumentProviders().add(() -> {
+                if (test.getJavaVersion().compareTo(JavaVersion.VERSION_23) <= 0) {
+                    return List.of("-Djava.security.manager=allow");
+                } else {
+                    return List.of();
+                }
+            });
 
             String argline = System.getProperty("tests.jvm.argline");
             if (argline != null) {
diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/test/GradleTestPolicySetupPlugin.java b/build-tools/src/main/java/org/elasticsearch/gradle/test/GradleTestPolicySetupPlugin.java
@@ -9,11 +9,14 @@
 
 package org.elasticsearch.gradle.test;
 
+import org.gradle.api.JavaVersion;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
 import org.gradle.api.invocation.Gradle;
 import org.gradle.api.tasks.testing.Test;
 
+import java.util.List;
+
 public class GradleTestPolicySetupPlugin implements Plugin<Project> {
 
     @Override
@@ -23,8 +26,13 @@ public void apply(Project project) {
             test.systemProperty("tests.gradle", true);
             test.systemProperty("tests.task", test.getPath());
 
-            // Flag is required for later Java versions since our tests use a custom security manager
-            test.jvmArgs("-Djava.security.manager=allow");
+            test.getJvmArgumentProviders().add(() -> {
+                if (test.getJavaVersion().compareTo(JavaVersion.VERSION_23) <= 0) {
+                    return List.of("-Djava.security.manager=allow");
+                } else {
+                    return List.of();
+                }
+            });
 
             SystemPropertyCommandLineArgumentProvider nonInputProperties = new SystemPropertyCommandLineArgumentProvider();
             // don't track these as inputs since they contain absolute paths and break cache relocatability
diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java
@@ -12,6 +12,7 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.EsExecutors;
 import org.elasticsearch.core.UpdateForV9;
+import org.elasticsearch.jdk.RuntimeVersionFeature;
 
 import java.io.IOException;
 import java.nio.file.Files;
@@ -149,8 +150,11 @@ private static Stream<String> maybeWorkaroundG1Bug() {
     }
 
     private static Stream<String> maybeAllowSecurityManager() {
-        // Will become conditional on useEntitlements once entitlements can run without SM
-        return Stream.of("-Djava.security.manager=allow");
+        if (RuntimeVersionFeature.isSecurityManagerAvailable()) {
+            // Will become conditional on useEntitlements once entitlements can run without SM
+            return Stream.of("-Djava.security.manager=allow");
+        }
+        return Stream.of();
     }
 
     private static Stream<String> maybeAttachEntitlementAgent(boolean useEntitlements) {
diff --git a/libs/core/src/main/java/org/elasticsearch/jdk/RuntimeVersionFeature.java b/libs/core/src/main/java/org/elasticsearch/jdk/RuntimeVersionFeature.java
@@ -0,0 +1,18 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.jdk;
+
+public class RuntimeVersionFeature {
+    private RuntimeVersionFeature() {}
+
+    public static boolean isSecurityManagerAvailable() {
+        return Runtime.version().feature() < 24;
+    }
+}
diff --git a/libs/secure-sm/build.gradle b/libs/secure-sm/build.gradle
@@ -28,4 +28,5 @@ tasks.named('forbiddenApisMain').configure {
 tasks.named("jarHell").configure { enabled = false }
 tasks.named("testTestingConventions").configure {
   baseClass 'junit.framework.TestCase'
+  baseClass 'org.junit.Assert'
 }
diff --git a/libs/secure-sm/src/test/java/org/elasticsearch/secure_sm/SecureSMTests.java b/libs/secure-sm/src/test/java/org/elasticsearch/secure_sm/SecureSMTests.java
@@ -9,27 +9,43 @@
 
 package org.elasticsearch.secure_sm;
 
-import junit.framework.TestCase;
+import com.carrotsearch.randomizedtesting.JUnit3MethodProvider;
+import com.carrotsearch.randomizedtesting.RandomizedRunner;
+import com.carrotsearch.randomizedtesting.RandomizedTest;
+import com.carrotsearch.randomizedtesting.annotations.TestMethodProviders;
+
+import org.elasticsearch.jdk.RuntimeVersionFeature;
+import org.junit.BeforeClass;
+import org.junit.runner.RunWith;
 
 import java.security.Permission;
 import java.security.Policy;
 import java.security.ProtectionDomain;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
 
 /** Simple tests for SecureSM */
-public class SecureSMTests extends TestCase {
-    static {
+@TestMethodProviders({ JUnit3MethodProvider.class })
+@RunWith(RandomizedRunner.class)
+public class SecureSMTests extends org.junit.Assert {
+
+    @BeforeClass
+    public static void initialize() {
+        RandomizedTest.assumeFalse(
+            "SecurityManager has been permanently removed in JDK 24",
+            RuntimeVersionFeature.isSecurityManagerAvailable() == false
+        );
         // install a mock security policy:
         // AllPermission to source code
         // ThreadPermission not granted anywhere else
-        final ProtectionDomain sourceCode = SecureSM.class.getProtectionDomain();
+        final var sourceCode = Set.of(SecureSM.class.getProtectionDomain(), RandomizedRunner.class.getProtectionDomain());
         Policy.setPolicy(new Policy() {
             @Override
             public boolean implies(ProtectionDomain domain, Permission permission) {
-                if (domain == sourceCode) {
+                if (sourceCode.contains(domain)) {
                     return true;
                 } else if (permission instanceof ThreadPermission) {
                     return false;
diff --git a/server/src/main/java/org/elasticsearch/bootstrap/BootstrapChecks.java b/server/src/main/java/org/elasticsearch/bootstrap/BootstrapChecks.java
@@ -21,6 +21,7 @@
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.discovery.DiscoveryModule;
 import org.elasticsearch.index.IndexModule;
+import org.elasticsearch.jdk.RuntimeVersionFeature;
 import org.elasticsearch.monitor.jvm.JvmInfo;
 import org.elasticsearch.monitor.process.ProcessProbe;
 import org.elasticsearch.nativeaccess.NativeAccess;
@@ -721,6 +722,9 @@ public final BootstrapCheckResult check(BootstrapContext context) {
         }
 
         boolean isAllPermissionGranted() {
+            if (RuntimeVersionFeature.isSecurityManagerAvailable() == false) {
+                return false;
+            }
             final SecurityManager sm = System.getSecurityManager();
             assert sm != null;
             try {
diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java
@@ -34,6 +34,7 @@
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.jdk.JarHell;
+import org.elasticsearch.jdk.RuntimeVersionFeature;
 import org.elasticsearch.monitor.jvm.HotThreads;
 import org.elasticsearch.monitor.jvm.JvmInfo;
 import org.elasticsearch.monitor.os.OsProbe;
@@ -115,7 +116,7 @@ private static Bootstrap initPhase1() {
              * the presence of a security manager or lack thereof act as if there is a security manager present (e.g., DNS cache policy).
              * This forces such policies to take effect immediately.
              */
-            if (useEntitlements == false) {
+            if (useEntitlements == false && RuntimeVersionFeature.isSecurityManagerAvailable()) {
                 org.elasticsearch.bootstrap.Security.setSecurityManager(new SecurityManager() {
                     @Override
                     public void checkPermission(Permission perm) {
@@ -224,14 +225,16 @@ private static void initPhase2(Bootstrap bootstrap) throws IOException {
             var pluginsResolver = PluginsResolver.create(pluginsLoader);
 
             EntitlementBootstrap.bootstrap(pluginData, pluginsResolver::resolveClassToPluginName);
-        } else {
+        } else if (RuntimeVersionFeature.isSecurityManagerAvailable()) {
             // install SM after natives, shutdown hooks, etc.
             LogManager.getLogger(Elasticsearch.class).info("Bootstrapping java SecurityManager");
             org.elasticsearch.bootstrap.Security.configure(
                 nodeEnv,
                 SECURITY_FILTER_BAD_DEFAULTS_SETTING.get(args.nodeSettings()),
                 args.pidFile()
             );
+        } else {
+            LogManager.getLogger(Elasticsearch.class).warn("Bootstrapping without any protection");
         }
     }
 
diff --git a/server/src/test/java/org/elasticsearch/bootstrap/ESPolicyTests.java b/server/src/test/java/org/elasticsearch/bootstrap/ESPolicyTests.java
@@ -9,6 +9,7 @@
 
 package org.elasticsearch.bootstrap;
 
+import org.elasticsearch.jdk.RuntimeVersionFeature;
 import org.elasticsearch.test.ESTestCase;
 
 import java.security.AccessControlContext;
@@ -27,7 +28,10 @@ public class ESPolicyTests extends ESTestCase {
      * test restricting privileges to no permissions actually works
      */
     public void testRestrictPrivileges() {
-        assumeTrue("test requires security manager", System.getSecurityManager() != null);
+        assumeTrue(
+            "test requires security manager",
+            RuntimeVersionFeature.isSecurityManagerAvailable() && System.getSecurityManager() != null
+        );
         try {
             System.getProperty("user.home");
         } catch (SecurityException e) {
diff --git a/server/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java b/server/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java
@@ -9,6 +9,7 @@
 
 package org.elasticsearch.bootstrap;
 
+import org.elasticsearch.jdk.RuntimeVersionFeature;
 import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
@@ -50,7 +51,10 @@ public void testEnsureRegularFile() throws IOException {
 
     /** can't execute processes */
     public void testProcessExecution() throws Exception {
-        assumeTrue("test requires security manager", System.getSecurityManager() != null);
+        assumeTrue(
+            "test requires security manager",
+            RuntimeVersionFeature.isSecurityManagerAvailable() && System.getSecurityManager() != null
+        );
         try {
             Runtime.getRuntime().exec("ls");
             fail("didn't get expected exception");
diff --git a/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java b/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java
@@ -23,6 +23,7 @@
 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;
@@ -118,8 +119,8 @@ public class BootstrapForTesting {
         // Log ifconfig output before SecurityManager is installed
         IfConfig.logIfNecessary();
 
-        // install security manager if requested
-        if (systemPropertyAsBoolean("tests.security.manager", true)) {
+        // 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();
diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java
@@ -116,6 +116,7 @@
 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.plugins.AnalysisPlugin;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.plugins.scanners.StablePluginsRegistry;
@@ -508,8 +509,10 @@ protected void afterIfSuccessful() throws Exception {}
 
     @BeforeClass
     public static void maybeStashClassSecurityManager() {
-        if (getTestClass().isAnnotationPresent(WithoutSecurityManager.class)) {
-            securityManagerRestorer = BootstrapForTesting.disableTestSecurityManager();
+        if (RuntimeVersionFeature.isSecurityManagerAvailable()) {
+            if (getTestClass().isAnnotationPresent(WithoutSecurityManager.class)) {
+                securityManagerRestorer = BootstrapForTesting.disableTestSecurityManager();
+            }
         }
     }
 
diff --git a/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java b/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java
@@ -11,6 +11,7 @@
 import org.elasticsearch.example.realm.CustomRealm;
 import org.elasticsearch.example.realm.CustomRoleMappingRealm;
 import org.elasticsearch.example.role.CustomInMemoryRolesProvider;
+import org.elasticsearch.jdk.RuntimeVersionFeature;
 import org.elasticsearch.xpack.core.security.SecurityExtension;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationFailureHandler;
 import org.elasticsearch.xpack.core.security.authc.Realm;
@@ -36,7 +37,7 @@ public class ExampleSecurityExtension implements SecurityExtension {
 
     static {
         final boolean useEntitlements = Boolean.parseBoolean(System.getProperty("es.entitlements.enabled"));
-        if (useEntitlements == false) {
+        if (useEntitlements == false && RuntimeVersionFeature.isSecurityManagerAvailable()) {
             // check that the extension's policy works.
             AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
                 System.getSecurityManager().checkPropertyAccess("myproperty");

Original file line number	Diff line number	Diff line change
`@@ -28,4 +28,5 @@ tasks.named('forbiddenApisMain').configure {`
`28`	`28`	`tasks.named("jarHell").configure { enabled = false }`
`29`	`29`	`tasks.named("testTestingConventions").configure {`
`30`	`30`	`baseClass 'junit.framework.TestCase'`
	`31`	`+ baseClass 'org.junit.Assert'`
`31`	`32`	`}`