From 894a3ca48e88529b443ad8847841ead0564bd096 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 25 Feb 2025 00:05:36 -0800 Subject: [PATCH] Consider entitlement lib as system module (#123315) * Consider entitlement lib as system module Entitlements sometimes needs to perform sensitive operations, particularly within the FileAccessTree. This commit expands the trivially allowed check to include entitlements as one of the system modules alongside the jdk. One consequence is that the self test must be moved outside entitlements. * [CI] Auto commit changes from spotless * remove old method call --------- Co-authored-by: elasticsearchmachine Co-authored-by: Elastic Machine --- .../bootstrap/EntitlementBootstrap.java | 49 ------------------- .../runtime/policy/PolicyManager.java | 16 +++--- .../bootstrap/Elasticsearch.java | 32 ++++++++++++ 3 files changed, 38 insertions(+), 59 deletions(-) diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/EntitlementBootstrap.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/EntitlementBootstrap.java index 4f37362d9325a..8610d9f3be66f 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/EntitlementBootstrap.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/EntitlementBootstrap.java @@ -14,16 +14,13 @@ import com.sun.tools.attach.AttachNotSupportedException; import com.sun.tools.attach.VirtualMachine; -import org.elasticsearch.core.CheckedConsumer; import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.entitlement.initialization.EntitlementInitialization; -import org.elasticsearch.entitlement.runtime.api.NotEntitledException; import org.elasticsearch.entitlement.runtime.policy.Policy; import org.elasticsearch.logging.LogManager; import org.elasticsearch.logging.Logger; import java.io.IOException; -import java.lang.reflect.InvocationTargetException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Map; @@ -114,7 +111,6 @@ public static void bootstrap( ); exportInitializationToAgent(); loadAgent(findAgentJar()); - selfTest(); } @SuppressForbidden(reason = "The VirtualMachine API is the only way to attach a java agent dynamically") @@ -160,50 +156,5 @@ private static String findAgentJar() { } } - /** - * Attempt a few sensitive operations to ensure that some are permitted and some are forbidden. - *

- * - * This serves two purposes: - * - *

    - *
  1. - * a smoke test to make sure the entitlements system is not completely broken, and - *
    2. - *
  2. - * an early test of certain important operations so they don't fail later on at an awkward time. - *
    3. - *
- * - * @throws IllegalStateException if the entitlements system can't prevent an unauthorized action of our choosing - */ - private static void selfTest() { - ensureCannotStartProcess(ProcessBuilder::start); - // Try again with reflection - ensureCannotStartProcess(EntitlementBootstrap::reflectiveStartProcess); - } - - private static void ensureCannotStartProcess(CheckedConsumer startProcess) { - try { - // The command doesn't matter; it doesn't even need to exist - startProcess.accept(new ProcessBuilder("")); - } catch (NotEntitledException e) { - logger.debug("Success: Entitlement protection correctly prevented process creation"); - return; - } catch (Exception e) { - throw new IllegalStateException("Failed entitlement protection self-test", e); - } - throw new IllegalStateException("Entitlement protection self-test was incorrectly permitted"); - } - - private static void reflectiveStartProcess(ProcessBuilder pb) throws Exception { - try { - var start = ProcessBuilder.class.getMethod("start"); - start.invoke(pb); - } catch (InvocationTargetException e) { - throw (Exception) e.getCause(); - } - } - private static final Logger logger = LogManager.getLogger(EntitlementBootstrap.class); } diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java index 2aafcfc594abd..b6296fe5d4713 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java @@ -12,7 +12,6 @@ import org.elasticsearch.core.Strings; import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.entitlement.bootstrap.EntitlementBootstrap; -import org.elasticsearch.entitlement.bridge.EntitlementChecker; import org.elasticsearch.entitlement.instrumentation.InstrumentationService; import org.elasticsearch.entitlement.runtime.api.NotEntitledException; import org.elasticsearch.entitlement.runtime.policy.entitlements.CreateClassLoaderEntitlement; @@ -126,11 +125,12 @@ private static Set findSystemModules() { .stream() .map(ModuleReference::descriptor) .collect(Collectors.toUnmodifiableSet()); - return ModuleLayer.boot() - .modules() - .stream() - .filter(m -> systemModulesDescriptors.contains(m.getDescriptor())) - .collect(Collectors.toUnmodifiableSet()); + return Stream.concat( + // entitlements is a "system" module, we can do anything from it + Stream.of(PolicyManager.class.getModule()), + // anything in the boot layer is also part of the system + ModuleLayer.boot().modules().stream().filter(m -> systemModulesDescriptors.contains(m.getDescriptor())) + ).collect(Collectors.toUnmodifiableSet()); } /** @@ -564,10 +564,6 @@ private static boolean isTriviallyAllowed(Class requestingClass) { logger.debug("Entitlement trivially allowed from system module [{}]", requestingClass.getModule().getName()); return true; } - if (EntitlementChecker.class.isAssignableFrom(requestingClass)) { - logger.debug("Entitlement trivially allowed for EntitlementChecker class"); - return true; - } logger.trace("Entitlement not trivially allowed"); return false; } diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index 85a1e87983238..2fdf24597ad6f 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -29,9 +29,11 @@ import org.elasticsearch.common.util.concurrent.RunOnce; import org.elasticsearch.core.AbstractRefCounted; import org.elasticsearch.core.Booleans; +import org.elasticsearch.core.CheckedConsumer; import org.elasticsearch.core.IOUtils; import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.entitlement.bootstrap.EntitlementBootstrap; +import org.elasticsearch.entitlement.runtime.api.NotEntitledException; import org.elasticsearch.entitlement.runtime.policy.Policy; import org.elasticsearch.entitlement.runtime.policy.PolicyParserUtils; import org.elasticsearch.entitlement.runtime.policy.entitlements.LoadNativeLibrariesEntitlement; @@ -52,6 +54,7 @@ import java.io.InputStream; import java.io.PrintStream; import java.lang.invoke.MethodHandles; +import java.lang.reflect.InvocationTargetException; import java.nio.file.Files; import java.nio.file.Path; import java.security.Permission; @@ -248,6 +251,7 @@ private static void initPhase2(Bootstrap bootstrap) throws IOException { nodeEnv.logsDir(), nodeEnv.tmpDir() ); + entitlementSelfTest(); } else { assert RuntimeVersionFeature.isSecurityManagerAvailable(); // no need to explicitly enable native access for legacy code @@ -264,6 +268,34 @@ private static void initPhase2(Bootstrap bootstrap) throws IOException { bootstrap.setPluginsLoader(pluginsLoader); } + // check entitlements were loaded correctly. note this must be outside the entitlements lib. + private static void entitlementSelfTest() { + ensureCannotStartProcess(ProcessBuilder::start); + // Try again with reflection + ensureCannotStartProcess(Elasticsearch::reflectiveStartProcess); + } + + private static void ensureCannotStartProcess(CheckedConsumer startProcess) { + try { + // The command doesn't matter; it doesn't even need to exist + startProcess.accept(new ProcessBuilder("")); + } catch (NotEntitledException e) { + return; + } catch (Exception e) { + throw new IllegalStateException("Failed entitlement protection self-test", e); + } + throw new IllegalStateException("Entitlement protection self-test was incorrectly permitted"); + } + + private static void reflectiveStartProcess(ProcessBuilder pb) throws Exception { + try { + var start = ProcessBuilder.class.getMethod("start"); + start.invoke(pb); + } catch (InvocationTargetException e) { + throw (Exception) e.getCause(); + } + } + private static void ensureInitialized(Class... classes) { for (final var clazz : classes) { try {