Add validation in policy files for missing codebases (#64841)

Elasticsearch plugins can add a java security policy file to grant
additional permissions. These policy files can contain permission grants
for specific jar files, which are specified through system properties.
Unfortunately the java policy parser is lenient when a system property
is missing, meaning we can't know if there is a typo or grant for a no
longer relevant jar file.

This commit adds validation to the policy parsing by overriding the
system properties and tracking when a missing system property is used.

modules/systemd/src/test/resources/plugin-security.codebases

@@ -0,0 +1 @@
+systemd: org.elasticsearch.systemd.SystemdPlugin

qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/ESPolicyUnitTests.java

@@ -24,6 +24,7 @@ import org.elasticsearch.test.ESTestCase;
 
 import java.io.FilePermission;
 import java.net.SocketPermission;
+import java.net.URL;
 import java.security.AllPermission;
 import java.security.CodeSource;
 import java.security.Permission;
@@ -32,12 +33,15 @@ import java.security.Permissions;
 import java.security.ProtectionDomain;
 import java.security.cert.Certificate;
 import java.util.Collections;
+import java.util.Map;
 
 /**
  * Unit tests for ESPolicy: these cannot run with security manager,
  * we don't allow messing with the policy
  */
 public class ESPolicyUnitTests extends ESTestCase {
+
+    static final Map<String, URL> TEST_CODEBASES = BootstrapForTesting.getCodebases();
     /**
      * Test policy with null codesource.
      * <p>
@@ -52,7 +56,7 @@ public class ESPolicyUnitTests extends ESTestCase {
         Permission all = new AllPermission();
         PermissionCollection allCollection = all.newPermissionCollection();
         allCollection.add(all);
-        ESPolicy policy = new ESPolicy(Collections.emptyMap(), allCollection, Collections.emptyMap(), true, new Permissions());
+        ESPolicy policy = new ESPolicy(TEST_CODEBASES, allCollection, Collections.emptyMap(), true, new Permissions());
         // restrict ourselves to NoPermission
         PermissionCollection noPermissions = new Permissions();
         assertFalse(policy.implies(new ProtectionDomain(null, noPermissions), new FilePermission("foo", "read")));
@@ -67,7 +71,7 @@ public class ESPolicyUnitTests extends ESTestCase {
     public void testNullLocation() throws Exception {
         assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
         PermissionCollection noPermissions = new Permissions();
-        ESPolicy policy = new ESPolicy(Collections.emptyMap(), noPermissions, Collections.emptyMap(), true, new Permissions());
+        ESPolicy policy = new ESPolicy(TEST_CODEBASES, noPermissions, Collections.emptyMap(), true, new Permissions());
         assertFalse(policy.implies(new ProtectionDomain(new CodeSource(null, (Certificate[]) null), noPermissions),
                 new FilePermission("foo", "read")));
     }
@@ -75,7 +79,7 @@ public class ESPolicyUnitTests extends ESTestCase {
     public void testListen() {
         assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
         final PermissionCollection noPermissions = new Permissions();
-        final ESPolicy policy = new ESPolicy(Collections.emptyMap(), noPermissions, Collections.emptyMap(), true, new Permissions());
+        final ESPolicy policy = new ESPolicy(TEST_CODEBASES, noPermissions, Collections.emptyMap(), true, new Permissions());
         assertFalse(
             policy.implies(
                 new ProtectionDomain(ESPolicyUnitTests.class.getProtectionDomain().getCodeSource(), noPermissions),
@@ -87,7 +91,7 @@ public class ESPolicyUnitTests extends ESTestCase {
         assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
         final PermissionCollection dataPathPermission = new Permissions();
         dataPathPermission.add(new FilePermission("/home/elasticsearch/data/-", "read"));
-        final ESPolicy policy = new ESPolicy(Collections.emptyMap(), new Permissions(), Collections.emptyMap(), true, dataPathPermission);
+        final ESPolicy policy = new ESPolicy(TEST_CODEBASES, new Permissions(), Collections.emptyMap(), true, dataPathPermission);
         assertTrue(
             policy.implies(
                     new ProtectionDomain(new CodeSource(null, (Certificate[]) null), new Permissions()),

qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/PolicyUtilTests.java

@@ -38,6 +38,7 @@ import java.util.Map;
 import java.util.Set;
 
 import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.emptyIterable;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
@@ -119,6 +120,13 @@ public class PolicyUtilTests extends ESTestCase {
             plugin.resolve("bar.jar").toUri().toURL()));
     }
 
+    public void testPolicyMissingCodebaseProperty() throws Exception {
+        Path plugin = makeDummyPlugin("missing-codebase.policy", "foo.jar");
+        URL policyFile = plugin.resolve(PluginInfo.ES_PLUGIN_POLICY).toUri().toURL();
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> PolicyUtil.readPolicy(policyFile, Map.of()));
+        assertThat(e.getMessage(), containsString("Unknown codebases [codebase.doesnotexist] in policy file"));
+    }
+
     public void testPolicyPermissions() throws Exception {
         Path plugin = makeDummyPlugin("global-and-jar.policy", "foo.jar", "bar.jar");
         Path tmpDir = createTempDir();

qa/evil-tests/src/test/resources/org/elasticsearch/bootstrap/missing-codebase.policy

@@ -0,0 +1,3 @@
+grant codeBase "${codebase.doesnotexist}" {
+  permission java.lang.RuntimePermission "getClassLoader";
+};

server/src/main/java/org/elasticsearch/bootstrap/PolicyUtil.java

@@ -38,13 +38,13 @@ import java.security.Policy;
 import java.security.ProtectionDomain;
 import java.security.URIParameter;
 import java.security.cert.Certificate;
-import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
-import java.util.List;
 import java.util.Map;
+import java.util.Properties;
 import java.util.Set;
 
 public class PolicyUtil {
@@ -79,8 +79,27 @@ public class PolicyUtil {
     @SuppressForbidden(reason = "accesses fully qualified URLs to configure security")
     public static Policy readPolicy(URL policyFile, Map<String, URL> codebases) {
         try {
-            List<String> propertiesSet = new ArrayList<>();
+            Properties originalProps = System.getProperties();
+            // allow missing while still setting values
+            Set<String> unknownCodebases = new HashSet<>();
+            Map<String, String> codebaseProperties = new HashMap<>();
+            Properties tempProps = new Properties(originalProps) {
+                @Override
+                public String getProperty(String key) {
+                    if (key.startsWith("codebase.")) {
+                        String value = codebaseProperties.get(key);
+                        if (value == null) {
+                            unknownCodebases.add(key);
+                        }
+                        return value;
+                    } else {
+                        return super.getProperty(key);
+                    }
+                }
+            };
+
             try {
+                System.setProperties(tempProps);
                 // set codebase properties
                 for (Map.Entry<String,URL> codebase : codebases.entrySet()) {
                     String name = codebase.getKey();
@@ -92,26 +111,27 @@ public class PolicyUtil {
                     String property = "codebase." + name;
                     String aliasProperty = "codebase." + name.replaceFirst("-\\d+\\.\\d+.*\\.jar", "");
                     if (aliasProperty.equals(property) == false) {
-                        propertiesSet.add(aliasProperty);
-                        String previous = System.setProperty(aliasProperty, url.toString());
+
+                        Object previous = codebaseProperties.put(aliasProperty, url.toString());
                         if (previous != null) {
                             throw new IllegalStateException("codebase property already set: " + aliasProperty + " -> " + previous +
-                                                            ", cannot set to " + url.toString());
+                                ", cannot set to " + url.toString());
                         }
                     }
-                    propertiesSet.add(property);
-                    String previous = System.setProperty(property, url.toString());
+                    Object previous = codebaseProperties.put(property, url.toString());
                     if (previous != null) {
                         throw new IllegalStateException("codebase property already set: " + property + " -> " + previous +
                                                         ", cannot set to " + url.toString());
                     }
                 }
-                return Policy.getInstance("JavaPolicy", new URIParameter(policyFile.toURI()));
-            } finally {
-                // clear codebase properties
-                for (String property : propertiesSet) {
-                    System.clearProperty(property);
+                Policy policy = Policy.getInstance("JavaPolicy", new URIParameter(policyFile.toURI()));
+                if (unknownCodebases.isEmpty() == false) {
+                    throw new IllegalArgumentException("Unknown codebases " + unknownCodebases + " in policy file [" + policyFile + "]" +
+                        "\nAvailable codebases: " + codebaseProperties.keySet());
                 }
+                return policy;
+            } finally {
+                System.setProperties(originalProps);
             }
         } catch (NoSuchAlgorithmException | URISyntaxException e) {
             throw new IllegalArgumentException("unable to parse policy file `" + policyFile + "`", e);

test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java

@@ -53,6 +53,7 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Properties;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean;
 import static org.elasticsearch.bootstrap.FilePermissionUtils.addDirectoryPath;
@@ -138,22 +139,12 @@ public class BootstrapForTesting {
                 // TODO: cut over all tests to bind to ephemeral ports
                 perms.add(new SocketPermission("localhost:1024-", "listen,resolve"));
 
-                boolean inGradle = System.getProperty("tests.gradle") != null;
-
                 // read test-framework permissions
-                Map<String, URL> codebases = PolicyUtil.getCodebaseJarMap(JarHell.parseClassPath());
-                // when testing server, the main elasticsearch code is not yet in a jar, so we need to manually add it
-                addClassCodebase(codebases,"elasticsearch", "org.elasticsearch.plugins.PluginsService");
-                if (inGradle == false) {
-                    // intellij and eclipse don't package our internal libs, so we need to set the codebases for them manually
-                    addClassCodebase(codebases,"elasticsearch-plugin-classloader", "org.elasticsearch.plugins.ExtendedPluginsClassLoader");
-                    addClassCodebase(codebases,"elasticsearch-nio", "org.elasticsearch.nio.ChannelFactory");
-                    addClassCodebase(codebases, "elasticsearch-secure-sm", "org.elasticsearch.secure_sm.SecureSM");
-                    addClassCodebase(codebases, "elasticsearch-rest-client", "org.elasticsearch.client.RestClient");
-                }
+                Map<String, URL> codebases = getCodebases();
+
                 final Policy testFramework = PolicyUtil.readPolicy(Bootstrap.class.getResource("test-framework.policy"), codebases);
                 final Policy runnerPolicy;
-                if (inGradle) {
+                if (System.getProperty("tests.gradle") != null) {
                     runnerPolicy = PolicyUtil.readPolicy(Bootstrap.class.getResource("gradle.policy"), codebases);
                 } else {
                     runnerPolicy = PolicyUtil.readPolicy(Bootstrap.class.getResource("intellij.policy"), codebases);
@@ -192,9 +183,23 @@ public class BootstrapForTesting {
         }
     }
 
+    static Map<String, URL> getCodebases() {
+        Map<String, URL> codebases = PolicyUtil.getCodebaseJarMap(JarHell.parseClassPath());
+        // when testing server, the main elasticsearch code is not yet in a jar, so we need to manually add it
+        addClassCodebase(codebases,"elasticsearch", "org.elasticsearch.plugins.PluginsService");
+        addClassCodebase(codebases,"elasticsearch-plugin-classloader", "org.elasticsearch.plugins.ExtendedPluginsClassLoader");
+        addClassCodebase(codebases,"elasticsearch-nio", "org.elasticsearch.nio.ChannelFactory");
+        addClassCodebase(codebases, "elasticsearch-secure-sm", "org.elasticsearch.secure_sm.SecureSM");
+        addClassCodebase(codebases, "elasticsearch-rest-client", "org.elasticsearch.client.RestClient");
+        return codebases;
+    }
+
     /** Add the codebase url of the given classname to the codebases map, if the class exists. */
     private static void addClassCodebase(Map<String, URL> codebases, String name, String classname) {
         try {
+            if (codebases.containsKey(name)) {
+                return; // the codebase already exists, from the classpath
+            }
             Class<?> clazz = BootstrapForTesting.class.getClassLoader().loadClass(classname);
             URL location = clazz.getProtectionDomain().getCodeSource().getLocation();
             if (location.toString().endsWith(".jar") == false) {
@@ -234,11 +239,30 @@ public class BootstrapForTesting {
                 Assert.class.getProtectionDomain().getCodeSource().getLocation()
         ));
         codebases.removeAll(excluded);
+        final Map<String, URL> codebasesMap = PolicyUtil.getCodebaseJarMap(codebases);
 
         // parse each policy file, with codebase substitution from the classpath
         final List<Policy> policies = new ArrayList<>(pluginPolicies.size());
         for (URL policyFile : pluginPolicies) {
-            policies.add(PolicyUtil.readPolicy(policyFile, PolicyUtil.getCodebaseJarMap(codebases)));
+            Map<String, URL> policyCodebases = codebasesMap;
+
+            // if the codebases file is inside a jar, then we don't need to load it since the jar will
+            // have already been read from the classpath
+            if (policyFile.toString().contains(".jar!") == false) {
+                Path policyPath = PathUtils.get(policyFile.toURI());
+                Path codebasesPath = policyPath.getParent().resolve("plugin-security.codebases");
+
+                if (Files.exists(codebasesPath)) {
+                    // load codebase to class map used for tests
+                    policyCodebases = new HashMap<>(codebasesMap);
+                    Map<String, String> codebasesProps = parsePropertiesFile(codebasesPath);
+                    for (var entry : codebasesProps.entrySet()) {
+                        addClassCodebase(policyCodebases, entry.getKey(), entry.getValue());
+                    }
+                }
+            }
+
+            policies.add(PolicyUtil.readPolicy(policyFile, policyCodebases));
         }
 
         // consult each policy file for those codebases
@@ -260,6 +284,14 @@ public class BootstrapForTesting {
         return Collections.unmodifiableMap(map);
     }
 
+    static Map<String, String> parsePropertiesFile(Path propertiesFile) throws Exception {
+        Properties props = new Properties();
+        try (InputStream is = Files.newInputStream(propertiesFile)) {
+            props.load(is);
+        }
+        return props.entrySet().stream().collect(Collectors.toMap(e -> e.getKey().toString(), e -> e.getValue().toString()));
+    }
+
     /**
      * return parsed classpath, but with symlinks resolved to destination files for matching
      * this is for matching the toRealPath() in the code where we have a proper plugin structure