Skip to content

Commit fb98d84

Browse files
authored
Reduce duplicate and dead entitlements code (#121409) (#121425)
* Refactor: remove duplicate canWrite methods. This serves as a good example of how Path and File handling could be specialized in the future, but as long as they are identical, the duplication causes more harm than good. * Refactor: just one neverEntitled. The original motivation was to avoid allocating a lambda object on each call, but since that's a highly optimized operation in the JVM, it's unlikely to make a difference in practice, and this smacks of premature optimization. We're pretty liberal about lambdas elsewhere, so let's not sweat it here until we have some evidence that it matters. * Remove dead code
1 parent 5fe99a1 commit fb98d84

File tree

3 files changed

+3
-76
lines changed

3 files changed

+3
-76
lines changed

libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@
99

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

12-
import org.elasticsearch.core.SuppressForbidden;
1312
import org.elasticsearch.entitlement.runtime.policy.entitlements.FileEntitlement;
1413

15-
import java.io.File;
1614
import java.nio.file.Path;
1715
import java.util.ArrayList;
1816
import java.util.Arrays;
@@ -51,20 +49,10 @@ boolean canRead(Path path) {
5149
return checkPath(normalize(path), readPaths);
5250
}
5351

54-
@SuppressForbidden(reason = "Explicitly checking File apis")
55-
boolean canRead(File file) {
56-
return checkPath(normalize(file.toPath()), readPaths);
57-
}
58-
5952
boolean canWrite(Path path) {
6053
return checkPath(normalize(path), writePaths);
6154
}
6255

63-
@SuppressForbidden(reason = "Explicitly checking File apis")
64-
boolean canWrite(File file) {
65-
return checkPath(normalize(file.toPath()), writePaths);
66-
}
67-
6856
private static String normalize(Path path) {
6957
return path.toAbsolutePath().normalize().toString();
7058
}

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

Lines changed: 3 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -169,23 +169,7 @@ private static void validateEntitlementsPerModule(String sourceName, String modu
169169
}
170170

171171
public void checkStartProcess(Class<?> callerClass) {
172-
neverEntitled(callerClass, "start process");
173-
}
174-
175-
private void neverEntitled(Class<?> callerClass, String operationDescription) {
176-
var requestingClass = requestingClass(callerClass);
177-
if (isTriviallyAllowed(requestingClass)) {
178-
return;
179-
}
180-
181-
throw new NotEntitledException(
182-
Strings.format(
183-
"Not entitled: caller [%s], module [%s], operation [%s]",
184-
callerClass,
185-
requestingClass.getModule() == null ? "<none>" : requestingClass.getModule().getName(),
186-
operationDescription
187-
)
188-
);
172+
neverEntitled(callerClass, () -> "start process");
189173
}
190174

191175
/**
@@ -241,31 +225,9 @@ public void checkChangeNetworkHandling(Class<?> callerClass) {
241225
checkChangeJVMGlobalState(callerClass);
242226
}
243227

244-
/**
245-
* Check for operations that can access sensitive network information, e.g. secrets, tokens or SSL sessions
246-
*/
247-
public void checkReadSensitiveNetworkInformation(Class<?> callerClass) {
248-
neverEntitled(callerClass, "access sensitive network information");
249-
}
250-
251228
@SuppressForbidden(reason = "Explicitly checking File apis")
252229
public void checkFileRead(Class<?> callerClass, File file) {
253-
var requestingClass = requestingClass(callerClass);
254-
if (isTriviallyAllowed(requestingClass)) {
255-
return;
256-
}
257-
258-
ModuleEntitlements entitlements = getEntitlements(requestingClass);
259-
if (entitlements.fileAccess().canRead(file) == false) {
260-
throw new NotEntitledException(
261-
Strings.format(
262-
"Not entitled: caller [%s], module [%s], entitlement [file], operation [read], path [%s]",
263-
callerClass,
264-
requestingClass.getModule(),
265-
file
266-
)
267-
);
268-
}
230+
checkFileRead(callerClass, file.toPath());
269231
}
270232

271233
public void checkFileRead(Class<?> callerClass, Path path) {
@@ -289,22 +251,7 @@ public void checkFileRead(Class<?> callerClass, Path path) {
289251

290252
@SuppressForbidden(reason = "Explicitly checking File apis")
291253
public void checkFileWrite(Class<?> callerClass, File file) {
292-
var requestingClass = requestingClass(callerClass);
293-
if (isTriviallyAllowed(requestingClass)) {
294-
return;
295-
}
296-
297-
ModuleEntitlements entitlements = getEntitlements(requestingClass);
298-
if (entitlements.fileAccess().canWrite(file) == false) {
299-
throw new NotEntitledException(
300-
Strings.format(
301-
"Not entitled: caller [%s], module [%s], entitlement [file], operation [write], path [%s]",
302-
callerClass,
303-
requestingClass.getModule(),
304-
file
305-
)
306-
);
307-
}
254+
checkFileWrite(callerClass, file.toPath());
308255
}
309256

310257
public void checkFileWrite(Class<?> callerClass, Path path) {

libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,6 @@ public void testRequestingClassFastPath() throws IOException, ClassNotFoundExcep
238238
}
239239

240240
public void testRequestingModuleWithStackWalk() throws IOException, ClassNotFoundException {
241-
var agentsClass = new TestAgent();
242241
var entitlementsClass = makeClassInItsOwnModule(); // A class in the entitlements library itself
243242
var requestingClass = makeClassInItsOwnModule(); // This guy is always the right answer
244243
var instrumentedClass = makeClassInItsOwnModule(); // The class that called the check method
@@ -365,13 +364,6 @@ private static Class<?> makeClassInItsOwnModule() throws IOException, ClassNotFo
365364
return layer.findLoader("org.example.plugin").loadClass("q.B");
366365
}
367366

368-
private static Class<?> makeClassInItsOwnUnnamedModule() throws IOException, ClassNotFoundException {
369-
final Path home = createTempDir();
370-
Path jar = createMockPluginJar(home);
371-
var layer = createLayerForJar(jar, "org.example.plugin");
372-
return layer.findLoader("org.example.plugin").loadClass("q.B");
373-
}
374-
375367
private static PolicyManager policyManager(String agentsPackageName, Module entitlementsModule) {
376368
return new PolicyManager(createEmptyTestServerPolicy(), List.of(), Map.of(), c -> "test", agentsPackageName, entitlementsModule);
377369
}

0 commit comments

Comments
 (0)