From f90b01597c3fc6d2ae6ac2a71540cbf166c0e49f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenzo=20Dematt=C3=A9?= Date: Mon, 5 May 2025 17:41:22 +0200 Subject: [PATCH] Move FilesEntitlements validation to a separate class (#127703) Moves FilesEntitlements validation to a separate class. This is the final PR to make EntitlementsInitialization a simpler "orchestrator" of the various steps in the initialization phase. --- .../EntitlementInitialization.java | 73 +------------- .../FilesEntitlementsValidation.java | 98 +++++++++++++++++++ ... => FilesEntitlementsValidationTests.java} | 10 +- 3 files changed, 104 insertions(+), 77 deletions(-) create mode 100644 libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/FilesEntitlementsValidation.java rename libs/entitlement/src/test/java/org/elasticsearch/entitlement/initialization/{EntitlementInitializationTests.java => FilesEntitlementsValidationTests.java} (91%) diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java index 2e1c6c8f753b..7c939dba333b 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java @@ -10,11 +10,9 @@ package org.elasticsearch.entitlement.initialization; import org.elasticsearch.core.Booleans; -import org.elasticsearch.core.Strings; import org.elasticsearch.entitlement.bootstrap.EntitlementBootstrap; import org.elasticsearch.entitlement.bridge.EntitlementChecker; import org.elasticsearch.entitlement.runtime.api.ElasticsearchEntitlementChecker; -import org.elasticsearch.entitlement.runtime.policy.FileAccessTree; import org.elasticsearch.entitlement.runtime.policy.PathLookup; import org.elasticsearch.entitlement.runtime.policy.Policy; import org.elasticsearch.entitlement.runtime.policy.PolicyManager; @@ -39,7 +37,6 @@ import java.lang.reflect.InvocationTargetException; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -248,7 +245,7 @@ public class EntitlementInitialization { ) ); - validateFilesEntitlements(pluginPolicies, pathLookup); + FilesEntitlementsValidation.validate(pluginPolicies, pathLookup); return new PolicyManager( serverPolicy, @@ -262,74 +259,6 @@ public class EntitlementInitialization { ); } - // package visible for tests - static void validateFilesEntitlements(Map pluginPolicies, PathLookup pathLookup) { - Set readAccessForbidden = new HashSet<>(); - pathLookup.getBaseDirPaths(PLUGINS).forEach(p -> readAccessForbidden.add(p.toAbsolutePath().normalize())); - pathLookup.getBaseDirPaths(MODULES).forEach(p -> readAccessForbidden.add(p.toAbsolutePath().normalize())); - pathLookup.getBaseDirPaths(LIB).forEach(p -> readAccessForbidden.add(p.toAbsolutePath().normalize())); - Set writeAccessForbidden = new HashSet<>(); - pathLookup.getBaseDirPaths(CONFIG).forEach(p -> writeAccessForbidden.add(p.toAbsolutePath().normalize())); - for (var pluginPolicy : pluginPolicies.entrySet()) { - for (var scope : pluginPolicy.getValue().scopes()) { - var filesEntitlement = scope.entitlements() - .stream() - .filter(x -> x instanceof FilesEntitlement) - .map(x -> ((FilesEntitlement) x)) - .findFirst(); - if (filesEntitlement.isPresent()) { - var fileAccessTree = FileAccessTree.withoutExclusivePaths(filesEntitlement.get(), pathLookup, null); - validateReadFilesEntitlements(pluginPolicy.getKey(), scope.moduleName(), fileAccessTree, readAccessForbidden); - validateWriteFilesEntitlements(pluginPolicy.getKey(), scope.moduleName(), fileAccessTree, writeAccessForbidden); - } - } - } - } - - private static IllegalArgumentException buildValidationException( - String componentName, - String moduleName, - Path forbiddenPath, - FilesEntitlement.Mode mode - ) { - return new IllegalArgumentException( - Strings.format( - "policy for module [%s] in [%s] has an invalid file entitlement. Any path under [%s] is forbidden for mode [%s].", - moduleName, - componentName, - forbiddenPath, - mode - ) - ); - } - - private static void validateReadFilesEntitlements( - String componentName, - String moduleName, - FileAccessTree fileAccessTree, - Set readForbiddenPaths - ) { - - for (Path forbiddenPath : readForbiddenPaths) { - if (fileAccessTree.canRead(forbiddenPath)) { - throw buildValidationException(componentName, moduleName, forbiddenPath, READ); - } - } - } - - private static void validateWriteFilesEntitlements( - String componentName, - String moduleName, - FileAccessTree fileAccessTree, - Set writeForbiddenPaths - ) { - for (Path forbiddenPath : writeForbiddenPaths) { - if (fileAccessTree.canWrite(forbiddenPath)) { - throw buildValidationException(componentName, moduleName, forbiddenPath, READ_WRITE); - } - } - } - /** * If bytecode verification is enabled, ensure these classes get loaded before transforming/retransforming them. * For these classes, the order in which we transform and verify them matters. Verification during class transformation is at least an diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/FilesEntitlementsValidation.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/FilesEntitlementsValidation.java new file mode 100644 index 000000000000..4e0cc8f3a0a8 --- /dev/null +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/FilesEntitlementsValidation.java @@ -0,0 +1,98 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.entitlement.initialization; + +import org.elasticsearch.core.Strings; +import org.elasticsearch.entitlement.runtime.policy.FileAccessTree; +import org.elasticsearch.entitlement.runtime.policy.PathLookup; +import org.elasticsearch.entitlement.runtime.policy.Policy; +import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement; + +import java.nio.file.Path; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import static org.elasticsearch.entitlement.runtime.policy.PathLookup.BaseDir.CONFIG; +import static org.elasticsearch.entitlement.runtime.policy.PathLookup.BaseDir.LIB; +import static org.elasticsearch.entitlement.runtime.policy.PathLookup.BaseDir.MODULES; +import static org.elasticsearch.entitlement.runtime.policy.PathLookup.BaseDir.PLUGINS; +import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ; +import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ_WRITE; + +class FilesEntitlementsValidation { + + static void validate(Map pluginPolicies, PathLookup pathLookup) { + Set readAccessForbidden = new HashSet<>(); + pathLookup.getBaseDirPaths(PLUGINS).forEach(p -> readAccessForbidden.add(p.toAbsolutePath().normalize())); + pathLookup.getBaseDirPaths(MODULES).forEach(p -> readAccessForbidden.add(p.toAbsolutePath().normalize())); + pathLookup.getBaseDirPaths(LIB).forEach(p -> readAccessForbidden.add(p.toAbsolutePath().normalize())); + Set writeAccessForbidden = new HashSet<>(); + pathLookup.getBaseDirPaths(CONFIG).forEach(p -> writeAccessForbidden.add(p.toAbsolutePath().normalize())); + for (var pluginPolicy : pluginPolicies.entrySet()) { + for (var scope : pluginPolicy.getValue().scopes()) { + var filesEntitlement = scope.entitlements() + .stream() + .filter(x -> x instanceof FilesEntitlement) + .map(x -> ((FilesEntitlement) x)) + .findFirst(); + if (filesEntitlement.isPresent()) { + var fileAccessTree = FileAccessTree.withoutExclusivePaths(filesEntitlement.get(), pathLookup, null); + validateReadFilesEntitlements(pluginPolicy.getKey(), scope.moduleName(), fileAccessTree, readAccessForbidden); + validateWriteFilesEntitlements(pluginPolicy.getKey(), scope.moduleName(), fileAccessTree, writeAccessForbidden); + } + } + } + } + + private static IllegalArgumentException buildValidationException( + String componentName, + String moduleName, + Path forbiddenPath, + FilesEntitlement.Mode mode + ) { + return new IllegalArgumentException( + Strings.format( + "policy for module [%s] in [%s] has an invalid file entitlement. Any path under [%s] is forbidden for mode [%s].", + moduleName, + componentName, + forbiddenPath, + mode + ) + ); + } + + private static void validateReadFilesEntitlements( + String componentName, + String moduleName, + FileAccessTree fileAccessTree, + Set readForbiddenPaths + ) { + + for (Path forbiddenPath : readForbiddenPaths) { + if (fileAccessTree.canRead(forbiddenPath)) { + throw buildValidationException(componentName, moduleName, forbiddenPath, READ); + } + } + } + + private static void validateWriteFilesEntitlements( + String componentName, + String moduleName, + FileAccessTree fileAccessTree, + Set writeForbiddenPaths + ) { + for (Path forbiddenPath : writeForbiddenPaths) { + if (fileAccessTree.canWrite(forbiddenPath)) { + throw buildValidationException(componentName, moduleName, forbiddenPath, READ_WRITE); + } + } + } +} diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/initialization/EntitlementInitializationTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/initialization/FilesEntitlementsValidationTests.java similarity index 91% rename from libs/entitlement/src/test/java/org/elasticsearch/entitlement/initialization/EntitlementInitializationTests.java rename to libs/entitlement/src/test/java/org/elasticsearch/entitlement/initialization/FilesEntitlementsValidationTests.java index 6bbcec9cc400..4ca57a99e0a3 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/initialization/EntitlementInitializationTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/initialization/FilesEntitlementsValidationTests.java @@ -27,7 +27,7 @@ import static org.hamcrest.Matchers.both; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.startsWith; -public class EntitlementInitializationTests extends ESTestCase { +public class FilesEntitlementsValidationTests extends ESTestCase { private static PathLookup TEST_PATH_LOOKUP; @@ -75,7 +75,7 @@ public class EntitlementInitializationTests extends ESTestCase { ) ) ); - EntitlementInitialization.validateFilesEntitlements(Map.of("plugin", policy), TEST_PATH_LOOKUP); + FilesEntitlementsValidation.validate(Map.of("plugin", policy), TEST_PATH_LOOKUP); } public void testValidationFailForRead() { @@ -94,7 +94,7 @@ public class EntitlementInitializationTests extends ESTestCase { var ex = expectThrows( IllegalArgumentException.class, - () -> EntitlementInitialization.validateFilesEntitlements(Map.of("plugin", policy), TEST_PATH_LOOKUP) + () -> FilesEntitlementsValidation.validate(Map.of("plugin", policy), TEST_PATH_LOOKUP) ); assertThat( ex.getMessage(), @@ -119,7 +119,7 @@ public class EntitlementInitializationTests extends ESTestCase { ex = expectThrows( IllegalArgumentException.class, - () -> EntitlementInitialization.validateFilesEntitlements(Map.of("plugin2", policy2), TEST_PATH_LOOKUP) + () -> FilesEntitlementsValidation.validate(Map.of("plugin2", policy2), TEST_PATH_LOOKUP) ); assertThat( ex.getMessage(), @@ -145,7 +145,7 @@ public class EntitlementInitializationTests extends ESTestCase { var ex = expectThrows( IllegalArgumentException.class, - () -> EntitlementInitialization.validateFilesEntitlements(Map.of("plugin", policy), TEST_PATH_LOOKUP) + () -> FilesEntitlementsValidation.validate(Map.of("plugin", policy), TEST_PATH_LOOKUP) ); assertThat( ex.getMessage(),