Skip to content

Commit ffae8d7

Browse files
authored
[Entitlements] Fix: consider case sensitiveness differences (#126990) (#127274)
Our path comparison for file access is string based, due to the fact that we need to support Paths created for different file systems/platforms. However, Windows files and paths are (sort of) case insensitive. This PR fixes the problem by abstracting String comparison operations and making them case sensitive or not based on the host OS.
1 parent 663dd2b commit ffae8d7

File tree

10 files changed

+317
-132
lines changed

10 files changed

+317
-132
lines changed

docs/changelog/126990.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
pr: 126990
2+
summary: "Fix: consider case sensitiveness differences in Windows/Unix-like filesystems\
3+
\ for files entitlements"
4+
area: Infra/Core
5+
type: bug
6+
issues:
7+
- 127047
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
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.runtime.policy;
11+
12+
class CaseInsensitiveComparison extends FileAccessTreeComparison {
13+
14+
CaseInsensitiveComparison(char separatorChar) {
15+
super(CaseInsensitiveComparison::caseInsensitiveCharacterComparator, separatorChar);
16+
}
17+
18+
private static int caseInsensitiveCharacterComparator(char c1, char c2) {
19+
return Character.compare(Character.toLowerCase(c1), Character.toLowerCase(c2));
20+
}
21+
22+
@Override
23+
protected boolean pathStartsWith(String pathString, String pathPrefix) {
24+
return pathString.regionMatches(true, 0, pathPrefix, 0, pathPrefix.length());
25+
}
26+
27+
@Override
28+
protected boolean pathsAreEqual(String path1, String path2) {
29+
return path1.equalsIgnoreCase(path2);
30+
}
31+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
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.runtime.policy;
11+
12+
class CaseSensitiveComparison extends FileAccessTreeComparison {
13+
14+
CaseSensitiveComparison(char separatorChar) {
15+
super(Character::compare, separatorChar);
16+
}
17+
18+
@Override
19+
protected boolean pathStartsWith(String pathString, String pathPrefix) {
20+
return pathString.startsWith(pathPrefix);
21+
}
22+
23+
@Override
24+
protected boolean pathsAreEqual(String path1, String path2) {
25+
return path1.equals(path2);
26+
}
27+
}

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

Lines changed: 47 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@
1111

1212
import org.elasticsearch.core.Nullable;
1313
import org.elasticsearch.core.Strings;
14+
import org.elasticsearch.core.SuppressForbidden;
1415
import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement;
1516
import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode;
1617
import org.elasticsearch.logging.LogManager;
1718
import org.elasticsearch.logging.Logger;
1819

20+
import java.io.File;
1921
import java.io.IOException;
2022
import java.io.UncheckedIOException;
2123
import java.nio.file.Files;
@@ -33,7 +35,6 @@
3335

3436
import static java.util.Comparator.comparing;
3537
import static org.elasticsearch.core.PathUtils.getDefaultFileSystem;
36-
import static org.elasticsearch.entitlement.runtime.policy.FileUtils.PATH_ORDER;
3738
import static org.elasticsearch.entitlement.runtime.policy.PathLookup.BaseDir.CONFIG;
3839
import static org.elasticsearch.entitlement.runtime.policy.PathLookup.BaseDir.TEMP;
3940
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ_WRITE;
@@ -70,9 +71,9 @@
7071
* Secondly, the path separator (whether slash or backslash) sorts after the dot character. This means, for example, if the array contains
7172
* {@code ["/a", "/a.xml"]} and we look up {@code "/a/b"} using normal string comparison, the binary search would land on {@code "/a.xml"},
7273
* which is neither an exact match nor a containing directory, and so the lookup would incorrectly report no match, even though
73-
* {@code "/a"} is in the array. To fix this, we define {@link FileUtils#PATH_ORDER} which sorts path separators before any other
74-
* character. In the example, this would cause {@code "/a/b"} to sort between {@code "/a"} and {@code "/a.xml"} so that it correctly
75-
* finds {@code "/a"}.
74+
* {@code "/a"} is in the array. To fix this, we define {@link FileAccessTreeComparison#pathComparator()} which sorts path separators
75+
* before any other character. In the example, this would cause {@code "/a/b"} to sort between {@code "/a"} and {@code "/a.xml"} so that
76+
* it correctly finds {@code "/a"}.
7677
* </p>
7778
* With the paths pruned, sorted, and segregated by permission, each binary search has the following properties:
7879
* <ul>
@@ -118,7 +119,11 @@ public String toString() {
118119
}
119120
}
120121

121-
static List<ExclusivePath> buildExclusivePathList(List<ExclusiveFileEntitlement> exclusiveFileEntitlements, PathLookup pathLookup) {
122+
static List<ExclusivePath> buildExclusivePathList(
123+
List<ExclusiveFileEntitlement> exclusiveFileEntitlements,
124+
PathLookup pathLookup,
125+
FileAccessTreeComparison comparison
126+
) {
122127
Map<String, ExclusivePath> exclusivePaths = new HashMap<>();
123128
for (ExclusiveFileEntitlement efe : exclusiveFileEntitlements) {
124129
for (FilesEntitlement.FileData fd : efe.filesEntitlement().filesData()) {
@@ -150,15 +155,16 @@ static List<ExclusivePath> buildExclusivePathList(List<ExclusiveFileEntitlement>
150155
}
151156
}
152157
}
153-
return exclusivePaths.values().stream().sorted(comparing(ExclusivePath::path, PATH_ORDER)).distinct().toList();
158+
return exclusivePaths.values().stream().sorted(comparing(ExclusivePath::path, comparison.pathComparator())).distinct().toList();
154159
}
155160

156-
static void validateExclusivePaths(List<ExclusivePath> exclusivePaths) {
161+
static void validateExclusivePaths(List<ExclusivePath> exclusivePaths, FileAccessTreeComparison comparison) {
157162
if (exclusivePaths.isEmpty() == false) {
158163
ExclusivePath currentExclusivePath = exclusivePaths.get(0);
159164
for (int i = 1; i < exclusivePaths.size(); ++i) {
160165
ExclusivePath nextPath = exclusivePaths.get(i);
161-
if (currentExclusivePath.path().equals(nextPath.path) || isParent(currentExclusivePath.path(), nextPath.path())) {
166+
if (comparison.samePath(currentExclusivePath.path(), nextPath.path)
167+
|| comparison.isParent(currentExclusivePath.path(), nextPath.path())) {
162168
throw new IllegalArgumentException(
163169
"duplicate/overlapping exclusive paths found in files entitlements: " + currentExclusivePath + " and " + nextPath
164170
);
@@ -168,9 +174,18 @@ static void validateExclusivePaths(List<ExclusivePath> exclusivePaths) {
168174
}
169175
}
170176

177+
@SuppressForbidden(reason = "we need the separator as a char, not a string")
178+
static char separatorChar() {
179+
return File.separatorChar;
180+
}
181+
171182
private static final Logger logger = LogManager.getLogger(FileAccessTree.class);
172183
private static final String FILE_SEPARATOR = getDefaultFileSystem().getSeparator();
184+
static final FileAccessTreeComparison DEFAULT_COMPARISON = Platform.LINUX.isCurrent()
185+
? new CaseSensitiveComparison(separatorChar())
186+
: new CaseInsensitiveComparison(separatorChar());
173187

188+
private final FileAccessTreeComparison comparison;
174189
/**
175190
* lists paths that are forbidden for this component+module because some other component has granted exclusive access to one of its
176191
* modules
@@ -188,19 +203,27 @@ static void validateExclusivePaths(List<ExclusivePath> exclusivePaths) {
188203
private static String[] buildUpdatedAndSortedExclusivePaths(
189204
String componentName,
190205
String moduleName,
191-
List<ExclusivePath> exclusivePaths
206+
List<ExclusivePath> exclusivePaths,
207+
FileAccessTreeComparison comparison
192208
) {
193209
List<String> updatedExclusivePaths = new ArrayList<>();
194210
for (ExclusivePath exclusivePath : exclusivePaths) {
195211
if (exclusivePath.componentName().equals(componentName) == false || exclusivePath.moduleNames().contains(moduleName) == false) {
196212
updatedExclusivePaths.add(exclusivePath.path());
197213
}
198214
}
199-
updatedExclusivePaths.sort(PATH_ORDER);
215+
updatedExclusivePaths.sort(comparison.pathComparator());
200216
return updatedExclusivePaths.toArray(new String[0]);
201217
}
202218

203-
private FileAccessTree(FilesEntitlement filesEntitlement, PathLookup pathLookup, Path componentPath, String[] sortedExclusivePaths) {
219+
FileAccessTree(
220+
FilesEntitlement filesEntitlement,
221+
PathLookup pathLookup,
222+
Path componentPath,
223+
String[] sortedExclusivePaths,
224+
FileAccessTreeComparison comparison
225+
) {
226+
this.comparison = comparison;
204227
List<String> readPaths = new ArrayList<>();
205228
List<String> writePaths = new ArrayList<>();
206229
BiConsumer<Path, Mode> addPath = (path, mode) -> {
@@ -252,12 +275,12 @@ private FileAccessTree(FilesEntitlement filesEntitlement, PathLookup pathLookup,
252275
Path jdk = Paths.get(System.getProperty("java.home"));
253276
addPathAndMaybeLink.accept(jdk.resolve("conf"), Mode.READ);
254277

255-
readPaths.sort(PATH_ORDER);
256-
writePaths.sort(PATH_ORDER);
278+
readPaths.sort(comparison.pathComparator());
279+
writePaths.sort(comparison.pathComparator());
257280

258281
this.exclusivePaths = sortedExclusivePaths;
259-
this.readPaths = pruneSortedPaths(readPaths).toArray(new String[0]);
260-
this.writePaths = pruneSortedPaths(writePaths).toArray(new String[0]);
282+
this.readPaths = pruneSortedPaths(readPaths, comparison).toArray(new String[0]);
283+
this.writePaths = pruneSortedPaths(writePaths, comparison).toArray(new String[0]);
261284

262285
logger.debug(
263286
() -> Strings.format(
@@ -270,14 +293,14 @@ private FileAccessTree(FilesEntitlement filesEntitlement, PathLookup pathLookup,
270293
}
271294

272295
// package private for testing
273-
static List<String> pruneSortedPaths(List<String> paths) {
296+
static List<String> pruneSortedPaths(List<String> paths, FileAccessTreeComparison comparison) {
274297
List<String> prunedReadPaths = new ArrayList<>();
275298
if (paths.isEmpty() == false) {
276299
String currentPath = paths.get(0);
277300
prunedReadPaths.add(currentPath);
278301
for (int i = 1; i < paths.size(); ++i) {
279302
String nextPath = paths.get(i);
280-
if (currentPath.equals(nextPath) == false && isParent(currentPath, nextPath) == false) {
303+
if (comparison.samePath(currentPath, nextPath) == false && comparison.isParent(currentPath, nextPath) == false) {
281304
prunedReadPaths.add(nextPath);
282305
currentPath = nextPath;
283306
}
@@ -298,7 +321,8 @@ static FileAccessTree of(
298321
filesEntitlement,
299322
pathLookup,
300323
componentPath,
301-
buildUpdatedAndSortedExclusivePaths(componentName, moduleName, exclusivePaths)
324+
buildUpdatedAndSortedExclusivePaths(componentName, moduleName, exclusivePaths, DEFAULT_COMPARISON),
325+
DEFAULT_COMPARISON
302326
);
303327
}
304328

@@ -310,7 +334,7 @@ public static FileAccessTree withoutExclusivePaths(
310334
PathLookup pathLookup,
311335
@Nullable Path componentPath
312336
) {
313-
return new FileAccessTree(filesEntitlement, pathLookup, componentPath, new String[0]);
337+
return new FileAccessTree(filesEntitlement, pathLookup, componentPath, new String[0], DEFAULT_COMPARISON);
314338
}
315339

316340
public boolean canRead(Path path) {
@@ -346,24 +370,18 @@ private boolean checkPath(String path, String[] paths) {
346370
return false;
347371
}
348372

349-
int endx = Arrays.binarySearch(exclusivePaths, path, PATH_ORDER);
350-
if (endx < -1 && isParent(exclusivePaths[-endx - 2], path) || endx >= 0) {
373+
int endx = Arrays.binarySearch(exclusivePaths, path, comparison.pathComparator());
374+
if (endx < -1 && comparison.isParent(exclusivePaths[-endx - 2], path) || endx >= 0) {
351375
return false;
352376
}
353377

354-
int ndx = Arrays.binarySearch(paths, path, PATH_ORDER);
378+
int ndx = Arrays.binarySearch(paths, path, comparison.pathComparator());
355379
if (ndx < -1) {
356-
return isParent(paths[-ndx - 2], path);
380+
return comparison.isParent(paths[-ndx - 2], path);
357381
}
358382
return ndx >= 0;
359383
}
360384

361-
private static boolean isParent(String maybeParent, String path) {
362-
var isParent = path.startsWith(maybeParent) && path.startsWith(FILE_SEPARATOR, maybeParent.length());
363-
logger.trace(() -> Strings.format("checking isParent [%s] for [%s]: %b", maybeParent, path, isParent));
364-
return isParent;
365-
}
366-
367385
@Override
368386
public boolean equals(Object o) {
369387
if (o == null || getClass() != o.getClass()) return false;
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
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.runtime.policy;
11+
12+
import org.elasticsearch.core.Strings;
13+
import org.elasticsearch.logging.LogManager;
14+
import org.elasticsearch.logging.Logger;
15+
16+
import java.util.Comparator;
17+
18+
abstract class FileAccessTreeComparison {
19+
20+
private static final Logger logger = LogManager.getLogger(FileAccessTreeComparison.class);
21+
22+
interface CharComparator {
23+
int compare(char c1, char c2);
24+
}
25+
26+
private final Comparator<String> pathComparator;
27+
28+
private final char separatorChar;
29+
30+
/**
31+
* For our lexicographic sort trick to work correctly, we must have path separators sort before
32+
* any other character so that files in a directory appear immediately after that directory.
33+
* For example, we require [/a, /a/b, /a.xml] rather than the natural order [/a, /a.xml, /a/b].
34+
*/
35+
FileAccessTreeComparison(CharComparator charComparator, char separatorChar) {
36+
pathComparator = (s1, s2) -> {
37+
int len1 = s1.length();
38+
int len2 = s2.length();
39+
int lim = Math.min(len1, len2);
40+
for (int k = 0; k < lim; k++) {
41+
char c1 = s1.charAt(k);
42+
char c2 = s2.charAt(k);
43+
var comp = charComparator.compare(c1, c2);
44+
if (comp == 0) {
45+
continue;
46+
}
47+
boolean c1IsSeparator = c1 == separatorChar;
48+
boolean c2IsSeparator = c2 == separatorChar;
49+
if (c1IsSeparator == false || c2IsSeparator == false) {
50+
if (c1IsSeparator) {
51+
return -1;
52+
}
53+
if (c2IsSeparator) {
54+
return 1;
55+
}
56+
return comp;
57+
}
58+
}
59+
return len1 - len2;
60+
};
61+
this.separatorChar = separatorChar;
62+
}
63+
64+
Comparator<String> pathComparator() {
65+
return pathComparator;
66+
}
67+
68+
boolean isParent(String maybeParent, String path) {
69+
logger.trace(() -> Strings.format("checking isParent [%s] for [%s]", maybeParent, path));
70+
return pathStartsWith(path, maybeParent)
71+
&& (path.length() > maybeParent.length() && path.charAt(maybeParent.length()) == separatorChar);
72+
}
73+
74+
boolean samePath(String currentPath, String nextPath) {
75+
return pathsAreEqual(currentPath, nextPath);
76+
}
77+
78+
protected abstract boolean pathStartsWith(String pathString, String pathPrefix);
79+
80+
protected abstract boolean pathsAreEqual(String path1, String path2);
81+
}

0 commit comments

Comments
 (0)