[WIP] Non-modular plugins with synthetic module

modules/ingest-attachment/src/main/java/org/elasticsearch/ingest/attachment/TikaImpl.java

@@ -21,6 +21,7 @@
 import org.elasticsearch.core.PathUtils;
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.jdk.JarHell;
+import org.elasticsearch.plugins.UberModuleClassLoader;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
@@ -140,14 +141,17 @@ static PermissionCollection getRestrictedPermissions() {
             // classpath
             addReadPermissions(perms, JarHell.parseClassPath());
             // plugin jars
-            if (TikaImpl.class.getClassLoader() instanceof URLClassLoader urlClassLoader) {
-                URL[] urls = urlClassLoader.getURLs();
-                Set<URL> set = new LinkedHashSet<>(Arrays.asList(urls));
-                if (set.size() != urls.length) {
-                    throw new AssertionError("duplicate jars: " + Arrays.toString(urls));
-                }
-                addReadPermissions(perms, set);
+            final URL[] urls = switch (TikaImpl.class.getClassLoader()) {
+                case URLClassLoader urlClassLoader -> urlClassLoader.getURLs();
+                case UberModuleClassLoader uberClassLoader -> uberClassLoader.getInternalLoader().getURLs();
+                default -> throw new IllegalStateException("Unexpected value: " + TikaImpl.class.getClassLoader());
+            };
+            Set<URL> set = new LinkedHashSet<>(Arrays.asList(urls));
+            if (set.size() != urls.length) {
+                throw new AssertionError("duplicate jars: " + Arrays.toString(urls));
             }
+            addReadPermissions(perms, set);
+
             // jvm's java.io.tmpdir (needs read/write)
             FilePermissionUtils.addDirectoryPath(
                 perms,

server/src/main/java/org/elasticsearch/plugins/ModuleSupport.java

@@ -10,6 +10,8 @@
 package org.elasticsearch.plugins;
 
 import org.elasticsearch.core.SuppressForbidden;
+import org.elasticsearch.logging.LogManager;
+import org.elasticsearch.logging.Logger;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -42,6 +44,10 @@
  */
 public class ModuleSupport {
 
+    private static final Logger logger = LogManager.getLogger(ModuleSupport.class);
+
+    private static final Set<String> FORBIDDEN_PACKAGE_NAMES = Set.of("org.apache.commons.lang.enum");
+
     private ModuleSupport() {
         throw new AssertionError("Utility class, should not be instantiated");
     }
@@ -82,31 +88,44 @@ static ModuleDescriptor createModuleDescriptor(
         Map<String, List<String>> allBundledProviders = new HashMap<>();
         Set<String> servicesUsedInBundle = new HashSet<>();
         for (Path path : jarPaths) {
+            logger.info("Scanning JAR " + path + " for services");
             assert path.getFileName().toString().endsWith(".jar") : "expected jars suffix, in path: " + path;
             try (JarFile jf = new JarFile(path.toFile(), true, ZipFile.OPEN_READ, Runtime.version())) {
                 // if we have a module declaration, trust its uses/provides
                 JarEntry moduleInfo = jf.getJarEntry("module-info.class");
                 if (moduleInfo != null) {
                     var descriptor = getDescriptorForModularJar(path);
-                    pkgs.addAll(descriptor.packages());
+                    pkgs.addAll(descriptor.packages().stream().filter(Predicate.not(FORBIDDEN_PACKAGE_NAMES::contains)).toList());
                     servicesUsedInBundle.addAll(descriptor.uses());
+                    logger.info("Adding modular uses " + String.join(";", descriptor.uses()));
+
                     for (ModuleDescriptor.Provides p : descriptor.provides()) {
                         String serviceName = p.service();
                         List<String> providersInModule = p.providers();
+                        logger.info("Adding modular providers " + String.join(";", providersInModule));
 
                         allBundledProviders.compute(serviceName, (k, v) -> createListOrAppend(v, providersInModule));
                         servicesUsedInBundle.add(serviceName);
                     }
                 } else {
                     var scan = scan(jf);
-                    scan.classFiles().stream().map(cf -> toPackageName(cf, "/")).flatMap(Optional::stream).forEach(pkgs::add);
+                    scan.classFiles()
+                        .stream()
+                        .map(cf -> toPackageName(cf, "/"))
+                        .flatMap(Optional::stream)
+                        .filter(Predicate.not(FORBIDDEN_PACKAGE_NAMES::contains))
+                        .forEach(pkgs::add);
 
                     // read providers from the list of service files
                     for (String serviceFileName : scan.serviceFiles()) {
                         String serviceName = getServiceName(serviceFileName);
                         List<String> providersInJar = getProvidersFromServiceFile(jf, serviceFileName);
 
-                        allBundledProviders.compute(serviceName, (k, v) -> createListOrAppend(v, providersInJar));
+                        if (providersInJar.isEmpty() == false) {
+                            logger.info("Adding non-modular providers " + String.join(";", providersInJar));
+                            allBundledProviders.compute(serviceName, (k, v) -> createListOrAppend(v, providersInJar));
+                        }
+                        logger.info("Adding non-modular uses " + serviceName);
                         servicesUsedInBundle.add(serviceName);
                     }
                 }

server/src/main/java/org/elasticsearch/plugins/PluginsLoader.java

@@ -38,6 +38,7 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static org.elasticsearch.common.io.FileSystemUtils.isAccessibleDirectory;
@@ -100,8 +101,8 @@ public record LayerAndLoader(ModuleLayer layer, ClassLoader loader) {
             Objects.requireNonNull(loader);
         }
 
-        public static LayerAndLoader ofLoader(ClassLoader loader) {
-            return new LayerAndLoader(ModuleLayer.boot(), loader);
+        public static LayerAndLoader ofLoader(UberModuleClassLoader loader) {
+            return new LayerAndLoader(loader.getLayer(), loader);
         }
     }
 
@@ -273,7 +274,7 @@ static LayerAndLoader createSPI(
             );
         } else {
             logger.debug(() -> "Loading bundle: " + plugin.getName() + ", creating spi, non-modular");
-            return LayerAndLoader.ofLoader(URLClassLoader.newInstance(bundle.spiUrls.toArray(new URL[0]), parentLoader));
+            return new LayerAndLoader(ModuleLayer.boot(), URLClassLoader.newInstance(bundle.spiUrls.toArray(new URL[0]), parentLoader));
         }
     }
 
@@ -286,26 +287,39 @@ static LayerAndLoader createPlugin(
     ) {
         final PluginDescriptor plugin = bundle.plugin;
         if (plugin.getModuleName().isPresent()) {
-            logger.debug(() -> "Loading bundle: " + plugin.getName() + ", modular");
+            logger.info(() -> "Loading bundle: " + plugin.getName() + ", modular");
             var parentLayers = Stream.concat(
                 Stream.ofNullable(spiLayerAndLoader != null ? spiLayerAndLoader.layer() : null),
                 extendedPlugins.stream().map(LoadedPluginLayer::spiModuleLayer)
             ).toList();
             return createPluginModuleLayer(bundle, pluginParentLoader, parentLayers, qualifiedExports);
         } else if (plugin.isStable()) {
-            logger.debug(() -> "Loading bundle: " + plugin.getName() + ", non-modular as synthetic module");
+            logger.info(() -> "Loading bundle: " + plugin.getName() + ", non-modular as synthetic module");
             return LayerAndLoader.ofLoader(
                 UberModuleClassLoader.getInstance(
                     pluginParentLoader,
-                    ModuleLayer.boot(),
+                    List.of(ModuleLayer.boot()),
                     "synthetic." + toModuleName(plugin.getName()),
                     bundle.allUrls,
                     Set.of("org.elasticsearch.server") // TODO: instead of denying server, allow only jvm + stable API modules
                 )
             );
         } else {
-            logger.debug(() -> "Loading bundle: " + plugin.getName() + ", non-modular");
-            return LayerAndLoader.ofLoader(URLClassLoader.newInstance(bundle.urls.toArray(URL[]::new), pluginParentLoader));
+            var syntheticName = "synthetic." + toModuleName2(plugin.getName());
+            logger.info(() -> "Loading bundle: " + plugin.getName() + ", non-modular, as synthetic module " + syntheticName);
+            var parentLayers = Stream.concat(
+                Stream.ofNullable(spiLayerAndLoader != null ? spiLayerAndLoader.layer() : null),
+                Stream.concat(extendedPlugins.stream().map(LoadedPluginLayer::spiModuleLayer), Stream.of(ModuleLayer.boot()))
+            ).toList();
+            var parentModuleNames = parentLayers.stream()
+                .flatMap(l -> l.modules().stream())
+                .map(Module::getName)
+                .distinct()
+                .collect(Collectors.joining(";"));
+            logger.info("Parent modules for {}: {}", syntheticName, parentModuleNames);
+            return LayerAndLoader.ofLoader(
+                UberModuleClassLoader.getInstance(pluginParentLoader, parentLayers, syntheticName, bundle.allUrls, Set.of())
+            );
         }
     }
 
@@ -343,7 +357,7 @@ static LayerAndLoader createPluginModuleLayer(
         );
     }
 
-    static LayerAndLoader createModuleLayer(
+    private static LayerAndLoader createModuleLayer(
         String className,
         String moduleName,
         Path[] paths,
@@ -389,6 +403,16 @@ static String toModuleName(String name) {
         return result;
     }
 
+    static String toModuleName2(String name) {
+        String result = name.replace('-', '_')
+            .replaceAll("\\W+", ".") // replace non-alphanumeric character strings with dots
+            .replaceAll("(^[^A-Za-z_]*)", "") // trim non-alpha or underscore characters from start
+            .replaceAll("\\.$", "") // trim trailing dot
+            .toLowerCase(Locale.getDefault());
+        assert ModuleSupport.isPackageName(result);
+        return result;
+    }
+
     static final String toPackageName(String className) {
         assert className.endsWith(".") == false;
         int index = className.lastIndexOf('.');

server/src/main/java/org/elasticsearch/plugins/UberModuleClassLoader.java

@@ -28,6 +28,7 @@
 import java.security.PrivilegedAction;
 import java.security.SecureClassLoader;
 import java.util.Enumeration;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -58,43 +59,74 @@ public class UberModuleClassLoader extends SecureClassLoader implements AutoClos
     private final ModuleLayer.Controller moduleController;
     private final Set<String> packageNames;
 
-    private static Map<String, Set<String>> getModuleToServiceMap(ModuleLayer moduleLayer) {
-        Set<String> unqualifiedExports = moduleLayer.modules()
+    private static Set<String> unqualifiedExportsForLayer(ModuleLayer layer) {
+        return layer.modules()
             .stream()
             .flatMap(module -> module.getDescriptor().exports().stream())
             .filter(Predicate.not(ModuleDescriptor.Exports::isQualified))
             .map(ModuleDescriptor.Exports::source)
             .collect(Collectors.toSet());
-        return moduleLayer.modules()
+    }
+
+    private static Set<ModuleDescriptor> readableModulesForLayer(ModuleLayer layer) {
+        return layer.modules()
             .stream()
             .map(Module::getDescriptor)
-            .filter(ModuleSupport::hasAtLeastOneUnqualifiedExport)
+            .filter(md -> ModuleSupport.hasAtLeastOneUnqualifiedExport(md) || md.isOpen() || md.isAutomatic())
+            .collect(Collectors.toSet());
+    }
+
+    private static Map<String, Set<String>> getModuleToServiceMap(List<ModuleLayer> moduleLayers) {
+        Set<String> allUnqualifiedExports = new HashSet<>();
+        Set<ModuleDescriptor> allReadableModules = new HashSet<>();
+        collectLayersInfo(moduleLayers, allUnqualifiedExports, allReadableModules, true);
+
+        return allReadableModules.stream()
             .collect(
                 Collectors.toMap(
                     ModuleDescriptor::name,
                     md -> md.provides()
                         .stream()
                         .map(ModuleDescriptor.Provides::service)
-                        .filter(name -> unqualifiedExports.contains(packageName(name)))
+                        .filter(name -> allUnqualifiedExports.contains(packageName(name)))
                         .collect(Collectors.toSet())
                 )
             );
     }
 
+    private static void collectLayersInfo(
+        List<ModuleLayer> moduleLayers,
+        Set<String> allUnqualifiedExports,
+        Set<ModuleDescriptor> allReadableModules,
+        boolean allowBootLayer
+    ) {
+        if (moduleLayers == null || moduleLayers.isEmpty()) {
+            return;
+        }
+        for (var layer : moduleLayers) {
+            // ModuleLayer.boot() contains all ES libs too, so we want it to be an explicit parent
+            if (allowBootLayer || layer != ModuleLayer.boot()) {
+                allUnqualifiedExports.addAll(unqualifiedExportsForLayer(layer));
+                allReadableModules.addAll(readableModulesForLayer(layer));
+                collectLayersInfo(layer.parents(), allUnqualifiedExports, allReadableModules, false);
+            }
+        }
+    }
+
     static UberModuleClassLoader getInstance(ClassLoader parent, String moduleName, Set<URL> jarUrls) {
-        return getInstance(parent, ModuleLayer.boot(), moduleName, jarUrls, Set.of());
+        return getInstance(parent, List.of(ModuleLayer.boot()), moduleName, jarUrls, Set.of());
     }
 
     @SuppressWarnings("removal")
     static UberModuleClassLoader getInstance(
         ClassLoader parent,
-        ModuleLayer parentLayer,
+        List<ModuleLayer> parentLayers,
         String moduleName,
         Set<URL> jarUrls,
         Set<String> moduleDenyList
     ) {
         Path[] jarPaths = jarUrls.stream().map(UberModuleClassLoader::urlToPathUnchecked).toArray(Path[]::new);
-        var parentLayerModuleToServiceMap = getModuleToServiceMap(parentLayer);
+        var parentLayerModuleToServiceMap = getModuleToServiceMap(parentLayers);
         Set<String> requires = parentLayerModuleToServiceMap.keySet()
             .stream()
             .filter(Predicate.not(moduleDenyList::contains))
@@ -110,10 +142,15 @@ static UberModuleClassLoader getInstance(
             jarPaths,
             requires,
             uses,
-            s -> isPackageInLayers(s, parentLayer)
+            s -> isPackageInLayers(s, parentLayers)
         );
         // TODO: check that denied modules are not brought as transitive dependencies (or switch to allow-list?)
-        Configuration cf = parentLayer.configuration().resolve(finder, ModuleFinder.of(), Set.of(moduleName));
+        Configuration cf = Configuration.resolve(
+            finder,
+            parentLayers.stream().map(ModuleLayer::configuration).toList(),
+            ModuleFinder.of(),
+            Set.of(moduleName)
+        );
 
         Set<String> packageNames = finder.find(moduleName).map(ModuleReference::descriptor).map(ModuleDescriptor::packages).orElseThrow();
 
@@ -122,20 +159,25 @@ static UberModuleClassLoader getInstance(
             moduleName,
             jarUrls.toArray(new URL[0]),
             cf,
-            parentLayer,
+            parentLayers,
             packageNames
         );
         return AccessController.doPrivileged(pa);
     }
 
-    private static boolean isPackageInLayers(String packageName, ModuleLayer moduleLayer) {
-        if (moduleLayer.modules().stream().map(Module::getPackages).anyMatch(p -> p.contains(packageName))) {
+    private static boolean isPackageInLayers(String packageName, List<ModuleLayer> moduleLayers) {
+        if (moduleLayers.stream().flatMap(x -> x.modules().stream()).map(Module::getPackages).anyMatch(p -> p.contains(packageName))) {
             return true;
         }
-        if (moduleLayer.parents().equals(List.of(ModuleLayer.empty()))) {
-            return false;
+        for (var moduleLayer : moduleLayers) {
+            if (moduleLayer.parents().equals(List.of(ModuleLayer.empty()))) {
+                continue;
+            }
+            if (moduleLayer.parents().stream().anyMatch(ml -> isPackageInLayers(packageName, List.of(ml)))) {
+                return true;
+            }
         }
-        return moduleLayer.parents().stream().anyMatch(ml -> isPackageInLayers(packageName, ml));
+        return false;
     }
 
     /**
@@ -146,7 +188,7 @@ private UberModuleClassLoader(
         String moduleName,
         URL[] jarURLs,
         Configuration cf,
-        ModuleLayer mparent,
+        List<ModuleLayer> mparent,
         Set<String> packageNames
     ) {
         super(parent);
@@ -157,7 +199,7 @@ private UberModuleClassLoader(
         // Defining a module layer tells the Java virtual machine about the
         // classes that may be loaded from the module, and is what makes the
         // Class::getModule call return the name of our ubermodule.
-        this.moduleController = ModuleLayer.defineModules(cf, List.of(mparent), s -> this);
+        this.moduleController = ModuleLayer.defineModules(cf, mparent, s -> this);
         this.module = this.moduleController.layer().findModule(moduleName).orElseThrow();
 
         this.packageNames = packageNames;

server/src/test/java/org/elasticsearch/plugins/UberModuleClassLoaderTests.java

@@ -308,7 +308,7 @@ public String toString() {
         try (
             UberModuleClassLoader denyListLoader = UberModuleClassLoader.getInstance(
                 UberModuleClassLoaderTests.class.getClassLoader(),
-                ModuleLayer.boot(),
+                List.of(ModuleLayer.boot()),
                 "synthetic",
                 Set.of(toUrl(jar)),
                 Set.of("java.sql", "java.sql.rowset") // if present, java.sql.rowset requires java.sql transitively
@@ -614,7 +614,7 @@ private static UberModuleClassLoader getServiceTestLoader(boolean includeOptiona
         Set<Path> jarPaths = new HashSet<>(Set.of(modularJar, nonModularJar, serviceCallerJar));
         return UberModuleClassLoader.getInstance(
             parentLayer.findLoader(includeOptionalDeps ? "p.optional" : "p.required"),
-            parentLayer,
+            List.of(parentLayer),
             "synthetic",
             jarPaths.stream().map(UberModuleClassLoaderTests::pathToUrlUnchecked).collect(Collectors.toSet()),
             Set.of()

test/external-modules/jvm-crash/src/main/java/org/elasticsearch/test/jvm_crash/RestJvmCrashAction.java

@@ -30,7 +30,8 @@ public class RestJvmCrashAction implements RestHandler {
     static {
         try {
             AccessController.doPrivileged((PrivilegedExceptionAction<?>) () -> {
-                Class<?> unsafe = Class.forName("sun.misc.Unsafe");
+                // Bypass UberModuleClassLoader, as server does not have our very dangerous permissions
+                Class<?> unsafe = ClassLoader.getSystemClassLoader().loadClass("sun.misc.Unsafe");
 
                 FREE_MEMORY = unsafe.getMethod("freeMemory", long.class);
                 Field f = unsafe.getDeclaredField("theUnsafe");

test/external-modules/jvm-crash/src/main/plugin-metadata/plugin-security.policy

@@ -3,4 +3,5 @@ grant {
   permission java.lang.RuntimePermission "accessDeclaredMembers";
   permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
   permission java.lang.RuntimePermission "accessClassInPackage.sun.misc";
+  permission java.lang.RuntimePermission "getClassLoader";
 };