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

Author: ldematte

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java

@@ -20,6 +20,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;
@@ -112,7 +113,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
@@ -127,6 +127,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) {

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

distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java

@@ -11,6 +11,8 @@
 
 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;
@@ -137,9 +139,13 @@ private static Stream<String> maybeWorkaroundG1Bug() {
         return Stream.of();
     }
 
+    @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA)
     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) {

libs/core/src/main/java/org/elasticsearch/jdk/RuntimeVersionFeature.java

@@ -0,0 +1,21 @@
+/*
+ * 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;
+
+import org.elasticsearch.core.UpdateForV9;
+
+public class RuntimeVersionFeature {
+    private RuntimeVersionFeature() {}
+
+    @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) // Remove once we removed all references to SecurityManager in code
+    public static boolean isSecurityManagerAvailable() {
+        return Runtime.version().feature() < 24;
+    }
+}

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'
 }

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;

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;
@@ -722,6 +723,9 @@ public final BootstrapCheckResult check(BootstrapContext context) {
         }
 
         boolean isAllPermissionGranted() {
+            if (RuntimeVersionFeature.isSecurityManagerAvailable() == false) {
+                return false;
+            }
             final SecurityManager sm = System.getSecurityManager();
             assert sm != null;
             try {

server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java

@@ -35,6 +35,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;
@@ -113,12 +114,14 @@ 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.
              */
-            org.elasticsearch.bootstrap.Security.setSecurityManager(new SecurityManager() {
-                @Override
-                public void checkPermission(Permission perm) {
-                    // grant all permissions so that we can later set the security manager to the one that we want
-                }
-            });
+            if (RuntimeVersionFeature.isSecurityManagerAvailable()) {
+                org.elasticsearch.bootstrap.Security.setSecurityManager(new SecurityManager() {
+                    @Override
+                    public void checkPermission(Permission perm) {
+                        // grant all permissions so that we can later set the security manager to the one that we want
+                    }
+                });
+            }
             LogConfigurator.registerErrorListener();
 
             BootstrapInfo.init();
@@ -215,14 +218,16 @@ private static void initPhase2(Bootstrap bootstrap) throws IOException {
                 .toList();
 
             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");
         }
     }
 

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) {

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");