From aeaec170d58d6765459462110e8846cb5cedf047 Mon Sep 17 00:00:00 2001 From: Mark Payne Date: Fri, 1 Apr 2022 12:59:01 -0400 Subject: [PATCH] NIFI-9861: Removed BlockListClassLoader in favor of AllowListClassLoader NIFI-9861: Fixed stateless-processor-tests assembly to ensure that all necessary libraries were included; removed BlockListClassLoader NIFI-9861: Fixed issue in which we would list .class files as files that we allow through the AllowListClassLoader but didn't allow them. This closes #5925. Signed-off-by: Peter Turcsanyi --- .../nifi-stateless-processor-tests/pom.xml | 5 + .../src/test/assembly/dependencies.xml | 2 + ...sLoader.java => AllowListClassLoader.java} | 33 ++-- .../bootstrap/StatelessBootstrap.java | 167 ++++++++++-------- .../nifi-stateless-system-test-suite/pom.xml | 11 +- 5 files changed, 124 insertions(+), 94 deletions(-) rename nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/{BlockListClassLoader.java => AllowListClassLoader.java} (66%) diff --git a/nifi-nar-bundles/nifi-stateless-processor-bundle/nifi-stateless-processor-tests/pom.xml b/nifi-nar-bundles/nifi-stateless-processor-bundle/nifi-stateless-processor-tests/pom.xml index f6d4d103ca..a6fb1702ae 100644 --- a/nifi-nar-bundles/nifi-stateless-processor-bundle/nifi-stateless-processor-tests/pom.xml +++ b/nifi-nar-bundles/nifi-stateless-processor-bundle/nifi-stateless-processor-tests/pom.xml @@ -74,6 +74,11 @@ nifi-registry-data-model 1.17.0-SNAPSHOT + + org.slf4j + slf4j-api + runtime + org.apache.nifi diff --git a/nifi-nar-bundles/nifi-stateless-processor-bundle/nifi-stateless-processor-tests/src/test/assembly/dependencies.xml b/nifi-nar-bundles/nifi-stateless-processor-bundle/nifi-stateless-processor-tests/src/test/assembly/dependencies.xml index 743db977de..68a2e24931 100644 --- a/nifi-nar-bundles/nifi-stateless-processor-bundle/nifi-stateless-processor-tests/src/test/assembly/dependencies.xml +++ b/nifi-nar-bundles/nifi-stateless-processor-bundle/nifi-stateless-processor-tests/src/test/assembly/dependencies.xml @@ -39,6 +39,8 @@ nifi-server-api nifi-runtime nifi-nar-utils + nifi-stateless-api + slf4j-api diff --git a/nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/BlockListClassLoader.java b/nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/AllowListClassLoader.java similarity index 66% rename from nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/BlockListClassLoader.java rename to nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/AllowListClassLoader.java index ae72f6fe15..795da323ea 100644 --- a/nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/BlockListClassLoader.java +++ b/nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/AllowListClassLoader.java @@ -22,7 +22,7 @@ import java.util.Set; /** *

- * A ClassLoader that blocks a specific set of selected classes from being loaded by its parent. This ClassLoader does not load any classes itself + * A ClassLoader that allows only a specific set of selected classes to be loaded by its parent. This ClassLoader does not load any classes itself * but serves as a mechanism for preventing unwanted classes from a parent from being used. *

*

@@ -32,40 +32,41 @@ import java.util.Set; *

*

