Consider entitlement lib as system module (elastic#123315) (elastic#123338)

* 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 <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>

Author: 3 people

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.
-     * <p>
-     *
-     * This serves two purposes:
-     *
-     * <ol>
-     *     <li>
-     *         a smoke test to make sure the entitlements system is not completely broken, and
-     *     </li>
-     *     <li>
-     *         an early test of certain important operations so they don't fail later on at an awkward time.
-     *     </li>
-     * </ol>
-     *
-     * @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<ProcessBuilder, ?> 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);
 }

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<Module> 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;
     }

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<ProcessBuilder, ?> 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 {