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.
This commit is contained in:
Ryan Ernst 2025-06-25 17:31:48 -07:00 committed by GitHub
parent f4e7ce935f
commit 2df9dd42fb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 51 additions and 9 deletions

View file

@ -98,6 +98,10 @@ public class EntitlementBootstrap {
); );
exportInitializationToAgent(); exportInitializationToAgent();
loadAgent(findAgentJar(), EntitlementInitialization.class.getName()); loadAgent(findAgentJar(), EntitlementInitialization.class.getName());
if (EntitlementInitialization.getError() != null) {
throw EntitlementInitialization.getError();
}
} }
private static Path getUserHome() { private static Path getUserHome() {

View file

@ -117,6 +117,10 @@ class DynamicInstrumentation {
// We should have failed already in the loop above, but just in case we did not, rethrow. // We should have failed already in the loop above, but just in case we did not, rethrow.
throw e; 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, private static Map<MethodKey, CheckMethod> getMethodsToInstrument(Class<?> checkerInterface) throws ClassNotFoundException,

View file

@ -16,11 +16,14 @@ import org.elasticsearch.entitlement.runtime.policy.PathLookup;
import org.elasticsearch.entitlement.runtime.policy.PolicyChecker; import org.elasticsearch.entitlement.runtime.policy.PolicyChecker;
import org.elasticsearch.entitlement.runtime.policy.PolicyCheckerImpl; import org.elasticsearch.entitlement.runtime.policy.PolicyCheckerImpl;
import org.elasticsearch.entitlement.runtime.policy.PolicyManager; 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.instrument.Instrumentation;
import java.lang.reflect.Constructor; import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException; import java.lang.reflect.InvocationTargetException;
import java.util.Set; import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import static java.util.Objects.requireNonNull; import static java.util.Objects.requireNonNull;
@ -32,17 +35,26 @@ import static java.util.Objects.requireNonNull;
* to begin injecting our instrumentation. * to begin injecting our instrumentation.
*/ */
public class EntitlementInitialization { public class EntitlementInitialization {
private static final Logger logger = LogManager.getLogger(EntitlementInitialization.class);
private static final Module ENTITLEMENTS_MODULE = PolicyManager.class.getModule(); private static final Module ENTITLEMENTS_MODULE = PolicyManager.class.getModule();
public static InitializeArgs initializeArgs; public static InitializeArgs initializeArgs;
private static ElasticsearchEntitlementChecker checker; private static ElasticsearchEntitlementChecker checker;
private static AtomicReference<RuntimeException> error = new AtomicReference<>();
// Note: referenced by bridge reflectively // Note: referenced by bridge reflectively
public static EntitlementChecker checker() { public static EntitlementChecker checker() {
return checker; return checker;
} }
/**
* Return any exception that occurred during initialization
*/
public static RuntimeException getError() {
return error.get();
}
/** /**
* Initializes the Entitlement system: * Initializes the Entitlement system:
* <ol> * <ol>
@ -62,10 +74,16 @@ public class EntitlementInitialization {
* *
* @param inst the JVM instrumentation class instance * @param inst the JVM instrumentation class instance
*/ */
public static void initialize(Instrumentation inst) throws Exception { public static void initialize(Instrumentation inst) {
// the checker _MUST_ be set before _any_ instrumentation is done try {
checker = initChecker(initializeArgs.policyManager()); // the checker _MUST_ be set before _any_ instrumentation is done
initInstrumentation(inst); 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));
}
} }
/** /**

View file

@ -9,16 +9,22 @@
package org.elasticsearch.entitlement.instrumentation; package org.elasticsearch.entitlement.instrumentation;
import org.elasticsearch.logging.LogManager;
import org.elasticsearch.logging.Logger;
import java.lang.instrument.ClassFileTransformer; import java.lang.instrument.ClassFileTransformer;
import java.security.ProtectionDomain; import java.security.ProtectionDomain;
import java.util.Set; import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
/** /**
* A {@link ClassFileTransformer} that applies an {@link Instrumenter} to the appropriate classes. * A {@link ClassFileTransformer} that applies an {@link Instrumenter} to the appropriate classes.
*/ */
public class Transformer implements ClassFileTransformer { public class Transformer implements ClassFileTransformer {
private static final Logger logger = LogManager.getLogger(Transformer.class);
private final Instrumenter instrumenter; private final Instrumenter instrumenter;
private final Set<String> classesToTransform; private final Set<String> classesToTransform;
private final AtomicBoolean hadErrors = new AtomicBoolean(false);
private boolean verifyClasses; private boolean verifyClasses;
@ -33,6 +39,10 @@ public class Transformer implements ClassFileTransformer {
this.verifyClasses = true; this.verifyClasses = true;
} }
public boolean hadErrors() {
return hadErrors.get();
}
@Override @Override
public byte[] transform( public byte[] transform(
ClassLoader loader, ClassLoader loader,
@ -42,13 +52,19 @@ public class Transformer implements ClassFileTransformer {
byte[] classfileBuffer byte[] classfileBuffer
) { ) {
if (classesToTransform.contains(className)) { if (classesToTransform.contains(className)) {
// System.out.println("Transforming " + className); logger.debug("Transforming " + className);
return instrumenter.instrumentClass(className, classfileBuffer, verifyClasses); 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 { } else {
// System.out.println("Not transforming " + className); logger.trace("Not transforming " + className);
return null; return null;
} }
} }
// private static final Logger LOGGER = LogManager.getLogger(Transformer.class);
} }