Fail startup if entitlement instrumentation failed (#130051) (#130121)

* Fail startup if entitlement instrumentation failed (#130051)

Java class transformers swallow exceptions, so any instrumentation
failures, for example due to a java version mismatch, will silently
proceed with startup, which then will cryptically fail the entitlement
self test. This commit logs exceptions that occur during
instrumentation, as well as plumb through the fact that any occured so
that bootstrap can fail rather than allow startup to proceed.

* [CI] Auto commit changes from spotless

---------

Co-authored-by: elasticsearchmachine <[email protected]>

Author: rjernst

libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/EntitlementBootstrap.java

@@ -122,7 +122,12 @@ public static void bootstrap(
             suppressFailureLogPackages
         );
         exportInitializationToAgent();
+
         loadAgent(findAgentJar());
+
+        if (EntitlementInitialization.getError() != null) {
+            throw EntitlementInitialization.getError();
+        }
     }
 
     private static Path getUserHome() {

libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/DynamicInstrumentation.java

@@ -119,6 +119,10 @@ static void initialize(Instrumentation inst, Class<?> checkerInterface, boolean
             // We should have failed already in the loop above, but just in case we did not, rethrow.
             throw e;
         }
+
+        if (transformer.hadErrors()) {
+            throw new RuntimeException("Failed to transform JDK classes for entitlements");
+        }
     }
 
     private static Map<MethodKey, CheckMethod> getMethodsToInstrument(Class<?> checkerInterface) throws ClassNotFoundException,

libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java

@@ -16,12 +16,15 @@
 import org.elasticsearch.entitlement.runtime.policy.PathLookup;
 import org.elasticsearch.entitlement.runtime.policy.Policy;
 import org.elasticsearch.entitlement.runtime.policy.PolicyManager;
+import org.elasticsearch.logging.LogManager;
+import org.elasticsearch.logging.Logger;
 
 import java.lang.instrument.Instrumentation;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * Called by the agent during {@code agentmain} to configure the entitlement system,
@@ -31,16 +34,25 @@
  * to begin injecting our instrumentation.
  */
 public class EntitlementInitialization {
+    private static final Logger logger = LogManager.getLogger(EntitlementInitialization.class);
 
     private static final Module ENTITLEMENTS_MODULE = PolicyManager.class.getModule();
 
     private static ElasticsearchEntitlementChecker manager;
+    private static AtomicReference<RuntimeException> error = new AtomicReference<>();
 
     // Note: referenced by bridge reflectively
     public static EntitlementChecker checker() {
         return manager;
     }
 
+    /**
+     * Return any exception that occurred during initialization
+     */
+    public static RuntimeException getError() {
+        return error.get();
+    }
+
     /**
      * Initializes the Entitlement system:
      * <ol>
@@ -60,19 +72,25 @@ public static EntitlementChecker checker() {
      *
      * @param inst the JVM instrumentation class instance
      */
-    public static void initialize(Instrumentation inst) throws Exception {
-        manager = initChecker();
+    public static void initialize(Instrumentation inst) {
+        try {
+            manager = initChecker();
 
-        var verifyBytecode = Booleans.parseBoolean(System.getProperty("es.entitlements.verify_bytecode", "false"));
-        if (verifyBytecode) {
-            ensureClassesSensitiveToVerificationAreInitialized();
-        }
+            var verifyBytecode = Booleans.parseBoolean(System.getProperty("es.entitlements.verify_bytecode", "false"));
+            if (verifyBytecode) {
+                ensureClassesSensitiveToVerificationAreInitialized();
+            }
 
-        DynamicInstrumentation.initialize(
-            inst,
-            getVersionSpecificCheckerClass(EntitlementChecker.class, Runtime.version().feature()),
-            verifyBytecode
-        );
+            DynamicInstrumentation.initialize(
+                inst,
+                getVersionSpecificCheckerClass(EntitlementChecker.class, Runtime.version().feature()),
+                verifyBytecode
+            );
+        } catch (Exception e) {
+            // exceptions thrown within the agent will be swallowed, so capture it here
+            // instead so that it can be retrieved by bootstrap
+            error.set(new RuntimeException("Failed to initialize entitlements", e));
+        }
     }
 
     private static PolicyManager createPolicyManager() {

libs/entitlement/src/main/java/org/elasticsearch/entitlement/instrumentation/Transformer.java

@@ -9,16 +9,22 @@
 
 package org.elasticsearch.entitlement.instrumentation;
 
+import org.elasticsearch.logging.LogManager;
+import org.elasticsearch.logging.Logger;
+
 import java.lang.instrument.ClassFileTransformer;
 import java.security.ProtectionDomain;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
  * A {@link ClassFileTransformer} that applies an {@link Instrumenter} to the appropriate classes.
  */
 public class Transformer implements ClassFileTransformer {
+    private static final Logger logger = LogManager.getLogger(Transformer.class);
     private final Instrumenter instrumenter;
     private final Set<String> classesToTransform;
+    private final AtomicBoolean hadErrors = new AtomicBoolean(false);
 
     private boolean verifyClasses;
 
@@ -33,6 +39,10 @@ public void enableClassVerification() {
         this.verifyClasses = true;
     }
 
+    public boolean hadErrors() {
+        return hadErrors.get();
+    }
+
     @Override
     public byte[] transform(
         ClassLoader loader,
@@ -42,13 +52,19 @@ public byte[] transform(
         byte[] classfileBuffer
     ) {
         if (classesToTransform.contains(className)) {
-            // System.out.println("Transforming " + className);
-            return instrumenter.instrumentClass(className, classfileBuffer, verifyClasses);
+            logger.debug("Transforming " + className);
+            try {
+                return instrumenter.instrumentClass(className, classfileBuffer, verifyClasses);
+            } catch (Throwable t) {
+                hadErrors.set(true);
+                logger.error("Failed to instrument class " + className, t);
+                // throwing an exception from a transformer results in the exception being swallowed,
+                // effectively the same as returning null anyways, so we instead log it here completely
+                return null;
+            }
         } else {
-            // System.out.println("Not transforming " + className);
+            logger.trace("Not transforming " + className);
             return null;
         }
     }
-
-    // private static final Logger LOGGER = LogManager.getLogger(Transformer.class);
 }