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

joegallo · Dec 10, 2024 · ba9e0ce · ba9e0ce
1 parent 8aa49d9
commit ba9e0ce
Show file tree

Hide file tree

Showing 13 changed files with 111 additions and 28 deletions.
diff --git a/...internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java b/...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) {

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/...ibution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java b/...ibution/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) {

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,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;
+    }
+}
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;
@@ -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 {

diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/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");
         }
     }
 

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
@@ -118,6 +118,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;
@@ -505,8 +506,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/...ample-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java b/...ample-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;
@@ -35,11 +36,13 @@
 public class ExampleSecurityExtension implements SecurityExtension {
 
     static {
-        // check that the extension's policy works.
-        AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
-            System.getSecurityManager().checkPropertyAccess("myproperty");
-            return null;
-        });
+        if (RuntimeVersionFeature.isSecurityManagerAvailable()) {
+            // check that the extension's policy works.
+            AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+                System.getSecurityManager().checkPropertyAccess("myproperty");
+                return null;
+            });
+        }
     }
 
     @Override