Skip to content

Commit fcd1dd0

Browse files
moschemosche
committed
Revert "move notEntitled into policManager to possibly ignore in tests"
This reverts commit bbfb065.
1 parent c94bf75 commit fcd1dd0

File tree

8 files changed

+50
-43
lines changed

8 files changed

+50
-43
lines changed

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ public static void bootstrap(
9393
);
9494
EntitlementInitialization.initializeArgs = new EntitlementInitialization.InitializeArgs(
9595
pathLookup,
96-
createPolicyManager(pluginPolicies, pathLookup, serverPolicyPatch, scopeResolver, pluginSourcePaths, suppressFailureLogPackages)
96+
suppressFailureLogPackages,
97+
createPolicyManager(pluginPolicies, pathLookup, serverPolicyPatch, scopeResolver, pluginSourcePaths)
9798
);
9899
exportInitializationToAgent();
99100
loadAgent(findAgentJar(), EntitlementInitialization.class.getName());
@@ -160,8 +161,7 @@ private static PolicyManager createPolicyManager(
160161
PathLookup pathLookup,
161162
Policy serverPolicyPatch,
162163
Function<Class<?>, PolicyManager.PolicyScope> scopeResolver,
163-
Map<String, Collection<Path>> pluginSourcePathsResolver,
164-
Set<Package> suppressFailureLogPackages
164+
Map<String, Collection<Path>> pluginSourcePathsResolver
165165
) {
166166
FilesEntitlementsValidation.validate(pluginPolicies, pathLookup);
167167

@@ -171,8 +171,7 @@ private static PolicyManager createPolicyManager(
171171
pluginPolicies,
172172
scopeResolver,
173173
pluginSourcePathsResolver::get,
174-
pathLookup,
175-
suppressFailureLogPackages
174+
pathLookup
176175
);
177176
}
178177

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,17 +91,24 @@ public static void initialize(Instrumentation inst) {
9191
* we have no way to pass arguments directly, so we stuff them in here.
9292
*
9393
* @param pathLookup
94+
* @param suppressFailureLogPackages
9495
* @param policyManager
9596
*/
96-
public record InitializeArgs(PathLookup pathLookup, PolicyManager policyManager) {
97+
public record InitializeArgs(PathLookup pathLookup, Set<Package> suppressFailureLogPackages, PolicyManager policyManager) {
9798
public InitializeArgs {
9899
requireNonNull(pathLookup);
100+
requireNonNull(suppressFailureLogPackages);
99101
requireNonNull(policyManager);
100102
}
101103
}
102104

103105
private static PolicyCheckerImpl createPolicyChecker(PolicyManager policyManager) {
104-
return new PolicyCheckerImpl(ENTITLEMENTS_MODULE, policyManager, initializeArgs.pathLookup());
106+
return new PolicyCheckerImpl(
107+
initializeArgs.suppressFailureLogPackages(),
108+
ENTITLEMENTS_MODULE,
109+
policyManager,
110+
initializeArgs.pathLookup()
111+
);
105112
}
106113

107114
/**

‎libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyCheckerImpl.java‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@
5757
*/
5858
@SuppressForbidden(reason = "Explicitly checking APIs that are forbidden")
5959
public class PolicyCheckerImpl implements PolicyChecker {
60+
61+
protected final Set<Package> suppressFailureLogPackages;
6062
/**
6163
* Frames originating from this module are ignored in the permission logic.
6264
*/
@@ -66,7 +68,13 @@ public class PolicyCheckerImpl implements PolicyChecker {
6668

6769
private final PathLookup pathLookup;
6870

69-
public PolicyCheckerImpl(Module entitlementsModule, PolicyManager policyManager, PathLookup pathLookup) {
71+
public PolicyCheckerImpl(
72+
Set<Package> suppressFailureLogPackages,
73+
Module entitlementsModule,
74+
PolicyManager policyManager,
75+
PathLookup pathLookup
76+
) {
77+
this.suppressFailureLogPackages = suppressFailureLogPackages;
7078
this.entitlementsModule = entitlementsModule;
7179
this.policyManager = policyManager;
7280
this.pathLookup = pathLookup;
@@ -121,7 +129,7 @@ private void neverEntitled(Class<?> callerClass, Supplier<String> operationDescr
121129
}
122130

123131
ModuleEntitlements entitlements = policyManager.getEntitlements(requestingClass);
124-
policyManager.notEntitled(
132+
notEntitled(
125133
Strings.format(
126134
"component [%s], module [%s], class [%s], operation [%s]",
127135
entitlements.componentName(),
@@ -233,7 +241,7 @@ public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks)
233241
}
234242

235243
if (canRead == false) {
236-
policyManager.notEntitled(
244+
notEntitled(
237245
Strings.format(
238246
"component [%s], module [%s], class [%s], entitlement [file], operation [read], path [%s]",
239247
entitlements.componentName(),
@@ -265,7 +273,7 @@ public void checkFileWrite(Class<?> callerClass, Path path) {
265273

266274
ModuleEntitlements entitlements = policyManager.getEntitlements(requestingClass);
267275
if (entitlements.fileAccess().canWrite(path) == false) {
268-
policyManager.notEntitled(
276+
notEntitled(
269277
Strings.format(
270278
"component [%s], module [%s], class [%s], entitlement [file], operation [write], path [%s]",
271279
entitlements.componentName(),
@@ -379,7 +387,7 @@ public void checkWriteProperty(Class<?> callerClass, String property) {
379387
);
380388
return;
381389
}
382-
policyManager.notEntitled(
390+
notEntitled(
383391
Strings.format(
384392
"component [%s], module [%s], class [%s], entitlement [write_system_properties], property [%s]",
385393
entitlements.componentName(),
@@ -431,7 +439,7 @@ private void checkFlagEntitlement(
431439
Class<?> requestingClass
432440
) {
433441
if (classEntitlements.hasEntitlement(entitlementClass) == false) {
434-
policyManager.notEntitled(
442+
notEntitled(
435443
Strings.format(
436444
"component [%s], module [%s], class [%s], entitlement [%s]",
437445
classEntitlements.componentName(),
@@ -454,6 +462,15 @@ private void checkFlagEntitlement(
454462
);
455463
}
456464

465+
private void notEntitled(String message, Class<?> requestingClass, ModuleEntitlements entitlements) {
466+
var exception = new NotEntitledException(message);
467+
// Don't emit a log for suppressed packages, e.g. packages containing self tests
468+
if (suppressFailureLogPackages.contains(requestingClass.getPackage()) == false) {
469+
entitlements.logger(requestingClass).warn("Not entitled: {}", message, exception);
470+
}
471+
throw exception;
472+
}
473+
457474
@Override
458475
public void checkEntitlementPresent(Class<?> callerClass, Class<? extends Entitlement> entitlementClass) {
459476
var requestingClass = requestingClass(callerClass);

‎libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
package org.elasticsearch.entitlement.runtime.policy;
1111

12-
import org.elasticsearch.entitlement.runtime.api.NotEntitledException;
1312
import org.elasticsearch.entitlement.runtime.policy.FileAccessTree.ExclusiveFileEntitlement;
1413
import org.elasticsearch.entitlement.runtime.policy.FileAccessTree.ExclusivePath;
1514
import org.elasticsearch.entitlement.runtime.policy.entitlements.Entitlement;
@@ -144,7 +143,7 @@ public <E extends Entitlement> Stream<E> getEntitlements(Class<E> entitlementCla
144143
return entitlements.stream().map(entitlementClass::cast);
145144
}
146145

147-
private Logger logger(Class<?> requestingClass) {
146+
Logger logger(Class<?> requestingClass) {
148147
var packageName = requestingClass.getPackageName();
149148
var loggerSuffix = "." + componentName + "." + ((moduleName == null) ? ALL_UNNAMED : moduleName) + "." + packageName;
150149
return LogManager.getLogger(PolicyManager.class.getName() + loggerSuffix);
@@ -183,7 +182,6 @@ ModuleEntitlements policyEntitlements(
183182

184183
final Map<Module, ModuleEntitlements> moduleEntitlementsMap = new ConcurrentHashMap<>();
185184

186-
private final Set<Package> suppressFailureLogPackages;
187185
private final Map<String, List<Entitlement>> serverEntitlements;
188186
private final List<Entitlement> apmAgentEntitlements;
189187
private final Map<String, Map<String, List<Entitlement>>> pluginsEntitlements;
@@ -234,8 +232,7 @@ public PolicyManager(
234232
Map<String, Policy> pluginPolicies,
235233
Function<Class<?>, PolicyScope> scopeResolver,
236234
Function<String, Collection<Path>> pluginSourcePathsResolver,
237-
PathLookup pathLookup,
238-
Set<Package> suppressFailureLogPackages
235+
PathLookup pathLookup
239236
) {
240237
this.serverEntitlements = buildScopeEntitlementsMap(requireNonNull(serverPolicy));
241238
this.apmAgentEntitlements = apmAgentEntitlements;
@@ -245,7 +242,6 @@ public PolicyManager(
245242
this.scopeResolver = scopeResolver;
246243
this.pluginSourcePathsResolver = pluginSourcePathsResolver;
247244
this.pathLookup = requireNonNull(pathLookup);
248-
this.suppressFailureLogPackages = suppressFailureLogPackages;
249245

250246
List<ExclusiveFileEntitlement> exclusiveFileEntitlements = new ArrayList<>();
251247
for (var e : serverEntitlements.entrySet()) {
@@ -354,15 +350,6 @@ protected Collection<Path> getComponentPathsFromClass(Class<?> requestingClass)
354350
}
355351
}
356352

357-
void notEntitled(String message, Class<?> requestingClass, ModuleEntitlements entitlements) {
358-
var exception = new NotEntitledException(message);
359-
// Don't emit a log for suppressed packages, e.g. packages containing self tests
360-
if (suppressFailureLogPackages.contains(requestingClass.getPackage()) == false) {
361-
entitlements.logger(requestingClass).warn("Not entitled: {}", message, exception);
362-
}
363-
throw exception;
364-
}
365-
366353
private ModuleEntitlements getModuleScopeEntitlements(
367354
Map<String, List<Entitlement>> scopeEntitlements,
368355
String scopeName,

‎libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyCheckerImplTests.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.test.ESTestCase;
1313

1414
import java.io.IOException;
15+
import java.util.Set;
1516
import java.util.stream.Stream;
1617

1718
import static org.elasticsearch.entitlement.runtime.policy.PolicyManagerTests.NO_ENTITLEMENTS_MODULE;
@@ -54,7 +55,7 @@ public void testRequestingModuleWithStackWalk() throws IOException, ClassNotFoun
5455
}
5556

5657
private static PolicyCheckerImpl checker(Module entitlementsModule) {
57-
return new PolicyCheckerImpl(entitlementsModule, null, TEST_PATH_LOOKUP);
58+
return new PolicyCheckerImpl(Set.of(), entitlementsModule, null, TEST_PATH_LOOKUP);
5859
}
5960

6061
}

‎test/framework/src/main/java/org/elasticsearch/entitlement/bootstrap/TestEntitlementBootstrap.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public static void bootstrap(@Nullable Path tempDir) throws IOException {
7373
assert previousTempDir == null : "Test entitlement bootstrap called multiple times";
7474
TestPathLookup pathLookup = new TestPathLookup(baseDirPaths);
7575
policyManager = createPolicyManager(pathLookup);
76-
EntitlementInitialization.initializeArgs = new EntitlementInitialization.InitializeArgs(pathLookup, policyManager);
76+
EntitlementInitialization.initializeArgs = new EntitlementInitialization.InitializeArgs(pathLookup, Set.of(), policyManager);
7777
logger.debug("Loading entitlement agent");
7878
EntitlementBootstrap.loadAgent(EntitlementBootstrap.findAgentJar(), EntitlementInitialization.class.getName());
7979
}

‎test/framework/src/main/java/org/elasticsearch/entitlement/runtime/policy/TestPolicyManager.java‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.security.ProtectionDomain;
2121
import java.util.Arrays;
2222
import java.util.Collection;
23-
import java.util.Collections;
2423
import java.util.List;
2524
import java.util.Map;
2625
import java.util.concurrent.ConcurrentHashMap;
@@ -52,7 +51,7 @@ public TestPolicyManager(
5251
Collection<Path> classpath,
5352
Collection<URI> testOnlyClasspath
5453
) {
55-
super(serverPolicy, apmAgentEntitlements, pluginPolicies, scopeResolver, name -> classpath, pathLookup, Collections.emptySet());
54+
super(serverPolicy, apmAgentEntitlements, pluginPolicies, scopeResolver, name -> classpath, pathLookup);
5655
this.classpath = classpath;
5756
this.testOnlyClasspath = testOnlyClasspath;
5857
reset();
@@ -86,13 +85,6 @@ public final void reset() {
8685
isTriviallyAllowingTestCode = true;
8786
}
8887

89-
@Override
90-
void notEntitled(String message, Class<?> requestingClass, ModuleEntitlements entitlements) {
91-
// ignore if a class on the stack is annotated with WithoutEntitlements
92-
if (hasEntitlementsDisabledForStack()) return;
93-
super.notEntitled(message, requestingClass, entitlements);
94-
}
95-
9688
@Override
9789
protected boolean isTrustedSystemClass(Class<?> requestingClass) {
9890
ClassLoader loader = requestingClass.getClassLoader();
@@ -125,7 +117,11 @@ boolean isTriviallyAllowed(Class<?> requestingClass) {
125117
if (isTriviallyAllowingTestCode && isTestCode(requestingClass)) {
126118
return true;
127119
}
128-
return super.isTriviallyAllowed(requestingClass);
120+
if (super.isTriviallyAllowed(requestingClass)) {
121+
return true;
122+
}
123+
;
124+
return isStackWithoutEntitlements();
129125
}
130126

131127
@Override
@@ -137,7 +133,7 @@ private static boolean hasWithoutEntitlements(Class<?> clazz) {
137133
return clazz.getAnnotation(ESTestCase.WithoutEntitlements.class) != null;
138134
}
139135

140-
private static boolean hasEntitlementsDisabledForStack() {
136+
private static boolean isStackWithoutEntitlements() {
141137
return StackWalker.getInstance(RETAIN_CLASS_REFERENCE)
142138
.walk(
143139
frames -> frames.map(StackWalker.StackFrame::getDeclaringClass)

‎test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ protected void afterIfFailed(List<Throwable> errors) {}
499499
protected void afterIfSuccessful() throws Exception {}
500500

501501
/**
502-
* Marks a test suite or a test utility that should run without checking for entitlements.
502+
* Marks a test suite or a test method that should run without checking for entitlements.
503503
*/
504504
@Retention(RetentionPolicy.RUNTIME)
505505
@Target(ElementType.TYPE)
@@ -508,7 +508,7 @@ protected void afterIfSuccessful() throws Exception {}
508508
}
509509

510510
/**
511-
* Marks a test suite to enforce entitlements on the test code itself.
511+
* Marks a test suite or a test method that enforce entitlements on the test code itself.
512512
* Useful for testing the enforcement of entitlements; for any other test cases, this probably isn't what you want.
513513
*/
514514
@Retention(RetentionPolicy.RUNTIME)

0 commit comments

Comments
 (0)