From 4c1877b333e95ab8de9243ea8c08367ff61c98ae Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Mon, 10 Mar 2025 14:20:36 +0100 Subject: [PATCH 1/4] Rethrow NoSuchFileException if encountering an invalid symlink when checking file entitlements --- .../bridge/EntitlementChecker.java | 3 +- .../qa/entitled/EntitledActions.java | 6 ++- .../qa/entitlement-test-plugin/build.gradle | 1 + .../src/main/java/module-info.java | 1 + .../entitlement/qa/test/EntitlementTest.java | 4 ++ .../entitlement/qa/test/FileCheckActions.java | 4 ++ .../entitlement/qa/test/PathActions.java | 8 ++++ .../qa/test/RestEntitlementsCheckAction.java | 41 +++++++++++++++---- .../qa/AbstractEntitlementsIT.java | 33 +++++++++++++-- .../entitlement/qa/EntitlementsDeniedIT.java | 2 +- .../api/ElasticsearchEntitlementChecker.java | 3 +- .../runtime/policy/PolicyManager.java | 22 +++++++--- 12 files changed, 107 insertions(+), 21 deletions(-) diff --git a/libs/entitlement/bridge/src/main/java/org/elasticsearch/entitlement/bridge/EntitlementChecker.java b/libs/entitlement/bridge/src/main/java/org/elasticsearch/entitlement/bridge/EntitlementChecker.java index 4ead99356ab00..85fc0235d0aeb 100644 --- a/libs/entitlement/bridge/src/main/java/org/elasticsearch/entitlement/bridge/EntitlementChecker.java +++ b/libs/entitlement/bridge/src/main/java/org/elasticsearch/entitlement/bridge/EntitlementChecker.java @@ -64,6 +64,7 @@ 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 @@ void checkNewByteChannel( 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); diff --git a/libs/entitlement/qa/entitled-plugin/src/main/java/org/elasticsearch/entitlement/qa/entitled/EntitledActions.java b/libs/entitlement/qa/entitled-plugin/src/main/java/org/elasticsearch/entitlement/qa/entitled/EntitledActions.java index aa2e3497c3c04..a33d2a56d2615 100644 --- a/libs/entitlement/qa/entitled-plugin/src/main/java/org/elasticsearch/entitlement/qa/entitled/EntitledActions.java +++ b/libs/entitlement/qa/entitled-plugin/src/main/java/org/elasticsearch/entitlement/qa/entitled/EntitledActions.java @@ -63,7 +63,11 @@ public static Path createTempDirectoryForWrite() throws IOException { } public static Path createTempSymbolicLink() throws IOException { - return Files.createSymbolicLink(readDir().resolve("entitlements-link-" + random.nextLong()), readWriteDir()); + return createTempSymbolicLink(readDir().resolve("entitlements-link-" + random.nextLong()), readWriteDir()); + } + + public static Path createTempSymbolicLink(Path link, Path target) throws IOException { + return Files.createSymbolicLink(link, target); } public static Path createK8sLikeMount() throws IOException { diff --git a/libs/entitlement/qa/entitlement-test-plugin/build.gradle b/libs/entitlement/qa/entitlement-test-plugin/build.gradle index 3ee9b510089ba..d678ac7a50a32 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/build.gradle +++ b/libs/entitlement/qa/entitlement-test-plugin/build.gradle @@ -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 { diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/module-info.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/module-info.java index ee2ae33d34890..e59fb20a54861 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/module-info.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/module-info.java @@ -11,6 +11,7 @@ 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 diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/EntitlementTest.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/EntitlementTest.java index ee4dfa26743bc..840a031b4b672 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/EntitlementTest.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/EntitlementTest.java @@ -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 @@ enum ExpectedAccess { ExpectedAccess expectedAccess(); + Class expectedExceptionIfDenied() default NotEntitledException.class; + int fromJavaVersion() default -1; } diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/FileCheckActions.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/FileCheckActions.java index b22643c90064e..b288b4b0039c9 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/FileCheckActions.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/FileCheckActions.java @@ -62,6 +62,10 @@ static Path readFile() { return testRootDir.resolve("read_file"); } + static Path readFileAlwaysAllowed() { + return readDir().resolve("always_allowed"); + } + static Path readWriteFile() { return testRootDir.resolve("read_write_file"); } diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/PathActions.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/PathActions.java index 63cdbd6c9d19c..c01cacaf49208 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/PathActions.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/PathActions.java @@ -14,8 +14,10 @@ import java.io.IOException; import java.nio.file.FileSystems; import java.nio.file.LinkOption; +import java.nio.file.NoSuchFileException; import java.nio.file.WatchEvent; +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 +28,12 @@ static void checkToRealPath() throws IOException { FileCheckActions.readFile().toRealPath(); } + @EntitlementTest(expectedAccess = ALWAYS_DENIED, expectedExceptionIfDenied = NoSuchFileException.class) + static void checkToRealPathForInvalidTarget() throws IOException { + EntitledActions.createTempSymbolicLink(FileCheckActions.readFileAlwaysAllowed(), FileCheckActions.readDir().resolve("invalid")) + .toRealPath(); + } + @EntitlementTest(expectedAccess = PLUGINS) static void checkToRealPathWithK8sLikeMount() throws IOException, Exception { EntitledActions.createK8sLikeMount().toRealPath(); diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java index 5af9df8f749a6..199343709934d 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java @@ -13,6 +13,7 @@ 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 @@ public class RestEntitlementsCheckAction extends BaseRestHandler { private static final Logger logger = LogManager.getLogger(RestEntitlementsCheckAction.class); - record CheckAction(CheckedRunnable action, EntitlementTest.ExpectedAccess expectedAccess, Integer fromJavaVersion) { + record CheckAction( + CheckedRunnable action, + EntitlementTest.ExpectedAccess expectedAccess, + Class expectedExceptionIfDenied, + Integer fromJavaVersion + ) { /** * These cannot be granted to plugins, so our test plugins cannot test the "allowed" case. */ static CheckAction deniedToPlugins(CheckedRunnable action) { - return new CheckAction(action, SERVER_ONLY, null); + return new CheckAction(action, SERVER_ONLY, NotEntitledException.class, null); } static CheckAction forPlugins(CheckedRunnable action) { - return new CheckAction(action, PLUGINS, null); + return new CheckAction(action, PLUGINS, NotEntitledException.class, null); } static CheckAction alwaysDenied(CheckedRunnable action) { - return new CheckAction(action, ALWAYS_DENIED, null); + return new CheckAction(action, ALWAYS_DENIED, NotEntitledException.class, null); } } @@ -128,7 +134,12 @@ static CheckAction alwaysDenied(CheckedRunnable action) { 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 @@ private static Stream> getTestEntries(Class action } }; 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(); } @@ -436,8 +452,17 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli } return channel -> { - logger.info("Calling check action [{}]", actionName); - checkAction.action().run(); + logger.info("Calling check action [{}]", checkAction); + try { + checkAction.action().run(); + } catch (Exception e) { + if (checkAction.expectedExceptionIfDenied.isInstance(e) == false) { + throw e; + } + logger.debug("Check action [{}] denied with expected exception", actionName, e); + channel.sendResponse(new RestResponse(channel, RestStatus.FORBIDDEN, e)); + return; + } logger.debug("Check action [{}] returned", actionName); channel.sendResponse(new RestResponse(RestStatus.OK, Strings.format("Succesfully executed action [%s]", actionName))); }; diff --git a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java index bd88c23fc5b91..3354ae76e9c6d 100644 --- a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java +++ b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java @@ -11,18 +11,25 @@ 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 { + static final PolicyBuilder ALWAYS_ALLOWED_TEST_ENTITLEMENTS = (builder, tempDir) -> builder.value( + Map.of("files", List.of(Map.of("path", tempDir.resolve("read_dir").resolve("always_allowed"), "mode", "read"))) + ); + static final PolicyBuilder ALLOWED_TEST_ENTITLEMENTS = (builder, tempDir) -> { builder.value("create_class_loader"); builder.value("set_https_connection_properties"); @@ -69,8 +76,28 @@ public void testAction() throws IOException { 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, FORBIDDEN_STATUS_CODE_MATCHER); } } + + private static final Matcher FORBIDDEN_STATUS_CODE_MATCHER = new TypeSafeMatcher<>() { + @Override + protected boolean matchesSafely(ResponseException item) { + return item.getResponse().getStatusLine().getStatusCode() == 403; + } + + @Override + public void describeTo(Description description) { + description.appendText("403 (FORBIDDEN)"); + } + + @Override + protected void describeMismatchSafely(ResponseException item, Description description) { + description.appendText("was ") + .appendValue(item.getResponse().getStatusLine().getStatusCode()) + .appendText("\n") + .appendValue(item.getMessage()); + } + }; } diff --git a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedIT.java b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedIT.java index 5d31afbd8a5b3..97c5f93d7e3fd 100644 --- a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedIT.java +++ b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedIT.java @@ -18,7 +18,7 @@ public class EntitlementsDeniedIT extends AbstractEntitlementsIT { @ClassRule - public static EntitlementsTestRule testRule = new EntitlementsTestRule(true, null); + public static EntitlementsTestRule testRule = new EntitlementsTestRule(true, ALWAYS_ALLOWED_TEST_ENTITLEMENTS); public EntitlementsDeniedIT(@Name("actionName") String actionName) { super(actionName, false); diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/api/ElasticsearchEntitlementChecker.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/api/ElasticsearchEntitlementChecker.java index a3851fbd59004..f891f2efc5419 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/api/ElasticsearchEntitlementChecker.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/api/ElasticsearchEntitlementChecker.java @@ -74,6 +74,7 @@ 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 void checkType(Class callerClass, FileStore that) { } @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) { diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java index 7aaa038600bbc..175fe2b5219cf 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java @@ -35,6 +35,7 @@ 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 @@ private static boolean isPathOnDefaultFilesystem(Path path) { } public void checkFileRead(Class callerClass, Path path) { - checkFileRead(callerClass, path, false); + 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,11 +353,13 @@ public void checkFileRead(Class callerClass, Path path, boolean followLinks) if (canRead && followLinks) { try { realPath = path.toRealPath(); + if (realPath.equals(path) == false) { + canRead = entitlements.fileAccess().canRead(realPath); + } + } catch (NoSuchFileException e) { + throw e; // rethrow } catch (IOException e) { - // target not found or other IO error - } - if (realPath != null && realPath.equals(path) == false) { - canRead = entitlements.fileAccess().canRead(realPath); + canRead = false; } } From f0c27181ec26965bb06c640a4efc9a1538280ebc Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Mon, 10 Mar 2025 15:54:55 +0100 Subject: [PATCH 2/4] Allow EntitlementsDeniedNonModularIT --- .../entitlement/qa/EntitlementsDeniedNonModularIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedNonModularIT.java b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedNonModularIT.java index ece18d4830387..057eb1d1d4d9f 100644 --- a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedNonModularIT.java +++ b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedNonModularIT.java @@ -18,7 +18,7 @@ public class EntitlementsDeniedNonModularIT extends AbstractEntitlementsIT { @ClassRule - public static EntitlementsTestRule testRule = new EntitlementsTestRule(false, null); + public static EntitlementsTestRule testRule = new EntitlementsTestRule(false, ALWAYS_ALLOWED_TEST_ENTITLEMENTS); public EntitlementsDeniedNonModularIT(@Name("actionName") String actionName) { super(actionName, false); From 00a09bd8b9ca239f8ffeba20a186a8cbde348c85 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Tue, 11 Mar 2025 09:27:35 +0100 Subject: [PATCH 3/4] feedback --- .../qa/entitled/EntitledActions.java | 10 ++-- .../entitlement/qa/test/FileCheckActions.java | 4 -- .../entitlement/qa/test/PathActions.java | 13 +++++- .../qa/test/RestEntitlementsCheckAction.java | 19 ++++---- .../qa/AbstractEntitlementsIT.java | 46 ++++++++++--------- .../entitlement/qa/EntitlementsDeniedIT.java | 2 +- .../qa/EntitlementsDeniedNonModularIT.java | 2 +- 7 files changed, 54 insertions(+), 42 deletions(-) diff --git a/libs/entitlement/qa/entitled-plugin/src/main/java/org/elasticsearch/entitlement/qa/entitled/EntitledActions.java b/libs/entitlement/qa/entitled-plugin/src/main/java/org/elasticsearch/entitlement/qa/entitled/EntitledActions.java index a33d2a56d2615..289ce2dc2fe32 100644 --- a/libs/entitlement/qa/entitled-plugin/src/main/java/org/elasticsearch/entitlement/qa/entitled/EntitledActions.java +++ b/libs/entitlement/qa/entitled-plugin/src/main/java/org/elasticsearch/entitlement/qa/entitled/EntitledActions.java @@ -63,11 +63,15 @@ public static Path createTempDirectoryForWrite() throws IOException { } public static Path createTempSymbolicLink() throws IOException { - return createTempSymbolicLink(readDir().resolve("entitlements-link-" + random.nextLong()), readWriteDir()); + return createTempSymbolicLink(readWriteDir()); } - public static Path createTempSymbolicLink(Path link, Path target) throws IOException { - return Files.createSymbolicLink(link, target); + 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 { diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/FileCheckActions.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/FileCheckActions.java index 457e4d2eba42d..eb8e572b53532 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/FileCheckActions.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/FileCheckActions.java @@ -62,10 +62,6 @@ static Path readFile() { return testRootDir.resolve("read_file"); } - static Path readFileAlwaysAllowed() { - return readDir().resolve("always_allowed"); - } - static Path readWriteFile() { return testRootDir.resolve("read_write_file"); } diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/PathActions.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/PathActions.java index c01cacaf49208..fa75395f62209 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/PathActions.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/PathActions.java @@ -10,12 +10,15 @@ 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; @@ -30,8 +33,14 @@ static void checkToRealPath() throws IOException { @EntitlementTest(expectedAccess = ALWAYS_DENIED, expectedExceptionIfDenied = NoSuchFileException.class) static void checkToRealPathForInvalidTarget() throws IOException { - EntitledActions.createTempSymbolicLink(FileCheckActions.readFileAlwaysAllowed(), FileCheckActions.readDir().resolve("invalid")) - .toRealPath(); + 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) diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java index 199343709934d..6905037b2f235 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java @@ -452,19 +452,20 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli } return channel -> { - logger.info("Calling check action [{}]", checkAction); + logger.info("Calling check action [{}]", actionName); + RestResponse response; try { checkAction.action().run(); + response = new RestResponse(RestStatus.OK, Strings.format("Succesfully executed action [%s]", actionName)); } catch (Exception e) { - if (checkAction.expectedExceptionIfDenied.isInstance(e) == false) { - throw e; - } - logger.debug("Check action [{}] denied with expected exception", actionName, e); - channel.sendResponse(new RestResponse(channel, RestStatus.FORBIDDEN, e)); - return; + 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", actionName); - channel.sendResponse(new RestResponse(RestStatus.OK, Strings.format("Succesfully executed action [%s]", actionName))); + logger.debug("Check action [{}] returned status [{}]", actionName, response.status().getStatus()); + channel.sendResponse(response); }; } diff --git a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java index 3354ae76e9c6d..c9b8965465829 100644 --- a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java +++ b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java @@ -26,10 +26,6 @@ public abstract class AbstractEntitlementsIT extends ESRestTestCase { - static final PolicyBuilder ALWAYS_ALLOWED_TEST_ENTITLEMENTS = (builder, tempDir) -> builder.value( - Map.of("files", List.of(Map.of("path", tempDir.resolve("read_dir").resolve("always_allowed"), "mode", "read"))) - ); - static final PolicyBuilder ALLOWED_TEST_ENTITLEMENTS = (builder, tempDir) -> { builder.value("create_class_loader"); builder.value("set_https_connection_properties"); @@ -77,27 +73,33 @@ public void testAction() throws IOException { assertThat(result.getStatusLine().getStatusCode(), equalTo(200)); } else { var exception = expectThrows(ResponseException.class, this::executeCheck); - assertThat(exception, FORBIDDEN_STATUS_CODE_MATCHER); + assertThat(exception, statusCodeMatcher(403)); } } - private static final Matcher FORBIDDEN_STATUS_CODE_MATCHER = new TypeSafeMatcher<>() { - @Override - protected boolean matchesSafely(ResponseException item) { - return item.getResponse().getStatusLine().getStatusCode() == 403; - } + private static Matcher statusCodeMatcher(int statusCode) { + return new TypeSafeMatcher<>() { + String expectedException = null; - @Override - public void describeTo(Description description) { - description.appendText("403 (FORBIDDEN)"); - } + @Override + protected boolean matchesSafely(ResponseException item) { + Response resp = item.getResponse(); + expectedException = resp.getHeader("expectedException"); + return resp.getStatusLine().getStatusCode() == statusCode && expectedException != null; + } - @Override - protected void describeMismatchSafely(ResponseException item, Description description) { - description.appendText("was ") - .appendValue(item.getResponse().getStatusLine().getStatusCode()) - .appendText("\n") - .appendValue(item.getMessage()); - } - }; + @Override + public void describeTo(Description description) { + description.appendText("403 (FORBIDDEN)").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()); + } + }; + } } diff --git a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedIT.java b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedIT.java index 97c5f93d7e3fd..5d31afbd8a5b3 100644 --- a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedIT.java +++ b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedIT.java @@ -18,7 +18,7 @@ public class EntitlementsDeniedIT extends AbstractEntitlementsIT { @ClassRule - public static EntitlementsTestRule testRule = new EntitlementsTestRule(true, ALWAYS_ALLOWED_TEST_ENTITLEMENTS); + public static EntitlementsTestRule testRule = new EntitlementsTestRule(true, null); public EntitlementsDeniedIT(@Name("actionName") String actionName) { super(actionName, false); diff --git a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedNonModularIT.java b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedNonModularIT.java index 057eb1d1d4d9f..ece18d4830387 100644 --- a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedNonModularIT.java +++ b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsDeniedNonModularIT.java @@ -18,7 +18,7 @@ public class EntitlementsDeniedNonModularIT extends AbstractEntitlementsIT { @ClassRule - public static EntitlementsTestRule testRule = new EntitlementsTestRule(false, ALWAYS_ALLOWED_TEST_ENTITLEMENTS); + public static EntitlementsTestRule testRule = new EntitlementsTestRule(false, null); public EntitlementsDeniedNonModularIT(@Name("actionName") String actionName) { super(actionName, false); From 5089e188be25a1283d976f339199f269b4a6b732 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Tue, 11 Mar 2025 09:33:34 +0100 Subject: [PATCH 4/4] feedback --- .../elasticsearch/entitlement/qa/AbstractEntitlementsIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java index c9b8965465829..d24fd32ade6ae 100644 --- a/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java +++ b/libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/AbstractEntitlementsIT.java @@ -90,7 +90,7 @@ protected boolean matchesSafely(ResponseException item) { @Override public void describeTo(Description description) { - description.appendText("403 (FORBIDDEN)").appendText(" due to ").appendText(expectedException); + description.appendValue(statusCode).appendText(" due to ").appendText(expectedException); } @Override