From 2df9dd42fbe494d9db844cc88832e8671a7f5bfc Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 25 Jun 2025 17:31:48 -0700 Subject: [PATCH] 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. --- .../bootstrap/EntitlementBootstrap.java | 4 +++ .../DynamicInstrumentation.java | 4 +++ .../EntitlementInitialization.java | 26 ++++++++++++++++--- .../instrumentation/Transformer.java | 26 +++++++++++++++---- 4 files changed, 51 insertions(+), 9 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 81f61aefdfb8..be9e8254f464 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 @@ -98,6 +98,10 @@ public class EntitlementBootstrap { ); exportInitializationToAgent(); loadAgent(findAgentJar(), EntitlementInitialization.class.getName()); + + if (EntitlementInitialization.getError() != null) { + throw EntitlementInitialization.getError(); + } } private static Path getUserHome() { diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/DynamicInstrumentation.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/DynamicInstrumentation.java index b7d92d351884..1802925d9625 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/DynamicInstrumentation.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/DynamicInstrumentation.java @@ -117,6 +117,10 @@ class DynamicInstrumentation { // 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 getMethodsToInstrument(Class checkerInterface) throws ClassNotFoundException, diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java index fccb84a0d908..871e4ade9748 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java @@ -16,11 +16,14 @@ import org.elasticsearch.entitlement.runtime.policy.PathLookup; import org.elasticsearch.entitlement.runtime.policy.PolicyChecker; import org.elasticsearch.entitlement.runtime.policy.PolicyCheckerImpl; 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.Set; +import java.util.concurrent.atomic.AtomicReference; import static java.util.Objects.requireNonNull; @@ -32,17 +35,26 @@ import static java.util.Objects.requireNonNull; * 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(); public static InitializeArgs initializeArgs; private static ElasticsearchEntitlementChecker checker; + private static AtomicReference error = new AtomicReference<>(); // Note: referenced by bridge reflectively public static EntitlementChecker checker() { return checker; } + /** + * Return any exception that occurred during initialization + */ + public static RuntimeException getError() { + return error.get(); + } + /** * Initializes the Entitlement system: *
    @@ -62,10 +74,16 @@ public class EntitlementInitialization { * * @param inst the JVM instrumentation class instance */ - public static void initialize(Instrumentation inst) throws Exception { - // the checker _MUST_ be set before _any_ instrumentation is done - checker = initChecker(initializeArgs.policyManager()); - initInstrumentation(inst); + public static void initialize(Instrumentation inst) { + try { + // the checker _MUST_ be set before _any_ instrumentation is done + checker = initChecker(initializeArgs.policyManager()); + initInstrumentation(inst); + } 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)); + } } /** diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/instrumentation/Transformer.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/instrumentation/Transformer.java index 6d4d4edaae16..bd9c5a06910f 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/instrumentation/Transformer.java +++ b/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 classesToTransform; + private final AtomicBoolean hadErrors = new AtomicBoolean(false); private boolean verifyClasses; @@ -33,6 +39,10 @@ public class Transformer implements ClassFileTransformer { this.verifyClasses = true; } + public boolean hadErrors() { + return hadErrors.get(); + } + @Override public byte[] transform( ClassLoader loader, @@ -42,13 +52,19 @@ public class Transformer implements ClassFileTransformer { 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); }