From ba9e0cef4d18c3f261238a3ce03639d11c3e3923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenzo=20Dematt=C3=A9?= Date: Tue, 10 Dec 2024 18:00:58 +0100 Subject: [PATCH] Do not try to enable SecurityManager on JDK 24 (#117999) --- .../internal/ElasticsearchTestBasePlugin.java | 9 ++++++- .../test/GradleTestPolicySetupPlugin.java | 12 +++++++-- .../server/cli/SystemJvmOptions.java | 10 +++++-- .../jdk/RuntimeVersionFeature.java | 21 +++++++++++++++ libs/secure-sm/build.gradle | 1 + .../secure_sm/SecureSMTests.java | 26 +++++++++++++++---- .../bootstrap/BootstrapChecks.java | 4 +++ .../bootstrap/Elasticsearch.java | 19 +++++++++----- .../bootstrap/ESPolicyTests.java | 6 ++++- .../bootstrap/SecurityTests.java | 6 ++++- .../bootstrap/BootstrapForTesting.java | 5 ++-- .../org/elasticsearch/test/ESTestCase.java | 7 +++-- .../example/ExampleSecurityExtension.java | 13 ++++++---- 13 files changed, 111 insertions(+), 28 deletions(-) create mode 100644 libs/core/src/main/java/org/elasticsearch/jdk/RuntimeVersionFeature.java 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 index 4446952fec2bb..720d6a7c2efb6 100644 --- 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 @@ -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 index 2068ee4447971..2107156902487 100644 --- 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 { @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 index b17ad7c87e3ff..fe0f82560894c 100644 --- 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 @@ -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 maybeWorkaroundG1Bug() { return Stream.of(); } + @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) private static Stream 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 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 new file mode 100644 index 0000000000000..fe6e73271599f --- /dev/null +++ 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 index 473a86215e91e..d93afcf84afed 100644 --- 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 index 69c6973f57cdf..965696d13613f 100644 --- 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 index 6a881163914e4..7b2f0c2c894be 100644 --- 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 index 27cbb39c05d38..04f93494a50f1 100644 --- 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,7 +218,7 @@ 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( @@ -223,6 +226,8 @@ private static void initPhase2(Bootstrap bootstrap) throws IOException { 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 index 6aa85fb132e28..1660eeee837b3 100644 --- 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 index 1d46bb7be33d5..98a1f577cfa3a 100644 --- 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 index 102797a963840..0ef16e7e9f555 100644 --- 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 index e869fc0836ba6..6612f0da0c43f 100644 --- 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/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 index eeec67a0580fa..8400e7df54f4d 100644 --- 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; @@ -35,11 +36,13 @@ public class ExampleSecurityExtension implements SecurityExtension { static { - // check that the extension's policy works. - AccessController.doPrivileged((PrivilegedAction) () -> { - System.getSecurityManager().checkPropertyAccess("myproperty"); - return null; - }); + if (RuntimeVersionFeature.isSecurityManagerAvailable()) { + // check that the extension's policy works. + AccessController.doPrivileged((PrivilegedAction) () -> { + System.getSecurityManager().checkPropertyAccess("myproperty"); + return null; + }); + } } @Override