Skip to content

Commit f790cb8

Browse files
moschemosche
authored
Rethrow NoSuchFileException if encountering an invalid symlink when checking file entitlements (#124483) (#124541)
This will rethrow the `NoSuchFileException` when encountering an invalid symbolic link when following links during file (read) entitlement checks. Relates to #124133 (ES-11019)
1 parent 0aaa8cd commit f790cb8

File tree

10 files changed

+119
-21
lines changed

10 files changed

+119
-21
lines changed

‎libs/entitlement/bridge/src/main/java/org/elasticsearch/entitlement/bridge/EntitlementChecker.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import java.nio.file.FileVisitOption;
5858
import java.nio.file.FileVisitor;
5959
import java.nio.file.LinkOption;
60+
import java.nio.file.NoSuchFileException;
6061
import java.nio.file.OpenOption;
6162
import java.nio.file.Path;
6263
import java.nio.file.WatchEvent;
@@ -1137,7 +1138,7 @@ void checkNewByteChannel(
11371138
void checkType(Class<?> callerClass, FileStore that);
11381139

11391140
// path
1140-
void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... options);
1141+
void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... options) throws NoSuchFileException;
11411142

11421143
void checkPathRegister(Class<?> callerClass, Path that, WatchService watcher, WatchEvent.Kind<?>... events);
11431144

‎libs/entitlement/qa/entitled-plugin/src/main/java/org/elasticsearch/entitlement/qa/entitled/EntitledActions.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,15 @@ public static Path createTempDirectoryForWrite() throws IOException {
6363
}
6464

6565
public static Path createTempSymbolicLink() throws IOException {
66-
return Files.createSymbolicLink(readDir().resolve("entitlements-link-" + random.nextLong()), readWriteDir());
66+
return createTempSymbolicLink(readWriteDir());
67+
}
68+
69+
public static Path createTempSymbolicLink(Path target) throws IOException {
70+
return Files.createSymbolicLink(readDir().resolve("entitlements-link-" + random.nextLong()), target);
71+
}
72+
73+
public static Path pathToRealPath(Path path) throws IOException {
74+
return path.toRealPath();
6775
}
6876

