Skip to content

Commit 0e5bd3c

Browse files
jdconradjdconrad
authored
Add exclusive file entitlement for settings (#125272)
Adds changes to ensure the correct caller's module is checked for exclusive file settings.
1 parent e6096a2 commit 0e5bd3c

File tree

7 files changed

+148
-17
lines changed

7 files changed

+148
-17
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
import java.util.stream.StreamSupport;
6969

7070
import static org.elasticsearch.entitlement.runtime.policy.Platform.LINUX;
71+
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.BaseDir.CONFIG;
7172
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.BaseDir.DATA;
7273
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.BaseDir.SHARED_REPO;
7374
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ;
@@ -182,7 +183,8 @@ private static PolicyManager createPolicyManager() {
182183
FileData.ofPath(bootstrapArgs.libDir(), READ),
183184
FileData.ofRelativePath(Path.of(""), DATA, READ_WRITE),
184185
FileData.ofRelativePath(Path.of(""), SHARED_REPO, READ_WRITE),
185-
186+
// exclusive settings file
187+
FileData.ofRelativePath(Path.of("operator/settings.json"), CONFIG, READ_WRITE).withExclusive(true),
186188
// OS release on Linux
187189
FileData.ofPath(Path.of("/etc/os-release"), READ).withPlatform(LINUX),
188190
FileData.ofPath(Path.of("/etc/system-release"), READ).withPlatform(LINUX),

‎server/src/main/java/org/elasticsearch/common/file/AbstractFileWatchingService.java

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,16 @@
1616
import org.elasticsearch.core.FixForMultiProject;
1717

1818
import java.io.IOException;
19+
import java.io.InputStream;
1920
import java.nio.file.ClosedWatchServiceException;
20-
import java.nio.file.Files;
2121
import java.nio.file.NoSuchFileException;
2222
import java.nio.file.Path;
2323
import java.nio.file.StandardWatchEventKinds;
2424
import java.nio.file.WatchEvent;
2525
import java.nio.file.WatchKey;
2626
import java.nio.file.WatchService;
2727
import java.nio.file.attribute.BasicFileAttributes;
28+
import java.nio.file.attribute.FileTime;
2829
import java.util.HashMap;
2930
import java.util.HashSet;
3031
import java.util.List;
@@ -62,8 +63,9 @@ public abstract class AbstractFileWatchingService extends AbstractLifecycleCompo
6263
private WatchKey settingsDirWatchKey;
6364
private WatchKey configDirWatchKey;
6465

66+
@SuppressWarnings("this-escape")
6567
public AbstractFileWatchingService(Path settingsDir) {
66-
if (Files.exists(settingsDir) && Files.isDirectory(settingsDir) == false) {
68+
if (filesExists(settingsDir) && filesIsDirectory(settingsDir) == false) {
6769
throw new IllegalArgumentException("settingsDir should be a directory");
6870
}
6971
this.settingsDir = settingsDir;
@@ -113,10 +115,10 @@ public final boolean watching() {
113115
return watcherThread != null;
114116
}
115117

116-
private static FileUpdateState readFileUpdateState(Path path) throws IOException {
118+
private FileUpdateState readFileUpdateState(Path path) throws IOException {
117119
BasicFileAttributes attr;
118120
try {
119-
attr = Files.readAttributes(path, BasicFileAttributes.class);
121+
attr = filesReadAttributes(path, BasicFileAttributes.class);
120122
} catch (NoSuchFileException e) {
121123
// file doesn't exist anymore
122124
return null;
@@ -141,7 +143,7 @@ final boolean fileChanged(Path path) throws IOException {
141143
}
142144

143145
protected final synchronized void startWatcher() {
144-
if (Files.exists(settingsDir.getParent()) == false) {
146+
if (filesExists(settingsDir.getParent()) == false) {
145147
logger.warn("File watcher for [{}] cannot start because parent directory does not exist", settingsDir);
146148
return;
147149
}
@@ -155,7 +157,7 @@ protected final synchronized void startWatcher() {
155157
*/
156158
try {
157159
this.watchService = settingsDir.getParent().getFileSystem().newWatchService();
158-
if (Files.exists(settingsDir)) {
160+
if (filesExists(settingsDir)) {
159161
settingsDirWatchKey = enableDirectoryWatcher(settingsDirWatchKey, settingsDir);
160162
} else {
161163
logger.debug("watched directory [{}] not found, will watch for its creation...", settingsDir);
@@ -188,8 +190,8 @@ protected final void watcherThread() {
188190
try {
189191
logger.info("file settings service up and running [tid={}]", Thread.currentThread().getId());
190192

191-
if (Files.exists(settingsDir)) {
192-
try (Stream<Path> files = Files.list(settingsDir)) {
193+
if (filesExists(settingsDir)) {
194+
try (Stream<Path> files = filesList(settingsDir)) {
193195
var f = files.iterator();
194196
if (f.hasNext() == false) {
195197
// no files in directory
@@ -248,7 +250,7 @@ protected final void watcherThread() {
248250
}
249251
}
250252
} else if (key == configDirWatchKey) {
251-
if (Files.exists(settingsDir)) {
253+
if (filesExists(settingsDir)) {
252254
// We re-register the settings directory watch key, because we don't know
253255
// if the file name maps to the same native file system file id. Symlinks
254256
// are one potential cause of inconsistency here, since their handling by
@@ -257,7 +259,7 @@ protected final void watcherThread() {
257259
settingsDirWatchKey = enableDirectoryWatcher(settingsDirWatchKey, settingsDir);
258260

259261
// re-read the settings directory, and ping for any changes
260-
try (Stream<Path> files = Files.list(settingsDir)) {
262+
try (Stream<Path> files = filesList(settingsDir)) {
261263
for (var f = files.iterator(); f.hasNext();) {
262264
Path file = f.next();
263265
if (fileChanged(file)) {
@@ -370,4 +372,19 @@ long retryDelayMillis(int failedCount) {
370372
* class to determine if a file has been changed.
371373
*/
372374
private record FileUpdateState(long timestamp, String path, Object fileKey) {}
375+
376+
// the following methods are a workaround to ensure exclusive access for files
377+
// required by child watchers; this is required because we only check the caller's module
378+
// not the entire stack
379+
protected abstract boolean filesExists(Path path);
380+
381+
protected abstract boolean filesIsDirectory(Path path);
382+
383+
protected abstract <A extends BasicFileAttributes> A filesReadAttributes(Path path, Class<A> clazz) throws IOException;
384+
385+
protected abstract Stream<Path> filesList(Path dir) throws IOException;
386+
387+
protected abstract Path filesSetLastModifiedTime(Path path, FileTime time) throws IOException;
388+
389+
protected abstract InputStream filesNewInputStream(Path path) throws IOException;
373390
}

‎server/src/main/java/org/elasticsearch/common/file/MasterNodeFileWatchingService.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import org.elasticsearch.gateway.GatewayService;
2121

2222
import java.io.IOException;
23-
import java.nio.file.Files;
2423
import java.nio.file.Path;
2524
import java.nio.file.attribute.FileTime;
2625
import java.time.Instant;
@@ -43,7 +42,7 @@ protected void doStart() {
4342
// We start the file watcher when we know we are master from a cluster state change notification.
4443
// We need the additional active flag, since cluster state can change after we've shutdown the service
4544
// causing the watcher to start again.
46-
this.active = Files.exists(watchedFileDir().getParent());
45+
this.active = filesExists(watchedFileDir().getParent());
4746
if (active == false) {
4847
// we don't have a config directory, we can't possibly launch the file settings service
4948
return;
@@ -92,10 +91,10 @@ public final void clusterChanged(ClusterChangedEvent event) {
9291
@FixForMultiProject // do we want to re-process everything all at once?
9392
private void refreshExistingFileStateIfNeeded(ClusterState clusterState) {
9493
if (shouldRefreshFileState(clusterState)) {
95-
try (Stream<Path> files = Files.list(watchedFileDir())) {
94+
try (Stream<Path> files = filesList(watchedFileDir())) {
9695
FileTime time = FileTime.from(Instant.now());
9796
for (var it = files.iterator(); it.hasNext();) {
98-
Files.setLastModifiedTime(it.next(), time);
97+
filesSetLastModifiedTime(it.next(), time);
9998
}
10099
} catch (IOException e) {
101100
logger.warn("encountered I/O error trying to update file settings timestamp", e);

‎server/src/main/java/org/elasticsearch/reservedstate/service/FileSettingsService.java

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,12 @@
4040
import java.io.InputStream;
4141
import java.nio.file.Files;
4242
import java.nio.file.Path;
43+
import java.nio.file.attribute.BasicFileAttributes;
44+
import java.nio.file.attribute.FileTime;
4345
import java.util.List;
4446
import java.util.Map;
4547
import java.util.concurrent.ExecutionException;
48+
import java.util.stream.Stream;
4649

4750
import static org.elasticsearch.health.HealthStatus.GREEN;
4851
import static org.elasticsearch.health.HealthStatus.YELLOW;
@@ -128,7 +131,7 @@ public void handleSnapshotRestore(ClusterState clusterState, Metadata.Builder md
128131
// since we don't know the current operator configuration, e.g. file settings could be disabled
129132
// on the target cluster. If file settings exist and the cluster state has lost it's reserved
130133
// state for the "file_settings" namespace, we touch our file settings file to cause it to re-process the file.
131-
if (watching() && Files.exists(watchedFile)) {
134+
if (watching() && filesExists(watchedFile)) {
132135
if (fileSettingsMetadata != null) {
133136
ReservedStateMetadata withResetVersion = new ReservedStateMetadata.Builder(fileSettingsMetadata).version(0L).build();
134137
mdBuilder.put(withResetVersion);
@@ -201,7 +204,7 @@ protected XContentParser createParser(InputStream stream) throws IOException {
201204

202205
private void processFileChanges(ReservedStateVersionCheck versionCheck) throws IOException, InterruptedException, ExecutionException {
203206
PlainActionFuture<Void> completion = new PlainActionFuture<>();
204-
try (var bis = new BufferedInputStream(Files.newInputStream(watchedFile)); var parser = createParser(bis)) {
207+
try (var bis = new BufferedInputStream(filesNewInputStream(watchedFile)); var parser = createParser(bis)) {
205208
stateService.process(NAMESPACE, parser, versionCheck, (e) -> completeProcessing(e, completion));
206209
}
207210
completion.get();
@@ -343,4 +346,37 @@ public synchronized HealthIndicatorResult calculate(boolean verbose, int maxAffe
343346
}
344347
}
345348
}
349+
350+
// the following methods are a workaround to ensure exclusive access for files
351+
// required by child watchers; this is required because we only check the caller's module
352+
// not the entire stack
353+
@Override
354+
protected boolean filesExists(Path path) {
355+
return Files.exists(path);
356+
}
357+
358+
@Override
359+
protected boolean filesIsDirectory(Path path) {
360+
return Files.isDirectory(path);
361+
}
362+
363+
@Override
364+
protected <A extends BasicFileAttributes> A filesReadAttributes(Path path, Class<A> clazz) throws IOException {
365+
return Files.readAttributes(path, clazz);
366+
}
367+
368+
@Override
369+
protected Stream<Path> filesList(Path dir) throws IOException {
370+
return Files.list(dir);
371+
}
372+
373+
@Override
374+
protected Path filesSetLastModifiedTime(Path path, FileTime time) throws IOException {
375+
return Files.setLastModifiedTime(path, time);
376+
}
377+
378+
@Override
379+
protected InputStream filesNewInputStream(Path path) throws IOException {
380+
return Files.newInputStream(path);
381+
}
346382
}

‎server/src/test/java/org/elasticsearch/common/file/AbstractFileWatchingServiceTests.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,13 @@
2424
import org.junit.Before;
2525

2626
import java.io.IOException;
27+
import java.io.InputStream;
2728
import java.nio.file.Files;
2829
import java.nio.file.Path;
2930
import java.nio.file.StandardCopyOption;
3031
import java.nio.file.StandardWatchEventKinds;
3132
import java.nio.file.WatchKey;
33+
import java.nio.file.attribute.BasicFileAttributes;
3234
import java.nio.file.attribute.FileTime;
3335
import java.time.Instant;
3436
import java.time.LocalDateTime;
@@ -40,6 +42,7 @@
4042
import java.util.concurrent.ExecutionException;
4143
import java.util.concurrent.TimeUnit;
4244
import java.util.function.Consumer;
45+
import java.util.stream.Stream;
4346

4447
import static org.elasticsearch.node.Node.NODE_NAME_SETTING;
4548
import static org.elasticsearch.test.hamcrest.OptionalMatchers.isEmpty;
@@ -81,6 +84,39 @@ protected void processInitialFilesMissing() {
8184
called.accept(null);
8285
}
8386
}
87+
88+
// the following methods are a workaround to ensure exclusive access for files
89+
// required by child watchers; this is required because we only check the caller's module
90+
// not the entire stack
91+
@Override
92+
protected boolean filesExists(Path path) {
93+
return Files.exists(path);
94+
}
95+
96+
@Override
97+
protected boolean filesIsDirectory(Path path) {
98+
return Files.isDirectory(path);
99+
}
100+
101+
@Override
102+
protected <A extends BasicFileAttributes> A filesReadAttributes(Path path, Class<A> clazz) throws IOException {
103+
return Files.readAttributes(path, clazz);
104+
}
105+
106+
@Override
107+
protected Stream<Path> filesList(Path dir) throws IOException {
108+
return Files.list(dir);
109+
}
110+
111+
@Override
112+
protected Path filesSetLastModifiedTime(Path path, FileTime time) throws IOException {
113+
return Files.setLastModifiedTime(path, time);
114+
}
115+
116+
@Override
117+
protected InputStream filesNewInputStream(Path path) throws IOException {
118+
return Files.newInputStream(path);
119+
}
84120
}
85121

86122
private Path watchedFile;

‎server/src/test/java/org/elasticsearch/common/file/MasterNodeFileWatchingServiceTests.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,14 @@
2626
import org.junit.Before;
2727

2828
import java.io.IOException;
29+
import java.io.InputStream;
30+
import java.nio.file.Files;
2931
import java.nio.file.Path;
32+
import java.nio.file.attribute.BasicFileAttributes;
33+
import java.nio.file.attribute.FileTime;
3034
import java.util.concurrent.ExecutionException;
3135
import java.util.function.Consumer;
36+
import java.util.stream.Stream;
3237

3338
import static org.hamcrest.Matchers.is;
3439
import static org.mockito.Mockito.mock;
@@ -62,6 +67,39 @@ protected void processFileChanges(Path file) throws InterruptedException, Execut
6267
protected void processInitialFilesMissing() throws InterruptedException, ExecutionException, IOException {
6368
// file always exists, but we don't care about the missing case for master node behavior
6469
}
70+
71+
// the following methods are a workaround to ensure exclusive access for files
72+
// required by child watchers; this is required because we only check the caller's module
73+
// not the entire stack
74+
@Override
75+
protected boolean filesExists(Path path) {
76+
return Files.exists(path);
77+
}
78+
79+
@Override
80+
protected boolean filesIsDirectory(Path path) {
81+
return Files.isDirectory(path);
82+
}
83+
84+
@Override
85+
protected <A extends BasicFileAttributes> A filesReadAttributes(Path path, Class<A> clazz) throws IOException {
86+
return Files.readAttributes(path, clazz);
87+
}
88+
89+
@Override
90+
protected Stream<Path> filesList(Path dir) throws IOException {
91+
return Files.list(dir);
92+
}
93+
94+
@Override
95+
protected Path filesSetLastModifiedTime(Path path, FileTime time) throws IOException {
96+
return Files.setLastModifiedTime(path, time);
97+
}
98+
99+
@Override
100+
protected InputStream filesNewInputStream(Path path) throws IOException {
101+
return Files.newInputStream(path);
102+
}
65103
};
66104
testService.start();
67105
}

‎x-pack/plugin/core/src/main/config/log4j2.properties

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,3 +115,6 @@ logger.samlxml_decrypt.name = org.opensaml.xmlsec.encryption.support.Decrypter
115115
logger.samlxml_decrypt.level = fatal
116116
logger.saml2_decrypt.name = org.opensaml.saml.saml2.encryption.Decrypter
117117
logger.saml2_decrypt.level = fatal
118+
119+
logger.entitlements_xpack_security.name = org.elasticsearch.entitlement.runtime.policy.PolicyManager.x-pack-security.org.elasticsearch.security
120+
logger.entitlements_xpack_security.level = error

0 commit comments

Comments
 (0)