From 71362d5a868ffc84f4a2c3dbacdce0c6d9b391ae Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Mon, 10 Mar 2025 15:33:58 -0400 Subject: [PATCH 1/8] Refactor: findRequestingFrame --- .../runtime/policy/PolicyManager.java | 11 ++-- .../runtime/policy/PolicyManagerTests.java | 55 +++++++++++++++++-- 2 files changed, 56 insertions(+), 10 deletions(-) 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 7aaa038600bbc..6fe6db1c0d02d 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 @@ -648,19 +648,18 @@ Class requestingClass(Class callerClass) { return callerClass; } Optional> result = StackWalker.getInstance(RETAIN_CLASS_REFERENCE) - .walk(frames -> findRequestingClass(frames.map(StackFrame::getDeclaringClass))); + .walk(frames -> findRequestingFrame(frames).map(StackFrame::getDeclaringClass)); return result.orElse(null); } /** - * Given a stream of classes corresponding to the frames from a {@link StackWalker}, - * returns the module whose entitlements should be checked. + * Given a stream of {@link StackFrame}s, identify the one whose entitlements should be checked. * * @throws NullPointerException if the requesting module is {@code null} */ - Optional> findRequestingClass(Stream> classes) { - return classes.filter(c -> c.getModule() != entitlementsModule) // Ignore the entitlements library - .skip(1) // Skip the sensitive caller method + Optional findRequestingFrame(Stream frames) { + return frames.filter(f -> f.getDeclaringClass().getModule() != entitlementsModule) // ignore entitlements library + .skip(1) // Skip the sensitive caller method .findFirst(); } diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java index 90230c9e7e279..e9b9d9df35f97 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java @@ -23,6 +23,7 @@ import org.junit.BeforeClass; import java.io.IOException; +import java.lang.StackWalker.StackFrame; import java.lang.module.Configuration; import java.lang.module.ModuleFinder; import java.net.URL; @@ -320,18 +321,21 @@ public void testRequestingModuleWithStackWalk() throws IOException, ClassNotFoun assertEquals( "Skip entitlement library and the instrumented method", requestingClass, - policyManager.findRequestingClass(Stream.of(entitlementsClass, instrumentedClass, requestingClass, ignorableClass)).orElse(null) + policyManager.findRequestingFrame( + Stream.of(entitlementsClass, instrumentedClass, requestingClass, ignorableClass).map(MockFrame::new) + ).map(StackFrame::getDeclaringClass).orElse(null) ); assertEquals( "Skip multiple library frames", requestingClass, - policyManager.findRequestingClass(Stream.of(entitlementsClass, entitlementsClass, instrumentedClass, requestingClass)) - .orElse(null) + policyManager.findRequestingFrame( + Stream.of(entitlementsClass, entitlementsClass, instrumentedClass, requestingClass).map(MockFrame::new) + ).map(StackFrame::getDeclaringClass).orElse(null) ); assertThrows( "Non-modular caller frames are not supported", NullPointerException.class, - () -> policyManager.findRequestingClass(Stream.of(entitlementsClass, null)) + () -> policyManager.findRequestingFrame(Stream.of(entitlementsClass, null).map(MockFrame::new)) ); } @@ -657,4 +661,47 @@ private static ModuleLayer createLayerForJar(Path jar, String moduleName) { ); return moduleController.layer(); } + + record MockFrame(Class declaringClass) implements StackFrame { + @Override + public String getClassName() { + return getDeclaringClass().getName(); + } + + @Override + public String getMethodName() { + throw new UnsupportedOperationException(); + } + + @Override + public Class getDeclaringClass() { + return declaringClass; + } + + @Override + public int getByteCodeIndex() { + throw new UnsupportedOperationException(); + } + + @Override + public String getFileName() { + throw new UnsupportedOperationException(); + } + + @Override + public int getLineNumber() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isNativeMethod() { + throw new UnsupportedOperationException(); + } + + @Override + public StackTraceElement toStackTraceElement() { + throw new UnsupportedOperationException(); + } + } + } From d45a622c5018525ebe85fbd2fb5c9173403c2b9a Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Mon, 10 Mar 2025 15:44:44 -0400 Subject: [PATCH 2/8] INFO instead of WARN for NotEntitledException. Some of these are expected, so an INFO seems more appropriate. The stack trace tends to attract attention even when entitlements are not the cause of a problem, so let's avoid the stack trace, but still include stack frame info from the frame of interest. --- .../entitlement/runtime/policy/PolicyManager.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 6fe6db1c0d02d..9c4cd4a1de0be 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 @@ -528,7 +528,11 @@ private void notEntitled(String message, Class callerClass) { var exception = new NotEntitledException(message); // Don't emit a log for muted classes, e.g. classes containing self tests if (mutedClasses.contains(callerClass) == false) { - logger.warn(message, exception); + String frameInfoSuffix = StackWalker.getInstance(RETAIN_CLASS_REFERENCE) + .walk(this::findRequestingFrame) + .map(StackFrame::toString) + .orElse(""); + logger.info(message + frameInfoSuffix); } throw exception; } From ec0118cb392cbf4bcff7bb447b3fb6a338eebfcb Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Tue, 11 Mar 2025 10:17:52 -0400 Subject: [PATCH 3/8] Use child loggers for Not Entitled logs --- .../entitlement/runtime/policy/PolicyManager.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 0c67d770e547f..5317efd9e01a6 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 @@ -538,11 +538,13 @@ private void notEntitled(String message, Class callerClass) { var exception = new NotEntitledException(message); // Don't emit a log for muted classes, e.g. classes containing self tests if (mutedClasses.contains(callerClass) == false) { + var moduleName = callerClass.getModule().getName(); + var notEntitledLogger = LogManager.getLogger(PolicyManager.class + "." + ((moduleName == null) ? "ALL_UNNAMED" : moduleName)); String frameInfoSuffix = StackWalker.getInstance(RETAIN_CLASS_REFERENCE) .walk(this::findRequestingFrame) .map(StackFrame::toString) .orElse(""); - logger.info(message + frameInfoSuffix); + notEntitledLogger.info(message + frameInfoSuffix); } throw exception; } From 853977f8d2c45db57640f48710319276e42c0208 Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Tue, 11 Mar 2025 10:58:57 -0400 Subject: [PATCH 4/8] Use warn, and include compoenent name --- .../runtime/policy/PolicyManager.java | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) 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 5317efd9e01a6..104a639ad1ab1 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 @@ -247,15 +247,17 @@ private void neverEntitled(Class callerClass, Supplier operationDescr return; } + String componentName = getEntitlements(requestingClass).componentName(); notEntitled( Strings.format( "Not entitled: component [%s], module [%s], class [%s], operation [%s]", - getEntitlements(requestingClass).componentName(), + componentName, requestingClass.getModule().getName(), requestingClass, operationDescription.get() ), - callerClass + callerClass, + componentName ); } @@ -372,7 +374,8 @@ public void checkFileRead(Class callerClass, Path path, boolean followLinks) requestingClass, realPath == null ? path : Strings.format("%s -> %s", path, realPath) ), - callerClass + callerClass, + entitlements.componentName() ); } } @@ -401,7 +404,8 @@ public void checkFileWrite(Class callerClass, Path path) { requestingClass, path ), - callerClass + callerClass, + entitlements.componentName() ); } } @@ -489,7 +493,8 @@ private void checkFlagEntitlement( requestingClass, PolicyParser.getEntitlementTypeName(entitlementClass) ), - callerClass + callerClass, + classEntitlements.componentName() ); } logger.debug( @@ -530,21 +535,23 @@ public void checkWriteProperty(Class callerClass, String property) { requestingClass, property ), - callerClass + callerClass, + entitlements.componentName() ); } - private void notEntitled(String message, Class callerClass) { + private void notEntitled(String message, Class callerClass, String componentName) { var exception = new NotEntitledException(message); // Don't emit a log for muted classes, e.g. classes containing self tests if (mutedClasses.contains(callerClass) == false) { var moduleName = callerClass.getModule().getName(); - var notEntitledLogger = LogManager.getLogger(PolicyManager.class + "." + ((moduleName == null) ? "ALL_UNNAMED" : moduleName)); + var loggerSuffix = "." + componentName + "." + ((moduleName == null) ? "ALL_UNNAMED" : moduleName); + var notEntitledLogger = LogManager.getLogger(PolicyManager.class.getName() + loggerSuffix); String frameInfoSuffix = StackWalker.getInstance(RETAIN_CLASS_REFERENCE) .walk(this::findRequestingFrame) .map(StackFrame::toString) .orElse(""); - notEntitledLogger.info(message + frameInfoSuffix); + notEntitledLogger.warn(message + frameInfoSuffix); } throw exception; } From 85611cf740cd3aa34d91fbd921153d65eb82f6c6 Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Tue, 11 Mar 2025 11:01:26 -0400 Subject: [PATCH 5/8] Fix ALL_UNNAMED --- .../elasticsearch/entitlement/runtime/policy/PolicyManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 104a639ad1ab1..746554bf568c3 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 @@ -545,7 +545,7 @@ private void notEntitled(String message, Class callerClass, String componentN // Don't emit a log for muted classes, e.g. classes containing self tests if (mutedClasses.contains(callerClass) == false) { var moduleName = callerClass.getModule().getName(); - var loggerSuffix = "." + componentName + "." + ((moduleName == null) ? "ALL_UNNAMED" : moduleName); + var loggerSuffix = "." + componentName + "." + ((moduleName == null) ? ALL_UNNAMED : moduleName); var notEntitledLogger = LogManager.getLogger(PolicyManager.class.getName() + loggerSuffix); String frameInfoSuffix = StackWalker.getInstance(RETAIN_CLASS_REFERENCE) .walk(this::findRequestingFrame) From 60a1ee9ecf5ae215297ce3c04e0332a8f5ba2bc6 Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Tue, 11 Mar 2025 11:28:01 -0400 Subject: [PATCH 6/8] Mute entitlement warnings from repositories --- modules/repository-gcs/src/main/config/log4j2.properties | 3 +++ modules/repository-s3/src/main/config/log4j2.properties | 4 ++++ 2 files changed, 7 insertions(+) create mode 100644 modules/repository-gcs/src/main/config/log4j2.properties diff --git a/modules/repository-gcs/src/main/config/log4j2.properties b/modules/repository-gcs/src/main/config/log4j2.properties new file mode 100644 index 0000000000000..922cf5c5c37e6 --- /dev/null +++ b/modules/repository-gcs/src/main/config/log4j2.properties @@ -0,0 +1,3 @@ +logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.name = org.elasticsearch.entitlement.runtime.policy.PolicyManager.repository-gcs +logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.level = error + diff --git a/modules/repository-s3/src/main/config/log4j2.properties b/modules/repository-s3/src/main/config/log4j2.properties index 36a38cf9d13a0..41b0079b5f378 100644 --- a/modules/repository-s3/src/main/config/log4j2.properties +++ b/modules/repository-s3/src/main/config/log4j2.properties @@ -12,3 +12,7 @@ logger.com_amazonaws_auth_profile_internal_BasicProfileConfigFileLoader.level = logger.com_amazonaws_services_s3_internal_UseArnRegionResolver.name = com.amazonaws.services.s3.internal.UseArnRegionResolver logger.com_amazonaws_services_s3_internal_UseArnRegionResolver.level = error + +logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.name = org.elasticsearch.entitlement.runtime.policy.PolicyManager.repository-s3 +logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.level = error + From 9c94c2abd20323f3c2ea20d47612f787707c72c2 Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Tue, 11 Mar 2025 12:53:30 -0400 Subject: [PATCH 7/8] PR feedback --- .../elasticsearch/entitlement/runtime/policy/PolicyManager.java | 2 +- modules/repository-gcs/src/main/config/log4j2.properties | 2 +- modules/repository-s3/src/main/config/log4j2.properties | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 746554bf568c3..ad6c3483db772 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 @@ -549,7 +549,7 @@ private void notEntitled(String message, Class callerClass, String componentN var notEntitledLogger = LogManager.getLogger(PolicyManager.class.getName() + loggerSuffix); String frameInfoSuffix = StackWalker.getInstance(RETAIN_CLASS_REFERENCE) .walk(this::findRequestingFrame) - .map(StackFrame::toString) + .map(frame -> "\n\tat " + frame) .orElse(""); notEntitledLogger.warn(message + frameInfoSuffix); } diff --git a/modules/repository-gcs/src/main/config/log4j2.properties b/modules/repository-gcs/src/main/config/log4j2.properties index 922cf5c5c37e6..ba596176f8956 100644 --- a/modules/repository-gcs/src/main/config/log4j2.properties +++ b/modules/repository-gcs/src/main/config/log4j2.properties @@ -1,3 +1,3 @@ -logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.name = org.elasticsearch.entitlement.runtime.policy.PolicyManager.repository-gcs +logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.name = org.elasticsearch.entitlement.runtime.policy.PolicyManager.repository-gcs.ALL-UNNAMED logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.level = error diff --git a/modules/repository-s3/src/main/config/log4j2.properties b/modules/repository-s3/src/main/config/log4j2.properties index 41b0079b5f378..357c52a122f3e 100644 --- a/modules/repository-s3/src/main/config/log4j2.properties +++ b/modules/repository-s3/src/main/config/log4j2.properties @@ -13,6 +13,6 @@ logger.com_amazonaws_auth_profile_internal_BasicProfileConfigFileLoader.level = logger.com_amazonaws_services_s3_internal_UseArnRegionResolver.name = com.amazonaws.services.s3.internal.UseArnRegionResolver logger.com_amazonaws_services_s3_internal_UseArnRegionResolver.level = error -logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.name = org.elasticsearch.entitlement.runtime.policy.PolicyManager.repository-s3 +logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.name = org.elasticsearch.entitlement.runtime.policy.PolicyManager.repository-s3.software.amazon.awssdk.profiles logger.org_elasticsearch_entitlement_runtime_policy_PolicyManager.level = error From a4fa2b61f2d4945c3d31c01481a73d23e20589db Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Tue, 11 Mar 2025 14:25:25 -0400 Subject: [PATCH 8/8] Common out the Not Entitled prefix. We're alerting on this, so let's not rely on every caller of notEntitled to remember it. --- .../entitlement/runtime/policy/PolicyManager.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 ad6c3483db772..931a8ba5d53fc 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 @@ -250,7 +250,7 @@ private void neverEntitled(Class callerClass, Supplier operationDescr String componentName = getEntitlements(requestingClass).componentName(); notEntitled( Strings.format( - "Not entitled: component [%s], module [%s], class [%s], operation [%s]", + "component [%s], module [%s], class [%s], operation [%s]", componentName, requestingClass.getModule().getName(), requestingClass, @@ -368,7 +368,7 @@ public void checkFileRead(Class callerClass, Path path, boolean followLinks) if (canRead == false) { notEntitled( Strings.format( - "Not entitled: component [%s], module [%s], class [%s], entitlement [file], operation [read], path [%s]", + "component [%s], module [%s], class [%s], entitlement [file], operation [read], path [%s]", entitlements.componentName(), requestingClass.getModule().getName(), requestingClass, @@ -398,7 +398,7 @@ public void checkFileWrite(Class callerClass, Path path) { if (entitlements.fileAccess().canWrite(path) == false) { notEntitled( Strings.format( - "Not entitled: component [%s], module [%s], class [%s], entitlement [file], operation [write], path [%s]", + "component [%s], module [%s], class [%s], entitlement [file], operation [write], path [%s]", entitlements.componentName(), requestingClass.getModule().getName(), requestingClass, @@ -487,7 +487,7 @@ private void checkFlagEntitlement( if (classEntitlements.hasEntitlement(entitlementClass) == false) { notEntitled( Strings.format( - "Not entitled: component [%s], module [%s], class [%s], entitlement [%s]", + "component [%s], module [%s], class [%s], entitlement [%s]", classEntitlements.componentName(), requestingClass.getModule().getName(), requestingClass, @@ -529,7 +529,7 @@ public void checkWriteProperty(Class callerClass, String property) { } notEntitled( Strings.format( - "Not entitled: component [%s], module [%s], class [%s], entitlement [write_system_properties], property [%s]", + "component [%s], module [%s], class [%s], entitlement [write_system_properties], property [%s]", entitlements.componentName(), requestingClass.getModule().getName(), requestingClass, @@ -551,7 +551,7 @@ private void notEntitled(String message, Class callerClass, String componentN .walk(this::findRequestingFrame) .map(frame -> "\n\tat " + frame) .orElse(""); - notEntitledLogger.warn(message + frameInfoSuffix); + notEntitledLogger.warn("Not entitled: " + message + frameInfoSuffix); } throw exception; }