6977
public static Path createK8sLikeMount() throws IOException {

‎libs/entitlement/qa/entitlement-test-plugin/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ dependencies {
2424
compileOnly project(':server')
2525
compileOnly project(':libs:logging')
2626
compileOnly project(":libs:entitlement:qa:entitled-plugin")
27+
implementation project(":libs:entitlement")
2728
}
2829

2930
tasks.named("javadoc").configure {

‎libs/entitlement/qa/entitlement-test-plugin/src/main/java/module-info.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
requires org.elasticsearch.server;
1212
requires org.elasticsearch.base;
1313
requires org.elasticsearch.logging;
14+
requires org.elasticsearch.entitlement;
1415
requires org.elasticsearch.entitlement.qa.entitled;
1516

1617
// Modules we'll attempt to use in order to exercise entitlements

‎libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/EntitlementTest.java‎

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

1010
package org.elasticsearch.entitlement.qa.test;
1111

12+
import org.elasticsearch.entitlement.runtime.api.NotEntitledException;
13+
1214
import java.lang.annotation.ElementType;
1315
import java.lang.annotation.Retention;
1416
import java.lang.annotation.RetentionPolicy;
@@ -27,5 +29,7 @@ enum ExpectedAccess {
2729

2830
ExpectedAccess expectedAccess();
2931

32+
Class<? extends Exception> expectedExceptionIfDenied() default NotEntitledException.class;
33+
3034
int fromJavaVersion() default -1;
3135
}

‎libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/PathActions.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,17 @@
1010
package org.elasticsearch.entitlement.qa.test;
1111

1212
import org.elasticsearch.entitlement.qa.entitled.EntitledActions;
13+
import org.elasticsearch.entitlement.runtime.policy.PolicyManager;
1314

1415
import java.io.IOException;
1516
import java.nio.file.FileSystems;
1617
import java.nio.file.LinkOption;
18+
import java.nio.file.NoSuchFileException;
19+
import java.nio.file.Path;
1720
import java.nio.file.WatchEvent;
21+
import java.util.Arrays;
1822

23+
import static org.elasticsearch.entitlement.qa.test.EntitlementTest.ExpectedAccess.ALWAYS_DENIED;
1924
import static org.elasticsearch.entitlement.qa.test.EntitlementTest.ExpectedAccess.PLUGINS;
2025

2126
@SuppressWarnings({ "unused" /* called via reflection */, "rawtypes" })
@@ -26,6 +31,18 @@ static void checkToRealPath() throws IOException {
2631
FileCheckActions.readFile().toRealPath();
2732
}
2833

34+
@EntitlementTest(expectedAccess = ALWAYS_DENIED, expectedExceptionIfDenied = NoSuchFileException.class)
35+
static void checkToRealPathForInvalidTarget() throws IOException {
36+
Path invalidLink = EntitledActions.createTempSymbolicLink(FileCheckActions.readDir().resolve("invalid"));
37+
try {
38+
EntitledActions.pathToRealPath(invalidLink); // throws NoSuchFileException when checking entitlements due to invalid target
39+
} catch (NoSuchFileException e) {
40+
assert Arrays.stream(e.getStackTrace()).anyMatch(t -> t.getClassName().equals(PolicyManager.class.getName()))
41+
: "Expected NoSuchFileException to be thrown by entitlements check";
42+
throw e;
43+
}
44+
}
45+
2946
@EntitlementTest(expectedAccess = PLUGINS)
3047
static void checkToRealPathWithK8sLikeMount() throws IOException, Exception {
3148
EntitledActions.createK8sLikeMount().toRealPath();

‎libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java‎

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.common.Strings;
1414
import org.elasticsearch.core.CheckedRunnable;
1515
import org.elasticsearch.core.SuppressForbidden;
16+
import org.elasticsearch.entitlement.runtime.api.NotEntitledException;
1617
import org.elasticsearch.logging.LogManager;
1718
import org.elasticsearch.logging.Logger;
1819
import org.elasticsearch.rest.BaseRestHandler;
@@ -68,20 +69,25 @@
6869
public class RestEntitlementsCheckAction extends BaseRestHandler {
6970
private static final Logger logger = LogManager.getLogger(RestEntitlementsCheckAction.class);
7071

71-
record CheckAction(CheckedRunnable<Exception> action, EntitlementTest.ExpectedAccess expectedAccess, Integer fromJavaVersion) {
72+
record CheckAction(
73+
CheckedRunnable<Exception> action,
74+
EntitlementTest.ExpectedAccess expectedAccess,
75+
Class<? extends Exception> expectedExceptionIfDenied,
76+
Integer fromJavaVersion
77+
) {
7278
/**
7379
* These cannot be granted to plugins, so our test plugins cannot test the "allowed" case.
7480
*/
7581
static CheckAction deniedToPlugins(CheckedRunnable<Exception> action) {
76-
return new CheckAction(action, SERVER_ONLY, null);
82+
return new CheckAction(action, SERVER_ONLY, NotEntitledException.class, null);
7783
}
7884

7985
static CheckAction forPlugins(CheckedRunnable<Exception> action) {
80-
return new CheckAction(action, PLUGINS, null);
86+
return new CheckAction(action, PLUGINS, NotEntitledException.class, null);
8187
}
8288

8389
static CheckAction alwaysDenied(CheckedRunnable<Exception> action) {
84-
return new CheckAction(action, ALWAYS_DENIED, null);
90+
return new CheckAction(action, ALWAYS_DENIED, NotEntitledException.class, null);
8591
}
8692
}
8793

@@ -128,7 +134,12 @@ static CheckAction alwaysDenied(CheckedRunnable<Exception> action) {
128134
entry("responseCache_setDefault", alwaysDenied(RestEntitlementsCheckAction::setDefaultResponseCache)),
129135
entry(
130136
"createInetAddressResolverProvider",
131-
new CheckAction(VersionSpecificNetworkChecks::createInetAddressResolverProvider, SERVER_ONLY, 18)
137+
new CheckAction(
138+
VersionSpecificNetworkChecks::createInetAddressResolverProvider,
139+
SERVER_ONLY,
140+
NotEntitledException.class,
141+
18
142+
)
132143
),
133144
entry("createURLStreamHandlerProvider", alwaysDenied(RestEntitlementsCheckAction::createURLStreamHandlerProvider)),
134145
entry("createURLWithURLStreamHandler", alwaysDenied(RestEntitlementsCheckAction::createURLWithURLStreamHandler)),
@@ -239,7 +250,12 @@ private static Stream<Entry<String, CheckAction>> getTestEntries(Class<?> action
239250
}
240251
};
241252
Integer fromJavaVersion = testAnnotation.fromJavaVersion() == -1 ? null : testAnnotation.fromJavaVersion();
242-
entries.add(entry(method.getName(), new CheckAction(runnable, testAnnotation.expectedAccess(), fromJavaVersion)));
253+
entries.add(
254+
entry(
255+
method.getName(),
256+
new CheckAction(runnable, testAnnotation.expectedAccess(), testAnnotation.expectedExceptionIfDenied(), fromJavaVersion)
257+
)
258+
);
243259
}
244260
return entries.stream();
245261
}
@@ -439,9 +455,19 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
439455

440456
return channel -> {
441457
logger.info("Calling check action [{}]", actionName);
442-
checkAction.action().run();
443-
logger.debug("Check action [{}] returned", actionName);
444-
channel.sendResponse(new RestResponse(RestStatus.OK, Strings.format("Succesfully executed action [%s]", actionName)));
458+
RestResponse response;
459+
try {
460+
checkAction.action().run();
461+
response = new RestResponse(RestStatus.OK, Strings.format("Succesfully executed action [%s]", actionName));
462+
} catch (Exception e) {
463+
var statusCode = checkAction.expectedExceptionIfDenied.isInstance(e)
464+
? RestStatus.FORBIDDEN
465+
: RestStatus.INTERNAL_SERVER_ERROR;
466+
response = new RestResponse(channel, statusCode, e);
467+
response.addHeader("expectedException", checkAction.expectedExceptionIfDenied.getName());
468+
}
469+
logger.debug("Check action [{}] returned status [{}]", actionName, response.status().getStatus());
470+
channel.sendResponse(response);
445471
};
446472
}
447473

‎libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,17 @@
1111

1212
import org.elasticsearch.client.Request;
1313
import org.elasticsearch.client.Response;
14+
import org.elasticsearch.client.ResponseException;
1415
import org.elasticsearch.entitlement.qa.EntitlementsTestRule.PolicyBuilder;
1516
import org.elasticsearch.test.rest.ESRestTestCase;
17+
import org.hamcrest.Description;
18+
import org.hamcrest.Matcher;
19+
import org.hamcrest.TypeSafeMatcher;
1620

1721
import java.io.IOException;
1822
import java.util.List;
1923
import java.util.Map;
2024

21-
import static org.hamcrest.Matchers.containsString;
2225
import static org.hamcrest.Matchers.equalTo;
2326

2427
public abstract class AbstractEntitlementsIT extends ESRestTestCase {
@@ -69,8 +72,34 @@ public void testAction() throws IOException {
6972
Response result = executeCheck();
7073
assertThat(result.getStatusLine().getStatusCode(), equalTo(200));
7174
} else {
72-
var exception = expectThrows(IOException.class, this::executeCheck);
73-
assertThat(exception.getMessage(), containsString("not_entitled_exception"));
75+
var exception = expectThrows(ResponseException.class, this::executeCheck);
76+
assertThat(exception, statusCodeMatcher(403));
7477
}
7578
}
79+
80+
private static Matcher<ResponseException> statusCodeMatcher(int statusCode) {
81+
return new TypeSafeMatcher<>() {
82+
String expectedException = null;
83+
84+
@Override
85+
protected boolean matchesSafely(ResponseException item) {
86+
Response resp = item.getResponse();
87+
expectedException = resp.getHeader("expectedException");
88+
return resp.getStatusLine().getStatusCode() == statusCode && expectedException != null;
89+
}
90+
91+
@Override
92+
public void describeTo(Description description) {
93+
description.appendValue(statusCode).appendText(" due to ").appendText(expectedException);
94+
}
95+
96+
@Override
97+
protected void describeMismatchSafely(ResponseException item, Description description) {
98+
description.appendText("was ")
99+
.appendValue(item.getResponse().getStatusLine().getStatusCode())
100+
.appendText("\n")
101+
.appendValue(item.getMessage());
102+
}
103+
};
104+
}
76105
}

‎libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/api/ElasticsearchEntitlementChecker.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
import java.nio.file.FileVisitOption;
6868
import java.nio.file.FileVisitor;
6969
import java.nio.file.LinkOption;
70+
import java.nio.file.NoSuchFileException;
7071
import java.nio.file.OpenOption;
7172
import java.nio.file.Path;
7273
import java.nio.file.Paths;
@@ -2641,7 +2642,7 @@ public void checkType(Class<?> callerClass, FileStore that) {
26412642
}
26422643

26432644
@Override
2644-
public void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... options) {
2645+
public void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... options) throws NoSuchFileException {
26452646
boolean followLinks = true;
26462647
for (LinkOption option : options) {
26472648
if (option == LinkOption.NOFOLLOW_LINKS) {

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.lang.StackWalker.StackFrame;
3636
import java.lang.module.ModuleFinder;
3737
import java.lang.module.ModuleReference;
38+
import java.nio.file.NoSuchFileException;
3839
import java.nio.file.Path;
3940
import java.nio.file.Paths;
4041
import java.util.ArrayList;
@@ -326,10 +327,17 @@ private static boolean isPathOnDefaultFilesystem(Path path) {
326327
}
327328

328329
public void checkFileRead(Class<?> callerClass, Path path) {
329-
checkFileRead(callerClass, path, false);
330+
try {
331+
checkFileRead(callerClass, path, false);
332+
} catch (NoSuchFileException e) {
333+
assert false : "NoSuchFileException should only be thrown when following links";
334+
var notEntitledException = new NotEntitledException(e.getMessage());
335+
notEntitledException.addSuppressed(e);
336+
throw notEntitledException;
337+
}
330338
}
331339

332-
public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks) {
340+
public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks) throws NoSuchFileException {
333341
if (isPathOnDefaultFilesystem(path) == false) {
334342
return;
335343
}
@@ -345,11 +353,13 @@ public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks)
345353
if (canRead && followLinks) {
346354
try {
347355
realPath = path.toRealPath();
356+
if (realPath.equals(path) == false) {
357+
canRead = entitlements.fileAccess().canRead(realPath);
358+
}
359+
} catch (NoSuchFileException e) {
360+
throw e; // rethrow
348361
} catch (IOException e) {
349-
// target not found or other IO error
350-
}
351-
if (realPath != null && realPath.equals(path) == false) {
352-
canRead = entitlements.fileAccess().canRead(realPath);
362+
canRead = false;
353363
}
354364
}
355365

0 commit comments

Comments
 (0)