* Because we cannot control what is loaded by the System ClassLoader (that's up to the embedding application), the best that we can do is to block NiFi's extensions' - * ClassLoaders from accessing those classes. This ClassLoader allows us to do just that, blocking specific classes that have been loaded by the parent ClassLoader - * from being accessible by child ClassLoaders. + * ClassLoaders from accessing those classes. This ClassLoader allows us to do just that, allowing only specific classes that have been loaded by the parent ClassLoader + * to be visible/accessible by child ClassLoaders. *

*/ -public class BlockListClassLoader extends ClassLoader { - private final Set blockList; +public class AllowListClassLoader extends ClassLoader { + private final Set allowed; - public BlockListClassLoader(final ClassLoader parent, final Set blockList) { + public AllowListClassLoader(final ClassLoader parent, final Set allowed) { super(parent); - this.blockList = blockList; + this.allowed = allowed; } /** - * @return the set of all Class names that will be blocked from loading by the parent + * @return the set of all Class names that will not be blocked from loading by the parent */ - public Set getClassesBlocked() { - return Collections.unmodifiableSet(blockList); + public Set getClassesAllowed() { + return Collections.unmodifiableSet(allowed); } @Override protected Class loadClass(final String name, final boolean resolve) throws ClassNotFoundException { - if (blockList.contains(name)) { - throw new ClassNotFoundException(name + " was blocked by BlockListClassLoader"); + if (allowed.contains(name)) { + return super.loadClass(name, resolve); } - return super.loadClass(name, resolve); + throw new ClassNotFoundException(name + " was blocked by AllowListClassLoader"); } @Override protected Class findClass(final String name) throws ClassNotFoundException { - if (blockList.contains(name)) { - throw new ClassNotFoundException(name + " was blocked by BlockListClassLoader"); + if (allowed.contains(name)) { + return super.findClass(name); } - return super.findClass(name); + throw new ClassNotFoundException(name + " was blocked by AllowListClassLoader"); } + } diff --git a/nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/StatelessBootstrap.java b/nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/StatelessBootstrap.java index ed1169c3be..b0e6ba6795 100644 --- a/nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/StatelessBootstrap.java +++ b/nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/StatelessBootstrap.java @@ -36,13 +36,8 @@ import org.slf4j.LoggerFactory; import java.io.File; import java.io.IOException; -import java.net.URISyntaxException; -import java.net.URL; -import java.net.URLClassLoader; import java.nio.file.Path; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.Enumeration; import java.util.HashSet; @@ -54,6 +49,7 @@ import java.util.function.Predicate; import java.util.jar.JarFile; import java.util.regex.Pattern; import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; public class StatelessBootstrap { private static final Logger logger = LoggerFactory.getLogger(StatelessBootstrap.class); @@ -120,7 +116,7 @@ public class StatelessBootstrap { final long unpackMillis = System.currentTimeMillis() - unpackStart; logger.info("Unpacked NAR files in {} millis", unpackMillis); - final BlockListClassLoader statelessClassLoader = createExtensionRootClassLoader(narDirectory, rootClassLoader); + final AllowListClassLoader statelessClassLoader = createExtensionRootClassLoader(narDirectory, rootClassLoader); final File statelessNarWorkingDir = locateStatelessNarWorkingDirectory(extensionsWorkingDir); final NarClassLoader engineClassLoader; @@ -136,7 +132,7 @@ public class StatelessBootstrap { /** * Creates a ClassLoader that is to be used as the 'root'/parent for all NiFi Extensions' ClassLoaders. The ClassLoader will inherit from its parent - * any classes that exist in JAR files that can be found in the given NAR Directory. However, it will not allow any other classes to be loaded from the parent. + * any classes that exist in JAR files that can be found in the given NAR Directory or within the Java home directory. However, it will not allow any other classes to be loaded from the parent. * This approach is important because we need to ensure that the ClassLoader that is provided to extensions when run from NiFi Stateless is the same as the ClassLoader * that will be provided to it in traditional NiFi. Whereas in traditional NiFi, we have the ability to control the System ClassLoader, Stateless NiFi is designed to be * embedded, so we cannot control the System ClassLoader of the embedding application. This gives us a way to ensure that we control what is available to Extensions and @@ -145,13 +141,9 @@ public class StatelessBootstrap { * * @param narDirectory the NAR directory whose .jar files should be made available via the parent. * @param parent the parent class loader that the given BlockListClassLoader should delegate to for classes that it does not block - * @return a BlockListClassLoader that allows only the appropriate classes to be loaded from the given parent + * @return an AllowListClassLoader that allows only the appropriate classes to be loaded from the given parent */ - private static BlockListClassLoader createExtensionRootClassLoader(final File narDirectory, final ClassLoader parent) throws IOException { - if (!(parent instanceof URLClassLoader)) { - return new BlockListClassLoader(parent, Collections.emptySet()); - } - + private static AllowListClassLoader createExtensionRootClassLoader(final File narDirectory, final ClassLoader parent) throws IOException { final File[] narDirectoryFiles = narDirectory.listFiles(); if (narDirectoryFiles == null) { throw new IOException("Could not get a listing of the NAR directory"); @@ -159,13 +151,6 @@ public class StatelessBootstrap { logger.debug("NAR directory used to find files to allow being loaded by Stateless Extension Classloaders from parent {}: {}", parent, narDirectory); - final Set urls = new HashSet<>(); - findClassLoaderUrls(parent, urls); - - final Set classesBlocked = new HashSet<>(); - final Set filesBlocked = new HashSet<>(); - findClassNamesInJars(urls, classesBlocked, filesBlocked); - final Set classesAllowed = new HashSet<>(); final Set filesAllowed = new HashSet<>(); for (final File file : narDirectoryFiles) { @@ -181,58 +166,75 @@ public class StatelessBootstrap { filesAllowed.add(file.getName()); } } - logger.debug("The following JAR files are proposed to be blocked from being loaded by Stateless Extensions ClassLoaders from parent {}: {}", parent, filesBlocked); - logger.debug("Of the full list above, the following JAR files will be explicitly allowed to be loaded by Stateless Extensions ClassLoaders from parent {}: {}", parent, filesAllowed); - classesBlocked.removeAll(classesAllowed); - filesBlocked.removeAll(filesAllowed); - logger.debug("The final list of JAR files blocked from being loaded by Stateless Extensions ClassLoaders from parent {}: {}", parent, filesBlocked); - - logger.debug("The final list of classes blocked from being loaded by Stateless Extension ClassLoaders from parent {}: {}", parent, classesBlocked); - - final BlockListClassLoader blockingClassLoader = new BlockListClassLoader(parent, classesBlocked); - return blockingClassLoader; - } - - private static void findClassNamesInJars(final Collection jarUrls, final Set classesFound, final Set jarFilesFound) throws IOException { - final String javaHome = System.getProperty("java.home"); - final File javaHomeDir = new File(javaHome); - final File javaLib = new File(javaHomeDir, "lib"); - final String javaLibPath = javaLib.getAbsolutePath(); - - for (final URL url : jarUrls) { - final File file; - try { - file = new File(url.toURI()); - } catch (URISyntaxException e) { - logger.warn("Could not find file for {} in classpath", url); - continue; - } - - final String absolutePath = file.getAbsolutePath(); - if (absolutePath.startsWith(javaLibPath)) { - continue; - } - - findClassNamesInJar(file, classesFound); - jarFilesFound.add(file.getName()); + final Set javaHomeFiles = findJavaHomeFiles(); + final Set javaHomeFilenames = new HashSet<>(); + for (final File file : javaHomeFiles) { + findLoadableClasses(file, classesAllowed); + javaHomeFilenames.add(file.getName()); } + + logger.debug("The following JAR files will be explicitly allowed to be loaded by Stateless Extensions ClassLoaders from parent {}: {}", parent, filesAllowed); + logger.debug("The following JAR/JMOD files from ${JAVA_HOME} will be explicitly allowed to be loaded by Stateless Extensions ClassLoaders from parent {}: {}", parent, javaHomeFilenames); + logger.debug("The final list of classes allowed to be loaded by Stateless Extension ClassLoaders from parent {}: {}", parent, classesAllowed); + + final AllowListClassLoader allowListClassLoader = new AllowListClassLoader(parent, classesAllowed); + return allowListClassLoader; } - private static void findClassLoaderUrls(final ClassLoader classLoader, final Set urls) { - if (classLoader == null) { + private static Set findJavaHomeFiles() { + final String javaHomeValue = System.getProperty("java.home"); + if (javaHomeValue == null) { + logger.warn("Could not find java.home system property so will not allow any classes explicitly from java.home in AllowListClassLoader"); + return Collections.emptySet(); + } + + final File javaHome = new File(javaHomeValue); + if (!javaHome.exists()) { + logger.warn("System property for java.home is {} but that directory does not exist so will not allow any classes explicitly from java.home in AllowListClassLoader", javaHomeValue); + return Collections.emptySet(); + } + + final File[] javaHomeFiles = javaHome.listFiles(); + if (javaHomeFiles == null) { + logger.warn("System property for java.home is {} but that directory is not readable so will not allow any classes explicitly from java.home in AllowListClassLoader", javaHomeValue); + return Collections.emptySet(); + } + + final Set loadableFiles = new HashSet<>(); + for (final File file : javaHomeFiles) { + findLoadableFiles(file, loadableFiles); + } + + return loadableFiles; + } + + private static void findLoadableFiles(final File file, final Set loadable) { + if (file.isDirectory()) { + final File[] children = file.listFiles(); + if (children == null) { + return; + } + + for (final File child : children) { + findLoadableFiles(child, loadable); + } + return; } - if (classLoader instanceof URLClassLoader) { - final URLClassLoader urlClassLoader = (URLClassLoader) classLoader; - urls.addAll(Arrays.asList(urlClassLoader.getURLs())); + final String filename = file.getName(); + if (filename.endsWith(".jar") || filename.endsWith(".jmod")) { + loadable.add(file); } + } - // If the classLoader is the system class loader, we are done. We don't want to process the parent of - // the system class loader (which would be the Launcher$ExtClassLoader that contains the JDK/JRE classes, etc) - if (classLoader != ClassLoader.getSystemClassLoader()) { - findClassLoaderUrls(classLoader.getParent(), urls); + private static void findLoadableClasses(final File file, final Set classNames) throws IOException { + final String filename = file.getName(); + if (filename.endsWith(".jar")) { + findClassNamesInJar(file, classNames); + } else if (filename.endsWith(".jmod")) { + findClassesInJmod(file, classNames); } } @@ -241,16 +243,37 @@ public class StatelessBootstrap { return; } - final JarFile jarFile = new JarFile(file); - final Enumeration enumeration = jarFile.entries(); - while (enumeration.hasMoreElements()) { - final ZipEntry zipEntry = enumeration.nextElement(); - final String entryName = zipEntry.getName(); + try (final JarFile jarFile = new JarFile(file)) { + final Enumeration enumeration = jarFile.entries(); + while (enumeration.hasMoreElements()) { + final ZipEntry zipEntry = enumeration.nextElement(); + final String entryName = zipEntry.getName(); - if (entryName.endsWith(".class")) { - final int lastIndex = entryName.lastIndexOf(".class"); - final String className = entryName.substring(0, lastIndex).replace("/", "."); - classNames.add(className); + if (entryName.endsWith(".class")) { + final int lastIndex = entryName.lastIndexOf(".class"); + final String className = entryName.substring(0, lastIndex).replace("/", "."); + classNames.add(className); + } + } + } + } + + private static void findClassesInJmod(final File file, final Set classNames) throws IOException { + if (!file.getName().endsWith(".jmod") || !file.isFile() || !file.exists()) { + return; + } + + try (final ZipFile zipFile = new ZipFile(file)) { + final Enumeration enumeration = zipFile.entries(); + while (enumeration.hasMoreElements()) { + final ZipEntry zipEntry = enumeration.nextElement(); + final String entryName = zipEntry.getName(); + + if (entryName.startsWith("classes/") && entryName.endsWith(".class")) { + final int lastIndex = entryName.lastIndexOf(".class"); + final String className = entryName.substring(8, lastIndex).replace("/", "."); + classNames.add(className); + } } } } diff --git a/nifi-system-tests/nifi-stateless-system-test-suite/pom.xml b/nifi-system-tests/nifi-stateless-system-test-suite/pom.xml index 8a483c9191..decd2f0f66 100644 --- a/nifi-system-tests/nifi-stateless-system-test-suite/pom.xml +++ b/nifi-system-tests/nifi-stateless-system-test-suite/pom.xml @@ -73,20 +73,19 @@ org.apache.nifi nifi-api 1.17.0-SNAPSHOT - test + runtime org.apache.nifi nifi-server-api 1.17.0-SNAPSHOT - test + runtime org.apache.nifi nifi-framework-api 1.17.0-SNAPSHOT - compile - true + runtime org.apache.nifi @@ -98,7 +97,7 @@ org.apache.nifi nifi-properties 1.17.0-SNAPSHOT - test + runtime org.slf4j @@ -113,7 +112,7 @@ javax.servlet javax.servlet-api 3.1.0 - test + runtime