Skip to content

Commit b9822c0

Browse files
ldematteldematte
committed
Addressing PR comments
1 parent 1116b90 commit b9822c0

File tree

10 files changed

+276
-95
lines changed

10 files changed

+276
-95
lines changed

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

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -130,22 +130,7 @@ private static PolicyManager createPolicyManager() {
130130
Map<String, Policy> pluginPolicies = bootstrapArgs.pluginPolicies();
131131
Path[] dataDirs = EntitlementBootstrap.bootstrapArgs().dataDirs();
132132

133-
var directoryResolver = new DirectoryResolver() {
134-
@Override
135-
public Path resolveConfig(Path path) {
136-
return bootstrapArgs.configDir().resolve(path);
137-
}
138-
139-
@Override
140-
public Stream<Path> resolveData(Path path) {
141-
return Arrays.stream(bootstrapArgs.dataDirs());
142-
}
143-
144-
@Override
145-
public Path resolveTemp(Path path) {
146-
return bootstrapArgs.tempDir();
147-
}
148-
};
133+
var directoryResolver = new EnvironmentDirectoryResolver(bootstrapArgs);
149134

150135
// TODO(ES-10031): Decide what goes in the elasticsearch default policy and extend it
151136
var serverPolicy = new Policy(
@@ -171,9 +156,7 @@ public Path resolveTemp(Path path) {
171156
"org.elasticsearch.nativeaccess",
172157
List.of(
173158
new LoadNativeLibrariesEntitlement(),
174-
new FilesEntitlement(
175-
Arrays.stream(dataDirs).map(d -> new FileData(d.toString(), READ_WRITE, FilesEntitlement.BaseDir.NONE)).toList()
176-
)
159+
new FilesEntitlement(Arrays.stream(dataDirs).map(d -> FileData.ofPath(Path.of(d.toString()), READ_WRITE)).toList())
177160
)
178161
)
179162
)
@@ -285,4 +268,27 @@ private static ElasticsearchEntitlementChecker initChecker() {
285268
"org.elasticsearch.entitlement.instrumentation",
286269
Set.of()
287270
).get();
271+
272+
static class EnvironmentDirectoryResolver implements DirectoryResolver {
273+
private final EntitlementBootstrap.BootstrapArgs bootstrapArgs;
274+
275+
EnvironmentDirectoryResolver(EntitlementBootstrap.BootstrapArgs bootstrapArgs) {
276+
this.bootstrapArgs = bootstrapArgs;
277+
}
278+
279+
@Override
280+
public Path resolveConfig(Path path) {
281+
return bootstrapArgs.configDir().resolve(path);
282+
}
283+
284+
@Override
285+
public Stream<Path> resolveData(Path path) {
286+
return Arrays.stream(bootstrapArgs.dataDirs()).map(dir -> dir.resolve(path));
287+
}
288+
289+
@Override
290+
public Path resolveTemp(Path path) {
291+
return bootstrapArgs.tempDir().resolve(path);
292+
}
293+
}
288294
}

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

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,21 @@ private static void resolvePath(
5151
DirectoryResolver directoryResolver,
5252
Consumer<String> resolvedPathReceiver
5353
) {
54-
Path path = Path.of(fileData.path());
55-
switch (fileData.baseDir()) {
56-
case NONE:
57-
resolvedPathReceiver.accept(normalizePath(path));
58-
break;
59-
case CONFIG:
60-
resolvedPathReceiver.accept(normalizePath(directoryResolver.resolveConfig(path)));
61-
break;
62-
case DATA:
63-
directoryResolver.resolveData(path).forEach(p -> resolvedPathReceiver.accept(normalizePath(p)));
64-
break;
65-
case TEMP:
66-
resolvedPathReceiver.accept(normalizePath(directoryResolver.resolveTemp(path)));
67-
break;
68-
default:
69-
throw new IllegalArgumentException();
54+
if (fileData.path() != null) {
55+
resolvedPathReceiver.accept(normalizePath(fileData.path()));
56+
} else if (fileData.relativePath() != null && fileData.baseDir() != null) {
57+
switch (fileData.baseDir()) {
58+
case CONFIG:
59+
resolvedPathReceiver.accept(normalizePath(directoryResolver.resolveConfig(fileData.relativePath())));
60+
break;
61+
case DATA:
62+
directoryResolver.resolveData(fileData.relativePath()).forEach(p -> resolvedPathReceiver.accept(normalizePath(p)));
63+
break;
64+
default:
65+
throw new IllegalArgumentException();
66+
}
67+
} else {
68+
throw new IllegalArgumentException();
7069
}
7170
}
7271

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

Lines changed: 89 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@
1212
import org.elasticsearch.entitlement.runtime.policy.ExternalEntitlement;
1313
import org.elasticsearch.entitlement.runtime.policy.PolicyValidationException;
1414

15+
import java.nio.file.Path;
1516
import java.util.ArrayList;
1617
import java.util.HashMap;
1718
import java.util.List;
1819
import java.util.Map;
20+
import java.util.Objects;
1921

2022
/**
2123
* Describes a file entitlement with a path and mode.
@@ -30,14 +32,64 @@ public enum Mode {
3032
}
3133

3234
public enum BaseDir {
33-
NONE,
3435
CONFIG,
35-
DATA,
36-
TEMP
36+
DATA
3737
}
3838

39-
public record FileData(String path, Mode mode, BaseDir baseDir) {
39+
public static final class FileData {
40+
private final Path path;
41+
private final Mode mode;
42+
private final Path relativePath;
43+
private final BaseDir baseDir;
4044

45+
private FileData(Path path, Mode mode, Path relativePath, BaseDir baseDir) {
46+
this.path = path;
47+
this.mode = mode;
48+
this.relativePath = relativePath;
49+
this.baseDir = baseDir;
50+
}
51+
52+
public static FileData ofPath(Path path, Mode mode) {
53+
assert path.isAbsolute();
54+
return new FileData(path, mode, null, null);
55+
}
56+
57+
public static FileData ofRelativePath(Path relativePath, BaseDir baseDir, Mode mode) {
58+
assert relativePath.isAbsolute() == false;
59+
return new FileData(null, mode, relativePath, baseDir);
60+
}
61+
62+
public Path path() {
63+
return path;
64+
}
65+
66+
public Mode mode() {
67+
return mode;
68+
}
69+
70+
public Path relativePath() {
71+
return relativePath;
72+
}
73+
74+
public BaseDir baseDir() {
75+
return baseDir;
76+
}
77+
78+
@Override
79+
public boolean equals(Object obj) {
80+
if (obj == this) return true;
81+
if (obj == null || obj.getClass() != this.getClass()) return false;
82+
var that = (FileData) obj;
83+
return Objects.equals(this.path, that.path)
84+
&& Objects.equals(this.mode, that.mode)
85+
&& Objects.equals(this.relativePath, that.relativePath)
86+
&& Objects.equals(this.baseDir, that.baseDir);
87+
}
88+
89+
@Override
90+
public int hashCode() {
91+
return Objects.hash(path, mode, relativePath, baseDir);
92+
}
4193
}
4294

4395
private static Mode parseMode(String mode) {
@@ -51,12 +103,12 @@ private static Mode parseMode(String mode) {
51103
}
52104

53105
private static BaseDir parseBaseDir(String baseDir) {
54-
return switch (baseDir) {
55-
case "config" -> BaseDir.CONFIG;
56-
case "data" -> BaseDir.DATA;
57-
case "temp" -> BaseDir.TEMP;
58-
default -> throw new PolicyValidationException("invalid base_dir: " + baseDir + ", valid values: [config, data, temp]");
59-
};
106+
if (baseDir.equals("config")) {
107+
return BaseDir.CONFIG;
108+
} else if (baseDir.equals("data")) {
109+
return BaseDir.DATA;
110+
}
111+
throw new PolicyValidationException("invalid relative directory: " + baseDir + ", valid values: [config, data]");
60112
}
61113

62114
@ExternalEntitlement(parameterNames = { "paths" }, esModulesOnly = false)
@@ -68,21 +120,38 @@ public static FilesEntitlement build(List<Object> paths) {
68120
List<FileData> filesData = new ArrayList<>();
69121
for (Object object : paths) {
70122
Map<String, String> file = new HashMap<>((Map<String, String>) object);
71-
String path = file.remove("path");
72-
if (path == null) {
73-
throw new PolicyValidationException("files entitlement must contain path for every listed file");
74-
}
123+
String pathAsString = file.remove("path");
124+
String relativePathAsString = file.remove("relative_path");
125+
String relativeTo = file.remove("relative_to");
75126
String mode = file.remove("mode");
127+
128+
if (file.isEmpty() == false) {
129+
throw new PolicyValidationException("unknown key(s) [" + file + "] in a listed file for files entitlement");
130+
}
76131
if (mode == null) {
77-
throw new PolicyValidationException("files entitlement must contain mode for every listed file");
132+
throw new PolicyValidationException("files entitlement must contain 'mode' for every listed file");
133+
}
134+
if ((pathAsString == null && relativePathAsString == null) || (pathAsString != null && relativePathAsString != null)) {
135+
throw new PolicyValidationException("files entitlement must contain either 'path' or 'relative_path' for every entry");
78136
}
79-
String baseDirString = file.remove("base_dir");
80-
final BaseDir baseDir = baseDirString == null ? BaseDir.NONE : parseBaseDir(baseDirString);
137+
if (relativePathAsString != null) {
138+
if (relativeTo == null) {
139+
throw new PolicyValidationException("files entitlement with a 'relative_path' must specify 'relative_to'");
140+
}
141+
final var baseDir = parseBaseDir(relativeTo);
81142

82-
if (file.isEmpty() == false) {
83-
throw new PolicyValidationException("unknown key(s) " + file + " in a listed file for files entitlement");
143+
Path relativePath = Path.of(relativePathAsString);
144+
if (relativePath.isAbsolute()) {
145+
throw new PolicyValidationException("'relative_path' must be relative");
146+
}
147+
filesData.add(FileData.ofRelativePath(relativePath, baseDir, parseMode(mode)));
148+
} else {
149+
Path path = Path.of(pathAsString);
150+
if (path.isAbsolute() == false) {
151+
throw new PolicyValidationException("'path' must be absolute");
152+
}
153+
filesData.add(FileData.ofPath(path, parseMode(mode)));
84154
}
85-
filesData.add(new FileData(path, parseMode(mode), baseDir));
86155
}
87156
return new FilesEntitlement(filesData);
88157
}

‎libs/entitlement/src/test/java/org/elasticsearch/entitlement/initialization/EnvironmentDirectoryResolverTests.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.entitlement.initialization;
11+
12+
import org.elasticsearch.entitlement.bootstrap.EntitlementBootstrap;
13+
import org.elasticsearch.entitlement.runtime.policy.DirectoryResolver;
14+
import org.elasticsearch.test.ESTestCase;
15+
16+
import java.nio.file.Path;
17+
import java.util.Map;
18+
19+
import static org.hamcrest.Matchers.contains;
20+
import static org.hamcrest.Matchers.is;
21+
22+
public class EnvironmentDirectoryResolverTests extends ESTestCase {
23+
24+
public void testDataDirs() {
25+
var resolver = createResolver();
26+
var dataDirs = resolver.resolveData(Path.of("foo")).toList();
27+
assertThat(dataDirs, contains(Path.of("/data1/foo"), Path.of("/data2/foo")));
28+
}
29+
30+
public void testConfigDir() {
31+
var resolver = createResolver();
32+
var configDir = resolver.resolveConfig(Path.of("foo"));
33+
assertThat(configDir, is(Path.of("/config/foo")));
34+
}
35+
36+
public void testTempDir() {
37+
var resolver = createResolver();
38+
var tempDir = resolver.resolveTemp(Path.of("foo"));
39+
assertThat(tempDir, is(Path.of("/tmp/foo")));
40+
}
41+
42+
private static DirectoryResolver createResolver() {
43+
return new EntitlementInitialization.EnvironmentDirectoryResolver(
44+
new EntitlementBootstrap.BootstrapArgs(
45+
Map.of(),
46+
c -> null,
47+
new Path[] { Path.of("/data1"), Path.of("/data2") },
48+
Path.of("/config"),
49+
Path.of("/tmp")
50+
)
51+
);
52+
}
53+
}

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -88,27 +88,27 @@ public void testReadWriteUnderRead() {
8888
assertThat(tree.canWrite(path("foo/bar")), is(true));
8989
}
9090

91-
public void testReadWithBaseDir() {
91+
public void testReadWithRelativePath() {
9292
var resolver = Mockito.mock(DirectoryResolver.class);
93-
when(resolver.resolveTemp(any(Path.class))).thenReturn(Path.of("/tmp/foo"));
94-
var tree = FileAccessTree.of(entitlement(Map.of("path", "foo", "mode", "read", "base_dir", "temp")), resolver);
93+
when(resolver.resolveConfig(any(Path.class))).thenReturn(Path.of("/config/foo"));
94+
var tree = FileAccessTree.of(entitlement(Map.of("relative_path", "foo", "mode", "read", "relative_to", "config")), resolver);
9595
assertThat(tree.canRead(path("foo")), is(false));
9696

97-
assertThat(tree.canRead(path("/tmp/foo")), is(true));
97+
assertThat(tree.canRead(path("/config/foo")), is(true));
9898

99-
assertThat(tree.canRead(path("/tmp/foo/subdir")), is(true));
100-
assertThat(tree.canRead(path("/tmp/food")), is(false));
101-
assertThat(tree.canWrite(path("/tmp/foo")), is(false));
99+
assertThat(tree.canRead(path("/config/foo/subdir")), is(true));
100+
assertThat(tree.canRead(path("/config/food")), is(false));
101+
assertThat(tree.canWrite(path("/config/foo")), is(false));
102102

103-
assertThat(tree.canRead(path("/tmp")), is(false));
104-
assertThat(tree.canRead(path("/tmp/before")), is(false));
105-
assertThat(tree.canRead(path("/tmp/later")), is(false));
103+
assertThat(tree.canRead(path("/config")), is(false));
104+
assertThat(tree.canRead(path("/config/before")), is(false));
105+
assertThat(tree.canRead(path("/config/later")), is(false));
106106
}
107107

108-
public void testWriteWithBaseDir() {
108+
public void testWriteWithRelativePath() {
109109
var resolver = Mockito.mock(DirectoryResolver.class);
110110
when(resolver.resolveConfig(any(Path.class))).thenReturn(Path.of("/config/foo"));
111-
var tree = FileAccessTree.of(entitlement(Map.of("path", "foo", "mode", "read_write", "base_dir", "config")), resolver);
111+
var tree = FileAccessTree.of(entitlement(Map.of("relative_path", "foo", "mode", "read_write", "relative_to", "config")), resolver);
112112
assertThat(tree.canWrite(path("/config/foo")), is(true));
113113
assertThat(tree.canWrite(path("/config/foo/subdir")), is(true));
114114
assertThat(tree.canWrite(path("foo")), is(false));
@@ -121,10 +121,10 @@ public void testWriteWithBaseDir() {
121121
assertThat(tree.canWrite(path("/config/later")), is(false));
122122
}
123123

124-
public void testMultipleDataBaseDir() {
124+
public void testMultipleDataDirs() {
125125
var resolver = Mockito.mock(DirectoryResolver.class);
126126
when(resolver.resolveData(any(Path.class))).thenReturn(Stream.of(Path.of("/data1/foo"), Path.of("/data2/foo")));
127-
var tree = FileAccessTree.of(entitlement(Map.of("path", "foo", "mode", "read_write", "base_dir", "data")), resolver);
127+
var tree = FileAccessTree.of(entitlement(Map.of("relative_path", "foo", "mode", "read_write", "relative_to", "data")), resolver);
128128
assertThat(tree.canWrite(path("/data1/foo")), is(true));
129129
assertThat(tree.canWrite(path("/data2/foo")), is(true));
130130
assertThat(tree.canWrite(path("/data3/foo")), is(false));

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -357,9 +357,7 @@ public void testDuplicateEntitlements() {
357357
FilesEntitlement.EMPTY,
358358
new CreateClassLoaderEntitlement(),
359359
new FilesEntitlement(
360-
List.of(
361-
new FilesEntitlement.FileData("test", FilesEntitlement.Mode.READ, FilesEntitlement.BaseDir.NONE)
362-
)
360+
List.of(FilesEntitlement.FileData.ofPath(Path.of("/tmp/test"), FilesEntitlement.Mode.READ))
363361
)
364362
)
365363
)
@@ -431,9 +429,7 @@ private static Policy createPluginPolicy(String... pluginModules) {
431429
name,
432430
List.of(
433431
new FilesEntitlement(
434-
List.of(
435-
new FilesEntitlement.FileData("/test/path", FilesEntitlement.Mode.READ, FilesEntitlement.BaseDir.NONE)
436-
)
432+
List.of(FilesEntitlement.FileData.ofPath(Path.of("/test/path"), FilesEntitlement.Mode.READ))
437433
),
438434
new CreateClassLoaderEntitlement()
439435
)

0 commit comments

Comments
 (0)