Rethrow NoSuchFileException if encountering an invalid symlink when checking file entitlements (#124483)

This will rethrow the `NoSuchFileException` when encountering an invalid
symbolic link when following links during file (read) entitlement
checks.

Relates to https://github.com/elastic/elasticsearch/pull/124133
(ES-11019)
This commit is contained in:
Moritz Mack 2025-03-11 10:38:03 +01:00 committed by GitHub
parent cda82554aa
commit c26d195120
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 119 additions and 21 deletions

View file

@ -64,6 +64,7 @@ import java.nio.file.FileStore;
import java.nio.file.FileVisitOption;
import java.nio.file.FileVisitor;
import java.nio.file.LinkOption;
import java.nio.file.NoSuchFileException;
import java.nio.file.OpenOption;
import java.nio.file.Path;
import java.nio.file.WatchEvent;
@ -1203,7 +1204,7 @@ public interface EntitlementChecker {
void checkType(Class<?> callerClass, FileStore that);
// path
void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... options);
void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... options) throws NoSuchFileException;
void checkPathRegister(Class<?> callerClass, Path that, WatchService watcher, WatchEvent.Kind<?>... events);

View file

@ -63,7 +63,15 @@ public final class EntitledActions {
}
public static Path createTempSymbolicLink() throws IOException {
return Files.createSymbolicLink(readDir().resolve("entitlements-link-" + random.nextLong()), readWriteDir());
return createTempSymbolicLink(readWriteDir());
}
public static Path createTempSymbolicLink(Path target) throws IOException {
return Files.createSymbolicLink(readDir().resolve("entitlements-link-" + random.nextLong()), target);
}
public static Path pathToRealPath(Path path) throws IOException {
return path.toRealPath();
}
public static Path createK8sLikeMount() throws IOException {

View file

@ -24,6 +24,7 @@ dependencies {
compileOnly project(':server')
compileOnly project(':libs:logging')
compileOnly project(":libs:entitlement:qa:entitled-plugin")
implementation project(":libs:entitlement")
}
tasks.named("javadoc").configure {

View file

@ -11,6 +11,7 @@ module org.elasticsearch.entitlement.qa.test {
requires org.elasticsearch.server;
requires org.elasticsearch.base;
requires org.elasticsearch.logging;
requires org.elasticsearch.entitlement;
requires org.elasticsearch.entitlement.qa.entitled;
// Modules we'll attempt to use in order to exercise entitlements

View file

@ -9,6 +9,8 @@
package org.elasticsearch.entitlement.qa.test;
import org.elasticsearch.entitlement.runtime.api.NotEntitledException;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
@ -27,5 +29,7 @@ public @interface EntitlementTest {
ExpectedAccess expectedAccess();
Class<? extends Exception> expectedExceptionIfDenied() default NotEntitledException.class;
int fromJavaVersion() default -1;
}

View file

@ -10,12 +10,17 @@
package org.elasticsearch.entitlement.qa.test;
import org.elasticsearch.entitlement.qa.entitled.EntitledActions;
import org.elasticsearch.entitlement.runtime.policy.PolicyManager;
import java.io.IOException;
import java.nio.file.FileSystems;
import java.nio.file.LinkOption;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.WatchEvent;
import java.util.Arrays;
import static org.elasticsearch.entitlement.qa.test.EntitlementTest.ExpectedAccess.ALWAYS_DENIED;
import static org.elasticsearch.entitlement.qa.test.EntitlementTest.ExpectedAccess.PLUGINS;
@SuppressWarnings({ "unused" /* called via reflection */, "rawtypes" })
@ -26,6 +31,18 @@ class PathActions {
FileCheckActions.readFile().toRealPath();
}
@EntitlementTest(expectedAccess = ALWAYS_DENIED, expectedExceptionIfDenied = NoSuchFileException.class)
static void checkToRealPathForInvalidTarget() throws IOException {
Path invalidLink = EntitledActions.createTempSymbolicLink(FileCheckActions.readDir().resolve("invalid"));
try {
EntitledActions.pathToRealPath(invalidLink); // throws NoSuchFileException when checking entitlements due to invalid target
} catch (NoSuchFileException e) {
assert Arrays.stream(e.getStackTrace()).anyMatch(t -> t.getClassName().equals(PolicyManager.class.getName()))
: "Expected NoSuchFileException to be thrown by entitlements check";
throw e;
}
}
@EntitlementTest(expectedAccess = PLUGINS)
static void checkToRealPathWithK8sLikeMount() throws IOException, Exception {
EntitledActions.createK8sLikeMount().toRealPath();

View file

@ -13,6 +13,7 @@ import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.common.Strings;
import org.elasticsearch.core.CheckedRunnable;
import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.entitlement.runtime.api.NotEntitledException;
import org.elasticsearch.logging.LogManager;
import org.elasticsearch.logging.Logger;
import org.elasticsearch.rest.BaseRestHandler;
@ -68,20 +69,25 @@ import static org.elasticsearch.rest.RestRequest.Method.GET;
public class RestEntitlementsCheckAction extends BaseRestHandler {
private static final Logger logger = LogManager.getLogger(RestEntitlementsCheckAction.class);
record CheckAction(CheckedRunnable<Exception> action, EntitlementTest.ExpectedAccess expectedAccess, Integer fromJavaVersion) {
record CheckAction(
CheckedRunnable<Exception> action,
EntitlementTest.ExpectedAccess expectedAccess,
Class<? extends Exception> expectedExceptionIfDenied,
Integer fromJavaVersion
) {
/**
* These cannot be granted to plugins, so our test plugins cannot test the "allowed" case.
*/
static CheckAction deniedToPlugins(CheckedRunnable<Exception> action) {
return new CheckAction(action, SERVER_ONLY, null);
return new CheckAction(action, SERVER_ONLY, NotEntitledException.class, null);
}
static CheckAction forPlugins(CheckedRunnable<Exception> action) {
return new CheckAction(action, PLUGINS, null);
return new CheckAction(action, PLUGINS, NotEntitledException.class, null);
}
static CheckAction alwaysDenied(CheckedRunnable<Exception> action) {
return new CheckAction(action, ALWAYS_DENIED, null);
return new CheckAction(action, ALWAYS_DENIED, NotEntitledException.class, null);
}
}
@ -128,7 +134,12 @@ public class RestEntitlementsCheckAction extends BaseRestHandler {
entry("responseCache_setDefault", alwaysDenied(RestEntitlementsCheckAction::setDefaultResponseCache)),
entry(
"createInetAddressResolverProvider",
new CheckAction(VersionSpecificNetworkChecks::createInetAddressResolverProvider, SERVER_ONLY, 18)
new CheckAction(
VersionSpecificNetworkChecks::createInetAddressResolverProvider,
SERVER_ONLY,
NotEntitledException.class,
18
)
),
entry("createURLStreamHandlerProvider", alwaysDenied(RestEntitlementsCheckAction::createURLStreamHandlerProvider)),
entry("createURLWithURLStreamHandler", alwaysDenied(RestEntitlementsCheckAction::createURLWithURLStreamHandler)),
@ -237,7 +248,12 @@ public class RestEntitlementsCheckAction extends BaseRestHandler {
}
};
Integer fromJavaVersion = testAnnotation.fromJavaVersion() == -1 ? null : testAnnotation.fromJavaVersion();
entries.add(entry(method.getName(), new CheckAction(runnable, testAnnotation.expectedAccess(), fromJavaVersion)));
entries.add(
entry(
method.getName(),
new CheckAction(runnable, testAnnotation.expectedAccess(), testAnnotation.expectedExceptionIfDenied(), fromJavaVersion)
)
);
}
return entries.stream();
}
@ -437,9 +453,19 @@ public class RestEntitlementsCheckAction extends BaseRestHandler {
return channel -> {
logger.info("Calling check action [{}]", actionName);
RestResponse response;
try {
checkAction.action().run();
logger.debug("Check action [{}] returned", actionName);
channel.sendResponse(new RestResponse(RestStatus.OK, Strings.format("Succesfully executed action [%s]", actionName)));
response = new RestResponse(RestStatus.OK, Strings.format("Succesfully executed action [%s]", actionName));
} catch (Exception e) {
var statusCode = checkAction.expectedExceptionIfDenied.isInstance(e)
? RestStatus.FORBIDDEN
: RestStatus.INTERNAL_SERVER_ERROR;
response = new RestResponse(channel, statusCode, e);
response.addHeader("expectedException", checkAction.expectedExceptionIfDenied.getName());
}
logger.debug("Check action [{}] returned status [{}]", actionName, response.status().getStatus());
channel.sendResponse(response);
};
}

View file

@ -11,14 +11,17 @@ package org.elasticsearch.entitlement.qa;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.entitlement.qa.EntitlementsTestRule.PolicyBuilder;
import org.elasticsearch.test.rest.ESRestTestCase;
import org.hamcrest.Description;
import org.hamcrest.Matcher;
import org.hamcrest.TypeSafeMatcher;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
public abstract class AbstractEntitlementsIT extends ESRestTestCase {
@ -69,8 +72,34 @@ public abstract class AbstractEntitlementsIT extends ESRestTestCase {
Response result = executeCheck();
assertThat(result.getStatusLine().getStatusCode(), equalTo(200));
} else {
var exception = expectThrows(IOException.class, this::executeCheck);
assertThat(exception.getMessage(), containsString("not_entitled_exception"));
var exception = expectThrows(ResponseException.class, this::executeCheck);
assertThat(exception, statusCodeMatcher(403));
}
}
private static Matcher<ResponseException> statusCodeMatcher(int statusCode) {
return new TypeSafeMatcher<>() {
String expectedException = null;
@Override
protected boolean matchesSafely(ResponseException item) {
Response resp = item.getResponse();
expectedException = resp.getHeader("expectedException");
return resp.getStatusLine().getStatusCode() == statusCode && expectedException != null;
}
@Override
public void describeTo(Description description) {
description.appendValue(statusCode).appendText(" due to ").appendText(expectedException);
}
@Override
protected void describeMismatchSafely(ResponseException item, Description description) {
description.appendText("was ")
.appendValue(item.getResponse().getStatusLine().getStatusCode())
.appendText("\n")
.appendValue(item.getMessage());
}
};
}
}

View file

@ -74,6 +74,7 @@ import java.nio.file.FileStore;
import java.nio.file.FileVisitOption;
import java.nio.file.FileVisitor;
import java.nio.file.LinkOption;
import java.nio.file.NoSuchFileException;
import java.nio.file.OpenOption;
import java.nio.file.Path;
import java.nio.file.Paths;
@ -2744,7 +2745,7 @@ public class ElasticsearchEntitlementChecker implements EntitlementChecker {
}
@Override
public void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... options) {
public void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... options) throws NoSuchFileException {
boolean followLinks = true;
for (LinkOption option : options) {
if (option == LinkOption.NOFOLLOW_LINKS) {

View file

@ -35,6 +35,7 @@ import java.io.IOException;
import java.lang.StackWalker.StackFrame;
import java.lang.module.ModuleFinder;
import java.lang.module.ModuleReference;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
@ -326,10 +327,17 @@ public class PolicyManager {
}
public void checkFileRead(Class<?> callerClass, Path path) {
try {
checkFileRead(callerClass, path, false);
} catch (NoSuchFileException e) {
assert false : "NoSuchFileException should only be thrown when following links";
var notEntitledException = new NotEntitledException(e.getMessage());
notEntitledException.addSuppressed(e);
throw notEntitledException;
}
}
public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks) {
public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks) throws NoSuchFileException {
if (isPathOnDefaultFilesystem(path) == false) {
return;
}
@ -345,12 +353,14 @@ public class PolicyManager {
if (canRead && followLinks) {
try {
realPath = path.toRealPath();
} catch (IOException e) {
// target not found or other IO error
}
if (realPath != null && realPath.equals(path) == false) {
if (realPath.equals(path) == false) {
canRead = entitlements.fileAccess().canRead(realPath);
}
} catch (NoSuchFileException e) {
throw e; // rethrow
} catch (IOException e) {
canRead = false;
}
}
if (canRead == false) {